From 09acbb2844ba600bebb5e50d279cfad34e0d9587 Mon Sep 17 00:00:00 2001 From: ancplua Date: Wed, 1 Jul 2026 17:25:15 +0200 Subject: [PATCH] =?UTF-8?q?fix(otel)!:=20single-owner-per-signal=20registr?= =?UTF-8?q?y=20=E2=80=94=20end=20double=20instrumentation=20(CODE=20RED=20?= =?UTF-8?q?#3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirmed HIGH double-count: when a consumer opted into more than one lane (source-generated interceptor, generated middleware, DiagnosticListener) for the same signal, the operation was instrumented twice — and the WebApiAotDemo "verified" fixture masked it (it captured both an aspnetcore.server and an http.server span for one endpoint and still passed). New QylSignalOwnership registry: each lane registers a priority for a signal (Interceptor 95 > GeneratedMiddleware 90 > DiagnosticListener 70); only the highest-priority registered lane emits, the rest defer — so exactly one span per operation. Race-free by construction: the interceptor path registers at endpoint-mapping time (Observe) and the interceptor helpers' static initializers register on first use — inside the intercepted call, before it reaches the framework code that raises the DiagnosticListener event — and the middleware registers at DI time. Wired: - DiagnosticListenerSubscriber: registers @70, defers in OnNext when a higher lane owns the signal. - AddQylAspNetCoreInstrumentation: registers @90; QylAspNetCoreStartupFilter middleware defers to the endpoint interceptor lane (95) so middleware + intercepted endpoints still yield one server span. - QylInterceptedAspNetCore.Observe: registers the ASP.NET Core interceptor lane @95. - QylInterceptedHttpClient / QylInterceptedGrpcNetClient: static ctors register HTTPCLIENT / GRPCNETCLIENT @95. Honest fixture: the WebApiAotDemo now emits exactly ONE server span per request (aspnetcore.server, route-backfilled); the http.server listener span is gone. The self-check + golden report were updated to assert the single-lane output, so any future re-double fails the gate. Verified: core.slnf 0/0; verify-webapi-aot-demo, source-interceptor-consumer, public-api-baseline (no API change — QylSignalOwnership is internal), generator-snapshots all pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../DiagnosticListenerSubscriber.cs | 7 ++- ...trumentationServiceCollectionExtensions.cs | 3 + .../QylAspNetCoreStartupFilter.cs | 8 ++- .../QylInterceptedAspNetCore.cs | 9 ++- .../QylInterceptedGrpcNetClient.cs | 4 ++ .../QylInterceptedHttpClient.cs | 4 ++ .../QylSignalOwnership.cs | 60 +++++++++++++++++++ .../Program.cs | 5 +- .../verified/report.json | 27 +-------- 9 files changed, 97 insertions(+), 30 deletions(-) create mode 100644 src/Qyl.OpenTelemetry.AutoInstrumentation/QylSignalOwnership.cs diff --git a/src/Qyl.OpenTelemetry.AutoInstrumentation.DiagnosticListeners/DiagnosticListenerSubscriber.cs b/src/Qyl.OpenTelemetry.AutoInstrumentation.DiagnosticListeners/DiagnosticListenerSubscriber.cs index 23fa6122..5632d6da 100644 --- a/src/Qyl.OpenTelemetry.AutoInstrumentation.DiagnosticListeners/DiagnosticListenerSubscriber.cs +++ b/src/Qyl.OpenTelemetry.AutoInstrumentation.DiagnosticListeners/DiagnosticListenerSubscriber.cs @@ -39,6 +39,10 @@ public void Subscribe() if (!QylAutoInstrumentationOptions.Current.IsInstrumentationEnabled(Signal, InstrumentationId)) return; + // Claim the DiagnosticListener lane for this signal. If a higher-priority lane (interceptor / + // generated middleware) also covers it, this subscriber defers in OnNext so the operation is + // instrumented exactly once. See QylSignalOwnership. + QylSignalOwnership.Register(InstrumentationId, QylSignalOwnership.DiagnosticListener); _allListenersSubscription ??= DiagnosticListener.AllListeners.Subscribe(new AllListenersObserver(this)); } @@ -47,7 +51,8 @@ public void Subscribe() void IObserver>.OnNext(KeyValuePair value) { - if (QylAutoInstrumentationOptions.Current.IsInstrumentationEnabled(Signal, InstrumentationId)) + if (QylAutoInstrumentationOptions.Current.IsInstrumentationEnabled(Signal, InstrumentationId) + && QylSignalOwnership.ShouldEmit(InstrumentationId, QylSignalOwnership.DiagnosticListener)) OnEvent(value.Key, value.Value); } diff --git a/src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreInstrumentationServiceCollectionExtensions.cs b/src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreInstrumentationServiceCollectionExtensions.cs index f68eebba..dd8ad563 100644 --- a/src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreInstrumentationServiceCollectionExtensions.cs +++ b/src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreInstrumentationServiceCollectionExtensions.cs @@ -26,6 +26,9 @@ public static class QylAspNetCoreInstrumentationServiceCollectionExtensions public static IServiceCollection AddQylAspNetCoreInstrumentation(this IServiceCollection services) { ArgumentNullException.ThrowIfNull(services); + // Claim the ASP.NET Core signal for the middleware lane so the DiagnosticListener lane (if the + // Hosting package is also referenced) defers and the server span is emitted exactly once. + QylSignalOwnership.Register(QylAutoInstrumentationIds.AspNetCore, QylSignalOwnership.GeneratedMiddleware); services.TryAddEnumerable(ServiceDescriptor.Singleton()); return services; } diff --git a/src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreStartupFilter.cs b/src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreStartupFilter.cs index 3b654d2e..0cb199e1 100644 --- a/src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreStartupFilter.cs +++ b/src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreStartupFilter.cs @@ -17,7 +17,13 @@ internal sealed class QylAspNetCoreStartupFilter : IStartupFilter public Action Configure(Action next) => app => { - app.Use(static (context, requestDelegate) => QylInterceptedAspNetCore.InvokeAsync(requestDelegate, context)); + // Middleware lane (priority 90). Defers to the endpoint interceptor lane (95) when it owns the + // ASP.NET Core signal, so a consumer that has both intercepted endpoints and this middleware + // still emits exactly one server span. Wins over the DiagnosticListener lane (70). + app.Use(static (context, requestDelegate) => + QylSignalOwnership.ShouldEmit(QylAutoInstrumentationIds.AspNetCore, QylSignalOwnership.GeneratedMiddleware) + ? QylInterceptedAspNetCore.InvokeAsync(requestDelegate, context) + : requestDelegate(context)); next(app); }; } diff --git a/src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedAspNetCore.cs b/src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedAspNetCore.cs index 3632799d..cd7316fa 100644 --- a/src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedAspNetCore.cs +++ b/src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedAspNetCore.cs @@ -122,7 +122,14 @@ private static void RecordResponse(Activity? activity, HttpContext context) } private static RequestDelegate Observe(RequestDelegate requestDelegate) - => requestDelegate is null ? null! : context => InvokeAsync(requestDelegate, context); + { + // The endpoint interceptor lane owns the ASP.NET Core server signal (priority 95). Registered here + // at endpoint-mapping time (before requests) so the middleware (90) and DiagnosticListener (70) + // lanes defer — exactly one server span per request. Registration is NOT done from InvokeAsync, + // which the middleware also calls; only actually-intercepted endpoints claim the interceptor lane. + QylSignalOwnership.Register(QylAutoInstrumentationIds.AspNetCore, QylSignalOwnership.Interceptor); + return requestDelegate is null ? null! : context => InvokeAsync(requestDelegate, context); + } private static string? GetRoute(HttpContext context) => context.GetEndpoint() is RouteEndpoint routeEndpoint diff --git a/src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedGrpcNetClient.cs b/src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedGrpcNetClient.cs index d9a14361..f1774428 100644 --- a/src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedGrpcNetClient.cs +++ b/src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedGrpcNetClient.cs @@ -9,6 +9,10 @@ namespace Qyl.OpenTelemetry.AutoInstrumentation; /// var apiType = typeof(QylInterceptedGrpcNetClient); public static class QylInterceptedGrpcNetClient { + // Registered on first use (inside an intercepted gRPC call, before the underlying call raises the + // Grpc.Net.Client DiagnosticListener event) so the listener lane defers — no double-count. + static QylInterceptedGrpcNetClient() + => QylSignalOwnership.Register(QylAutoInstrumentationIds.GrpcNetClient, QylSignalOwnership.Interceptor); /// Runs the Start Activity runtime helper used by source-generated qyl interceptors. public static Activity? StartActivity(string clientTypeName, string methodName, Metadata? requestMetadata) diff --git a/src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedHttpClient.cs b/src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedHttpClient.cs index 2c873e08..53914ae3 100644 --- a/src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedHttpClient.cs +++ b/src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedHttpClient.cs @@ -11,6 +11,10 @@ namespace Qyl.OpenTelemetry.AutoInstrumentation; /// public static class QylInterceptedHttpClient { + // Registered on first use — which is inside an intercepted HttpClient call, before that call reaches + // the BCL that raises the HttpClient DiagnosticListener event — so the listener lane defers (no double). + static QylInterceptedHttpClient() + => QylSignalOwnership.Register(QylAutoInstrumentationIds.HttpClient, QylSignalOwnership.Interceptor); /// Runs the Send runtime helper used by source-generated qyl interceptors. public static HttpResponseMessage Send(HttpClient client, HttpRequestMessage request) diff --git a/src/Qyl.OpenTelemetry.AutoInstrumentation/QylSignalOwnership.cs b/src/Qyl.OpenTelemetry.AutoInstrumentation/QylSignalOwnership.cs new file mode 100644 index 00000000..73bd21a5 --- /dev/null +++ b/src/Qyl.OpenTelemetry.AutoInstrumentation/QylSignalOwnership.cs @@ -0,0 +1,60 @@ +using System.Collections.Concurrent; + +namespace Qyl.OpenTelemetry.AutoInstrumentation; + +/// +/// Single-owner-per-signal registry. More than one lane can be able to produce a span for the same +/// instrumentation signal — a source-generated call-site interceptor, a generated middleware/wrapper, +/// the library's own native ActivitySource, or a DiagnosticListener subscription. Without +/// coordination a consumer that opts into two lanes emits the operation twice (double-counting). +/// +/// +/// Each lane s a priority for a signal; only the highest-priority registered lane +/// s, every other lane defers — so exactly one span is produced per operation. +/// Priorities follow the qyl instrumentation-mechanism ranking (higher = more precise, lower overhead, +/// more AOT-native): compiler-generated > interceptor > generated middleware > native +/// ActivitySource > DiagnosticListener. +/// +/// +/// +/// Registration is race-free by construction: the interceptor helper's static initializer registers on +/// first use — which happens inside the intercepted call, before that call reaches the underlying +/// framework code that raises the DiagnosticListener event — and the middleware registers at DI time, +/// before the first request. So by the time a lower lane observes an event, the higher lane is already +/// the recorded owner. +/// +/// +internal static class QylSignalOwnership +{ + /// Compiler-generated AOP (reserved for a future lane). + public const int CompilerGenerated = 100; + + /// Source-generated call-site interceptor — the qyl-preferred lane. + public const int Interceptor = 95; + + /// Generated middleware / wrapper (e.g. the ASP.NET Core server-span IStartupFilter). + public const int GeneratedMiddleware = 90; + + /// A span emitted natively by the instrumented library's own ActivitySource. + public const int NativeActivitySource = 85; + + /// A DiagnosticListener subscription — broad compatibility, coarser payload. + public const int DiagnosticListener = 70; + + private static readonly ConcurrentDictionary Owners = new(StringComparer.Ordinal); + + /// + /// Records that a lane of the given can produce + /// . The highest priority ever registered wins. + /// + public static void Register(string instrumentationId, int priority) + => Owners.AddOrUpdate(instrumentationId, priority, (_, existing) => existing >= priority ? existing : priority); + + /// + /// True when a lane of is the highest-priority producer registered for + /// and should therefore emit; false when a higher lane owns the + /// signal, so this lane defers. When nothing is registered the caller emits (it is the only lane). + /// + public static bool ShouldEmit(string instrumentationId, int priority) + => !Owners.TryGetValue(instrumentationId, out var max) || priority >= max; +} diff --git a/tools/Qyl.OpenTelemetry.AutoInstrumentation.WebApiAotDemo/Program.cs b/tools/Qyl.OpenTelemetry.AutoInstrumentation.WebApiAotDemo/Program.cs index 016c776f..3da041e8 100644 --- a/tools/Qyl.OpenTelemetry.AutoInstrumentation.WebApiAotDemo/Program.cs +++ b/tools/Qyl.OpenTelemetry.AutoInstrumentation.WebApiAotDemo/Program.cs @@ -148,8 +148,11 @@ public static WebApiAotReport Create(CapturedActivity[] activities) var failures = new List(); var signals = new List(); + // Server span is owned by the generated-middleware lane (priority 90) which wins over the + // DiagnosticListener lane (70) via QylSignalOwnership, so exactly one server span is emitted and + // it carries the aspnetcore.server domain (with the route backfilled after routing). AddRequired(signals, failures, "aspnetcore.server", activities.FirstOrDefault(static activity => - HasTag(activity, "qyl.instrumentation.domain", "http.server") && + HasTag(activity, "qyl.instrumentation.domain", "aspnetcore.server") && HasTag(activity, "http.route", "/probe/{id:int}"))); AddRequired(signals, failures, "httpclient.self", activities.FirstOrDefault(static activity => diff --git a/tools/Qyl.OpenTelemetry.AutoInstrumentation.WebApiAotDemo/verified/report.json b/tools/Qyl.OpenTelemetry.AutoInstrumentation.WebApiAotDemo/verified/report.json index 45641b1e..51b23429 100644 --- a/tools/Qyl.OpenTelemetry.AutoInstrumentation.WebApiAotDemo/verified/report.json +++ b/tools/Qyl.OpenTelemetry.AutoInstrumentation.WebApiAotDemo/verified/report.json @@ -50,19 +50,6 @@ "server.port": "" } }, - { - "Kind": "Client", - "Name": "GET", - "Signal": "activity", - "Status": "Unset", - "Tags": { - "http.request.method": "GET", - "http.response.status_code": "204", - "qyl.instrumentation.domain": "http.client", - "server.address": "127.0.0.1", - "server.port": "" - } - }, { "Kind": "Client", "Name": "GET", @@ -87,18 +74,6 @@ "qyl.instrumentation.domain": "aspnetcore.server" } }, - { - "Kind": "Server", - "Name": "GET /probe/{id:int}", - "Signal": "activity", - "Status": "Unset", - "Tags": { - "http.request.method": "GET", - "http.response.status_code": "204", - "http.route": "/probe/{id:int}", - "qyl.instrumentation.domain": "http.server" - } - }, { "Kind": "Client", "Name": "SQL SELECT", @@ -126,7 +101,7 @@ "http.request.method": "GET", "http.response.status_code": "204", "http.route": "/probe/{id:int}", - "qyl.instrumentation.domain": "http.server" + "qyl.instrumentation.domain": "aspnetcore.server" } }, {