Skip to content

Add fix for not re-reporting telemetry when there are "empty" dynamic config changes #7796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 26, 2025
+222 −83
Merged

Add fix for not re-reporting telemetry when there are "empty" dynamic config changes #7796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions tracer/src/Datadog.Trace/Configuration/MutableSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1074,10 +1074,10 @@ public static MutableSettings CreateInitialMutableSettings(
/// by excluding all the default sources. Effectively gives all the settings their default
/// values. Should only be used with the manual instrumentation source
/// </summary>
public static MutableSettings CreateWithoutDefaultSources(TracerSettings tracerSettings)
public static MutableSettings CreateWithoutDefaultSources(TracerSettings tracerSettings, ConfigurationTelemetry telemetry)
=> CreateInitialMutableSettings(
NullConfigurationSource.Instance,
new ConfigurationTelemetry(),
telemetry,
new OverrideErrorLog(),
tracerSettings);

Expand Down
85 changes: 72 additions & 13 deletions tracer/src/Datadog.Trace/Configuration/SettingsManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,41 +7,56 @@

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Threading;
using Datadog.Trace.Configuration.ConfigurationSources;
using Datadog.Trace.Configuration.ConfigurationSources.Telemetry;
using Datadog.Trace.Configuration.Telemetry;
using Datadog.Trace.Logging;

namespace Datadog.Trace.Configuration;

public partial record TracerSettings
{
internal class SettingsManager(
TracerSettings tracerSettings,
MutableSettings initialMutable,
ExporterSettings initialExporter)
internal class SettingsManager
{
private readonly TracerSettings _tracerSettings = tracerSettings;
private readonly TracerSettings _tracerSettings;
private readonly ConfigurationTelemetry _initialTelemetry;
private readonly List<SettingChangeSubscription> _subscribers = [];

private IConfigurationSource _dynamicConfigurationSource = NullConfigurationSource.Instance;
private ManualInstrumentationConfigurationSourceBase _manualConfigurationSource =
new ManualInstrumentationConfigurationSource(new Dictionary<string, object?>(), useDefaultSources: true);

// We delay creating these, as we likely won't need them
private ConfigurationTelemetry? _noDefaultSettingsTelemetry;
private MutableSettings? _noDefaultSourcesSettings;

private SettingChanges? _latest;

public SettingsManager(IConfigurationSource source, TracerSettings tracerSettings, IConfigurationTelemetry telemetry, OverrideErrorLog errorLog)
{
// We record the telemetry for the initial settings in a dedicated ConfigurationTelemetry,
// because we need to be able to reapply this configuration on dynamic config updates
// We don't re-record error logs, so we just use the built-in for that
var initialTelemetry = new ConfigurationTelemetry();
InitialMutableSettings = MutableSettings.CreateInitialMutableSettings(source, initialTelemetry, errorLog, tracerSettings);
InitialExporterSettings = new ExporterSettings(source, initialTelemetry);
_tracerSettings = tracerSettings;
_initialTelemetry = initialTelemetry;
initialTelemetry.CopyTo(telemetry);
}

/// <summary>
/// Gets the initial <see cref="MutableSettings"/>. On app startup, these will be the values read from
/// static sources. To subscribe to updates to these settings, from code or remote config, call <see cref="SubscribeToChanges"/>.
/// </summary>
public MutableSettings InitialMutableSettings { get; } = initialMutable;
public MutableSettings InitialMutableSettings { get; }

/// <summary>
/// Gets the initial <see cref="ExporterSettings"/>. On app startup, these will be the values read from
/// static sources. To subscribe to updates to these settings, from code or remote config, call <see cref="SubscribeToChanges"/>.
/// </summary>
public ExporterSettings InitialExporterSettings { get; } = initialExporter;
public ExporterSettings InitialExporterSettings { get; }

/// <summary>
/// Subscribe to changes in <see cref="MutableSettings"/> and/or <see cref="ExporterSettings"/>.
Expand Down Expand Up @@ -133,21 +148,45 @@ private bool UpdateSettings(
ManualInstrumentationConfigurationSourceBase manualSource,
IConfigurationTelemetry telemetry)
{
var initialSettings = manualSource.UseDefaultSources
? InitialMutableSettings
: MutableSettings.CreateWithoutDefaultSources(_tracerSettings);
// Set the correct default telemetry and initial settings depending
// on whether the manual config source explicitly disables using the default sources
ConfigurationTelemetry defaultTelemetry;
MutableSettings initialSettings;
if (manualSource.UseDefaultSources)
{
defaultTelemetry = _initialTelemetry;
initialSettings = InitialMutableSettings;
}
else
{
// We only need to initialize the "no default sources" settings once
// and we don't want to initialize them if we don't _need_ to
// so lazy-initialize here
if (_noDefaultSourcesSettings is null || _noDefaultSettingsTelemetry is null)
{
InitialiseNoDefaultSourceSettings();
}

defaultTelemetry = _noDefaultSettingsTelemetry;
initialSettings = _noDefaultSourcesSettings;
}

var current = _latest;
var currentMutable = current?.UpdatedMutable ?? current?.PreviousMutable ?? InitialMutableSettings;
var currentExporter = current?.UpdatedExporter ?? current?.PreviousExporter ?? InitialExporterSettings;

// we create a temporary ConfigurationTelemetry object to hold the changes to settings
// if nothing is actually written, and nothing changes compared to the default, then we
// don't need to report it to the provided telemetry
var tempTelemetry = new ConfigurationTelemetry();

var overrideErrorLog = new OverrideErrorLog();
var newMutableSettings = MutableSettings.CreateUpdatedMutableSettings(
dynamicConfigSource,
manualSource,
initialSettings,
_tracerSettings,
telemetry,
tempTelemetry,
overrideErrorLog); // TODO: We'll later report these

// The only exporter setting we currently _allow_ to change is the AgentUri, but if that does change,
Expand All @@ -159,7 +198,7 @@ private bool UpdateSettings(
var newRawExporterSettings = ExporterSettings.Raw.CreateUpdatedFromManualConfig(
currentExporter.RawSettings,
manualSource,
telemetry,
tempTelemetry,
manualSource.UseDefaultSources);

var isSameMutableSettings = currentMutable.Equals(newMutableSettings);
Expand All @@ -171,13 +210,33 @@ private bool UpdateSettings(
return null;
}

// we have changes, so we need to report them
// First record the "default"/fallback values, then record the "new" values
defaultTelemetry.CopyTo(telemetry);
tempTelemetry.CopyTo(telemetry);

Log.Information("Notifying consumers of new settings");
var updatedMutableSettings = isSameMutableSettings ? null : newMutableSettings;
var updatedExporterSettings = isSameExporterSettings ? null : new ExporterSettings(newRawExporterSettings, telemetry);

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

[MemberNotNull(nameof(_noDefaultSettingsTelemetry))]
[MemberNotNull(nameof(_noDefaultSourcesSettings))]
private void InitialiseNoDefaultSourceSettings()
{
if (_noDefaultSourcesSettings is not null
&& _noDefaultSettingsTelemetry is not null)
{
return;
}

var telemetry = new ConfigurationTelemetry();
_noDefaultSettingsTelemetry = telemetry;
_noDefaultSourcesSettings = MutableSettings.CreateWithoutDefaultSources(_tracerSettings, telemetry);
}

private void NotifySubscribers(SettingChanges settings)
{
_latest = settings;
Expand Down
129 changes: 64 additions & 65 deletions tracer/src/Datadog.Trace/Configuration/TracerSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ internal TracerSettings(IConfigurationSource? source, IConfigurationTelemetry te
.AsBoolResult()
.OverrideWith(in otelActivityListenerEnabled, ErrorLog, defaultValue: false);

var exporter = new ExporterSettings(source, _telemetry);

PeerServiceTagsEnabled = config
.WithKeys(ConfigurationKeys.PeerServiceDefaultsEnabled)
.AsBool(defaultValue: false);
Expand Down Expand Up @@ -335,66 +333,6 @@ not null when string.Equals(value, "otlp", StringComparison.OrdinalIgnoreCase) =

OpenTelemetryLogsEnabled = OpenTelemetryLogsEnabled && OtelLogsExporterEnabled;

DataPipelineEnabled = config
Copy link
Member Author

@andrewlock andrewlock Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has all just moved to the end of the constructor, cut and paste

.WithKeys(ConfigurationKeys.TraceDataPipelineEnabled)
.AsBool(defaultValue: EnvironmentHelpers.IsUsingAzureAppServicesSiteExtension() && !EnvironmentHelpers.IsAzureFunctions());

if (DataPipelineEnabled)
{
// Due to missing quantization and obfuscation in native side, we can't enable the native trace exporter
// as it may lead to different stats results than the managed one.
if (StatsComputationEnabled)
{
DataPipelineEnabled = false;
Log.Warning(
$"{ConfigurationKeys.TraceDataPipelineEnabled} is enabled, but {ConfigurationKeys.StatsComputationEnabled} is enabled. Disabling data pipeline.");
_telemetry.Record(ConfigurationKeys.TraceDataPipelineEnabled, false, ConfigurationOrigins.Calculated);
}

// Windows supports UnixDomainSocket https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
// but tokio hasn't added support for it yet https://github.com/tokio-rs/tokio/issues/2201
// There's an issue here, in that technically a user can initially be configured to send over TCP/named pipes,
// and so we allow and enable the datapipeline. Later, they could configure the app in code to send over UDS.
// This is a problem, as we currently don't support toggling the data pipeline at runtime, so we explicitly block
// this scenario in the public API.
if (exporter.TracesTransport == TracesTransportType.UnixDomainSocket && FrameworkDescription.Instance.IsWindows())
{
DataPipelineEnabled = false;
Log.Warning(
$"{ConfigurationKeys.TraceDataPipelineEnabled} is enabled, but TracesTransport is set to UnixDomainSocket which is not supported on Windows. Disabling data pipeline.");
_telemetry.Record(ConfigurationKeys.TraceDataPipelineEnabled, false, ConfigurationOrigins.Calculated);
}

if (!isLibDatadogAvailable.IsAvailable)
{
DataPipelineEnabled = false;
if (isLibDatadogAvailable.Exception is not null)
{
Log.Warning(
isLibDatadogAvailable.Exception,
$"{ConfigurationKeys.TraceDataPipelineEnabled} is enabled, but libdatadog is not available. Disabling data pipeline.");
}
else
{
Log.Warning(
$"{ConfigurationKeys.TraceDataPipelineEnabled} is enabled, but libdatadog is not available. Disabling data pipeline.");
}

_telemetry.Record(ConfigurationKeys.TraceDataPipelineEnabled, false, ConfigurationOrigins.Calculated);
}

// SSI already utilizes libdatadog. To prevent unexpected behavior,
// we proactively disable the data pipeline when SSI is enabled. Theoretically, this should not cause any issues,
// but as a precaution, we are taking a conservative approach during the initial rollout phase.
if (!string.IsNullOrEmpty(EnvironmentHelpers.GetEnvironmentVariable("DD_INJECTION_ENABLED")))
{
DataPipelineEnabled = false;
Log.Warning(
$"{ConfigurationKeys.TraceDataPipelineEnabled} is enabled, but SSI is enabled. Disabling data pipeline.");
_telemetry.Record(ConfigurationKeys.TraceDataPipelineEnabled, false, ConfigurationOrigins.Calculated);
}
}

// We should also be writing telemetry for OTEL_LOGS_EXPORTER similar to OTEL_METRICS_EXPORTER, but we don't have a corresponding Datadog config
// When we do, we can insert that here
CustomSamplingRulesFormat = config.WithKeys(ConfigurationKeys.CustomSamplingRulesFormat)
Expand Down Expand Up @@ -731,9 +669,70 @@ not null when string.Equals(value, "otlp", StringComparison.OrdinalIgnoreCase) =
// We create a lazy here because this is kind of expensive, and we want to avoid calling it if we can
_fallbackApplicationName = new(() => ApplicationNameHelpers.GetFallbackApplicationName(this));

// Move the creation of these settings inside SettingsManager?
var initialMutableSettings = MutableSettings.CreateInitialMutableSettings(source, telemetry, errorLog, this);
Manager = new(this, initialMutableSettings, exporter);
// There's a circular dependency here because DataPipeline depends on ExporterSettings,
// but the settings manager depends on TracerSettings. Basically this is all fine as long
// as nothing in the MutableSettings or ExporterSettings depends on the value of DataPipelineEnabled!
Manager = new(source, this, telemetry, errorLog);

DataPipelineEnabled = config
.WithKeys(ConfigurationKeys.TraceDataPipelineEnabled)
.AsBool(defaultValue: EnvironmentHelpers.IsUsingAzureAppServicesSiteExtension() && !EnvironmentHelpers.IsAzureFunctions());

if (DataPipelineEnabled)
{
// Due to missing quantization and obfuscation in native side, we can't enable the native trace exporter
// as it may lead to different stats results than the managed one.
if (StatsComputationEnabled)
{
DataPipelineEnabled = false;
Log.Warning(
$"{ConfigurationKeys.TraceDataPipelineEnabled} is enabled, but {ConfigurationKeys.StatsComputationEnabled} is enabled. Disabling data pipeline.");
_telemetry.Record(ConfigurationKeys.TraceDataPipelineEnabled, false, ConfigurationOrigins.Calculated);
}

// Windows supports UnixDomainSocket https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
// but tokio hasn't added support for it yet https://github.com/tokio-rs/tokio/issues/2201
// There's an issue here, in that technically a user can initially be configured to send over TCP/named pipes,
// and so we allow and enable the datapipeline. Later, they could configure the app in code to send over UDS.
// This is a problem, as we currently don't support toggling the data pipeline at runtime, so we explicitly block
// this scenario in the public API.
if (Manager.InitialExporterSettings.TracesTransport == TracesTransportType.UnixDomainSocket && FrameworkDescription.Instance.IsWindows())
Copy link
Member Author

@andrewlock andrewlock Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only change, we read the exporter settings that the Manager created, instead of creating the new ExporterSettings() and passing them to the Manager. It's the same thing overall, but it means the Manager can record the initial telemetry for the exporter settings (so it can be replayed again later)

{
DataPipelineEnabled = false;
Log.Warning(
$"{ConfigurationKeys.TraceDataPipelineEnabled} is enabled, but TracesTransport is set to UnixDomainSocket which is not supported on Windows. Disabling data pipeline.");
_telemetry.Record(ConfigurationKeys.TraceDataPipelineEnabled, false, ConfigurationOrigins.Calculated);
}

if (!isLibDatadogAvailable.IsAvailable)
{
DataPipelineEnabled = false;
if (isLibDatadogAvailable.Exception is not null)
{
Log.Warning(
isLibDatadogAvailable.Exception,
$"{ConfigurationKeys.TraceDataPipelineEnabled} is enabled, but libdatadog is not available. Disabling data pipeline.");
}
else
{
Log.Warning(
$"{ConfigurationKeys.TraceDataPipelineEnabled} is enabled, but libdatadog is not available. Disabling data pipeline.");
}

_telemetry.Record(ConfigurationKeys.TraceDataPipelineEnabled, false, ConfigurationOrigins.Calculated);
}

// SSI already utilizes libdatadog. To prevent unexpected behavior,
// we proactively disable the data pipeline when SSI is enabled. Theoretically, this should not cause any issues,
// but as a precaution, we are taking a conservative approach during the initial rollout phase.
if (!string.IsNullOrEmpty(EnvironmentHelpers.GetEnvironmentVariable("DD_INJECTION_ENABLED")))
{
DataPipelineEnabled = false;
Log.Warning(
$"{ConfigurationKeys.TraceDataPipelineEnabled} is enabled, but SSI is enabled. Disabling data pipeline.");
_telemetry.Record(ConfigurationKeys.TraceDataPipelineEnabled, false, ConfigurationOrigins.Calculated);
}
}
}

internal bool IsRunningInCiVisibility { get; }
Expand Down
2 changes: 2 additions & 0 deletions tracer/src/Datadog.Trace/RemoteConfigurationManagement/Transport/RemoteConfigurationApi.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#nullable enable

using System;
using System.IO;
Copy link
Collaborator

@NachoEchevarria NachoEchevarria Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Probably not needed

using System.Text;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -15,6 +16,7 @@
using Datadog.Trace.Logging;
using Datadog.Trace.PlatformHelpers;
using Datadog.Trace.RemoteConfigurationManagement.Protocol;
using Datadog.Trace.Util.Streams;
using Datadog.Trace.Vendors.Newtonsoft.Json;

namespace Datadog.Trace.RemoteConfigurationManagement.Transport
Expand Down
3 changes: 2 additions & 1 deletion tracer/test/Datadog.Trace.Security.Unit.Tests/EmptyDatadogTracer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System;
using Datadog.Trace.Configuration;
using Datadog.Trace.Configuration.Schema;
using Datadog.Trace.Configuration.Telemetry;
using Datadog.Trace.Sampling;
using Moq;

Expand All @@ -23,7 +24,7 @@ public EmptyDatadogTracer()
DefaultServiceName = "My Service Name";
Settings = new TracerSettings(NullConfigurationSource.Instance);
var namingSchema = new NamingSchema(SchemaVersion.V0, false, false, DefaultServiceName, null, null);
PerTraceSettings = new PerTraceSettings(null, null, namingSchema, MutableSettings.CreateWithoutDefaultSources(Settings));
PerTraceSettings = new PerTraceSettings(null, null, namingSchema, MutableSettings.CreateWithoutDefaultSources(Settings, new ConfigurationTelemetry()));
}

public string DefaultServiceName { get; }
Expand Down
Loading
Loading