Repository navigation
Introduce a ConfigureSampler API - #7780
Conversation
Pull request dashboard statusMerged · refreshed 2026-09-29 12:22 UTC Status above doesn't look right?
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7780 +/- ##
==========================================
+ Coverage 91.94% 91.99% +0.04%
==========================================
Files 343 343
Lines 18841 18865 +24
==========================================
+ Hits 17324 17354 +30
+ Misses 1517 1511 -6
Flags with carried forward coverage won't be shown. Click here to find out more.
|
martincostello
left a comment
There was a problem hiding this comment.
Just some comments around documentation.
…s.cs Co-authored-by: Martin Costello <martin@martincostello.com>
|
I understand why such changes might be needed, but before merging, I need to understand what otel-spec is saying about such methods. There is a chance that it should be agreed on the specificiation level first. @pellared, could you please advice here? |
|
No worries! Thanks @Kielek. From a quick reread myself, I don’t believe the spec blocks this API:
|
There was a problem hiding this comment.
In general LGTM. It will be great to have also @alanwest opinion as it creates new public API (and it is not so obvious as adding new mandatory Sampler).
Reviewed also with Codex, it states that it might be not obvious what is the precedence between SetSampler/ConfigureSampler/env.vars/other sources for this.
I am dropping here proposed changes, but feel free to fully ignore the wording/tests.
diff --git a/docs/trace/customizing-the-sdk/README.md b/docs/trace/customizing-the-sdk/README.md
index ac126a79c..0fa700694 100644
--- a/docs/trace/customizing-the-sdk/README.md
+++ b/docs/trace/customizing-the-sdk/README.md
@@ -390,10 +390,9 @@ to learn about writing custom samplers.
#### Adjusting the sampler
-`SetSampler` sets the sampler unconditionally: any `OTEL_TRACES_SAMPLER` value
-is ignored. This is appropriate for an application that fully controls its own
-configuration, but causes problems for a library that wants to influence
-sampling without affecting any explicit user configuration of the sampler.
+`SetSampler` selects the sampler ahead of any `OTEL_TRACES_SAMPLER` value.
+`ConfigureSampler` callbacks then receive that sampler and can keep, wrap, or
+replace it. A callback can therefore change a sampler selected by `SetSampler`.
`ConfigureSampler` registers a callback which receives the sampler the SDK
resolved - whether that came from `SetSampler`, from `OTEL_TRACES_SAMPLER`, or
@@ -409,10 +408,11 @@ var tracerProvider = Sdk.CreateTracerProviderBuilder()
.Build();
\`\`\`
-The callback runs once, while the provider is being built. Registering it several
-times chains the callbacks in registration order, each one receiving the sampler
-returned by the previous one, so several libraries can take part without
-conflicting. Returning `null` or throwing from a callback fails the build.
+The callback runs once, while the provider is being built, regardless of whether
+it was registered before or after `SetSampler`. Registering it several times
+chains the callbacks in registration order, each one receiving the sampler
+returned by the previous one. Returning `null` or throwing from a callback
+fails the build.
When a callback returns a sampler other than the one it was given, the provider
disposes only the final resolved sampler.
diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md
index b867d32ea..adc9c313f 100644
--- a/src/OpenTelemetry/CHANGELOG.md
+++ b/src/OpenTelemetry/CHANGELOG.md
@@ -10,10 +10,9 @@ Notes](../../RELEASENOTES.md).
inherited asynchronous execution contexts.
([#7803](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7803))
-* Added `TracerProviderBuilder.ConfigureSampler` which registers a callback
- invoked during provider construction with the resolved `Sampler`, allowing the
- sampler to be wrapped, conditionally replaced, or observed without discarding
- any sampler configured by the `OTEL_TRACES_SAMPLER` environment variable.
+* Added `TracerProviderBuilder.ConfigureSampler` to adjust the sampler selected by
+ `SetSampler`, `OTEL_TRACES_SAMPLER`, or the default during provider construction.
+ Its callbacks can wrap or replace that sampler.
([#7780](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7780))
## 1.19.1
diff --git a/src/OpenTelemetry/Trace/Builder/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry/Trace/Builder/TracerProviderBuilderExtensions.cs
index fc5373e4d..fea80c91f 100644
--- a/src/OpenTelemetry/Trace/Builder/TracerProviderBuilderExtensions.cs
+++ b/src/OpenTelemetry/Trace/Builder/TracerProviderBuilderExtensions.cs
@@ -52,9 +52,10 @@ public static class TracerProviderBuilderExtensions
/// <para sampler-precedence="true">Note: A sampler set programmatically
/// takes precedence over the <c>OTEL_TRACES_SAMPLER</c> environment
/// variable, which in turn takes precedence over the default sampler
- /// (<c>ParentBased(AlwaysOnSampler)</c>). To adjust the sampler the SDK
- /// resolves, instead of overriding it, use
- /// <see cref="ConfigureSampler(TracerProviderBuilder, Func{IServiceProvider, Sampler, Sampler})"/>.</para>
+ /// (<c>ParentBased(AlwaysOnSampler)</c>).
+ /// <see cref="ConfigureSampler(TracerProviderBuilder, Func{IServiceProvider, Sampler, Sampler})"/>
+ /// callbacks run after this selection and can wrap or replace the sampler,
+ /// regardless of registration order.</para>
/// </remarks>
/// <param name="tracerProviderBuilder"><see cref="TracerProviderBuilder"/>.</param>
/// <param name="sampler">Sampler instance.</param>
@@ -135,9 +136,11 @@ public static class TracerProviderBuilderExtensions
/// use.
/// </summary>
/// <remarks>
- /// <para>Note: Unlike <see cref="SetSampler(TracerProviderBuilder, Sampler)"/>,
- /// this method does not discard a sampler configured using the
- /// <c>OTEL_TRACES_SAMPLER</c> environment variable.</para>
+ /// <para>Callbacks run after the SDK selects a sampler from
+ /// <see cref="SetSampler(TracerProviderBuilder, Sampler)"/>, the
+ /// <c>OTEL_TRACES_SAMPLER</c> environment variable, or the default,
+ /// regardless of whether they are registered before or after
+ /// <c>SetSampler</c>.</para>
/// <para>The callback is invoked once, while the
/// <see cref="TracerProvider"/> is being built, and is passed the sampler
/// which would otherwise have been used. The callback may return the original
diff --git a/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTests.cs b/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTests.cs
index 22a8ee032..3ab8e1b26 100644
--- a/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTests.cs
+++ b/test/OpenTelemetry.Tests/Trace/TracerProviderBuilderExtensionsTests.cs
@@ -642,23 +642,63 @@ public class TracerProviderBuilderExtensionsTests
Assert.Same(receivedSampler, tracerProvider.Sampler);
}
- [Fact]
- public void ConfigureSamplerReceivesSamplerSetProgrammatically()
+ [Theory]
+ [InlineData(false, "instance")]
+ [InlineData(true, "instance")]
+ [InlineData(false, "type")]
+ [InlineData(true, "type")]
+ [InlineData(false, "factory")]
+ [InlineData(true, "factory")]
+ public void ConfigureSamplerOverridesSetSamplerRegardlessOfRegistrationOrder(bool configureSamplerFirst, string setSamplerOverload)
{
- var resolvedSampler = new MySampler();
-
+ using var activitySource = new ActivitySource(Utils.GetCurrentMethodName());
Sampler? receivedSampler = null;
+ var builder = Sdk.CreateTracerProviderBuilder()
+ .AddSource(activitySource.Name)
+ .AddSamplerConfiguration("always_on");
- using var tracerProvider = Sdk.CreateTracerProviderBuilder()
- .SetSampler(resolvedSampler)
- .ConfigureSampler((sp, sampler) =>
+ if (configureSamplerFirst)
+ {
+ RegisterConfigurator();
+ }
+
+ switch (setSamplerOverload)
+ {
+ case "instance":
+ builder.SetSampler(new MySampler());
+ break;
+ case "type":
+ builder.SetSampler<MySampler>();
+ break;
+ case "factory":
+ builder.SetSampler(sp => new MySampler());
+ break;
+ default:
+ throw new ArgumentOutOfRangeException(nameof(setSamplerOverload));
+ }
+
+ if (!configureSamplerFirst)
+ {
+ RegisterConfigurator();
+ }
+
+ using var tracerProvider = builder.Build() as TracerProviderSdk;
+
+ Assert.NotNull(tracerProvider);
+ Assert.IsType<MySampler>(receivedSampler);
+ Assert.IsType<AlwaysOffSampler>(tracerProvider.Sampler);
+
+ using var activity = activitySource.StartActivity("Activity");
+
+ Assert.NotNull(activity);
+ Assert.False(activity.Recorded);
+
+ void RegisterConfigurator() =>
+ builder.ConfigureSampler((sp, sampler) =>
{
receivedSampler = sampler;
- return sampler;
- })
- .Build();
-
- Assert.Same(resolvedSampler, receivedSampler);
+ return new AlwaysOffSampler();
+ });
}
[Fact]
@@ -734,25 +774,6 @@ public class TracerProviderBuilderExtensionsTests
messages);
}
- [Fact]
- public void ConfigureSamplerReceivesResolvedSamplerWhenRegisteredBeforeSetSampler()
- {
- var resolvedSampler = new MySampler();
-
- Sampler? receivedSampler = null;
-
- using var tracerProvider = Sdk.CreateTracerProviderBuilder()
- .ConfigureSampler((sp, sampler) =>
- {
- receivedSampler = sampler;
- return sampler;
- })
- .SetSampler(resolvedSampler)
- .Build();
-
- Assert.Same(resolvedSampler, receivedSampler);
- }
-
[Fact]
public void ConfigureSamplerReturningInputLeavesSamplerUnchanged()
{
Thanks, @Kielek. I've applied some of the docs suggestions and refined a couple of them + the improved testing. |
….0 (#1530) Updated [OpenTelemetry.Exporter.OpenTelemetryProtocol](https://github.com/open-telemetry/opentelemetry-dotnet) from 1.19.1 to 1.20.0. <details> <summary>Release notes</summary> _Sourced from [OpenTelemetry.Exporter.OpenTelemetryProtocol's releases](https://github.com/open-telemetry/opentelemetry-dotnet/releases)._ ## 1.20.0 For highlights and announcements pertaining to this release see: [Release Notes > 1.20.0](https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/RELEASENOTES.md#1200). The following changes are from the previous release [1.19.1](https://github.com/open-telemetry/opentelemetry-dotnet/releases/tag/core-1.19.1). * NuGet: [OpenTelemetry v1.20.0](https://www.nuget.org/packages/OpenTelemetry/1.20.0) * Fixed `SuppressInstrumentationScope` reference counts leaking between inherited asynchronous execution contexts. ([#7803](open-telemetry/opentelemetry-dotnet#7803)) * Self-diagnostic features are explicitly disabled on browser-based platforms (e.g. Blazor) and are now non-functional. Previously, attempting to enable self-diagnostics on such platforms would result in an exception at runtime. ([#7799](open-telemetry/opentelemetry-dotnet#7799)) * Added `TracerProviderBuilder.ConfigureSampler` which receives the resolved `Sampler` whether set programmatically, via `OTEL_TRACES_SAMPLER`, or by default, and returns a replacement. Intended for library authors who need to influence sampling without overriding a sampler the application has configured. ([#7780](open-telemetry/opentelemetry-dotnet#7780)) * Fixed `ResourceBuilder.CreateDefault` ignoring `OTEL_RESOURCE_ATTRIBUTES` and `OTEL_SERVICE_NAME` environment variables when the host `IConfiguration` does not contain those settings. ([#7855](open-telemetry/opentelemetry-dotnet#7855)) * Added improved self-diagnostics logging to improve the support experience. The new `SelfDiagnosticsOptions` class configures a rolling file sink and a console sink through the standard options pipeline. Sinks are off by default. ([#7588](open-telemetry/opentelemetry-dotnet#7588)) * Fixed a race that could incorrectly route a concurrent metric measurement to the overflow point when its tag set was published as the cardinality limit was reached. ([#7804](open-telemetry/opentelemetry-dotnet#7804)) * Added `OpenTelemetryLoggerOptions.Version` and `OpenTelemetryLoggerOptions.SchemaUrl` options to set an instrumentation library version and schema URL applied to logs emitted via the `Microsoft.Extensions.Logging` `ILogger` integration. ([#7727](open-telemetry/opentelemetry-dotnet#7727)) * Fixed `LogRecord.CategoryName` resetting the current `Logger`'s `Version` to `null` when set to a new value. ([#7727](open-telemetry/opentelemetry-dotnet#7727)) * Bounded the internal cache of `Logger` scopes keyed by instrumentation to avoid unbounded growth. ([#7727](open-telemetry/opentelemetry-dotnet#7727)) * Fixed the `MeterProvider` and its `IServiceProvider` being kept alive after the host is disposed when using `Microsoft.Extensions.Diagnostics` versions prior to 9.0, which never dispose their `MetricsSubscriptionManager` (and its underlying `MeterListener`). ([#7924](open-telemetry/opentelemetry-dotnet#7924)) * Fixed a race condition between recording a measurement and collecting a histogram with delta temporality that could cause an empty data point (with a count of zero) to be exported in the next collection cycle. ([#7923](open-telemetry/opentelemetry-dotnet#7923)) * Improved metric collection performance after metric streams are removed by compacting metric storage during collection. ([#7945](open-telemetry/opentelemetry-dotnet#7945)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/core-1.20.0/src/OpenTelemetry/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Api v1.20.0](https://www.nuget.org/packages/OpenTelemetry.Api/1.20.0) * Added `Logger.SchemaUrl` and a `LoggerProvider.GetLogger` overload accepting a schema URL parameter using the new `LoggerOptions` type. ([#7727](open-telemetry/opentelemetry-dotnet#7727)) * **Breaking change** (pre-release only versions): `LoggerProvider`'s protected `TryCreateLogger(string? name, out Logger? logger)` extensibility method has been replaced by `TryCreateLogger(LoggerOptions options, out Logger? logger)`. Custom `LoggerProvider` subclasses that override the previous overload must update to override the new one; the old overload is no longer called. ... (truncated) ## 1.20.0-rc.1 The following changes are from the previous release [1.19.1](https://github.com/open-telemetry/opentelemetry-dotnet/releases/tag/core-1.19.1). * NuGet: [OpenTelemetry v1.20.0-rc.1](https://www.nuget.org/packages/OpenTelemetry/1.20.0-rc.1) * Fixed `SuppressInstrumentationScope` reference counts leaking between inherited asynchronous execution contexts. ([#7803](open-telemetry/opentelemetry-dotnet#7803)) * Self-diagnostic features are explicitly disabled on browser-based platforms (e.g. Blazor) and are now non-functional. Previously, attempting to enable self-diagnostics on such platforms would result in an exception at runtime. ([#7799](open-telemetry/opentelemetry-dotnet#7799)) * Added `TracerProviderBuilder.ConfigureSampler` which receives the resolved `Sampler` whether set programmatically, via `OTEL_TRACES_SAMPLER`, or by default, and returns a replacement. Intended for library authors who need to influence sampling without overriding a sampler the application has configured. ([#7780](open-telemetry/opentelemetry-dotnet#7780)) * Fixed `ResourceBuilder.CreateDefault` ignoring `OTEL_RESOURCE_ATTRIBUTES` and `OTEL_SERVICE_NAME` environment variables when the host `IConfiguration` does not contain those settings. ([#7855](open-telemetry/opentelemetry-dotnet#7855)) * Added improved self-diagnostics logging to improve the support experience. The new `SelfDiagnosticsOptions` class configures a rolling file sink and a console sink through the standard options pipeline. Sinks are off by default. ([#7588](open-telemetry/opentelemetry-dotnet#7588)) * Fixed a race that could incorrectly route a concurrent metric measurement to the overflow point when its tag set was published as the cardinality limit was reached. ([#7804](open-telemetry/opentelemetry-dotnet#7804)) * Added `OpenTelemetryLoggerOptions.Version` and `OpenTelemetryLoggerOptions.SchemaUrl` options to set an instrumentation library version and schema URL applied to logs emitted via the `Microsoft.Extensions.Logging` `ILogger` integration. ([#7727](open-telemetry/opentelemetry-dotnet#7727)) * Fixed `LogRecord.CategoryName` resetting the current `Logger`'s `Version` to `null` when set to a new value. ([#7727](open-telemetry/opentelemetry-dotnet#7727)) * Bounded the internal cache of `Logger` scopes keyed by instrumentation to avoid unbounded growth. ([#7727](open-telemetry/opentelemetry-dotnet#7727)) * Fixed the `MeterProvider` and its `IServiceProvider` being kept alive after the host is disposed when using `Microsoft.Extensions.Diagnostics` versions prior to 9.0, which never dispose their `MetricsSubscriptionManager` (and its underlying `MeterListener`). ([#7924](open-telemetry/opentelemetry-dotnet#7924)) * Fixed a race condition between recording a measurement and collecting a histogram with delta temporality that could cause an empty data point (with a count of zero) to be exported in the next collection cycle. ([#7923](open-telemetry/opentelemetry-dotnet#7923)) * Improved metric collection performance after metric streams are removed by compacting metric storage during collection. ([#7945](open-telemetry/opentelemetry-dotnet#7945)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/core-1.20.0-rc.1/src/OpenTelemetry/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Api v1.20.0-rc.1](https://www.nuget.org/packages/OpenTelemetry.Api/1.20.0-rc.1) * Added `Logger.SchemaUrl` and a `LoggerProvider.GetLogger` overload accepting a schema URL parameter using the new `LoggerOptions` type. ([#7727](open-telemetry/opentelemetry-dotnet#7727)) * **Breaking change** (pre-release only versions): `LoggerProvider`'s protected `TryCreateLogger(string? name, out Logger? logger)` extensibility method has been replaced by `TryCreateLogger(LoggerOptions options, out Logger? logger)`. Custom `LoggerProvider` subclasses that override the previous overload must update to override the new one; the old overload is no longer called. ([#7727](open-telemetry/opentelemetry-dotnet#7727)) ... (truncated) ## 1.20.0-beta.1 The following changes are from the previous release [1.19.1-beta.1](https://github.com/open-telemetry/opentelemetry-dotnet/releases/tag/coreunstable-1.19.1-beta.1). * NuGet: [OpenTelemetry.Exporter.Prometheus.AspNetCore v1.20.0-beta.1](https://www.nuget.org/packages/OpenTelemetry.Exporter.Prometheus.AspNetCore/1.20.0-beta.1) * A scrape request whose connection is aborted, or whose `X-Prometheus-Scrape-Timeout-Seconds` deadline elapses, now stops waiting for an in-progress metrics collection instead of continuing to wait for it to finish. The collection itself is not cancelled and continues running in the background. ([#7856](open-telemetry/opentelemetry-dotnet#7856)) * Updated OpenTelemetry core component version(s) to `1.20.0`. ([#7950](open-telemetry/opentelemetry-dotnet#7950)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/coreunstable-1.20.0-beta.1/src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Exporter.Prometheus.HttpListener v1.20.0-beta.1](https://www.nuget.org/packages/OpenTelemetry.Exporter.Prometheus.HttpListener/1.20.0-beta.1) * Updated OpenTelemetry core component version(s) to `1.20.0`. ([#7950](open-telemetry/opentelemetry-dotnet#7950)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/coreunstable-1.20.0-beta.1/src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md) for details. * NuGet: [OpenTelemetry.Shims.OpenTracing v1.20.0-beta.1](https://www.nuget.org/packages/OpenTelemetry.Shims.OpenTracing/1.20.0-beta.1) * Updated OpenTelemetry core component version(s) to `1.20.0`. ([#7950](open-telemetry/opentelemetry-dotnet#7950)) See [CHANGELOG](https://github.com/open-telemetry/opentelemetry-dotnet/blob/coreunstable-1.20.0-beta.1/src/OpenTelemetry.Shims.OpenTracing/CHANGELOG.md) for details. Commits viewable in [compare view](open-telemetry/opentelemetry-dotnet@core-1.19.1...core-1.20.0). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Relates to #7192
Changes
Introduces
ConfigureSampleron theTracerProviderSdk.Today the only way to influence the SDK's sampler is
SetSampler, which wins outright: when a programmatic sampler is present, the user'sOTEL_TRACES_SAMPLERvalue is logged as ignored and discarded. So any library that supplies a sampler silently throws away the user's configuration, and two such libraries silently conflict (last writer wins, no diagnostic).ConfigureSamplerregisters a callback that receives the sampler the SDK resolved (programmatic, environment, or the default) and returns the sampler to use, so a library can wrap, inspect, conditionally replace, or just observe it instead of overwriting it. Multiple registrations chain in registration order, so libraries compose. This completes an existing pattern rather than introducing a concept: the resource already hasSetResourceBuilder,ConfigureResourceandGetResource, all stable, and the signature is the existing stableSetSampler(Func<IServiceProvider, Sampler>)overload with the resolved sampler as an extra input. Resolution order is unchanged, and behaviour is identical when no callback is registered.The callback runs once during
Build()and the sampler field stays immutable. Runtime swapping already works today without any SDK change, viaSetSamplerwith a delegating sampler holding a mutable delegate. What the SDK does not expose is the effective resolved sampler, and that is what this adds. This matters increasingly as declarative configuration moves sampler selection out of code, and it is the missing piece for dynamic control sampler configuration. A component that adjusts sampling remotely has to layer over the user's configured sampler and be able to restore it, which is impossible today. Nothing here is specific to that work, and the benefits it introduce are (I believe) useful for other library cases.I avoided introducing
GetSamplerin this PR as it adds less value. A type check would be needed to access which specificSamplerimplementation is set, and today most samplers do not expose public properties of their specific configuration. We could add it as a consistency follow up.Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changes