fix(otel): semconv completeness — url.scheme + method_original + Azure op (CODE RED #6/#7/#9) - #25
Merged
Merged
Conversation
…ginal, Azure operation (CODE RED #6/#7/#9) #6 (url.scheme, Required on HTTP server spans): StartServerActivity now sets url.scheme from context.Request.Scheme (threaded through QylInterceptedAspNetCore). Added to the AOT demo keep-list so the fixture asserts it. #7 (http.request.method_original): the interceptor client + server paths dropped the original method when Normalize collapsed a non-standard verb to _OTHER. New QylHttpMethod.Normalize(method, out original) returns the raw value for non-standard methods; SetRequestMethod emits http.request.method_original. Threaded through StartClientActivity / StartServerActivity and all three callers (HttpClient, HttpWebRequest, AspNetCore). #9 (Azure operation): QylInterceptedAzure.StartActivity received methodName and dropped it (every Azure span named the fixed "Azure SDK"). Now records it on code.function.name so spans are distinguishable (span name stays low-cardinality per Azure semconv). New public consts QylSemanticAttributes.UrlScheme + CodeFunctionName (PublicAPI.Unshipped updated). Verified: core 0/0; verify-webapi-aot-demo (fixture now asserts url.scheme=http on the server span), source-interceptor-consumer, public-api-baseline all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Addresses “CODE RED” semantic-convention completeness gaps by ensuring server spans always include url.scheme, threading http.request.method_original through HTTP client/server interceptor paths for non-standard verbs, and restoring the intercepted Azure method onto a stable semantic attribute so Azure spans remain distinguishable without changing span names.
Changes:
- Add
url.schemetagging for ASP.NET Core server spans and update the WebApi AOT demo verification outputs accordingly. - Thread
http.request.method_originalfrom method normalization into both HTTP client and ASP.NET Core server activity creation. - Record intercepted Azure operation on
code.function.nameand expose new public semantic attribute constants (UrlScheme,CodeFunctionName).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/Qyl.OpenTelemetry.AutoInstrumentation.WebApiAotDemo/verified/report.json | Updates verified output to include url.scheme on server spans. |
| tools/Qyl.OpenTelemetry.AutoInstrumentation.WebApiAotDemo/Program.cs | Extends canonicalization key set to account for url.scheme in demo verification. |
| src/Qyl.OpenTelemetry.AutoInstrumentation/QylSemanticAttributes.cs | Adds new public constants for url.scheme and code.function.name. |
| src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedHttpWebRequest.cs | Threads method_original into HTTP WebRequest client span creation. |
| src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedHttpClient.cs | Threads method_original into HttpClient client span creation. |
| src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedAzure.cs | Restores recording of intercepted Azure method on code.function.name. |
| src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedAspNetCore.cs | Passes scheme + method original into server span creation. |
| src/Qyl.OpenTelemetry.AutoInstrumentation/QylHttpMethod.cs | Adds Normalize(..., out original) overload for non-standard method reporting. |
| src/Qyl.OpenTelemetry.AutoInstrumentation/PublicAPI.Unshipped.txt | Updates public API baseline for the newly added constants. |
| src/Qyl.OpenTelemetry.AutoInstrumentation/Internal/QylHttpActivityPolicy.cs | Adds url.scheme tagging on server spans and http.request.method_original tagging on both client/server spans. |
Comment on lines
+40
to
+44
| /// <summary> | ||
| /// Normalizes <paramref name="method"/> and reports the raw value as <paramref name="original"/> when it | ||
| /// is non-standard. Per OTel, http.request.method_original MUST be set whenever http.request.method is | ||
| /// <c>_OTHER</c>; <paramref name="original"/> is null for the standard methods. | ||
| /// </summary> |
ANcpLua
added a commit
that referenced
this pull request
Jul 1, 2026
…v changes PR #23/#25 changed the aspnetcore server-span shape on main (qyl.instrumentation.domain http.server -> aspnetcore.server; new url.scheme attribute) without regenerating this fixture, and main's own verify runs are still queued — so any PR merge-commit run fails 'otlp verified fixtures'. Regenerated via verify-otlp-fixtures.py --update-verified; the diff is exactly those two attribute changes, unrelated to the descriptor refactor (generator snapshots byte-identical). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ANcpLua
added a commit
that referenced
this pull request
Jul 2, 2026
…elf-referential validation apparatus (#28) * fix(generator): throw on unknown DB instrumentation id in GetDbTraceContractKey The default arm silently mapped any unlisted instrumentationId to signals.traces.ADONET — a false trace-contract identity, inconsistent with the sibling GetDbMetricContractKeys (empty default) and the codebase-wide 'unknown -> throw' invariant. All 7 ids GetDbInstrumentationId can return are enumerated explicitly, so the default is unreachable today; this makes a future unlisted provider fail loudly instead of misfiling. Verified: full solution build green (0/0); AspNetCore + ILogger runtime interception verifiers still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(tcg): surface interceptor receiver types in the Telemetry Capability Graph InterceptorMatcherDescriptor.ReceiverTypePattern was a curated 25-row registry (incl. wildcard/pipe patterns like 'Azure.*Client' and the Kafka/MassTransit/Elastic unions) that nothing read — dead metadata. Rather than delete a curated column that is not recoverable from the matcher delegates, make it live: GetInterceptorReceiverSurface() reads it and the Telemetry Capability Graph now emits an 'interceptorReceivers' section, giving consumers a machine-readable map of the exact receiver surface qyl intercepts. Verified: solution build green (0/0); generator-snapshots-ok; tcg-publishing-demo-ok. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: task-state anchor for descriptor-metadata root fix Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(generator): body-descriptor hierarchy replaces policy metadata (cut 1/4) Delete the five validation-only enums (EmitterFamily, MethodShape, SignalOwnership, ErrorPolicy, DurationPolicy) — every read of them was inside validators that checked static data against redundant copies of itself. The 8 body descriptors become a closed sealed-record hierarchy under InterceptorBodyDescriptor; exactly-one-body is now structural. Matcher descriptor: 8 ctors -> 2, declaration metadata dropped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(generator): make descriptor invariants structural, delete the validation apparatus (cuts 2-4) Every read of TargetKindMask, matcher ContractKeys, matcher/emission Family and MethodShape, the three policy enums, and the target's three Matcher* fields was inside validators comparing static data against a redundant copy of itself. With the body hierarchy from cut 1: - emission catalog rows shrink to (Kind, Body); exactly-one-body and policy consistency are unrepresentable as errors, not runtime-checked - matcher rows shrink to (Name, ReceiverTypePattern, TryMatch) - emitter dispatch is a type switch on the body descriptor - ValidateDescriptorCatalog, Ensure* trio, ValidateEmissionDescriptorPolicy, ValidateSingleBodyDescriptor, ValidateMethodShape x2, ValidatePolicy and the Initialize()-time throw (the CS8785/TreatWarningsAsErrors build-break vector) are deleted, not relocated - GetDbTraceContractKey was 'signals.traces.' + id re-enumerated as a switch; the id set now lives only in GetDbInstrumentationId and the contract key is derived, so the unreachable default arm (silent-ADONET before 544ee8d, throw after) is gone entirely - InterceptorKinds()/GetInterceptorKindMask bitmask machinery deleted Verified: solution build 0/0 (TWAE), generator-snapshots-ok (byte-identical emitted source), real-aspnetcore-demo-ok, real-ilogger-demo-ok. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(invariants): verify the structural descriptor model, drop redundant-representation checks The python harness pinned the deleted validators as required tokens and re-derived SignalOwnership from target contract keys only to compare it against the declared enum — consistency checks between two encodings of one fact. Now that one encoding remains: - new parse_emission_descriptor_bodies: every InterceptorKind maps to exactly one typed body row, duplicates fail (the one real invariant ValidateDescriptorCatalog carried, now test-time instead of consumer build-time) - emitter dispatch check follows the body-type switch - DB trace contract keys derive from GetDbInstrumentationId (single source) instead of the deleted GetDbTraceContractKey switch - ownership/policy-matrix checks deleted with the enums they mirrored Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: tick task-state checklist (implementation verified) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(fixtures): update webapi-aot OTLP trace fixture for main's semconv changes PR #23/#25 changed the aspnetcore server-span shape on main (qyl.instrumentation.domain http.server -> aspnetcore.server; new url.scheme attribute) without regenerating this fixture, and main's own verify runs are still queued — so any PR merge-commit run fails 'otlp verified fixtures'. Regenerated via verify-otlp-fixtures.py --update-verified; the diff is exactly those two attribute changes, unrelated to the descriptor refactor (generator snapshots byte-identical). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(tcg): publish the NServiceBus receiver surface the matcher actually accepts IsNServiceBusEndpointType matches IMessageSession | IMessageHandlerContext; the curated pattern still claimed IMessageSession | IEndpointInstance | IPipelineContext from an earlier design. Now that GetInterceptorReceiverSurface serializes the pattern into the Telemetry Capability Graph, the string must tell the truth. All other 24 patterns spot-checked against their matchers - only NServiceBus drifted. Also MD022 blank lines in the task file. Reported-by: coderabbitai Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(generator)!: delete the unreachable AspNetCoreRequestDelegate interceptor path; truthful Redis receiver surface TryGetAspNetCoreRequestDelegateInvocation required MethodKind.Ordinary on RequestDelegate.Invoke — but a delegate's Invoke is always MethodKind.DelegateInvoke, so the matcher has been unmatchable by construction since 770a897 added the guard. Server-span coverage comes from the IStartupFilter middleware (d31d94f); the generator path was dead weight advertised as live surface in the TCG. Deleted: matcher row, emission row, detection method, enum member. The runtime QylInterceptedAspNetCore.InvokeAsync helper stays (startup filter uses it). Redis: the matcher gates StackExchange.Redis.IDatabaseAsync; publish that instead of the narrower IDatabase. Verified: build 0/0, generator-snapshots-ok (byte-identical), contract-invariants-ok, aspnetcore-middleware-delegate-ok, tcg-publishing-demo-ok, real-aspnetcore-demo-ok. Reported-by: coderabbitai (receiver-surface truthfulness) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: drop the task-state file — both tracked tasks verified complete The Build()-interceptor/IStartupFilter rewire (PR #20) is merged, released (v4.0.x on the feed), and pinned by qyl at 4.0.0; its two follow-up gaps were closed by later commits (route backfill after next() in QylInterceptedAspNetCore.RecordResponse, single-owner-per-signal registry in PR #23). The descriptor-metadata root fix is this PR — documented by the PR itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: CLAUDE.md becomes the single agent-rules file; truthful server-span combination docs - AGENTS.md deleted, CLAUDE.md is now the real file (was a symlink) — this repo's agent rules are consumed via CLAUDE.md only - rules updated to current tree: structural descriptor model + invariants routed to verify-contract-invariants.py; no-callsite-arbitration invariant (the deleted Build()-interceptor coordination protocol stays deleted); publish vs build version roles stated per version-sync; note on not racing local verifier runs against CI on the shared self-hosted hosts - AddQylAspNetCoreInstrumentation: remark predated the single-owner signal registry (PR #23) — combining with .Hosting is safe (listener lane defers), say so; document IStartupFilter registration-order contract (server span stays outermost) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(tcg): point the generated-files rule reference at CLAUDE.md Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CODE RED semconv MEDIUMs. #6 url.scheme on server spans (Required); #7 http.request.method_original threaded through the interceptor client+server paths for non-standard verbs; #9 Azure span records the intercepted method on code.function.name (was dropped). New public consts UrlScheme + CodeFunctionName. Verified: core 0/0, AOT demo (asserts url.scheme=http), source-interceptor-consumer, public-api-baseline pass.
🤖 Generated with Claude Code