Repository navigation
fix(version): bake scope version from build (no reflection), gate drift, sync to v4.0.0 - #21
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (6)**⚙️ CodeRabbit configuration file
Files:
⚙️ CodeRabbit configuration file
Files:
.github/workflows/nuget-publish.yml📄 CodeRabbit inference engine (AGENTS.md)
Files:
.github/workflows/**⚙️ CodeRabbit configuration file
Files:
**/*.csproj📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/PublicAPI.{Shipped,Unshipped}.txt📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/**/*.cs⚙️ CodeRabbit configuration file
Files:
🪛 ast-grep (0.44.0)tools/verify-version-sync.py[error] 49-54: Command coming from incoming request (subprocess-from-request) 🔇 Additional comments (7)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR switches version stamping to build-generated data, adds a sync verifier for props, README, and instrumentation source, and updates release/version references across build docs, workflow guidance, fixtures, and the changelog. ChangesVersion sync and release metadata
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Pull request overview
Aligns the runtime instrumentation-scope version (otel.scope.version) with the actual shipped package version by deriving it from build metadata, and adds a repository gate to prevent future drift between tags, props, and documentation. This fits the repo’s “version is CI-owned from v* tags” model by making runtime telemetry and docs follow the same source of truth.
Changes:
- Derive
QylInstrumentation.VersionfromAssemblyInformationalVersion(and update the public API baseline accordingly). - Add and wire a new
tools/verify-version-sync.pygate into the repo’s validation floor. - Sync version references across
Directory.Build.props, README install snippets, CHANGELOG, and NuGet publish workflow prompt text.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tools/verify-version-sync.py |
New drift-prevention gate comparing props floor, README snippets, and build-derived version behavior. |
tools/verify-otlp-collector-fixtures.py |
Updates commentary to reflect the new “props floor / CI overrides from tag” version model. |
tools/verify-aot-autoinstrumentation-goal.py |
Adds the new “version sync” gate to the orchestrated validation run. |
src/Qyl.OpenTelemetry.AutoInstrumentation/QylInstrumentation.cs |
Changes Version from a hardcoded const to build-derived runtime value. |
src/Qyl.OpenTelemetry.AutoInstrumentation/PublicAPI.Shipped.txt |
Updates baseline for the const → static readonly public API change. |
README.md |
Updates PackageReference examples to 3.1.2. |
Directory.Build.props |
Bumps the local-build version floor to 3.1.2 and documents CI override behavior. |
CHANGELOG.md |
Rewrites/structures release history and adds an [Unreleased] section plus dated releases. |
.github/workflows/nuget-publish.yml |
Updates the publish input description example version. |
| def check_props_floor(floor: str) -> None: | ||
| tag = latest_stable_tag() | ||
| if tag is None: | ||
| print(" - no stable v* tag reachable; skipping tag-floor comparison") | ||
| return | ||
| if semver(floor) < tag: | ||
| tag_text = ".".join(map(str, tag)) | ||
| fail( | ||
| f"Directory.Build.props <Version> ({floor}) is BEHIND the latest release tag " | ||
| f"v{tag_text}. Bump the floor to at least {tag_text}." | ||
| ) | ||
| print(f" - props floor {floor} >= latest tag v{'.'.join(map(str, tag))}") | ||
|
|
| def check_version_is_derived() -> None: | ||
| text = INSTRUMENTATION.read_text(encoding="utf-8") | ||
| literal = re.search(r'\bVersion\s*=\s*"\d', text) | ||
| if literal: | ||
| fail( | ||
| "QylInstrumentation.Version is a hardcoded semver literal again. It must derive from the " | ||
| "build (AssemblyInformationalVersion) so the scope version cannot drift from the package." | ||
| ) | ||
| if "AssemblyInformationalVersionAttribute" not in text: | ||
| fail( | ||
| "QylInstrumentation no longer derives its version from AssemblyInformationalVersion; " | ||
| "the scope version would be uncontrolled." | ||
| ) | ||
| print(" - QylInstrumentation.Version is build-derived (not a literal)") | ||
|
|
| except (OSError, subprocess.CalledProcessError): | ||
| return None # No git / no tags (fresh checkout): tag-floor check is skipped. | ||
| for line in out.splitlines(): |
![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 `@tools/verify-version-sync.py`:
- Around line 48-76: The version-sync check currently returns None from
latest_stable_tag() on any git/tag lookup failure, causing check_props_floor()
to silently skip the floor comparison. Update tools/verify-version-sync.py so
missing reachable stable tags fail the gate instead of passing, and either make
the git tag lookup error fatal or ensure the workflow fetches tags before
running the check. Use latest_stable_tag() and check_props_floor() to locate the
logic, and keep the success path unchanged when a stable tag is actually found.
🪄 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: aa7f7b4c-1840-414b-bdfa-7b7f9f51e39f
📒 Files selected for processing (9)
.github/workflows/nuget-publish.ymlCHANGELOG.mdDirectory.Build.propsREADME.mdsrc/Qyl.OpenTelemetry.AutoInstrumentation/PublicAPI.Shipped.txtsrc/Qyl.OpenTelemetry.AutoInstrumentation/QylInstrumentation.cstools/verify-aot-autoinstrumentation-goal.pytools/verify-otlp-collector-fixtures.pytools/verify-version-sync.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**
⚙️ 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:
Directory.Build.propstools/verify-otlp-collector-fixtures.pysrc/Qyl.OpenTelemetry.AutoInstrumentation/PublicAPI.Shipped.txttools/verify-aot-autoinstrumentation-goal.pyREADME.mdsrc/Qyl.OpenTelemetry.AutoInstrumentation/QylInstrumentation.cstools/verify-version-sync.pyCHANGELOG.md
⚙️ 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:
Directory.Build.propstools/verify-otlp-collector-fixtures.pysrc/Qyl.OpenTelemetry.AutoInstrumentation/PublicAPI.Shipped.txttools/verify-aot-autoinstrumentation-goal.pyREADME.mdsrc/Qyl.OpenTelemetry.AutoInstrumentation/QylInstrumentation.cstools/verify-version-sync.pyCHANGELOG.md
.github/workflows/nuget-publish.yml
📄 CodeRabbit inference engine (AGENTS.md)
NuGet publishing must use keyless OIDC Trusted Publishing in
nuget-publish.yml; do not add aNUGET_API_KEYsecret, and keep the publish job on the GitHub-hostedubuntu-latestrunner with the required OIDC/ghsetup.
Files:
.github/workflows/nuget-publish.yml
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: Check event triggers, permissions, token scope, concurrency, and runtime cost —
smoketest/fixture workflows must stay bounded.
Files:
.github/workflows/nuget-publish.yml
**/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.Shipped.txt
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/QylInstrumentation.cs
🪛 ast-grep (0.44.0)
tools/verify-version-sync.py
[error] 49-54: Command coming from incoming request
Context: subprocess.run(
["git", "-C", str(ROOT), "tag", "--list", "v*", "--sort=-v:refname"],
capture_output=True,
text=True,
check=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🔇 Additional comments (8)
CHANGELOG.md (1)
3-6: LGTM!Also applies to: 8-10, 12-87, 90-101
src/Qyl.OpenTelemetry.AutoInstrumentation/QylInstrumentation.cs (1)
1-35: LGTM!src/Qyl.OpenTelemetry.AutoInstrumentation/PublicAPI.Shipped.txt (1)
99-99: LGTM!Directory.Build.props (1)
41-46: LGTM!README.md (1)
30-43: LGTM!.github/workflows/nuget-publish.yml (1)
27-27: LGTM!tools/verify-otlp-collector-fixtures.py (1)
128-134: LGTM!tools/verify-aot-autoinstrumentation-goal.py (1)
21-21: LGTM!
| def latest_stable_tag() -> tuple[int, int, int] | None: | ||
| try: | ||
| out = subprocess.run( | ||
| ["git", "-C", str(ROOT), "tag", "--list", "v*", "--sort=-v:refname"], | ||
| capture_output=True, | ||
| text=True, | ||
| check=True, | ||
| ).stdout | ||
| except (OSError, subprocess.CalledProcessError): | ||
| return None # No git / no tags (fresh checkout): tag-floor check is skipped. | ||
| for line in out.splitlines(): | ||
| m = STABLE_TAG.match(line.strip()) | ||
| if m: | ||
| return int(m[1]), int(m[2]), int(m[3]) | ||
| return None | ||
|
|
||
|
|
||
| def check_props_floor(floor: str) -> None: | ||
| tag = latest_stable_tag() | ||
| if tag is None: | ||
| print(" - no stable v* tag reachable; skipping tag-floor comparison") | ||
| return | ||
| if semver(floor) < tag: | ||
| tag_text = ".".join(map(str, tag)) | ||
| fail( | ||
| f"Directory.Build.props <Version> ({floor}) is BEHIND the latest release tag " | ||
| f"v{tag_text}. Bump the floor to at least {tag_text}." | ||
| ) | ||
| print(f" - props floor {floor} >= latest tag v{'.'.join(map(str, tag))}") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether workflows that run verify-aot-autoinstrumentation-goal.py / verify-version-sync.py
# fetch full history/tags, which the tag-floor comparison depends on.
rg -n 'checkout|fetch-depth|fetch-tags' .github/workflows/*.yml -A3 -B1Repository: ANcpLua/Qyl.OpenTelemetry.AutoInstrumentation
Length of output: 4235
🏁 Script executed:
#!/bin/bash
rg -n 'verify-version-sync\.py|version-sync|verify-aot-autoinstrumentation-goal\.py' . -A3 -B3Repository: ANcpLua/Qyl.OpenTelemetry.AutoInstrumentation
Length of output: 5807
🏁 Script executed:
#!/bin/bash
sed -n '1,220p' .github/workflows/verify.ymlRepository: ANcpLua/Qyl.OpenTelemetry.AutoInstrumentation
Length of output: 1452
Fail closed when no stable tag is reachable
tools/verify-version-sync.py:48-76 skips the floor check on any git/tag lookup failure, and .github/workflows/verify.yml:21-34 uses the default shallow checkout with no tag fetch. That lets the version-sync gate pass without comparing <Version> to the latest release tag. Make missing tags fatal here, or fetch tags before running the gate.
🧰 Tools
🪛 ast-grep (0.44.0)
[error] 49-54: Command coming from incoming request
Context: subprocess.run(
["git", "-C", str(ROOT), "tag", "--list", "v*", "--sort=-v:refname"],
capture_output=True,
text=True,
check=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🤖 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 `@tools/verify-version-sync.py` around lines 48 - 76, The version-sync check
currently returns None from latest_stable_tag() on any git/tag lookup failure,
causing check_props_floor() to silently skip the floor comparison. Update
tools/verify-version-sync.py so missing reachable stable tags fail the gate
instead of passing, and either make the git tag lookup error fatal or ensure the
workflow fetches tags before running the check. Use latest_stable_tag() and
check_props_floor() to locate the logic, and keep the success path unchanged
when a stable tag is actually found.
…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>
de95cbc to
7d6311e
Compare
ANcpLua
changed the title
…sion (#24) PR #21 (v4.0.0 sync) merged just before PR #19 (docs), so #19's `### Documentation` block landed under the `[3.0.2]` initial-release heading and still claimed the README install example was corrected to `3.1.2` — which #21 had already advanced to `4.0.0`. Relocate the two doc bullets into `[Unreleased]`, alongside #21's version-bake entry (both were post-4.0.0 same-day merges), and correct the stale `0.3.0-pre.1 -> 3.1.2` note to `-> 4.0.0` to match the actual README. CHANGELOG prose only; no source, README, or props change, so the verify-version-sync gate is unaffected. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ckfill v4.0.1-v4.0.3, version floor sync - SemanticConventions(+Incubating) 3.0.2 -> 3.4.0: six db.system.name well-known values repointed to the Incubating assembly (3.3.0 removed non-registry-stable enum members from the stable assembly). Wire-neutral, proven by the OTLP verified fixture: the only diff is the instrumentation-scope version stamp. - Version floor 4.0.0 -> 4.1.0 (+3 README examples) — verify-version-sync had been red since v4.0.1 was tagged without bumping the floor. - CHANGELOG: backfilled the missing v4.0.1/v4.0.2/v4.0.3 sections; the old [Unreleased] content (#19/#21) moved into v4.0.3 where it shipped. - Full no-Docker validation floor green incl. both OTLP fixture gates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem
QylInstrumentation.Versionwas a hardcodedconst "0.3.0-pre.1"— andQylActivitySourcestamps it as the instrumentation-scope version on every emitted span/metric. The package now
ships as v4.0.0, so
otel.scope.versionwas lying. Four sources disagreed: the const(
0.3.0-pre.1), the props floor (3.0.2), the README (0.3.0-pre.1), and even the OTLP verifiedfixture (
3.0.2) — and none matched what the runtime actually emitted.Root fix — single source of truth, reflection-free
GenerateQylVersionInfoMSBuild target bakes<Version>into a generated internalconst(
QylVersionInfo.Version) thatQylInstrumentation.Versionreferences. NoSystem.Reflection— it's banned in product code by
verify-contract-invariants.py(my first attempt used areflection read of the assembly's informational version; that's clean under the AOT analyzers but
fails that gate, so it's gone). The target always regenerates, so a CI
-p:Versionoverride ishonoured;
const → static readonly; PublicAPI baseline updated.Directory.Build.props <Version>floor → 4.0.0 (matches the releasedv4.0.0tag); CI-owned.4.0.0.[4.0.0](theIStartupFilterrefactor refactor(aspnetcore)!: drop Build() interceptor; inject middleware via IStartupFilter #20 +core.slnfAdd core.slnf — solution filter without demo projects #18); theversion-derivation change sits under
[Unreleased].4.0.0— it now matches the runtime scope version, whichit never did before (fixture rendered
3.0.2while the runtime const emitted0.3.0-pre.1).Drift-proofing
tools/verify-version-sync.py(wired into the no-Docker floor) asserts props floor ≥ latestv*tag, README examples == floor, and the version stays generated-const-derived (no literal).
Negative-tested.
Verification
Rebased onto the merged refactor (
d31d94f). The full no-Docker validation floor passes locally— the exact suite the CI
verifyjob runs: contract invariants, release build, package layout,public API, version sync, generator snapshots, source interceptor, otlp verified fixtures, and
nativeaot consumer verified — all green. The generated const bakes
4.0.0.🤖 Generated with Claude Code