Skip to content

Commit 1b1f494

Browse files
andrewlockandrewlock
committed
Fix telemetry recording and log configuration when settings change
1 parent 830246c commit 1b1f494

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
@@ -97,7 +97,7 @@ public bool UpdateSettings(
9797
internal SettingChanges? BuildNewSettings(
9898
IConfigurationSource dynamicConfigSource,
9999
ManualInstrumentationConfigurationSourceBase manualSource,
100-
IConfigurationTelemetry centralTelemetry)
100+
IConfigurationTelemetry telemetry)
101101
{
102102
var initialSettings = manualSource.UseDefaultSources
103103
? InitialMutableSettings
@@ -107,26 +107,25 @@ public bool UpdateSettings(
107107
var currentMutable = current?.UpdatedMutable ?? current?.PreviousMutable ?? InitialMutableSettings;
108108
var currentExporter = current?.UpdatedExporter ?? current?.PreviousExporter ?? InitialExporterSettings;
109109

110-
var telemetry = new ConfigurationTelemetry();
110+
var overrideErrorLog = new OverrideErrorLog();
111111
var newMutableSettings = MutableSettings.CreateUpdatedMutableSettings(
112112
dynamicConfigSource,
113113
manualSource,
114114
initialSettings,
115115
_tracerSettings,
116116
telemetry,
117-
new OverrideErrorLog()); // TODO: We'll later report these
117+
overrideErrorLog); // TODO: We'll later report these
118118

119119
// The only exporter setting we currently _allow_ to change is the AgentUri, but if that does change,
120120
// it can mean that _everything_ about the exporter settings changes. To minimize the work to do, and
121121
// to simplify comparisons, we try to read the agent url from the manual setting. If it's missing, not
122122
// set, or unchanged, there's no need to update the exporter settings.
123123
// We only technically need to do this today if _manual_ config changes, not if remote config changes,
124124
// but for simplicity we don't distinguish currently.
125-
var exporterTelemetry = new ConfigurationTelemetry();
126125
var newRawExporterSettings = ExporterSettings.Raw.CreateUpdatedFromManualConfig(
127126
currentExporter.RawSettings,
128127
manualSource,
129-
exporterTelemetry,
128+
telemetry,
130129
manualSource.UseDefaultSources);
131130

132131
var isSameMutableSettings = currentMutable.Equals(newMutableSettings);
@@ -135,18 +134,12 @@ public bool UpdateSettings(
135134
if (isSameMutableSettings && isSameExporterSettings)
136135
{
137136
Log.Debug("No changes detected in the new configuration");
138-
// Even though there were no "real" changes, there may be _effective_ changes in telemetry that
139-
// need to be recorded (e.g. the customer set the value in code, but it was already set via
140-
// env vars). We _should_ record exporter settings too, but that introduces a bunch of complexity
141-
// which we'll resolve later anyway, so just have that gap for now (it's very niche).
142-
// If there are changes, they're recorded automatically in ConfigureInternal
143-
telemetry.CopyTo(centralTelemetry);
144137
return null;
145138
}
146139

147140
Log.Information("Notifying consumers of new settings");
148141
var updatedMutableSettings = isSameMutableSettings ? null : newMutableSettings;
149-
var updatedExporterSettings = isSameExporterSettings ? null : new ExporterSettings(newRawExporterSettings, exporterTelemetry);
142+
var updatedExporterSettings = isSameExporterSettings ? null : new ExporterSettings(newRawExporterSettings, telemetry);
150143

151144
return new SettingChanges(updatedMutableSettings, updatedExporterSettings, currentMutable, currentExporter);
152145
}

‎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)