refactor(aspnetcore)!: drop Build() interceptor; inject middleware via IStartupFilter - #20
Merged
Merged
Conversation
…; inject middleware via IStartupFilter The package intercepted WebApplicationBuilder.Build() to add its ASP.NET Core server-request middleware. That call site is the only place a cooperating generator (qyl's ServiceDefaults) can substitute the built WebApplication, so the two collided (CS9153) and needed a cross-generator opt-out knob to coexist. Root fix: the middleware never needed the Build() call site. Register it via an IStartupFilter off a non-colliding seam, preserving the exact middleware semantics (request/response header capture + query-string recording) that the DiagnosticListener path does not perform. Deletes the entire coordination layer: - QylInterceptedAspNetCore.Build (compose wrapper) - the generator's AspNetCoreWebApplicationBuilderBuild interceptor + dead BuilderInitialization shape - the QylAutoInstrumentationInterceptWebApplicationBuilderBuild opt-out (CompilerVisibleProperty in both .targets + generator read path) - tools/verify-build-interceptor-optout.py + its orchestrator registration Adds: - QylAspNetCoreStartupFilter : IStartupFilter - IServiceCollection.AddQylAspNetCoreInstrumentation() (idempotent, TryAddEnumerable) BREAKING: removes public QylInterceptedAspNetCore.Build(WebApplicationBuilder). Consumers wanting middleware-based server spans call AddQylAspNetCoreInstrumentation(); the zero-config DiagnosticListener path (.Hosting) is unchanged. Build green (core.slnf, 0/0); generator-snapshots, public-api-baseline, aspnetcore-middleware-delegate, source-interceptor-consumer verifiers 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. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 56 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRemoves the source generator's interception of ChangesBuild interceptor removal and IStartupFilter replacement
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant App as Application Startup
participant DI as ServiceCollectionExtensions
participant Filter as QylAspNetCoreStartupFilter
participant Core as QylInterceptedAspNetCore
App->>DI: AddQylAspNetCoreInstrumentation(services)
DI->>DI: TryAddEnumerable(IStartupFilter, QylAspNetCoreStartupFilter)
App->>Filter: Configure(next)
Filter->>App: app.Use(requestDelegate)
Filter->>Core: InvokeAsync(next, context) on request
Filter->>App: next(app)
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreInstrumentationServiceCollectionExtensions.cs`:
- Around line 12-16: The ASP.NET Core middleware registration in
QylAspNetCoreInstrumentationServiceCollectionExtensions is still emitting
duplicate server spans when the .Hosting lane is active, because the current DI
deduplication only covers the startup filter and not the DiagnosticListener
path. Add a shared suppression mechanism or enablement marker that both the
StartupFilter-based registration and the Hosting/DiagnosticListener server-span
creation path can consult before starting spans, and use it in the
AddQylAspNetCoreInstrumentation and related startup-filter registration flow to
prevent the two lanes from being active together.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1df98aeb-2ccb-46ac-8de2-d6d04a66eb33
📒 Files selected for processing (13)
.claude/TASK.mdsrc/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylAutoInstrumentationGenerator.Descriptors.cssrc/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylAutoInstrumentationGenerator.Detection.cssrc/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylAutoInstrumentationGenerator.cssrc/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylGeneratedSourceInterceptorCatalog.cssrc/Qyl.OpenTelemetry.AutoInstrumentation/PublicAPI.Unshipped.txtsrc/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreInstrumentationServiceCollectionExtensions.cssrc/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreStartupFilter.cssrc/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedAspNetCore.cssrc/Qyl.OpenTelemetry.AutoInstrumentation/build/Qyl.OpenTelemetry.AutoInstrumentation.targetssrc/Qyl.OpenTelemetry.AutoInstrumentation/buildTransitive/Qyl.OpenTelemetry.AutoInstrumentation.targetstools/verify-aot-autoinstrumentation-goal.pytools/verify-build-interceptor-optout.py
💤 Files with no reviewable changes (8)
- tools/verify-aot-autoinstrumentation-goal.py
- src/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylGeneratedSourceInterceptorCatalog.cs
- src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedAspNetCore.cs
- src/Qyl.OpenTelemetry.AutoInstrumentation/buildTransitive/Qyl.OpenTelemetry.AutoInstrumentation.targets
- src/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylAutoInstrumentationGenerator.Descriptors.cs
- tools/verify-build-interceptor-optout.py
- src/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylAutoInstrumentationGenerator.Detection.cs
- src/Qyl.OpenTelemetry.AutoInstrumentation/build/Qyl.OpenTelemetry.AutoInstrumentation.targets
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**
⚙️ CodeRabbit configuration file
AGENTS.md
**: # Qyl.OpenTelemetry.AutoInstrumentation agent rulesMission
This repository is the runtime AOT auto-instrumentation lane for qyl, evolving into a
self-describing observability substrate. The foundation is unchanged: .NET 10
NativeAOT-compatible zero-code instrumentation through managed build assets, source generation,
DiagnosticListener consumption, and module-initializer boot. The direction is the North Star
below.Keep this repository separate from:
- semantic-convention package generation (
Qyl.OpenTelemetry.SemanticConventionsis a referenced
vocabulary package, not generated here),- the old CLR-profiler/OpenTelemetry auto-instrumentation substrate.
North Star — declare and prove the whole stack
Every observability tool today is pull-by-observation: a backend learns what a service emits
by receiving samples over time, and never knows whether it has seen the whole surface. qyl has a
capability none of them have — because instrumentation is source-generated interceptors + a static
contract + a referenced semconv registry + (incrementally) DTO inference, the complete set of
telemetry a binary can ever produce is a compile-time-derivable fact, with provenance.The substrate goal: every qyl binary ships a complete, machine-readable Telemetry Capability
Graph (TCG) — the full possible OpenTelemetry surface for that exact binary, each capability
tagged compile-time-owned vs runtime-valued — and proves it by self-hosting (instrumenting its
own pipeline with its own mechanism, zero extra code). Any external entity consumes the TCG to know
the entire stack before a span is sampled. The contract becomes the shared semantic graph; an OTLP
backend is just one consumer.Three pillars:
- Self-host (the proof). qyl instruments qyl with qyl —
QylSelfTelemetry/
SemConvConformanceProcessorare the seed; the binary observing itself is how "declared TCG ==
runtime reality" is checked.- **Compile-time-complete ...
Files:
src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreStartupFilter.cssrc/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreInstrumentationServiceCollectionExtensions.cssrc/Qyl.OpenTelemetry.AutoInstrumentation/PublicAPI.Unshipped.txtsrc/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylAutoInstrumentationGenerator.cs
⚙️ CodeRabbit configuration file
**: Operating principles (solo-dev, agentic SDLC — reviews are advisory, agents act on them):
- LAZY: one self-contained, correct review beats ten partial ones. Every finding is
definitive — concrete evidence with file:line, a concrete fix, no "consider maybe",
no open or ambiguous questions back to the author. If you cannot decide a point
from the diff plus repo context, stay silent on it. Never cite a source, API, or
version you have not verified; an unverifiable claim is a dropped claim.- IMPATIENT: never stall a PR. There are no compatibility obligations here — internal
and dogfooding code has NO public-API contract; removing shims, breaking signatures,
and deleting dead paths are normal, desirable changes. Do not flag backward
compatibility, deprecation ceremony, or migration paths. (SemVer applies only to
commercially sold libraries — this repo has none.)- EGO: hold the bar of the best reviewer on the market — flag real correctness,
security, data-loss, and structural problems precisely; produce zero noise.
Files:
src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreStartupFilter.cssrc/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreInstrumentationServiceCollectionExtensions.cssrc/Qyl.OpenTelemetry.AutoInstrumentation/PublicAPI.Unshipped.txtsrc/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylAutoInstrumentationGenerator.cs
src/**/*.cs
⚙️ CodeRabbit configuration file
src/**/*.cs: Zero-code instrumentation runtime: this code runs inside EVERY request of host
applications. Top priorities, in order: (1) allocations and boxing on hot paths —
flag closures, LINQ, params arrays, string concat in listener/semantic-tag code;
(2) tag cardinality — any attribute value that is unbounded (raw URLs, user input,
exception messages) explodes at scale; (3) Activity/Meter lifecycle — undisposed
listeners, leaked subscriptions, double-Start/Stop; (4) thread safety of shared
listener state. PublicAPI.Shipped/Unshipped.txt are analyzer-managed: edits must
come from the analyzer flow, and API breaks are fine (internal product, no
compatibility contract).
Files:
src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreStartupFilter.cssrc/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreInstrumentationServiceCollectionExtensions.cssrc/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylAutoInstrumentationGenerator.cs
**/PublicAPI.{Shipped,Unshipped}.txt
📄 CodeRabbit inference engine (AGENTS.md)
When public APIs change, update the
PublicAPI.Shipped.txtandPublicAPI.Unshipped.txtbaselines next to each packaged project.
Files:
src/Qyl.OpenTelemetry.AutoInstrumentation/PublicAPI.Unshipped.txt
🪛 markdownlint-cli2 (0.22.1)
.claude/TASK.md
[warning] 7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 15-15: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 24-24: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 35-35: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 43-43: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 48-48: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (1)
.claude/TASK.md (1)
1-50: LGTM!
Comment on lines
+12
to
+16
| /// Adds a middleware-based server span (via <see cref="IStartupFilter"/>) that captures request and | ||
| /// response headers plus the query string. Prefer this when you want the richer middleware attributes; | ||
| /// the zero-config <c>Qyl.OpenTelemetry.AutoInstrumentation.Hosting</c> module-init path produces server | ||
| /// spans via the ASP.NET Core <c>DiagnosticListener</c> instead and must not be combined with this one | ||
| /// (doing so would emit two server spans per request). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Coordinate this registration with the existing .Hosting ASP.NET Core lane.
Line 12-16 already documents the failure mode: when Qyl.OpenTelemetry.AutoInstrumentation.Hosting is active, this API produces two server spans per request. Line 29 only deduplicates the startup filter inside DI, so the new public path still ships with deterministic double-emission. Add a shared suppression marker/option that the ASP.NET Core DiagnosticListener path reads before starting server spans, or otherwise block this registration when that lane is enabled.
Also applies to: 26-30
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreInstrumentationServiceCollectionExtensions.cs`
around lines 12 - 16, The ASP.NET Core middleware registration in
QylAspNetCoreInstrumentationServiceCollectionExtensions is still emitting
duplicate server spans when the .Hosting lane is active, because the current DI
deduplication only covers the startup filter and not the DiagnosticListener
path. Add a shared suppression mechanism or enablement marker that both the
StartupFilter-based registration and the Hosting/DiagnosticListener server-span
creation path can consult before starting spans, and use it in the
AddQylAspNetCoreInstrumentation and related startup-filter registration flow to
prevent the two lanes from being active together.
There was a problem hiding this comment.
Pull request overview
This PR removes the WebApplicationBuilder.Build() source-interceptor seam (which could collide with other generators and trigger CS9153) and replaces the ASP.NET Core server-span middleware injection with an IStartupFilter + explicit IServiceCollection registration surface in the core runtime package.
Changes:
- Deleted the
Build()interceptor path end-to-end (runtime helper, generator matcher/emitter + method-shape, MSBuild opt-out property, and verifier). - Added
QylAspNetCoreStartupFilter : IStartupFilterandAddQylAspNetCoreInstrumentation()(idempotent viaTryAddEnumerable) to inject the middleware without touchingBuild(). - Updated public API baselines to record the breaking removal and the new registration API.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/verify-build-interceptor-optout.py | Deleted verifier made obsolete by removing the Build() interceptor opt-out property. |
| tools/verify-aot-autoinstrumentation-goal.py | Removed the now-deleted verifier from the repo verification orchestrator. |
| src/Qyl.OpenTelemetry.AutoInstrumentation/QylInterceptedAspNetCore.cs | Removed the public Build(WebApplicationBuilder) helper used by the former interceptor seam. |
| src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreStartupFilter.cs | Added an IStartupFilter to inject the existing request middleware without intercepting Build(). |
| src/Qyl.OpenTelemetry.AutoInstrumentation/QylAspNetCoreInstrumentationServiceCollectionExtensions.cs | Added AddQylAspNetCoreInstrumentation() registration surface for the startup filter. |
| src/Qyl.OpenTelemetry.AutoInstrumentation/PublicAPI.Unshipped.txt | Recorded the new extension API and the breaking removal of QylInterceptedAspNetCore.Build. |
| src/Qyl.OpenTelemetry.AutoInstrumentation/buildTransitive/Qyl.OpenTelemetry.AutoInstrumentation.targets | Removed CompilerVisibleProperty for the eliminated opt-out MSBuild coordination layer. |
| src/Qyl.OpenTelemetry.AutoInstrumentation/build/Qyl.OpenTelemetry.AutoInstrumentation.targets | Same as above for non-transitive build assets. |
| src/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylGeneratedSourceInterceptorCatalog.cs | Removed the Build matcher and emission descriptor from the generator catalog. |
| src/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylAutoInstrumentationGenerator.Detection.cs | Removed invocation detection for WebApplicationBuilder.Build(). |
| src/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylAutoInstrumentationGenerator.Descriptors.cs | Removed AspNetCoreWebApplicationBuilderBuild kind and BuilderInitialization method shape. |
| src/Qyl.OpenTelemetry.AutoInstrumentation.SourceGenerators/QylAutoInstrumentationGenerator.cs | Removed the opt-out property read + filtering logic and simplified the incremental pipeline. |
| .claude/TASK.md | Added an internal task/cut manifest describing the refactor and breaking-release plan. |
Comment on lines
+12
to
+16
| /// Adds a middleware-based server span (via <see cref="IStartupFilter"/>) that captures request and | ||
| /// response headers plus the query string. Prefer this when you want the richer middleware attributes; | ||
| /// the zero-config <c>Qyl.OpenTelemetry.AutoInstrumentation.Hosting</c> module-init path produces server | ||
| /// spans via the ASP.NET Core <c>DiagnosticListener</c> instead and must not be combined with this one | ||
| /// (doing so would emit two server spans per request). |
Comment on lines
+26
to
+31
| public static IServiceCollection AddQylAspNetCoreInstrumentation(this IServiceCollection services) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(services); | ||
| services.TryAddEnumerable(ServiceDescriptor.Singleton<IStartupFilter, QylAspNetCoreStartupFilter>()); | ||
| return services; | ||
| } |
…Filter middleware The server-span middleware, now injected via IStartupFilter, runs OUTSIDE WebApplication's auto-UseRouting — so at activity start the endpoint is not yet resolved and the route template is unknown (span "GET" instead of "GET /route", no http.route). The former Build() interceptor's app.Use ran after auto-UseRouting, so it saw the route. Fix: resolve the route after the pipeline runs (in RecordResponse) and backfill http.route + refine the span DisplayName. Position-independent and no-op on the per-endpoint interceptor path (route already set at start). Restores the exact WebApiAotDemo fixture (verify-webapi-aot-demo: ok, 0 warnings). Also register AddQylAspNetCoreInstrumentation() in the WebApiAotDemo, which relied on the removed Build() interceptor for its server span. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ANcpLua
added a commit
that referenced
this pull request
Jul 1, 2026
…ft, sync to v4.0.0 QylInstrumentation.Version was a hardcoded const "0.3.0-pre.1" stamped onto every emitted span/metric via QylActivitySource, while the package now ships as v4.0.0. Props floor said 3.0.2, README said 0.3.0-pre.1, and the OTLP verified fixture said 3.0.2 — four version stories, none matching what the runtime actually emitted. Root fix (single source of truth; reflection-free, per the contract-invariants gate): - New GenerateQylVersionInfo MSBuild target bakes <Version> into a generated internal const (QylVersionInfo.Version) that QylInstrumentation.Version references. No System.Reflection (banned in product code by verify-contract-invariants.py). Always regenerates, so a CI -p:Version override is honoured. const -> static readonly; PublicAPI baseline updated. - Directory.Build.props <Version> floor -> 4.0.0 (matches the released v4.0.0 tag); CI-owned. - README install examples -> 4.0.0. - CHANGELOG: added [4.0.0] (IStartupFilter refactor #20 + core.slnf #18); the version- derivation change sits under [Unreleased]. - Regenerated the OTLP verified fixture to 4.0.0 — it now matches the runtime scope version, which it never did before (fixture rendered 3.0.2 while the runtime const emitted 0.3.0-pre.1). Drift-proofing: - tools/verify-version-sync.py (in the no-Docker floor) asserts props floor >= latest v* tag, README examples == floor, and the version stays generated-const-derived (no literal). Negative-tested. Verified: the FULL no-Docker validation floor passes locally (contract invariants, release build, public API, version sync, generator snapshots, source interceptor, otlp verified fixtures, nativeaot consumer verified — all green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ANcpLua
added a commit
that referenced
this pull request
Jul 1, 2026
…ft, sync to v4.0.0 (#21) QylInstrumentation.Version was a hardcoded const "0.3.0-pre.1" stamped onto every emitted span/metric via QylActivitySource, while the package now ships as v4.0.0. Props floor said 3.0.2, README said 0.3.0-pre.1, and the OTLP verified fixture said 3.0.2 — four version stories, none matching what the runtime actually emitted. Root fix (single source of truth; reflection-free, per the contract-invariants gate): - New GenerateQylVersionInfo MSBuild target bakes <Version> into a generated internal const (QylVersionInfo.Version) that QylInstrumentation.Version references. No System.Reflection (banned in product code by verify-contract-invariants.py). Always regenerates, so a CI -p:Version override is honoured. const -> static readonly; PublicAPI baseline updated. - Directory.Build.props <Version> floor -> 4.0.0 (matches the released v4.0.0 tag); CI-owned. - README install examples -> 4.0.0. - CHANGELOG: added [4.0.0] (IStartupFilter refactor #20 + core.slnf #18); the version- derivation change sits under [Unreleased]. - Regenerated the OTLP verified fixture to 4.0.0 — it now matches the runtime scope version, which it never did before (fixture rendered 3.0.2 while the runtime const emitted 0.3.0-pre.1). Drift-proofing: - tools/verify-version-sync.py (in the no-Docker floor) asserts props floor >= latest v* tag, README examples == floor, and the version stays generated-const-derived (no literal). Negative-tested. Verified: the FULL no-Docker validation floor passes locally (contract invariants, release build, public API, version sync, generator snapshots, source interceptor, otlp verified fixtures, nativeaot consumer verified — all green). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ANcpLua
added a commit
that referenced
this pull request
Jul 1, 2026
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>
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>
ANcpLua
added a commit
that referenced
this pull request
Jul 2, 2026
…directing artifacts (#30) The gate's --artifacts-path isolation broke every demo on a fresh workspace (22x CS0006 on both OS in run 28594231868): under PublishAot the demos swap their generator ProjectReference for a literal artifacts/bin/...SourceGenerators/release Analyzer path, so redirecting build output guarantees the DLL is never where the compiler looks. The prior local verification passed only against a stale pre-populated artifacts/ tree — the exact shared-state class the flag was meant to fix. Rewritten, no fallback: drop the redirect entirely, prebuild the generator into the standard artifacts layout before the demo loop (the contract every other workflow satisfies incidentally via full-solution builds), keep --disable-build-servers (the real cross-job leak). Verified on a virgin clone: prebuild -> Qyl.RealHttpClientDemo PASS warning-clean. Also drop .claude/TASK-aot-publish-gate.md: task complete, and it pinned a stale claim (TASK.md as active WIP; that task shipped in #20). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 6, 2026
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.
Root fix for the double
Build()interceptor (CS9153)The package intercepted
WebApplicationBuilder.Build()to add its ASP.NET Core server-requestmiddleware. That call site is the only place a cooperating generator (qyl's ServiceDefaults) can
substitute the built
WebApplication, so the two collided (CS9153) and needed a cross-generatoropt-out MSBuild property to coexist.
The middleware never needed the
Build()call site. It's now injected via anIStartupFilteroff a non-colliding seam, preserving the exact middleware semantics (request/response header capture
Deleted (the whole coordination layer)
QylInterceptedAspNetCore.Buildcompose wrapperAspNetCoreWebApplicationBuilderBuildinterceptor + now-deadBuilderInitializationshapeQylAutoInstrumentationInterceptWebApplicationBuilderBuildopt-out (CompilerVisiblePropertyinboth
.targets+ the generator read path)tools/verify-build-interceptor-optout.py+ its orchestrator registrationAdded
QylAspNetCoreStartupFilter : IStartupFilterIServiceCollection.AddQylAspNetCoreInstrumentation()(idempotent viaTryAddEnumerable)Breaking
Removes public
QylInterceptedAspNetCore.Build(WebApplicationBuilder)→ major bump (4.0.0).Consumers wanting middleware-based server spans call
AddQylAspNetCoreInstrumentation(); thezero-config DiagnosticListener path (
.Hosting) is unchanged. Recorded inPublicAPI.Unshipped.txt.Verification
core.slnfRelease build: 0 warnings / 0 errorssource-interceptor-consumer
.Hosting/DiagnosticListener path, not the removed interceptor)Downstream
qylcollector PR follows after this indexes on nuget.org.🤖 Generated with Claude Code