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

Change http.request.body.size and http.response.body.size attributes from recommended to opt-in #9799

Merged
merged 2 commits into from
Nov 2, 2023

Conversation

mateuszrzeszutek
Copy link
Member

@mateuszrzeszutek mateuszrzeszutek requested a review from a team November 2, 2023 15:44
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heads up, I think my proposed change open-telemetry/semantic-conventions#461 would result in deleting the code (but makes sense to keep it until then)

@mateuszrzeszutek
Copy link
Member Author

Hmm, the body size metrics too?
The only reason I introduced an experimental attributes extractor is to keep the experimental metrics working

@trask
Copy link
Member

trask commented Nov 2, 2023

Hmm, the body size metrics too?

I'm not sure, it's a good question, maybe we need *.header.content_length metrics, or maybe we could relax the metric definition to allow populating it just from content-length header

The only reason I introduced an experimental attributes extractor is to keep the experimental metrics working

👍

@mateuszrzeszutek
Copy link
Member Author

maybe we could relax the metric definition to allow populating it just from content-length header

Metric definition is one thing -- the limiting factor here is the OperationListener interface, actually. We can only produce metrics from Context and Attributes unfortunately

@trask trask merged commit d6c2aff into open-telemetry:main Nov 2, 2023
47 checks passed
@mateuszrzeszutek mateuszrzeszutek deleted the http-body-size branch November 6, 2023 07:32
Abhishekkr3003 pushed a commit to Abhishekkr3003/opentelemetry-java-instrumentation that referenced this pull request Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants