Skip to content

Commit da150c9

Browse files
andrewlockveerbia
authored andcommitted
Update config-in-code manual instrumentation to use an IConfigurationSource (#6397)
Change how we "apply" settings from manual configuration to the automatic side to use `IConfigurationSource` ## Reason for change The previous design of how we "apply" settings made in code using manual instrumentation required mutating a `TracerSettings` object after it was already built. In fact, this is pretty much the _only_ place that we mutate the settings. By switching to using a "configuration source" approach, so that the settings are built _once_ with the correct values opens up the option to make these immutable (and therefore delete all of the `Immutable*` settings we currently have. This reduces both code duplication and work we do on startup, and opens the path to further refactoring improvements. Note that the public API does not change, so consumers of Datadog.Trace.Manual are still working with a mutable object initially. ## Implementation details Currently we pass a `Dictionary<string, object>` between the manual and automatic side. Previously, we then iterated the keys, compared against known values, and modified `TracerSettings` as required. With the changes, we have a `ManualInstrumentationConfigurationSource` which just "wraps" the `Dictionary<>`, and returns the correct value as required for a given key. Overall, I think this is much cleaner. Where things get messy is how we handle disabling specific integrations. The existing dictionary is optimised for looping through the provided values, fetching the setting that needs to be modified, and changing all the required properties. Unfortunately, the `IConfigurationSource` approach where we're looking up by a key like `DD_TRACE_NPGSQL_ENABLED` works _horribly_ for this pattern 🙁. So I introduced an additional approach which explicitly _additionally_ transfers the settings using these values, making them just "standard" lookups. > Note that due to backwards + forwards compatibility requirements > - We _still_ need to send the "old" version of integration settings from the manual side, in case it's used with an old version of the auto instrumentation > - We _still_ need to handle the "old" version of integration settings in the auto side, in case it's used with an old version of the manual instrumentation. > - At least in this case we can use the more efficient `IConfigurationSource` reader, so we don't pay the expense of retrieving the settings. The only downside is a couple of extra allocations when they _do_ disable integrations in code. Minor other changes: - Add helper ctor to `CompositeConfigurationSource` for creating the internal list from a collection of `IConfigurationSource` - Tweak `DictionaryObjectConfigurationSource` so we can derive from it - Create a separate integration for <3.7.0 manual instrumentation that uses the legacy settings, otherwise use the new settings objects ## Test coverage Mostly covered by existing unit tests (and indirectly by integration tests). Tweaked the test to test both the new and legacy configuration source. ## Other details Requires #6393 to fix how we read integration settings Part of stack - #6370 - #6376 - #6385 - #6386 - #6397 👈 This PR - #6399 - #6400 - #6405 - #6408
1 parent ec65c33 commit da150c9

File tree

15 files changed

+746
-321
lines changed

15 files changed

+746
-321
lines changed

tracer/src/Datadog.Trace.Manual/Configuration/IntegrationSettings.cs

+18
Original file line numberDiff line numberDiff line change
@@ -75,4 +75,22 @@ public double AnalyticsSampleRate
7575
_analyticsEnabled.IsOverridden ? _analyticsEnabled.Value : null,
7676
_analyticsSampleRate.IsOverridden,
7777
_analyticsSampleRate.IsOverridden ? _analyticsSampleRate.Value : null);
78+
79+
internal void RecordChangedKeys(Dictionary<string, object?> results)
80+
{
81+
if (_enabled.IsOverridden)
82+
{
83+
results[$"DD_TRACE_{IntegrationName.ToUpperInvariant()}_ENABLED"] = _enabled.Value;
84+
}
85+
86+
if (_analyticsEnabled.IsOverridden)
87+
{
88+
results[$"DD_TRACE_{IntegrationName.ToUpperInvariant()}_ANALYTICS_ENABLED"] = _analyticsEnabled.Value;
89+
}
90+
91+
if (_analyticsSampleRate.IsOverridden)
92+
{
93+
results[$"DD_TRACE_{IntegrationName.ToUpperInvariant()}_ANALYTICS_SAMPLE_RATE"] = _analyticsSampleRate.Value;
94+
}
95+
}
7896
}

tracer/src/Datadog.Trace.Manual/Configuration/TracerSettings.cs

+9-5
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,9 @@ public void SetServiceNameMappings(IEnumerable<KeyValuePair<string, string>> map
471471

472472
// Always set
473473
results[TracerSettingKeyConstants.IsFromDefaultSourcesKey] = _isFromDefaultSources;
474-
if (BuildIntegrationSettings(Integrations) is { } integrations)
474+
// We include both the "legacy" way of specifying the integration changes
475+
// as well as the "new" way, so that we are both backward and forward compatible
476+
if (BuildIntegrationSettings(Integrations, results) is { } integrations)
475477
{
476478
results[TracerSettingKeyConstants.IntegrationSettingsKey] = integrations;
477479
}
@@ -549,24 +551,26 @@ static bool HasChanges(in OverrideValue<IDictionary<string, string>> updated)
549551
}
550552
}
551553

552-
static Dictionary<string, object?[]>? BuildIntegrationSettings(IntegrationSettingsCollection settings)
554+
static Dictionary<string, object?[]>? BuildIntegrationSettings(IntegrationSettingsCollection settings, Dictionary<string, object?> results)
553555
{
554556
if (settings.Settings.Count == 0)
555557
{
556558
return null;
557559
}
558560

559-
var results = new Dictionary<string, object?[]>(settings.Settings.Count, StringComparer.OrdinalIgnoreCase);
561+
var integrationArrays = new Dictionary<string, object?[]>(settings.Settings.Count, StringComparer.OrdinalIgnoreCase);
560562
foreach (var pair in settings.Settings)
561563
{
562564
var setting = pair.Value;
563565
if (setting.GetChangeDetails() is { } changes)
564566
{
565-
results[setting.IntegrationName] = changes;
567+
integrationArrays[setting.IntegrationName] = changes;
568+
// set the values that have changed explicitly using the "new" way
569+
setting.RecordChangedKeys(results);
566570
}
567571
}
568572

569-
return results;
573+
return integrationArrays;
570574
}
571575
}
572576
}

tracer/src/Datadog.Trace/ClrProfiler/AutoInstrumentation/ManualInstrumentation/Tracer/ConfigureIntegration.cs

+15-300
Large diffs are not rendered by default.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// <copyright file="ConfigureIntegration_Pre3_7.cs" company="Datadog">
2+
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
4+
// </copyright>
5+
6+
#nullable enable
7+
using System.Collections.Generic;
8+
using System.ComponentModel;
9+
using Datadog.Trace.ClrProfiler.CallTarget;
10+
11+
namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation.Tracer;
12+
13+
/// <summary>
14+
/// System.Void Datadog.Trace.Tracer::Configure(System.Collections.Generic.Dictionary`2[System.String,System.Object]) calltarget instrumentation
15+
/// </summary>
16+
[InstrumentMethod(
17+
AssemblyName = "Datadog.Trace.Manual",
18+
TypeName = "Datadog.Trace.Tracer",
19+
MethodName = "Configure",
20+
ReturnTypeName = ClrNames.Void,
21+
ParameterTypeNames = ["System.Collections.Generic.Dictionary`2[System.String,System.Object]"],
22+
MinimumVersion = ManualInstrumentationConstants.MinVersion,
23+
MaximumVersion = "3.6.*",
24+
IntegrationName = ManualInstrumentationConstants.IntegrationName)]
25+
[Browsable(false)]
26+
[EditorBrowsable(EditorBrowsableState.Never)]
27+
public class ConfigureIntegration_Pre3_7
28+
{
29+
internal static CallTargetState OnMethodBegin<TTarget>(Dictionary<string, object?> values)
30+
{
31+
ConfigureIntegration.ConfigureSettingsWithManualOverrides(values, useLegacySettings: true);
32+
return CallTargetState.GetDefault();
33+
}
34+
}

tracer/src/Datadog.Trace/Configuration/ConfigurationSources/CompositeConfigurationSource.cs

+11-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,17 @@ namespace Datadog.Trace.Configuration
2323
/// </summary>
2424
internal class CompositeConfigurationSource : IConfigurationSource, IEnumerable<IConfigurationSource>
2525
{
26-
private readonly List<IConfigurationSource> _sources = new();
26+
private readonly List<IConfigurationSource> _sources;
27+
28+
public CompositeConfigurationSource()
29+
{
30+
_sources = new();
31+
}
32+
33+
public CompositeConfigurationSource(IEnumerable<IConfigurationSource> sources)
34+
{
35+
_sources = [..sources];
36+
}
2737

2838
/// <summary>
2939
/// Adds a new configuration source to this instance.

tracer/src/Datadog.Trace/Configuration/ConfigurationSources/DictionaryObjectConfigurationSource.cs

+13-9
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ namespace Datadog.Trace.Configuration;
1515

1616
internal class DictionaryObjectConfigurationSource : IConfigurationSource
1717
{
18-
private readonly IReadOnlyDictionary<string, object?> _dictionary;
1918
private readonly ConfigurationOrigins _origin;
2019

2120
public DictionaryObjectConfigurationSource(IReadOnlyDictionary<string, object?> dictionary)
@@ -25,15 +24,20 @@ public DictionaryObjectConfigurationSource(IReadOnlyDictionary<string, object?>
2524

2625
public DictionaryObjectConfigurationSource(IReadOnlyDictionary<string, object?> dictionary, ConfigurationOrigins origin)
2726
{
28-
_dictionary = dictionary;
27+
Dictionary = dictionary;
2928
_origin = origin;
3029
}
3130

32-
public bool IsPresent(string key) => _dictionary.ContainsKey(key);
31+
protected IReadOnlyDictionary<string, object?> Dictionary { get; }
32+
33+
protected virtual bool TryGetValue(string key, out object? value)
34+
=> Dictionary.TryGetValue(key, out value);
35+
36+
public bool IsPresent(string key) => Dictionary.ContainsKey(key);
3337

3438
public ConfigurationResult<string> GetString(string key, IConfigurationTelemetry telemetry, Func<string, bool>? validator, bool recordValue)
3539
{
36-
if (_dictionary.TryGetValue(key, out var objValue) && objValue is not null)
40+
if (TryGetValue(key, out var objValue) && objValue is not null)
3741
{
3842
if (objValue is not string value)
3943
{
@@ -56,7 +60,7 @@ public ConfigurationResult<string> GetString(string key, IConfigurationTelemetry
5660

5761
public ConfigurationResult<int> GetInt32(string key, IConfigurationTelemetry telemetry, Func<int, bool>? validator)
5862
{
59-
if (_dictionary.TryGetValue(key, out var objValue) && objValue is not null)
63+
if (TryGetValue(key, out var objValue) && objValue is not null)
6064
{
6165
if (objValue is not int value)
6266
{
@@ -79,7 +83,7 @@ public ConfigurationResult<int> GetInt32(string key, IConfigurationTelemetry tel
7983

8084
public ConfigurationResult<double> GetDouble(string key, IConfigurationTelemetry telemetry, Func<double, bool>? validator)
8185
{
82-
if (_dictionary.TryGetValue(key, out var objValue) && objValue is not null)
86+
if (TryGetValue(key, out var objValue) && objValue is not null)
8387
{
8488
if (objValue is not double value)
8589
{
@@ -102,7 +106,7 @@ public ConfigurationResult<double> GetDouble(string key, IConfigurationTelemetry
102106

103107
public ConfigurationResult<bool> GetBool(string key, IConfigurationTelemetry telemetry, Func<bool, bool>? validator)
104108
{
105-
if (_dictionary.TryGetValue(key, out var objValue) && objValue is not null)
109+
if (TryGetValue(key, out var objValue) && objValue is not null)
106110
{
107111
if (objValue is not bool value)
108112
{
@@ -128,7 +132,7 @@ public ConfigurationResult<IDictionary<string, string>> GetDictionary(string key
128132

129133
public ConfigurationResult<IDictionary<string, string>> GetDictionary(string key, IConfigurationTelemetry telemetry, Func<IDictionary<string, string>, bool>? validator, bool allowOptionalMappings, char separator)
130134
{
131-
if (_dictionary.TryGetValue(key, out var objValue) && objValue is not null)
135+
if (TryGetValue(key, out var objValue) && objValue is not null)
132136
{
133137
if (objValue is not IDictionary<string, string> value)
134138
{
@@ -152,7 +156,7 @@ public ConfigurationResult<IDictionary<string, string>> GetDictionary(string key
152156

153157
public ConfigurationResult<T> GetAs<T>(string key, IConfigurationTelemetry telemetry, Func<string, ParsingResult<T>> converter, Func<T, bool>? validator, bool recordValue)
154158
{
155-
if (_dictionary.TryGetValue(key, out var objValue) && objValue is not null)
159+
if (TryGetValue(key, out var objValue) && objValue is not null)
156160
{
157161
// Handle conversion
158162
var valueAsString = objValue.ToString()!;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// <copyright file="ManualInstrumentationConfigurationSource.cs" company="Datadog">
2+
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
4+
// </copyright>
5+
6+
#nullable enable
7+
8+
using System;
9+
using System.Collections.Generic;
10+
using System.Globalization;
11+
using System.Linq;
12+
using Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation;
13+
using Datadog.Trace.Configuration.Telemetry;
14+
using Datadog.Trace.Telemetry;
15+
using Datadog.Trace.Telemetry.Metrics;
16+
17+
namespace Datadog.Trace.Configuration.ConfigurationSources;
18+
19+
/// <summary>
20+
/// Wraps the settings passed in from the manual instrumentation API in a configuration source, to make it easier to integrate
21+
/// </summary>
22+
internal class ManualInstrumentationConfigurationSource : DictionaryObjectConfigurationSource
23+
{
24+
public ManualInstrumentationConfigurationSource(IReadOnlyDictionary<string, object?> dictionary)
25+
: base(dictionary, ConfigurationOrigins.Code)
26+
{
27+
}
28+
29+
protected override bool TryGetValue(string key, out object? value)
30+
{
31+
// Get the value for the given key, but also record telemetry
32+
// This is also where any "remapping" should be done, in cases
33+
// where either the manual-instrumentation key differs or the
34+
// type stored in the dictionary differs
35+
36+
var result = base.TryGetValue(key, out value);
37+
if (result)
38+
{
39+
if (GetTelemetryKey(key) is { } telemetryKey)
40+
{
41+
TelemetryFactory.Metrics.Record(telemetryKey);
42+
}
43+
44+
if (value is not null)
45+
{
46+
value = RemapResult(key, value);
47+
}
48+
}
49+
50+
return result;
51+
}
52+
53+
internal static PublicApiUsage? GetTelemetryKey(string key) => key switch
54+
{
55+
TracerSettingKeyConstants.AgentUriKey => PublicApiUsage.ExporterSettings_AgentUri_Set,
56+
TracerSettingKeyConstants.AnalyticsEnabledKey => PublicApiUsage.TracerSettings_AnalyticsEnabled_Set,
57+
TracerSettingKeyConstants.CustomSamplingRules => PublicApiUsage.TracerSettings_CustomSamplingRules_Set,
58+
TracerSettingKeyConstants.DiagnosticSourceEnabledKey => PublicApiUsage.TracerSettings_DiagnosticSourceEnabled_Set,
59+
TracerSettingKeyConstants.DisabledIntegrationNamesKey => PublicApiUsage.TracerSettings_DisabledIntegrationNames_Set,
60+
TracerSettingKeyConstants.EnvironmentKey => PublicApiUsage.TracerSettings_Environment_Set,
61+
TracerSettingKeyConstants.GlobalSamplingRateKey => PublicApiUsage.TracerSettings_GlobalSamplingRate_Set,
62+
TracerSettingKeyConstants.GrpcTags => PublicApiUsage.TracerSettings_GrpcTags_Set,
63+
TracerSettingKeyConstants.HeaderTags => PublicApiUsage.TracerSettings_HeaderTags_Set,
64+
TracerSettingKeyConstants.GlobalTagsKey => PublicApiUsage.TracerSettings_GlobalTags_Set,
65+
TracerSettingKeyConstants.HttpClientErrorCodesKey => PublicApiUsage.TracerSettings_SetHttpClientErrorStatusCodes,
66+
TracerSettingKeyConstants.HttpServerErrorCodesKey => PublicApiUsage.TracerSettings_SetHttpServerErrorStatusCodes,
67+
TracerSettingKeyConstants.KafkaCreateConsumerScopeEnabledKey => PublicApiUsage.TracerSettings_KafkaCreateConsumerScopeEnabled_Set,
68+
TracerSettingKeyConstants.LogsInjectionEnabledKey => PublicApiUsage.TracerSettings_LogsInjectionEnabled_Set,
69+
TracerSettingKeyConstants.ServiceNameKey => PublicApiUsage.TracerSettings_ServiceName_Set,
70+
TracerSettingKeyConstants.ServiceNameMappingsKey => PublicApiUsage.TracerSettings_SetServiceNameMappings,
71+
TracerSettingKeyConstants.MaxTracesSubmittedPerSecondKey => PublicApiUsage.TracerSettings_MaxTracesSubmittedPerSecond_Set,
72+
TracerSettingKeyConstants.ServiceVersionKey => PublicApiUsage.TracerSettings_ServiceVersion_Set,
73+
TracerSettingKeyConstants.StartupDiagnosticLogEnabledKey => PublicApiUsage.TracerSettings_StartupDiagnosticLogEnabled_Set,
74+
TracerSettingKeyConstants.StatsComputationEnabledKey => PublicApiUsage.TracerSettings_StatsComputationEnabled_Set,
75+
TracerSettingKeyConstants.TraceEnabledKey => PublicApiUsage.TracerSettings_TraceEnabled_Set,
76+
TracerSettingKeyConstants.TracerMetricsEnabledKey => PublicApiUsage.TracerSettings_TracerMetricsEnabled_Set,
77+
TracerSettingKeyConstants.IntegrationSettingsKey => PublicApiUsage.TracerSettings_TracerMetricsEnabled_Set,
78+
// These are pretty hacky but about the best we can do
79+
_ when key.EndsWith("_ANALYTICS_ENABLED", StringComparison.Ordinal) => PublicApiUsage.IntegrationSettings_AnalyticsEnabled_Set,
80+
_ when key.EndsWith("_ANALYTICS_SAMPLE_RATE", StringComparison.Ordinal) => PublicApiUsage.IntegrationSettings_AnalyticsSampleRate_Set,
81+
_ when key.StartsWith("DD_TRACE_", StringComparison.Ordinal) && key.EndsWith("_ANALYTICS_SAMPLE_RATE", StringComparison.Ordinal)
82+
=> PublicApiUsage.IntegrationSettings_Enabled_Set, // this could definitely be too broad, but about the best we can reasonably do
83+
_ => null
84+
};
85+
86+
private static object RemapResult(string key, object value) => key switch
87+
{
88+
TracerSettingKeyConstants.AgentUriKey => value is Uri uri ? uri.ToString() : value,
89+
TracerSettingKeyConstants.HttpServerErrorCodesKey => value is List<int> list
90+
? string.Join(",", list.Select(i => i.ToString(CultureInfo.InvariantCulture)))
91+
: value,
92+
TracerSettingKeyConstants.HttpClientErrorCodesKey => value is List<int> list
93+
? string.Join(",", list.Select(i => i.ToString(CultureInfo.InvariantCulture)))
94+
: value,
95+
_ => value,
96+
};
97+
}

0 commit comments

Comments
 (0)