feat(component-inventory): implement phase 2, 3 and 4 compiler checks and baseline compliance (#3435)#3562
Conversation
…C 0001 Phase 1)
Implements Phase 1 of RFC 0001 (component inventory): the attribute macro and
its runtime metadata surface, mirroring the existing #[capability] ->
KNOWN_CAPABILITIES mechanism. No components are annotated yet (Phase 2) and
there is no xtask command yet (Phase 3).
otap-df-engine (new `inventory` module):
- ComponentMeta { id, category, description, file, line, attributes } and a
#[linkme::distributed_slice] COMPONENT_INVENTORY populated at link time.
- Category enum, Phase 1 factory variants only: Receiver/Exporter/Processor/
Extension, each with a urn_segment() used for the macro's URN cross-check.
Non-factory categories (Admin/Controller/Cli/Subsystem/Safety) are deferred
to Phase 2 when those components are annotated and their synthetic-URN scheme
is settled with the SIG.
- attrs key constants (RFC "Option A"): free-form attribute map plus PORT/
PROTOCOL/AUTH/... key constants. Value validation ("Option C") is not
implemented in Phase 1. Per-signal stability is intentionally omitted (see
TODO(stability)): components may have zero or multiple signal types, so a
single stability field does not fit; revisit with the SIG if wanted.
otap-df-engine-macros (new #[component_inventory] proc macro):
- Re-emits the annotated item unchanged and appends one COMPONENT_INVENTORY
entry using fully-qualified ::otap_df_engine::inventory::* paths, so it can
be invoked from any node crate (unlike #[capability], which is engine-local).
- Factory case: `id` is derived from the factory static's `name` (URN) field,
so contributors write no id. Non-factory items require an explicit id.
- `category` is a validated bare identifier (a misspelling is a compile error);
when the URN is a string literal, category is cross-checked against the URN
segment. For const-path URNs the value is not visible at macro time, so the
full cross-check is deferred to the xtask scanner (Phase 3).
- Propagates the annotated item's #[cfg(...)] to the emitted entry.
Tests:
- Hand-rolled expansion unit tests (arg parsing, id derivation, cfg
propagation, URN cross-check).
- trybuild compile-fail UI tests (unknown/missing category, missing id,
URN/category mismatch).
- End-to-end test: annotate factory + non-factory items and read back
COMPONENT_INVENTORY, validating the cross-crate link-time path.
Relates to open-telemetry#3435.
The trybuild compile-fail UI tests failed in CI: the repo runs tests via
`cargo nextest` from a prebuilt archive in `--offline` mode, and trybuild
spawns a nested `cargo` build for its fixture crate that cannot resolve
dependencies offline ("no matching package named `proc-macro2` found"). No
other crate in the workspace uses trybuild.
Remove the trybuild harness, the tests/ui fixtures + .stderr snapshots, and the
trybuild dev-dependency. All four compile-fail paths (unknown/missing category,
missing id, URN/category mismatch) are already covered by the hand-rolled
expansion unit tests in component_inventory.rs, which assert on the generated
error text and are environment-independent. A comment documents why trybuild is
not used here.
This also resolves the license-header-check failure, which was flagging the
now-removed tests/ui/*.rs fixtures.
Per rust/otap-dataflow/AGENTS.md and CONTRIBUTING.md, every test must be documented immediately above its declaration with '/// Scenario:' and '/// Guarantees:' doc comments. Add them to all component_inventory macro unit tests and the e2e tests. Addresses GitLabDuo/Copilot review feedback on open-telemetry#3487.
… and baseline compliance (open-telemetry#3435)
Pull request dashboard statusStatus last refreshed: 2026-07-24 21:24:19 UTC.
This automated status or its linked feedback items may be incorrect. If something looks wrong, please report it with the result you expected. |
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (69.29%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3562 +/- ##
==========================================
- Coverage 86.91% 86.85% -0.07%
==========================================
Files 782 786 +4
Lines 319143 320156 +1013
==========================================
+ Hits 277389 278060 +671
- Misses 41230 41572 +342
Partials 524 524
🚀 New features to boost your workflow:
|
…ntory-phase2 # Conflicts: # .github/codecov.yaml
… link-time oracle
Make the RFC 0001 component-inventory drift check dependable by removing the
hand-rolled character scanner in favor of a real `syn` AST parse, a single
shared attribute parser, workspace-wide URN resolution, and a compiler-resolved
reliability oracle.
- Add crate `otap-df-engine-inventory-syntax` holding the one
`ComponentInventoryArgs` `Parse` impl + category validation. Both the
`#[component_inventory]` proc-macro and the xtask scanner use it, so the
macro and the tool can never disagree about the annotation grammar.
- Rewrite `xtask` `component-inventory` on `syn::parse_file`:
* Two passes over parsed ASTs: (1) build a workspace-wide `const NAME: &str`
table (indexed by identifier, so cross-crate `pub use`d URN consts resolve),
(2) extract annotated components and detect OTAP_* factory statics missing
the annotation via the item's real attribute list (no line-window heuristic).
* Unresolved URNs become a loud `urn:UNRESOLVED:<detail>` marker that fails
the check and is refused by `--update-baseline` -- never a silent value.
* `run_check` returns `Result` instead of `process::exit(1)`.
* UTF-8-safe table truncation (no byte-index slicing).
- Fix the frozen-wrong baseline entry: `INTERNAL_TELEMETRY_RECEIVER_URN` now
resolves to `urn:otel:receiver:internal_telemetry` (was
`urn:derived:unknown:...`, because its URN const is `pub use`d from
`otap-df-config`). Regenerated baseline; added trailing newline.
- Annotate `WASM_PROCESSOR_FACTORY` (urn:otel:processor:wasm_processor): a real
OTAP_* factory the old scanner missed. Baseline now has 45 components.
- Add `otap` link-time oracle test: iterates the compiler-resolved
`COMPONENT_INVENTORY` slice for all linked components and asserts each matches
`components-baseline.json`. This is the reliability half (correctness for the
linked subset); the syn scan owns completeness (sees cfg/feature/target-gated
components a single link build cannot).
- Add unit tests for URN resolution (same-file / cross-crate / unresolved),
factory detection, cfg-gated recording, missing-annotation, baseline diff,
and UTF-8 truncation; remove the codecov ignore for the scanner.
Relates to open-telemetry#3435.
| component: engine | ||
|
|
||
| # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
| note: Add the `#[component_inventory]` attribute macro and `inventory` module (RFC 0001, Phase 1) |
There was a problem hiding this comment.
Phase 1 or phases 2,3, and 4 as indicated in the PR title?
| @@ -0,0 +1,32 @@ | |||
| # Use this changelog template to create an entry for release notes. | |||
There was a problem hiding this comment.
Why having to change log entry into the same PR?
| @@ -0,0 +1,360 @@ | |||
| // Copyright The OpenTelemetry Authors | |||
There was a problem hiding this comment.
I was wondering why you didn’t name this folder component-inventory.
| pub const KNOWN_CATEGORIES: &[&str] = &[ | ||
| "Receiver", | ||
| "Exporter", | ||
| "Processor", | ||
| "Extension", | ||
| "Admin", | ||
| "Controller", | ||
| "Cli", | ||
| "Subsystem", | ||
| "Safety", | ||
| ]; |
There was a problem hiding this comment.
Why not represent these different categories as an enum?
Also, for variants such as Admin, Safety, and Subsystem, I think we should provide definitions that clarify when each one should be used. Another question I have (though this may be explained later) is whether a variant like Subsystem is a bit too much of a catch-all.
| //! # Accepted forms | ||
| //! | ||
| //! Factory `static` (the common case) -- `id` is derived from the factory's | ||
| //! `name` (URN) field, so the author writes no `id`: |
| /// synthetic-URN scheme for them is settled with the SIG. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| #[non_exhaustive] | ||
| pub enum Category { |
There was a problem hiding this comment.
See one of my previous comment, we should use this enum instead of the array of strings used in the other crate.
| // stability field does not fit. Revisit with the SIG if a component-level | ||
| // stability key is wanted later. | ||
| pub mod attrs { | ||
| /// Network port the component listens on or connects to (e.g. `"4317"`). |
There was a problem hiding this comment.
Should we distinguish between a listening port and a destination port?
Proposal:
listen_port/remote_portinbound_port/outbound_port
| /// Network port the component listens on or connects to (e.g. `"4317"`). | ||
| pub const PORT: &str = "port"; | ||
| /// Wire protocol (e.g. `"gRPC (HTTP/2)"`, `"HTTP"`). | ||
| pub const PROTOCOL: &str = "protocol"; |
There was a problem hiding this comment.
Should we codify these protocols somewhere? May be an enum that we can easily control and enrich as needed?
| /// Wire protocol (e.g. `"gRPC (HTTP/2)"`, `"HTTP"`). | ||
| pub const PROTOCOL: &str = "protocol"; | ||
| /// Authentication mechanism (e.g. `"mTLS (opt-in)"`, `"NONE"`). | ||
| pub const AUTH: &str = "auth"; |
| /// Authentication mechanism (e.g. `"mTLS (opt-in)"`, `"NONE"`). | ||
| pub const AUTH: &str = "auth"; | ||
| /// Whether/how the component accesses the local filesystem. | ||
| pub const FILESYSTEM_ACCESS: &str = "filesystem_access"; |
| /// Whether/how the component accesses the local filesystem. | ||
| pub const FILESYSTEM_ACCESS: &str = "filesystem_access"; | ||
| /// Cloud API the component talks to, if any. | ||
| pub const CLOUD_API: &str = "cloud_api"; |
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| //! Reliability oracle for the component inventory (RFC 0001). |
There was a problem hiding this comment.
Why having this module in the otap crate?
| 1. Link-Time Registration (The Proc-Macro): | ||
| When components are compiled, the #[component_inventory] macro generates | ||
| a static metadata block (ComponentMeta) and emits it into a unified link | ||
| section via the linkme crate. This guarantees that compiled binaries | ||
| carry a highly accurate inventory of exactly what feature/platform gates | ||
| are active. |
There was a problem hiding this comment.
Since this information is available in the final binary, I think the engine’s CLI should provide a way to display this list in both human-readable and JSON formats.
| 2. Source-Text Analysis (The AST Scanner): | ||
| In local development and CI (cargo xtask check), an offline scanner parses | ||
| the workspace .rs source code using the syn parser: | ||
| * Pass 1 (URN Resolution): It indexes all const NAME: &str = "..." | ||
| declarations workspace-wide to resolve factory name constants back to | ||
| their actual string values. | ||
| * Pass 2 (Verification): It maps all #[distributed_slice] factory | ||
| registries and verifies that each one has a matching, well-formed | ||
| #[component_inventory] annotation. |
There was a problem hiding this comment.
Have you measured the impact of this pass on the execution time of cargo xtask check?
|
|
||
| Use well-known attribute key constants from otap_df_engine::inventory::attrs: | ||
|
|
||
| | Key Constant | Description | Example Values | |
There was a problem hiding this comment.
In the last column, are we talking about "Example values" or "Default values" or something else?
| /// Directory names that are never scanned for components. | ||
| const SKIP_DIRS: &[&str] = &[ | ||
| "target", | ||
| ".git", | ||
| "tests", | ||
| "benches", | ||
| "examples", | ||
| "engine-macros", | ||
| "engine-inventory-syntax", | ||
| "telemetry-macros", | ||
| "validation", | ||
| "otap-test-net", | ||
| "otap-test-tls-certs", | ||
| "quiver-e2e", | ||
| ]; |
There was a problem hiding this comment.
Not urgent but at some these folders should be in a config file.
| } | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Pass 1: URN const table |
There was a problem hiding this comment.
Could you add to the PR description how long these two passes take on the current project?
lquerel
left a comment
There was a problem hiding this comment.
I’ve left a number of comments and requests that need to be addressed before we can merge, but I think this is heading in the right direction!
Change Summary
This PR implements the Component Inventory System (RFC 0001) for Phases 2, 3, and 4. It establishes a static-analysis source scanner, delta/baseline comparison mechanism, baseline definition, and strict verification checks inside the CI pipeline.
Core Changes:
cargo xtask component-inventorysubcommand: Created a fast, offline Rust code parser to inspect#[component_inventory]attributes and resolve factory URN constant identifiers in place of runtime lookups.cargo xtask checkandcargo xtask quick-checkdirectly after layout structure validation.#[otap_df_engine::component_inventory(category = ...)].components-baseline.json.What issue does this PR close?
How are these changes tested?
xtask/src/component_inventory.rsvalidating string unescaping, attribute map parsing, and parameter key-value extraction. All tests are documented withScenarioandGuaranteesrequirements.cargo xtask check.cargo clippy --workspace --all-targetsandcargo test.Are there any user-facing changes?
No (internal CI enforcement and build instrumentation checks).
Changelog
.chloggen/*.yamlentrychore(indicated in title)