[processor/resourcedetection] add internal telemetry - #49128
Conversation
17ab104 to
20dee6f
Compare
There was a problem hiding this comment.
Pull request overview
Adds internal telemetry for the resource_detection (resourcedetectionprocessor) detector execution path to support component stability requirements by exposing per-detector results and latency, plus a gauge of detected resource attribute count.
Changes:
- Instrument per-detector detection with
otelcol.resourcedetection.detector.results(counter) andotelcol.resourcedetection.detector.duration(histogram), labeled by{detector, outcome}. - Add an async gauge
otelcol.resourcedetection.attributes.detectedreporting the number of attributes in the currently cached detected resource. - Document and publish the metrics via
metadata.yaml+ generated documentation, and add unit tests validating emitted telemetry.
Reviewed changes
Copilot reviewed 8 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| processor/resourcedetectionprocessor/README.md | Links to internal telemetry documentation. |
| processor/resourcedetectionprocessor/metadata.yaml | Defines metric metadata and attribute enums for generated telemetry/docs. |
| processor/resourcedetectionprocessor/internal/resourcedetection.go | Implements metric recording per detector and registers async gauge callback; shuts down telemetry on stop. |
| processor/resourcedetectionprocessor/internal/resourcedetection_test.go | Adds tests validating success/failure metric emission and updates helpers for new provider signature. |
| processor/resourcedetectionprocessor/internal/metadatatest/generated_telemetrytest.go | Generated helpers to assert metric outputs in tests. |
| processor/resourcedetectionprocessor/internal/metadatatest/generated_telemetrytest_test.go | Generated test verifying telemetry setup and metric emission. |
| processor/resourcedetectionprocessor/internal/metadata/generated_telemetry.go | Generated telemetry builder and instruments for the new metrics. |
| processor/resourcedetectionprocessor/internal/metadata/generated_telemetry_test.go | Generated tests for providers and telemetry builder options. |
| processor/resourcedetectionprocessor/internal/benchmark_test.go | Updates benchmark to use new provider construction path. |
| processor/resourcedetectionprocessor/go.mod | Adds direct module deps required for metrics/tracing packages used by new code/tests. |
| processor/resourcedetectionprocessor/documentation.md | Generated internal telemetry documentation for the new metrics. |
| .chloggen/rdp_metrics.yaml | Changelog entry announcing the new internal telemetry metrics. |
Files not reviewed (4)
- processor/resourcedetectionprocessor/internal/metadata/generated_telemetry.go: Generated file
- processor/resourcedetectionprocessor/internal/metadata/generated_telemetry_test.go: Generated file
- processor/resourcedetectionprocessor/internal/metadatatest/generated_telemetrytest.go: Generated file
- processor/resourcedetectionprocessor/internal/metadatatest/generated_telemetrytest_test.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Paulo Dias <44772900+paulojmdias@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 12 changed files in this pull request and generated 4 comments.
Files not reviewed (4)
- processor/resourcedetectionprocessor/internal/metadata/generated_telemetry.go: Generated file
- processor/resourcedetectionprocessor/internal/metadata/generated_telemetry_test.go: Generated file
- processor/resourcedetectionprocessor/internal/metadatatest/generated_telemetrytest.go: Generated file
- processor/resourcedetectionprocessor/internal/metadatatest/generated_telemetrytest_test.go: Generated file
| error.type: | ||
| description: The type of error that caused detection to fail. Absent on success. | ||
| type: string | ||
| outcome: |
There was a problem hiding this comment.
At #44718 (comment), we used status="success|error".
Since the context looks similar, I wonder if we could standardise sth here.
There was a problem hiding this comment.
I'm happy to switch to status, but I think outcome is more prevalent from what I've seen. It's used in receiverhelper, kafkaexporter, kafkareceiver, and elasticsearchexporter. Let me know your preference.
There was a problem hiding this comment.
Shall we file an issue for this so we can come to an alignment across the Collector project? The decision should be documented at https://opentelemetry.io/docs/collector/internal-telemetry.
No need to block this PR though.
There was a problem hiding this comment.
I created #49630 to align outcome vs status for internal telemetry.
|
This PR has review comments. Review suggestions, whether from maintainers or automated reviewers, aren't always correct or required. Please evaluate each comment on its merits, then make sure each thread has a clear outcome. For example, link to the commit if you applied a suggestion, explain why it wasn't applied, or ask a follow-up question. Automation flags a PR for human review once every review thread has a reply or is marked as resolved. Status across open PRs is visible on the pull request dashboard. |
Description
Adds internal telemetry to the resourcedetection processor in support of promoting it to stable. Detection runs at startup/refresh (not per-item), so this instruments the detection operation rather than item throughput. The following three metrics are recorded:
Link to tracking issue
Fixes
#44595
Testing
Units, manual.
Documentation
Metrics added to metadata.yaml and the generated documentation.md is linked to from the README.
Authorship