Skip to content

feat(instrumentation): generate collector routing - #619

Open
johanpel wants to merge 2 commits into
rapidsai:mainfrom
johanpel:schema-generated-collector-integration
Open

feat(instrumentation): generate collector routing#619
johanpel wants to merge 2 commits into
rapidsai:mainfrom
johanpel:schema-generated-collector-integration

Conversation

@johanpel

@johanpel johanpel commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Description

Add schema-generated routing from collector entity stream names to typed instrumentation observers.

  • Introduce CollectorRouter and implement CollectorSink for generated model contexts.
  • Generate event deserialization and dispatch for each schema entity.
  • Expose collector wire-format serialization helpers.
  • Enable quent-events/serde directly in the collector client.

Testing

  • pixi run cargo fmt --all -- --check
  • pixi run cargo test -p quent-instrumentation-build
  • pixi run cargo check -p quent-instrumentation --features io-collector
  • pixi run cargo test -p quent-instrumentation --features io-collector --test collector_roundtrip
  • pixi run cargo test -p quent-collector-client
  • pixi run cargo test -p quent-collector -p quent-io-collector

Written by Codex.

@johanpel
johanpel force-pushed the schema-generated-collector-integration branch from 6eaca4e to f5e948a Compare August 26, 2026 12:03
@johanpel
johanpel marked this pull request as ready for review August 26, 2026 12:34
@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds shared collector event serialization and feature-gated collector sink support. Build options enable generated routing, which deserializes events by entity name and forwards them to matching observers.

Changes

Collector sink integration

Layer / File(s) Summary
Event serialization API
crates/collector/client/Cargo.toml, crates/collector/client/src/lib.rs, crates/collector/server/src/lib.rs
The collector client exposes serialize_event and uses it for normal processing and shutdown draining. The server re-exports the helper.
Generated collector routing
crates/instrumentation-build/src/lib.rs, crates/instrumentation-build/src/runtime/context.rs, crates/instrumentation-build/src/runtime/mod.rs
Build options enable collector sinks, reject configurations without serde, and pass the setting into generated models. Generated routing deserializes events, forwards recognized entities, and rejects unknown streams.
Instrumentation collector integration
crates/instrumentation/Cargo.toml, crates/instrumentation/src/collector.rs, crates/instrumentation/src/entity.rs, crates/instrumentation/src/lib.rs
The instrumentation crate enables the collector client, adds collector routing and sink APIs, restricts observer internals, and exposes feature-gated collector exports.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🟡 Moderate · up to f5e94

The new collector and instrumentation integrations bypass workspace-managed dependency declarations, which can allow dependency versions and features to drift between crates; update the manifests to use workspace dependencies before merging.

Suggested reviewers: cmatzenbach, 9prady9

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 8 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: generating collector routing for instrumentation.
Description check ✅ Passed The description explains the implementation and includes detailed testing commands. It omits the Related Issues and Screenshots sections, but these omissions are non-critical for this Rust change.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 8 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant