Skip to content

Commit 4626c34

Browse files
andrewlockandrewlock
authored
Rebuild MutableSettings on dynamic config changes (#7525)
## Summary of changes Rebuild and re-assign `MutableSettings` when dynamic config (remote or config in code) changes. ## Reason for change This is part of a general stack to extract the "mutable" configuration from static config that is fixed for the lifetime of the app. In the [previous PR](#7522) we moved mutable settings to their own type, but otherwise left things unchanged and just rebuilt everything whenever anything changes. In this PR we move towards combining the dynamic/code configuration, handling changes by _only_ rebuilding the `MutableSettings` (not `TracerSettings`) and handling all the fallout that causes for telemetry. This is very much still a "stop gap"; we still rebuild everything (_except_ the `TracerSettings` object) when these settings changes. ## Implementation details - Create "global" config sources for dynamic settings and code settings - There's actually a bug today where we clobber dynamic settings if we change things in code because we're not storing the sources globally. - When dynamic config or config in code changes - Create a new `MutableSettings` object based only on dynamic sources (with a fallback for the "static" values) - For config in code, we also check for changes to `ExporterSettings` - If there's no discernable changes, bail out - no need to tear down the world - If there _are_ changes, Mutate `TracerSettings`, and do the normal "reconfigure everything" - Remove the (now unused `ImmutableDynamicSettingsTests`) Note that there are technically some behaviour changes in this PR: - `useDefaultSources: false` only ignores env vars etc for values that can be set through code. Other settings will always use the default sources. - `StatsComutationEnabled` can not be _set_ via code. ## Test coverage This is still all technically a "refactoring", so should be covered by existing tests 🤞 ## Other details https://datadoghq.atlassian.net/browse/LANGPLAT-819 Part of a config stack - #7522 - #7652 - #7525 👈 - #7530 - #7532 - #7543 - #7544
1 parent 8ad16a0 commit 4626c34

File tree

12 files changed

+488
-250
lines changed

12 files changed

+488
-250
lines changed

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

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using Datadog.Trace.Configuration.ConfigurationSources;
1212
using Datadog.Trace.Configuration.ConfigurationSources.Telemetry;
1313
using Datadog.Trace.Configuration.Telemetry;
14+
using Datadog.Trace.Logging;
1415
using Datadog.Trace.Telemetry;
1516
using Datadog.Trace.Telemetry.Metrics;
1617

@@ -32,6 +33,8 @@ namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.ManualInstrumentation.Tr
3233
[EditorBrowsable(EditorBrowsableState.Never)]
3334
public class ConfigureIntegration
3435
{
36+
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor<ConfigureIntegration>();
37+
3538
internal static CallTargetState OnMethodBegin<TTarget>(Dictionary<string, object?> values)
3639
{
3740
ConfigureSettingsWithManualOverrides(values, useLegacySettings: false);
@@ -47,18 +50,74 @@ internal static void ConfigureSettingsWithManualOverrides(Dictionary<string, obj
4750
var isFromDefaults = values.TryGetValue(TracerSettingKeyConstants.IsFromDefaultSourcesKey, out var value) && value is true;
4851

4952
// Build the configuration sources, including our manual instrumentation values
50-
ManualInstrumentationConfigurationSourceBase manualConfigSource =
53+
ManualInstrumentationConfigurationSourceBase manualConfig =
5154
useLegacySettings
5255
? new ManualInstrumentationLegacyConfigurationSource(values, isFromDefaults)
5356
: new ManualInstrumentationConfigurationSource(values, isFromDefaults);
5457

55-
IConfigurationSource source = isFromDefaults
56-
? new CompositeConfigurationSource([manualConfigSource, GlobalConfigurationSource.Instance])
57-
: manualConfigSource;
58+
// We need to save this immediately, even if there's no manifest changes in the final settings
59+
GlobalConfigurationSource.UpdateManualConfigurationSource(manualConfig);
60+
61+
var tracerSettings = Datadog.Trace.Tracer.Instance.Settings;
62+
var dynamicConfig = GlobalConfigurationSource.DynamicConfigurationSource;
63+
var initialSettings = isFromDefaults
64+
? tracerSettings.InitialMutableSettings
65+
: MutableSettings.CreateWithoutDefaultSources(tracerSettings);
66+
67+
// TODO: these will eventually live elsewhere
68+
var currentSettings = tracerSettings.MutableSettings;
69+
70+
var manualTelemetry = new ConfigurationTelemetry();
71+
var newMutableSettings = MutableSettings.CreateUpdatedMutableSettings(
72+
dynamicConfig,
73+
manualConfig,
74+
initialSettings,
75+
tracerSettings,
76+
manualTelemetry,
77+
new OverrideErrorLog()); // TODO: We'll later report these
78+
79+
var isSameMutableSettings = currentSettings.Equals(newMutableSettings);
80+
81+
// The only exporter setting we currently _allow_ to change is the AgentUri, but if that does change,
82+
// it can mean that _everything_ about the exporter settings changes. To minimize the work to do, and
83+
// to simplify comparisons, we try to read the agent url from the manual setting. If it's missing, not
84+
// set, or unchanged, there's no need to update the exporter settings. In the future, ExporterSettings
85+
// will live separate from TracerSettings entirely.
86+
var exporterTelemetry = new ConfigurationTelemetry();
87+
var newRawExporterSettings = ExporterSettings.Raw.CreateUpdatedFromManualConfig(
88+
tracerSettings.Exporter.RawSettings,
89+
manualConfig,
90+
exporterTelemetry,
91+
isFromDefaults);
92+
var isSameExporterSettings = tracerSettings.Exporter.RawSettings.Equals(newRawExporterSettings);
93+
94+
if (isSameMutableSettings && isSameExporterSettings)
95+
{
96+
Log.Debug("No changes detected in the new configuration in code");
97+
// Even though there were no "real" changes, there may be _effective_ changes in telemetry that
98+
// need to be recorded (e.g. the customer set the value in code but it was already set via
99+
// env vars). We _should_ record exporter settings too, but that introduces a bunch of complexity
100+
// which we'll resolve later anyway, so just have that gap for now (it's very niche).
101+
// If there are changes, they're recorded automatically in ConfigureInternal
102+
manualTelemetry.CopyTo(TelemetryFactory.Config);
103+
return;
104+
}
58105

59-
var settings = new TracerSettings(source, new ConfigurationTelemetry(), new OverrideErrorLog());
106+
Log.Information("Applying new configuration in code");
107+
TracerSettings newSettings;
108+
if (isSameExporterSettings)
109+
{
110+
newSettings = tracerSettings with { MutableSettings = newMutableSettings };
111+
}
112+
else
113+
{
114+
var exporterSettings = new ExporterSettings(newRawExporterSettings, exporterTelemetry);
115+
newSettings = isSameMutableSettings
116+
? tracerSettings with { Exporter = exporterSettings }
117+
: tracerSettings with { MutableSettings = newMutableSettings, Exporter = exporterSettings };
118+
}
60119

61120
// Update the global instance
62-
Trace.Tracer.Configure(settings);
121+
Trace.Tracer.Configure(newSettings);
63122
}
64123
}

‎tracer/src/Datadog.Trace/Configuration/ConfigurationSources/GlobalConfigurationSource.cs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
#nullable enable
77

88
using System;
9+
using System.Collections.Generic;
910
using System.Diagnostics.CodeAnalysis;
1011
using System.IO;
12+
using System.Threading;
1113
using Datadog.Trace.Configuration.ConfigurationSources;
1214
using Datadog.Trace.Configuration.Telemetry;
1315
using Datadog.Trace.LibDatadog.HandsOffConfiguration;
@@ -20,13 +22,20 @@ namespace Datadog.Trace.Configuration;
2022
/// </summary>
2123
internal class GlobalConfigurationSource
2224
{
25+
private static IConfigurationSource _dynamicConfigConfigurationSource = NullConfigurationSource.Instance;
26+
private static ManualInstrumentationConfigurationSourceBase _manualConfigurationSource = new ManualInstrumentationConfigurationSource(new Dictionary<string, object?>(), useDefaultSources: true);
27+
2328
/// <summary>
2429
/// Gets the configuration source instance.
2530
/// </summary>
2631
internal static IConfigurationSource Instance => CreationResult.ConfigurationSource;
2732

2833
internal static GlobalConfigurationSourceResult CreationResult { get; private set; } = CreateDefaultConfigurationSource();
2934

35+
internal static IConfigurationSource DynamicConfigurationSource => _dynamicConfigConfigurationSource;
36+
37+
internal static ManualInstrumentationConfigurationSourceBase ManualConfigurationSource => _manualConfigurationSource;
38+
3039
/// <summary>
3140
/// Creates a <see cref="IConfigurationSource"/> by combining environment variables,
3241
/// Precedence is as follows:
@@ -133,4 +142,14 @@ private static string GetCurrentDirectory()
133142
{
134143
return AppDomain.CurrentDomain.BaseDirectory ?? Directory.GetCurrentDirectory();
135144
}
145+
146+
public static void UpdateDynamicConfigConfigurationSource(IConfigurationSource dynamic)
147+
{
148+
Interlocked.Exchange(ref _dynamicConfigConfigurationSource, dynamic);
149+
}
150+
151+
public static void UpdateManualConfigurationSource(ManualInstrumentationConfigurationSourceBase manual)
152+
{
153+
Interlocked.Exchange(ref _manualConfigurationSource, manual);
154+
}
136155
}

‎tracer/src/Datadog.Trace/Configuration/DynamicConfigurationManager.cs‎

Lines changed: 52 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using System.Threading;
1414
using System.Threading.Tasks;
1515
using Datadog.Trace.Configuration.ConfigurationSources;
16+
using Datadog.Trace.Configuration.ConfigurationSources.Telemetry;
1617
using Datadog.Trace.Configuration.Telemetry;
1718
using Datadog.Trace.Debugger;
1819
using Datadog.Trace.Debugger.Configurations;
@@ -30,14 +31,12 @@ internal class DynamicConfigurationManager : IDynamicConfigurationManager
3031
private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor<DynamicConfigurationManager>();
3132

3233
private readonly IRcmSubscriptionManager _subscriptionManager;
33-
private readonly ConfigurationTelemetry _configurationTelemetry;
3434
private readonly Dictionary<string, RemoteConfiguration> _activeConfigurations = new();
3535
private ISubscription? _subscription;
3636

3737
public DynamicConfigurationManager(IRcmSubscriptionManager subscriptionManager)
3838
{
3939
_subscriptionManager = subscriptionManager;
40-
_configurationTelemetry = new ConfigurationTelemetry();
4140
}
4241

4342
public void Start()
@@ -68,50 +67,67 @@ public void Dispose()
6867
}
6968
}
7069

71-
internal static void OnlyForTests_ApplyConfiguration(ConfigurationBuilder settings)
70+
internal static void OnlyForTests_ApplyConfiguration(IConfigurationSource dynamicConfig)
7271
{
73-
OnConfigurationChanged(settings);
72+
OnConfigurationChanged(dynamicConfig);
7473
}
7574

76-
private static void OnConfigurationChanged(ConfigurationBuilder settings)
75+
private static void OnConfigurationChanged(IConfigurationSource dynamicConfig)
7776
{
78-
var oldSettings = Tracer.Instance.Settings;
79-
80-
var headerTags = MutableSettings.InitializeHeaderTags(settings, ConfigurationKeys.HeaderTags, headerTagsNormalizationFixEnabled: true);
81-
// var serviceNameMappings = TracerSettings.InitializeServiceNameMappings(settings, ConfigurationKeys.ServiceNameMappings);
82-
83-
var globalTags = settings.WithKeys(ConfigurationKeys.GlobalTags).AsDictionary();
84-
85-
var dynamicSettings = new ImmutableDynamicSettings
86-
{
87-
TraceEnabled = settings.WithKeys(ConfigurationKeys.TraceEnabled).AsBool(),
88-
// RuntimeMetricsEnabled = settings.WithKeys(ConfigurationKeys.RuntimeMetricsEnabled).AsBool(),
89-
// DataStreamsMonitoringEnabled = settings.WithKeys(ConfigurationKeys.DataStreamsMonitoring.Enabled).AsBool(),
90-
// Note: Calling GetAsClass<string>() here instead of GetAsString() as we need to get the
91-
// "serialized JToken", which in JsonConfigurationSource is different, as it allows for non-string tokens
92-
SamplingRules = settings.WithKeys(ConfigurationKeys.CustomSamplingRules).GetAsClass<string>(validator: null, converter: s => s),
93-
GlobalSamplingRate = settings.WithKeys(ConfigurationKeys.GlobalSamplingRate).AsDouble(),
94-
// SpanSamplingRules = settings.WithKeys(ConfigurationKeys.SpanSamplingRules).AsString(),
95-
LogsInjectionEnabled = settings.WithKeys(ConfigurationKeys.LogsInjectionEnabled).AsBool(),
96-
HeaderTags = headerTags,
97-
// ServiceNameMappings = serviceNameMappings == null ? null : new ReadOnlyDictionary<string, string>(serviceNameMappings)
98-
GlobalTags = globalTags == null ? null : new ReadOnlyDictionary<string, string>(globalTags)
99-
};
77+
var tracerSettings = Tracer.Instance.Settings;
78+
var manualSource = GlobalConfigurationSource.ManualConfigurationSource;
79+
var mutableSettings = manualSource.UseDefaultSources
80+
? tracerSettings.InitialMutableSettings
81+
: MutableSettings.CreateWithoutDefaultSources(tracerSettings);
82+
83+
// We save this immediately, even if there's no manifest changes in the final settings
84+
GlobalConfigurationSource.UpdateDynamicConfigConfigurationSource(dynamicConfig);
85+
86+
OnConfigurationChanged(
87+
dynamicConfig,
88+
manualSource,
89+
mutableSettings,
90+
tracerSettings,
91+
// TODO: In the future this will 'live' elsewhere
92+
currentSettings: tracerSettings.MutableSettings,
93+
new ConfigurationTelemetry(),
94+
new OverrideErrorLog()); // TODO: We'll later report these
95+
}
10096

101-
// Needs to be done before returning, to feed the value to the telemetry
102-
// var debugLogsEnabled = settings.WithKeys(ConfigurationKeys.DebugEnabled).AsBool();
97+
private static void OnConfigurationChanged(
98+
IConfigurationSource dynamicConfig,
99+
ManualInstrumentationConfigurationSourceBase manualConfig,
100+
MutableSettings initialSettings,
101+
TracerSettings tracerSettings,
102+
MutableSettings currentSettings,
103+
ConfigurationTelemetry telemetry,
104+
OverrideErrorLog errorLog)
105+
{
106+
var newMutableSettings = MutableSettings.CreateUpdatedMutableSettings(
107+
dynamicConfig,
108+
manualConfig,
109+
initialSettings,
110+
tracerSettings,
111+
telemetry,
112+
errorLog);
103113

104114
TracerSettings newSettings;
105-
if (dynamicSettings.Equals(oldSettings.DynamicSettings))
115+
if (currentSettings.Equals(newMutableSettings))
106116
{
107117
Log.Debug("No changes detected in the new dynamic configuration");
108-
newSettings = oldSettings;
118+
// Even though there were no "real" changes, there may be _effective_ changes in telemetry that
119+
// need to be recorded (e.g. the customer set the value in code but it was already set via
120+
// env vars). We _should_ record exporter settings too, but that introduces a bunch of complexity
121+
// which we'll resolve later anyway, so just have that gap for now (it's very niche).
122+
// If there are changes, they're recorded automatically in Tracer.Configure()
123+
telemetry.CopyTo(TelemetryFactory.Config);
124+
newSettings = tracerSettings;
109125
}
110126
else
111127
{
112128
Log.Information("Applying new dynamic configuration");
113129

114-
newSettings = oldSettings with { DynamicSettings = dynamicSettings };
130+
newSettings = tracerSettings with { MutableSettings = newMutableSettings };
115131

116132
/*
117133
if (debugLogsEnabled != null && debugLogsEnabled.Value != GlobalSettings.Instance.DebugEnabled)
@@ -126,6 +142,9 @@ private static void OnConfigurationChanged(ConfigurationBuilder settings)
126142
Tracer.Configure(newSettings);
127143
}
128144

145+
// TODO: This might not record the config in the correct order in future, but would require
146+
// a big refactoring of debugger settings to resolve
147+
var settings = new ConfigurationBuilder(dynamicConfig, TelemetryFactory.Config);
129148
var dynamicDebuggerSettings = new ImmutableDynamicDebuggerSettings
130149
{
131150
DynamicInstrumentationEnabled = settings.WithKeys(ConfigurationKeys.Debugger.DynamicInstrumentationEnabled).AsBool(),
@@ -238,12 +257,8 @@ private void ApplyMergedConfiguration(List<RemoteConfiguration> remoteConfigurat
238257
environment);
239258

240259
var configurationSource = new DynamicConfigConfigurationSource(mergedConfigJToken, ConfigurationOrigins.RemoteConfig);
241-
var configurationBuilder = new ConfigurationBuilder(configurationSource, _configurationTelemetry);
242-
243-
OnConfigurationChanged(configurationBuilder);
244260

245-
_configurationTelemetry.CopyTo(TelemetryFactory.Config);
246-
_configurationTelemetry.Clear();
261+
OnConfigurationChanged(configurationSource);
247262
}
248263
}
249264
}

0 commit comments

Comments
 (0)