Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[receiver/prometheusremotewrite] Parse labels #35656

Merged
merged 10 commits into from
Dec 15, 2024

Conversation

ArthurSens
Copy link
Member

@ArthurSens ArthurSens commented Oct 7, 2024

Description

This PR builds on top of #35535, #35565 and #35624.

Here we're parsing labels into resource/metric attributes. It's still not great because resource attributes (with exception to service.namespace, service.name and service.name.id) are encoded into a special metric called target_info. Metrics related to specific target infos may arrive in separate write requests, so it may be impossible to build the full OTLP metric in a stateless way.

In this PR I'm ignoring this problem 😛, and transforming job and instance labels into resource attributes, while all other labels become scope attributes.

Please focus on the latest commit when reviewing this PR :)
1c9ff80

@ArthurSens ArthurSens requested review from dashpole and a team as code owners October 7, 2024 18:34
@ArthurSens ArthurSens force-pushed the prwreceiver-parselabels branch 4 times, most recently from 5f5d327 to b36ce75 Compare October 8, 2024 13:05
@dashpole
Copy link
Contributor

dashpole commented Oct 9, 2024

I don't think we should be putting metric labels into the resource attributes.

@ArthurSens
Copy link
Member Author

I don't think we should be putting metric labels into the resource attributes.

I was in doubt about this one. Since there are three layers of attributes, how do we know where labels should go when transformed into OpenTelemetry metrics?

@dashpole
Copy link
Contributor

I was in doubt about this one. Since there are three layers of attributes, how do we know where labels should go when transformed into OpenTelemetry metrics?

Job + Info become service.* labels. Other labels on prom metrics become labels on the otel metric. Labels on target_info become attributes on the resource. The tricky part is that target_info might end up in a different request? We can deal with that in a follow-up

@ArthurSens
Copy link
Member Author

The tricky part is that target_info might end up in a different request? We can deal with that in a follow-up

Oof, I honestly haven't thought about that. This makes this receiver a lot more difficult to write 😬

@ArthurSens ArthurSens force-pushed the prwreceiver-parselabels branch from b36ce75 to 17df302 Compare October 18, 2024 14:38
@ArthurSens ArthurSens force-pushed the prwreceiver-parselabels branch from 17df302 to 3854a92 Compare October 30, 2024 13:55
@ArthurSens ArthurSens marked this pull request as draft October 30, 2024 13:55
@ArthurSens ArthurSens force-pushed the prwreceiver-parselabels branch from 3854a92 to 1d30b01 Compare October 30, 2024 15:30
@ArthurSens ArthurSens changed the title [receiver/prometheusremotewrite] Parse labels into resource attributes [receiver/prometheusremotewrite] Parse labels Oct 30, 2024
@ArthurSens ArthurSens force-pushed the prwreceiver-parselabels branch from 1d30b01 to b59d07e Compare October 30, 2024 15:37
@ArthurSens ArthurSens marked this pull request as ready for review October 30, 2024 15:37
@ArthurSens
Copy link
Member Author

ArthurSens commented Oct 30, 2024

Alright, PR updated to only handle job and instance as resource attributes. All the rest becomes scope attributes (I couldn't find attributes in the metrics object 🤔).

@ArthurSens ArthurSens force-pushed the prwreceiver-parselabels branch from b59d07e to 1c9ff80 Compare October 30, 2024 15:39
@dashpole dashpole removed the Stale label Nov 26, 2024
@ArthurSens ArthurSens force-pushed the prwreceiver-parselabels branch from f862fe3 to 99e2d15 Compare November 29, 2024 16:34
@ArthurSens ArthurSens force-pushed the prwreceiver-parselabels branch from 99e2d15 to 0f5a554 Compare December 10, 2024 14:10
Signed-off-by: Arthur Silva Sens <[email protected]>
Signed-off-by: Arthur Silva Sens <[email protected]>
@andrzej-stencel andrzej-stencel removed their request for review December 11, 2024 10:21
Signed-off-by: Arthur Silva Sens <[email protected]>
@dashpole dashpole added ready to merge Code review completed; ready to merge by maintainers enhancement New feature or request and removed cmd/opampsupervisor connector/routing connector/datadog labels Dec 12, 2024
@MovieStoreGuy MovieStoreGuy merged commit 5717550 into open-telemetry:main Dec 15, 2024
182 of 183 checks passed
@github-actions github-actions bot added this to the next release milestone Dec 15, 2024
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
#### Description
This PR builds on top of
open-telemetry#35535,
open-telemetry#35565
and
open-telemetry#35624.

Here we're parsing labels into resource/metric attributes. It's still
not great because resource attributes (with exception to
`service.namespace`, `service.name` and `service.name.id`) are encoded
into a special metric called `target_info`. Metrics related to specific
target infos may arrive in separate write requests, so it may be
impossible to build the full OTLP metric in a stateless way.

In this PR I'm ignoring this problem 😛, and transforming `job` and
`instance` labels into resource attributes, while all other labels
become scope attributes.

Please focus on the latest commit when reviewing this PR :) 
1c9ff80

---------

Signed-off-by: Arthur Silva Sens <[email protected]>
mterhar pushed a commit to mterhar/opentelemetry-collector-contrib that referenced this pull request Dec 19, 2024
#### Description
This PR builds on top of
open-telemetry#35535,
open-telemetry#35565
and
open-telemetry#35624.

Here we're parsing labels into resource/metric attributes. It's still
not great because resource attributes (with exception to
`service.namespace`, `service.name` and `service.name.id`) are encoded
into a special metric called `target_info`. Metrics related to specific
target infos may arrive in separate write requests, so it may be
impossible to build the full OTLP metric in a stateless way.

In this PR I'm ignoring this problem 😛, and transforming `job` and
`instance` labels into resource attributes, while all other labels
become scope attributes.

Please focus on the latest commit when reviewing this PR :) 
1c9ff80

---------

Signed-off-by: Arthur Silva Sens <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready to merge Code review completed; ready to merge by maintainers receiver/prometheusremotewrite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants