Skip to content

Fix OTel telemetry perf regression in CLI restore #54063

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

Closed
marcpopMSFT wants to merge 1 commit into from
+51 −42
Closed

Fix OTel telemetry perf regression in CLI restore #54063

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
91 changes: 51 additions & 40 deletions src/Cli/dotnet/Telemetry/TelemetryClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using Azure.Monitor.OpenTelemetry.Exporter;
using OpenTelemetry;
using OpenTelemetry.Context.Propagation;
using OpenTelemetry.Metrics;
using OpenTelemetry.Resources;
using OpenTelemetry.Trace;
#endif
Expand All @@ -23,9 +22,7 @@ public class TelemetryClient : ITelemetryClient
private Task? _trackEventTask;

#if TARGET_WINDOWS
private static readonly MeterProviderBuilder s_metricsProviderBuilder;
private static MeterProvider? s_metricsProvider;
private static readonly TracerProviderBuilder s_tracerProviderBuilder;
private static readonly object s_providerLock = new();
private static TracerProvider? s_tracerProvider;
private static readonly List<Activity> s_activities = [];

Expand Down Expand Up @@ -59,34 +56,10 @@ public static bool DisabledForTests

static TelemetryClient()
{
#if TARGET_WINDOWS
s_metricsProviderBuilder = Sdk.CreateMeterProviderBuilder()
.ConfigureResource(r => { r.AddService("dotnet-cli", serviceVersion: Product.Version); })
.AddMeter(Activities.Source.Name)
.AddHttpClientInstrumentation()
.AddRuntimeInstrumentation()
.AddOtlpExporter();

s_tracerProviderBuilder = Sdk.CreateTracerProviderBuilder()
.ConfigureResource(r => { r.AddService("dotnet-cli", serviceVersion: Product.Version); })
.AddSource(Activities.Source.Name)
.AddHttpClientInstrumentation()
.AddOtlpExporter()
.AddInMemoryExporter(s_activities)
.SetSampler(new AlwaysOnSampler());

if (!s_disableTraceExport)
{
var storageDirectory = string.IsNullOrWhiteSpace(s_environmentStoragePath) ? s_defaultStorageDirectory : s_environmentStoragePath;
s_tracerProviderBuilder.AddAzureMonitorTraceExporter(o =>
{
o.ConnectionString = s_connectionString;
o.EnableLiveMetrics = false;
o.StorageDirectory = storageDirectory;
});
}
#endif

// Only extract parent activity context in the static constructor.
// All OTel provider setup is deferred to the instance constructor,
// gated behind the telemetry opt-out check, to avoid paying initialization
// costs when telemetry is disabled.
var parentActivityContext = GetParentActivityContext();
ActivityKind = GetActivityKind(parentActivityContext);
ParentActivityContext = parentActivityContext ?? default;
Expand All @@ -113,19 +86,58 @@ public TelemetryClient(string? sessionId, IEnvironmentProvider? environmentProvi
}

#if TARGET_WINDOWS
if (s_metricsProvider is null || s_tracerProvider is null)
{
// Create a new OTel meter and tracer provider.
// It is important to keep the provider instances active throughout the process lifetime.
s_metricsProvider ??= s_metricsProviderBuilder.Build();
s_tracerProvider ??= s_tracerProviderBuilder.Build();
}
EnsureTracerProviderInitialized();
#endif

CurrentSessionId ??= !string.IsNullOrEmpty(sessionId) ? sessionId : Guid.NewGuid().ToString();
s_commonProperties = new TelemetryCommonProperties().GetTelemetryCommonProperties(CurrentSessionId);
}

#if TARGET_WINDOWS
private static void EnsureTracerProviderInitialized()
{
if (s_tracerProvider is not null)
{
return;
}

Comment on lines +99 to +103
lock (s_providerLock)
{
if (s_tracerProvider is not null)
{
return;
}

var tracerBuilder = Sdk.CreateTracerProviderBuilder()
.ConfigureResource(r => { r.AddService("dotnet-cli", serviceVersion: Product.Version); })
.AddSource(Activities.Source.Name)
.AddInMemoryExporter(s_activities)
Copy link
Copy Markdown
Member

@nagilson nagilson Apr 29, 2026

Choose a reason for hiding this comment

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

Seems that this also might be the root cause based on conversations with @baronfel and @MiYanni

.SetSampler(new AlwaysOnSampler());

// Only add the OTLP exporter when the user has explicitly configured an endpoint.
var otlpEndpoint = Env.GetEnvironmentVariable("OTEL_EXPORTER_OTLP_ENDPOINT")
?? Env.GetEnvironmentVariable("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT");
Comment on lines +118 to +119
Copy link
Copy Markdown
Member

@baronfel baronfel Apr 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This is bad for usability - most folks won't be defining these, they'll just be using tools that bind to the conventional OTel ports of 4317 and 4318. Put another way, this is very much not the expectation for the OTel ecosystem.

Copy link
Copy Markdown
Member

@nagilson nagilson Apr 28, 2026

Choose a reason for hiding this comment

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

I'm still learning how all of this works, but all of the OLTP exported data doesn't go anywhere we can observe since data-x-platform only takes in traces which are produced from logging events (activity events get sent as well). If this is only powering the aspire dashboard for local perf investigations, I don't think we should incur the perf hit of having this exporter by default. What do you think?

Copy link
Copy Markdown
Member

@nagilson nagilson Apr 29, 2026

Choose a reason for hiding this comment

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

Based on conversation, this is only pushing or sending anything if there is a specific otel endpoint configured and running such as localhost:4317/traces

if (!string.IsNullOrEmpty(otlpEndpoint))
{
tracerBuilder.AddOtlpExporter();
}
Comment on lines +117 to +123

if (!s_disableTraceExport)
{
var storageDirectory = string.IsNullOrWhiteSpace(s_environmentStoragePath) ? s_defaultStorageDirectory : s_environmentStoragePath;
tracerBuilder.AddAzureMonitorTraceExporter(o =>
{
o.ConnectionString = s_connectionString;
o.EnableLiveMetrics = false;
o.StorageDirectory = storageDirectory;
});
}

s_tracerProvider = tracerBuilder.Build();
}
}
#endif

/// <summary>
/// Uses the OpenTelemetry SDK's Propagation API to derive the parent activity context from the DOTNET_CLI_TRACEPARENT and DOTNET_CLI_TRACESTATE environment variables.
/// </summary>
Expand Down Expand Up @@ -166,7 +178,6 @@ public static void FlushProviders()
{
#if TARGET_WINDOWS
s_tracerProvider?.ForceFlush(s_flushTimeoutMs);
s_metricsProvider?.ForceFlush(s_flushTimeoutMs);
#endif
}

Expand Down
2 changes: 0 additions & 2 deletions src/Cli/dotnet/dotnet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,6 @@
<PackageReference Include="Azure.Monitor.OpenTelemetry.Exporter" />
<PackageReference Include="OpenTelemetry.Exporter.InMemory" />
<PackageReference Include="OpenTelemetry.Exporter.OpenTelemetryProtocol" />
<PackageReference Include="OpenTelemetry.Instrumentation.Http" />
<PackageReference Include="OpenTelemetry.Instrumentation.Runtime" />
</ItemGroup>

<ItemGroup Condition="'$(IncludeAspNetCoreRuntime)' != 'false'">
Expand Down
Loading