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

feat: receiver/prometheusremotewrite - Implement body unmarshaling #35624

Merged

Conversation

ArthurSens
Copy link
Member

@ArthurSens ArthurSens commented Oct 4, 2024

Description

This PR builds on top of #35535 and #35565. We're now making sure we're able to unmarshal a remote write request, while also exercising the decompression that is made by OTel's confighttp.

@ArthurSens ArthurSens requested review from dashpole and a team as code owners October 4, 2024 19:39
@ArthurSens ArthurSens force-pushed the prwreceiver-bodyunmarshal branch 3 times, most recently from df1caad to cd0941f Compare October 4, 2024 19:53
@ArthurSens
Copy link
Member Author

If everything looks good, one strategy is to merge this PR instead of the other two. This way we get all commits in without needing to rebase several PRs.

@ArthurSens ArthurSens force-pushed the prwreceiver-bodyunmarshal branch 2 times, most recently from 890e8e6 to 64def58 Compare October 17, 2024 19:40
@dashpole
Copy link
Contributor

looks like this needs some go mod updates. The diff currently downgrades otel dependences

@ArthurSens ArthurSens force-pushed the prwreceiver-bodyunmarshal branch from d4346dc to 48cd5d6 Compare October 30, 2024 13:49
@ArthurSens
Copy link
Member Author

PR rebased!

@dashpole dashpole added ready to merge Code review completed; ready to merge by maintainers and removed ready to merge Code review completed; ready to merge by maintainers labels Oct 30, 2024
@dashpole
Copy link
Contributor

--- a/receiver/prometheusremotewritereceiver/go.mod
+++ b/receiver/prometheusremotewritereceiver/go.mod
@@ -77,7 +77,7 @@ require (
 	go.opentelemetry.io/collector/pdata/pprofile v0.112.0 // indirect
 	go.opentelemetry.io/collector/pipeline v0.112.0 // indirect
 	go.opentelemetry.io/collector/receiver/receiverprofiles v0.112.0 // indirect
-	go.opentelemetry.io/collector/semconv v0.105.0 // indirect
+	go.opentelemetry.io/collector/semconv v0.112.0 // indirect
 	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.56.0 // indirect
 	go.opentelemetry.io/otel v1.31.0 // indirect
 	go.opentelemetry.io/otel/metric v1.31.0 // indirect

@ArthurSens ArthurSens force-pushed the prwreceiver-bodyunmarshal branch from 48cd5d6 to 8d9219e Compare October 30, 2024 15:41
@ArthurSens
Copy link
Member Author

Fixed :)

@dashpole dashpole added the ready to merge Code review completed; ready to merge by maintainers label Oct 31, 2024
@bogdandrutu bogdandrutu merged commit 85fe1ba into open-telemetry:main Nov 2, 2024
168 checks passed
@github-actions github-actions bot added this to the next release milestone Nov 2, 2024
ArthurSens added a commit to ArthurSens/opentelemetry-collector-contrib that referenced this pull request Nov 4, 2024
…pen-telemetry#35624)

This PR builds on top of open-telemetry#35535 and open-telemetry#35565. We're now making sure we're
able to unmarshal a remote write request, while also exercising the
decompression that is made by OTel's confighttp.

Signed-off-by: Arthur Silva Sens <[email protected]>
MovieStoreGuy pushed a commit that referenced this pull request Dec 15, 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

---------

Signed-off-by: Arthur Silva Sens <[email protected]>
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
…pen-telemetry#35624)

#### Description
This PR builds on top of open-telemetry#35535 and open-telemetry#35565. We're now making sure we're
able to unmarshal a remote write request, while also exercising the
decompression that is made by OTel's confighttp.

Signed-off-by: Arthur Silva Sens <[email protected]>
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
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.

3 participants