Skip to content

Commit 4b5118c

Browse files
andrewlockandrewlock
committed
Fix telemetry recording and log configuration when settings change
1 parent d0084f8 commit 4b5118c

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
@@ -30,6 +30,7 @@
3030
using Datadog.Trace.Sampling;
3131
using Datadog.Trace.SourceGenerators;
3232
using Datadog.Trace.Telemetry;
33+
using Datadog.Trace.Util;
3334
using Datadog.Trace.Util.Http;
3435
using Datadog.Trace.Vendors.Newtonsoft.Json;
3536
using Datadog.Trace.Vendors.StatsdClient;
@@ -314,7 +315,7 @@ private static async Task CleanUpOldTracerManager(TracerManager oldManager, Trac
314315
}
315316
}
316317

317-
private static async Task WriteDiagnosticLog(TracerManager instance)
318+
private static async Task WriteDiagnosticLog(TracerManager instance, MutableSettings mutableSettings, ExporterSettings exporterSettings)
318319
{
319320
try
320321
{
@@ -325,9 +326,6 @@ private static async Task WriteDiagnosticLog(TracerManager instance)
325326

326327
string agentError = null;
327328
var instanceSettings = instance.Settings;
328-
var mutableSettings = instance.PerTraceSettings.Settings;
329-
// TODO: this only writes the initial settings - we should make sure to record an "update" log on reconfiguration
330-
var exporterSettings = instanceSettings.Manager.InitialExporterSettings;
331329

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

613611
Log.Information("DATADOG TRACER CONFIGURATION - {Configuration}", stringWriter.ToString());
614612
OverrideErrorLog.Instance.ProcessAndClearActions(Log, TelemetryFactory.Metrics); // global errors, only logged once
615-
instanceSettings.ErrorLog.ProcessAndClearActions(Log, TelemetryFactory.Metrics); // global errors, only logged once
613+
instanceSettings.ErrorLog.ProcessAndClearActions(Log, TelemetryFactory.Metrics);
614+
mutableSettings.ErrorLog.ProcessAndClearActions(Log, TelemetryFactory.Metrics);
616615
}
617616
catch (Exception ex)
618617
{
@@ -685,11 +684,20 @@ private static TracerManager CreateInitializedTracer(TracerSettings settings, Tr
685684
OneTimeSetup(newManager.Settings);
686685
}
687686

688-
if (newManager.PerTraceSettings.Settings.StartupDiagnosticLogEnabled)
687+
if (newManager.Settings.Manager is { InitialMutableSettings: { StartupDiagnosticLogEnabled: true } mutable, InitialExporterSettings: { } exporter })
689688
{
690-
_ = Task.Run(() => WriteDiagnosticLog(newManager));
689+
_ = Task.Run(() => WriteDiagnosticLog(newManager, mutable, exporter));
691690
}
692691

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

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

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

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

8287
return tracer;
8388
}

0 commit comments

Comments
 (0)