Skip to content

Commit 23df0cb

Browse files
andrewlockandrewlock
committed
Fix telemetry recording and log configuration when settings change
1 parent d6e2e52 commit 23df0cb

File tree

4 files changed

+40
-40
lines changed

4 files changed

+40
-40
lines changed

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,7 @@ private MutableSettings(
6161
ReadOnlyDictionary<string, string> serviceNameMappings,
6262
string? gitRepositoryUrl,
6363
string? gitCommitSha,
64-
OverrideErrorLog errorLog,
65-
IConfigurationTelemetry telemetry)
64+
OverrideErrorLog errorLog)
6665
{
6766
IsInitialSettings = isInitialSettings;
6867
TraceEnabled = traceEnabled;
@@ -92,7 +91,6 @@ private MutableSettings(
9291
GitRepositoryUrl = gitRepositoryUrl;
9392
GitCommitSha = gitCommitSha;
9493
ErrorLog = errorLog;
95-
Telemetry = telemetry;
9694
}
9795

9896
// Settings that can be set via remote config
@@ -263,8 +261,6 @@ private MutableSettings(
263261

264262
internal OverrideErrorLog ErrorLog { get; }
265263

266-
internal IConfigurationTelemetry Telemetry { get; }
267-
268264
internal static ReadOnlyDictionary<string, string>? InitializeHeaderTags(ConfigurationBuilder config, string key, bool headerTagsNormalizationFixEnabled)
269265
=> InitializeHeaderTags(
270266
config.WithKeys(key).AsDictionaryResult(allowOptionalMappings: true),
@@ -737,8 +733,7 @@ public static MutableSettings CreateUpdatedMutableSettings(
737733
serviceNameMappings: serviceNameMappings,
738734
gitRepositoryUrl: gitRepositoryUrl,
739735
gitCommitSha: gitCommitSha,
740-
errorLog: errorLog,
741-
telemetry: telemetry);
736+
errorLog: errorLog);
742737

743738
static ReadOnlyDictionary<string, string> GetHeaderTagsResult(
744739
ConfigurationBuilder.ClassConfigurationResultWithKey<IDictionary<string, string>> result,
@@ -1071,8 +1066,7 @@ public static MutableSettings CreateInitialMutableSettings(
10711066
serviceNameMappings: serviceNameMappings,
10721067
gitRepositoryUrl: gitRepositoryUrl,
10731068
gitCommitSha: gitCommitSha,
1074-
errorLog: errorLog,
1075-
telemetry: telemetry);
1069+
errorLog: errorLog);
10761070
}
10771071

10781072
/// <summary>

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

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ private bool UpdateSettings(
131131
internal SettingChanges? BuildNewSettings(
132132
IConfigurationSource dynamicConfigSource,
133133
ManualInstrumentationConfigurationSourceBase manualSource,
134-
IConfigurationTelemetry centralTelemetry)
134+
IConfigurationTelemetry telemetry)
135135
{
136136
var initialSettings = manualSource.UseDefaultSources
137137
? InitialMutableSettings
@@ -141,26 +141,25 @@ private bool UpdateSettings(
141141
var currentMutable = current?.UpdatedMutable ?? current?.PreviousMutable ?? InitialMutableSettings;
142142
var currentExporter = current?.UpdatedExporter ?? current?.PreviousExporter ?? InitialExporterSettings;
143143

144-
var telemetry = new ConfigurationTelemetry();
144+
var overrideErrorLog = new OverrideErrorLog();
145145
var newMutableSettings = MutableSettings.CreateUpdatedMutableSettings(
146146
dynamicConfigSource,
147147
manualSource,
148148
initialSettings,
149149
_tracerSettings,
150150
telemetry,
151-
new OverrideErrorLog()); // TODO: We'll later report these
151+
overrideErrorLog); // TODO: We'll later report these
152152

153153
// The only exporter setting we currently _allow_ to change is the AgentUri, but if that does change,
154154
// it can mean that _everything_ about the exporter settings changes. To minimize the work to do, and
155155
// to simplify comparisons, we try to read the agent url from the manual setting. If it's missing, not
156156
// set, or unchanged, there's no need to update the exporter settings.
157157
// We only technically need to do this today if _manual_ config changes, not if remote config changes,
158158
// but for simplicity we don't distinguish currently.
159-
var exporterTelemetry = new ConfigurationTelemetry();
160159
var newRawExporterSettings = ExporterSettings.Raw.CreateUpdatedFromManualConfig(
161160
currentExporter.RawSettings,
162161
manualSource,
163-
exporterTelemetry,
162+
telemetry,
164163
manualSource.UseDefaultSources);
165164

166165
var isSameMutableSettings = currentMutable.Equals(newMutableSettings);
@@ -169,18 +168,12 @@ private bool UpdateSettings(
169168
if (isSameMutableSettings && isSameExporterSettings)
170169
{
171170
Log.Debug("No changes detected in the new configuration");
172-
// Even though there were no "real" changes, there may be _effective_ changes in telemetry that
173-
// need to be recorded (e.g. the customer set the value in code, but it was already set via
174-
// env vars). We _should_ record exporter settings too, but that introduces a bunch of complexity
175-
// which we'll resolve later anyway, so just have that gap for now (it's very niche).
176-
// If there are changes, they're recorded automatically in ConfigureInternal
177-
telemetry.CopyTo(centralTelemetry);
178171
return null;
179172
}
180173

181174
Log.Information("Notifying consumers of new settings");
182175
var updatedMutableSettings = isSameMutableSettings ? null : newMutableSettings;
183-
var updatedExporterSettings = isSameExporterSettings ? null : new ExporterSettings(newRawExporterSettings, exporterTelemetry);
176+
var updatedExporterSettings = isSameExporterSettings ? null : new ExporterSettings(newRawExporterSettings, telemetry);
184177

185178
return new SettingChanges(updatedMutableSettings, updatedExporterSettings, currentMutable, currentExporter);
186179
}

‎tracer/src/Datadog.Trace/TracerManager.cs‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
using Datadog.Trace.RuntimeMetrics;
3030
using Datadog.Trace.Sampling;
3131
using Datadog.Trace.Telemetry;
32+
using Datadog.Trace.Util;
3233
using Datadog.Trace.Util.Http;
3334
using Datadog.Trace.Vendors.Newtonsoft.Json;
3435
using Datadog.Trace.Vendors.StatsdClient;
@@ -312,7 +313,7 @@ private static async Task CleanUpOldTracerManager(TracerManager oldManager, Trac
312313
}
313314
}
314315

315-
private static async Task WriteDiagnosticLog(TracerManager instance)
316+
private static async Task WriteDiagnosticLog(TracerManager instance, MutableSettings mutableSettings, ExporterSettings exporterSettings)
316317
{
317318
try
318319
{
@@ -323,9 +324,6 @@ private static async Task WriteDiagnosticLog(TracerManager instance)
323324

324325
string agentError = null;
325326
var instanceSettings = instance.Settings;
326-
var mutableSettings = instance.PerTraceSettings.Settings;
327-
// TODO: this only writes the initial settings - we should make sure to record an "update" log on reconfiguration
328-
var exporterSettings = instanceSettings.Manager.InitialExporterSettings;
329327

330328
// In AAS, the trace agent is deployed alongside the tracer and managed by the tracer
331329
// Disable this check as it may hit the trace agent before it is ready to receive requests and give false negatives
@@ -610,7 +608,8 @@ void WriteDictionary(IReadOnlyDictionary<string, string> dictionary)
610608

611609
Log.Information("DATADOG TRACER CONFIGURATION - {Configuration}", stringWriter.ToString());
612610
OverrideErrorLog.Instance.ProcessAndClearActions(Log, TelemetryFactory.Metrics); // global errors, only logged once
613-
instanceSettings.ErrorLog.ProcessAndClearActions(Log, TelemetryFactory.Metrics); // global errors, only logged once
611+
instanceSettings.ErrorLog.ProcessAndClearActions(Log, TelemetryFactory.Metrics);
612+
mutableSettings.ErrorLog.ProcessAndClearActions(Log, TelemetryFactory.Metrics);
614613
}
615614
catch (Exception ex)
616615
{
@@ -683,11 +682,20 @@ private static TracerManager CreateInitializedTracer(TracerSettings settings, Tr
683682
OneTimeSetup(newManager.Settings);
684683
}
685684

686-
if (newManager.PerTraceSettings.Settings.StartupDiagnosticLogEnabled)
685+
if (newManager.Settings.Manager is { InitialMutableSettings: { StartupDiagnosticLogEnabled: true } mutable, InitialExporterSettings: { } exporter })
687686
{
688-
_ = Task.Run(() => WriteDiagnosticLog(newManager));
687+
_ = Task.Run(() => WriteDiagnosticLog(newManager, mutable, exporter));
689688
}
690689

690+
newManager.Settings.Manager.SubscribeToChanges(changes =>
691+
{
692+
var mutable = changes.UpdatedMutable ?? changes.PreviousMutable;
693+
if (mutable.StartupDiagnosticLogEnabled)
694+
{
695+
_ = Task.Run(() => WriteDiagnosticLog(newManager, mutable, changes.UpdatedExporter ?? changes.PreviousExporter));
696+
}
697+
});
698+
691699
return newManager;
692700
}
693701

‎tracer/src/Datadog.Trace/TracerManagerFactory.cs‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,21 +62,26 @@ internal TracerManager CreateTracerManager(TracerSettings settings, TracerManage
6262
tracerFlareManager: null,
6363
spanEventsManager: null);
6464

65-
try
65+
tracer.Settings.Manager.SubscribeToChanges(changes =>
6666
{
67-
if (Profiler.Instance.Status.IsProfilerReady)
67+
if (changes.UpdatedMutable is { } mutableSettings)
6868
{
69-
var mutableSettings = tracer.PerTraceSettings.Settings;
70-
NativeInterop.SetApplicationInfoForAppDomain(RuntimeId.Get(), mutableSettings.DefaultServiceName, mutableSettings.Environment, mutableSettings.ServiceVersion);
69+
try
70+
{
71+
if (Profiler.Instance.Status.IsProfilerReady)
72+
{
73+
NativeInterop.SetApplicationInfoForAppDomain(RuntimeId.Get(), mutableSettings.DefaultServiceName, mutableSettings.Environment, mutableSettings.ServiceVersion);
74+
}
75+
}
76+
catch (Exception ex)
77+
{
78+
// We failed to retrieve the runtime from native this can be because:
79+
// - P/Invoke issue (unknown dll, unknown entrypoint...)
80+
// - We are running in a partial trust environment
81+
Log.Warning(ex, "Failed to set the service name for native.");
82+
}
7183
}
72-
}
73-
catch (Exception ex)
74-
{
75-
// We failed to retrieve the runtime from native this can be because:
76-
// - P/Invoke issue (unknown dll, unknown entrypoint...)
77-
// - We are running in a partial trust environment
78-
Log.Warning(ex, "Failed to set the service name for native.");
79-
}
84+
});
8085

8186
return tracer;
8287
}

0 commit comments

Comments
 (0)