OTLP: Support including scope metadata as metric labels #16730
Conversation
7b47dc6 to
00598c5
Compare
c01b2c5 to
af944bb
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds optional support for including OTel scope metadata (name, version, schema URL, attributes) as Prometheus metric labels in the OTLP remote write translation pipeline.
Key changes:
- Introduce a
ConvertScopeMetadataflag in OTLP settings and config, propagating it through the write handler and converter. - Add a
scopetype and thread it through all converter methods to inject scope labels increateAttributes. - Update tests and documentation to cover scope metadata conversion for gauge, sum, and histogram metrics.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| storage/remote/write_handler.go | Wire new ConvertScopeMetadata option into exporter settings |
| storage/remote/otlptranslator/prometheusremotewrite/*.go | Add scope parameter to converter methods and inject scope labels via createAttributes |
| storage/remote/otlptranslator/prometheusremotewrite/helper.go | Update createAttributes to preallocate and append scope labels |
| storage/remote/otlptranslator/prometheusremotewrite/*_test.go | Extend tests for gauge, sum, and histogram conversion with and without scope metadata |
| docs/configuration/configuration.md | Document new convert_scope_metadata config flag |
| config/config.go, config/config_test.go, config/testdata/*.yml | Add parsing and test for new OTLP config flag |
| CHANGELOG.md | Document feature under unreleased section |
Comments suppressed due to low confidence (3)
storage/remote/otlptranslator/prometheusremotewrite/number_data_points_test.go:149
- Only the basic sum case is tested with
convertScope: true; recommend adding test cases forconvertScope: truein the exemplars, monotonic and non-monotonic sum scenarios to ensure full coverage.
tests := []struct {
storage/remote/otlptranslator/prometheusremotewrite/metrics_to_prw.go:95
- [nitpick] The unexported type name
scopeis very generic and can be confused with other concepts; consider renaming it to something more descriptive likescopeMetadataorotelScope.
type scope struct {
storage/remote/otlptranslator/prometheusremotewrite/helper.go:118
- [nitpick] Passing
scopethrough every converter method increases signature complexity; consider bundling scope metadata into the converter struct orSettingsto simplify method signatures.
func createAttributes(resource pcommon.Resource, attributes pcommon.Map, scope scope, settings Settings,
storage/remote/otlptranslator/prometheusremotewrite/number_data_points_test.go
Show resolved
Hide resolved
|
I think we can merge, as I've looked it over again and it checks out AFAICT. If there turns out to be any overlooked issues, we can address them in a new PR. |
|
I guess this should be ported to remote write exporter in the collector too right? |
@jmichalek132 Not really familiar with the OTel Collector side of things, but probably yes. Considering that OTel spec now stipulates that scope metadata (name/version/schema URL/attributes) should be part of Prometheus metric identity. |
|
Yes! |
Reverts #16730 and #16760 This is being done because we've noticed a problem in the spec that could lead to name collisions if attributes name, version or schema_url are added to the scope. They would collide with the already reserved labels otel_scope_name, otel_scope_version and otel_scope_schema_url. Since this new configuration option never made it into a release, we can safely remove it from the 3.5 release. We'll sort this out for the 3.6 release Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
Add support to the OTLP endpoint for including scope metadata as metric labels.
Fixes #16557.