feat(usage-collector): add OpenTelemetry metrics/observability layer#4181
Merged
Artifizer merged 1 commit intoJul 10, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (28)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (19)
📝 WalkthroughWalkthroughThis PR adds usage-collector metrics plumbing end to end: config and domain metric contracts, PDP and SPI instrumentation, an OpenTelemetry adapter, module bootstrap and refresh handling, and supporting tests. Documentation is updated to use ChangesUsage Collector metrics implementation
Documentation Updates
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
capybutler
force-pushed
the
usage-collector/metrics
branch
3 times, most recently
from
1096d16 to
038aabd
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
gears/system/usage-collector/usage-collector/src/domain/test_support.rs (1)
648-814: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate the six near-identical metric-readback traversal loops.
counter_sum_with_label,histogram_count,histogram_count_with_label,histogram_sum,histogram_sum_with_label, andgauge_lasteach repeat the samefor rm in &metrics { for sm in rm.scope_metrics() { for metric in sm.metrics() { if metric.name() == name ... } } }traversal, differing only in theAggregatedMetrics/MetricDatamatch arm and the data-point projection. Extracting a shared "find metric by name" helper would cut ~150 lines of duplication and centralize a change if the exporter API shifts.♻️ Suggested consolidation
+fn find_metric<'a>( + metrics: &'a [opentelemetry_sdk::metrics::data::ResourceMetrics], + name: &str, +) -> Option<&'a AggregatedMetrics> { + metrics + .iter() + .flat_map(|rm| rm.scope_metrics()) + .flat_map(|sm| sm.metrics()) + .find(|m| m.name() == name) + .map(|m| m.data()) +} + pub fn counter_sum_with_label( exporter: &InMemoryMetricExporter, name: &str, label_key: &str, label_value: &str, ) -> u64 { let metrics = exporter.get_finished_metrics().expect("finished metrics"); - for rm in &metrics { - for sm in rm.scope_metrics() { - for metric in sm.metrics() { - if metric.name() == name - && let AggregatedMetrics::U64(MetricData::Sum(sum)) = metric.data() - { - return sum - .data_points() - .filter(|dp| { - dp.attributes().any(|kv| { - kv.key.as_str() == label_key && kv.value.as_str() == label_value - }) - }) - .map(opentelemetry_sdk::metrics::data::SumDataPoint::value) - .sum(); - } - } - } - } - 0 + let Some(AggregatedMetrics::U64(MetricData::Sum(sum))) = find_metric(&metrics, name) else { + return 0; + }; + sum.data_points() + .filter(|dp| { + dp.attributes() + .any(|kv| kv.key.as_str() == label_key && kv.value.as_str() == label_value) + }) + .map(opentelemetry_sdk::metrics::data::SumDataPoint::value) + .sum() }The remaining five functions follow the same pattern.
🤖 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 `@gears/system/usage-collector/usage-collector/src/domain/test_support.rs` around lines 648 - 814, The six readback helpers duplicate the same exporter traversal and metric-name lookup logic, so consolidate that shared search into a helper in test_support.rs and have counter_sum_with_label, histogram_count, histogram_count_with_label, histogram_sum, histogram_sum_with_label, and gauge_last reuse it. Keep the helper focused on finding the matching metric by name across finished metrics/scope metrics, then let each public function handle only its specific AggregatedMetrics/MetricData projection and aggregation.
🤖 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.
Nitpick comments:
In `@gears/system/usage-collector/usage-collector/src/domain/test_support.rs`:
- Around line 648-814: The six readback helpers duplicate the same exporter
traversal and metric-name lookup logic, so consolidate that shared search into a
helper in test_support.rs and have counter_sum_with_label, histogram_count,
histogram_count_with_label, histogram_sum, histogram_sum_with_label, and
gauge_last reuse it. Keep the helper focused on finding the matching metric by
name across finished metrics/scope metrics, then let each public function handle
only its specific AggregatedMetrics/MetricData projection and aggregation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e74d0ba2-63d8-4194-affe-13118586a444
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
gears/system/usage-collector/docs/ADR/0001-pdp-centric-authorization.mdgears/system/usage-collector/docs/DECOMPOSITION.mdgears/system/usage-collector/docs/DESIGN.mdgears/system/usage-collector/docs/PRD.mdgears/system/usage-collector/docs/features/event-deactivation.mdgears/system/usage-collector/docs/features/foundation.mdgears/system/usage-collector/docs/features/usage-emission.mdgears/system/usage-collector/docs/features/usage-query.mdgears/system/usage-collector/docs/features/usage-type-lifecycle.mdgears/system/usage-collector/docs/plugin-spi.mdgears/system/usage-collector/docs/sdk-trait.mdgears/system/usage-collector/usage-collector/Cargo.tomlgears/system/usage-collector/usage-collector/src/config.rsgears/system/usage-collector/usage-collector/src/config_tests.rsgears/system/usage-collector/usage-collector/src/domain/authz.rsgears/system/usage-collector/usage-collector/src/domain/authz_tests.rsgears/system/usage-collector/usage-collector/src/domain/local_client.rsgears/system/usage-collector/usage-collector/src/domain/mod.rsgears/system/usage-collector/usage-collector/src/domain/ports/metrics.rsgears/system/usage-collector/usage-collector/src/domain/ports/mod.rsgears/system/usage-collector/usage-collector/src/domain/service.rsgears/system/usage-collector/usage-collector/src/domain/service_metrics_tests.rsgears/system/usage-collector/usage-collector/src/domain/test_support.rsgears/system/usage-collector/usage-collector/src/infra/metrics.rsgears/system/usage-collector/usage-collector/src/infra/metrics_tests.rsgears/system/usage-collector/usage-collector/src/infra/mod.rsgears/system/usage-collector/usage-collector/src/module.rsgears/system/usage-collector/usage-collector/src/module_tests.rs
Artifizer
reviewed
Jul 9, 2026
Instruments the usage-collector gear with the operational metric inventory from DESIGN §3.11.5, pushed via OTLP through ToolKit's global SdkMeterProvider. Domain - Add the `UsageCollectorMetrics` port (domain/ports/metrics.rs) with a no-op default impl, plus typed, bounded label vocabularies (PdpOp, PluginOp, PdpFailureCause, AuthzDecision, PluginErrorCategory, RequestOutcome, RecordKind, QueryKind, …) so the `uc_*` instruments only ever carry closed-set label values. - Instrument the service across every flow: PDP decisions/failures and readiness, plugin call duration / accept errors and readiness, ingestion (batch size, duration, per-record and per-request outcomes, metadata bytes), query (inflight gauge, result rows, request outcomes), deactivation requests, and usage-type requests. Infra - Implement the port over `opentelemetry::global::meter_with_scope`, constructing the full `uc_*` instrument set at gear bootstrap. uc_usage_types catalog gauge - Refresh the `uc_usage_types` gauge from a 60s serve-lifecycle loop rather than per-mutation, paginating the whole catalog via `CursorV1` and summing item counts across pages. Bounded by the refresh timeout, a page-size limit, and a page-count safety cap; any SPI error, undecodable cursor, or cap breach leaves the gauge at its prior value (best-effort, never a partial-count publish). Supports multi-instance aggregation. Supporting changes - Add gear config for the metrics layer. - Promote tokio-util to a runtime dependency for the serve lifecycle. - Add domain, infra, and module tests (incl. a multi-page list_usage_types queue in HappyPathPlugin) and update DESIGN / feature docs. Signed-off-by: capybutler <capybutler@gmail.com>
capybutler
force-pushed
the
usage-collector/metrics
branch
from
038aabd to
9b03ef1
Compare
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
2 participants
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.
Adds an OpenTelemetry metrics/observability layer to the usage-collector gear,
implementing the operational metric inventory defined in DESIGN §3.11.5. Metrics are
exported via OTLP through ToolKit's global
SdkMeterProvider.Domain
UsageCollectorMetricsport (domain/ports/metrics.rs) with a no-op defaultimplementation, plus typed, bounded label vocabularies (
PdpOp,PluginOp,PdpFailureCause,AuthzDecision,PluginErrorCategory,RequestOutcome,RecordKind,QueryKind, …) so everyuc_*instrument only carries closed-setlabel values.
plugin call duration / accept errors and readiness, ingestion (batch size, duration,
per-record and per-request outcomes, metadata bytes), query (inflight gauge, result
rows, request outcomes), deactivation requests, and usage-type requests.
Infra
opentelemetry::global::meter_with_scope, building the fulluc_*instrument set at gear bootstrap.uc_usage_typescatalog gaugewhole catalog via
CursorV1and summing item counts across pages. Bounded by arefresh timeout, page-size limit, and page-count safety cap; any SPI error,
undecodable cursor, or cap breach leaves the gauge at its prior value (best-effort,
never a partial-count publish). Supports multi-instance aggregation.
Supporting changes
tokio-utilpromoted to a runtime dependency for the serve lifecycle.list_usage_typesqueue in
HappyPathPlugin); DESIGN and feature docs updated.Type of Change
Testing
Documentation
Checklist
cargo clippy)cargo fmt)cargo test)Summary by CodeRabbit
uc) and full OTLP-based operational telemetry.uc_*metric inventory.