-
Notifications
You must be signed in to change notification settings - Fork 633
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
wsgi: always record span status code to have it available in metrics #3148
wsgi: always record span status code to have it available in metrics #3148
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, would it be possible to write a test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked carefully and the overhead impact should be minimal except if metrics are dropped. For example, I'm not sure if we have an api or type-check for when metrics are not in use (Noop). In this case, we are parsing strings for no reason where we weren't before. Not sure there's anything to do about that.
Description
The same bug reported here
The histograms records are set without status_code attribute due to the decision is based on the span
is_recording
method.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
python = ">=3.9,<4"
flask-restx = "1.3.0"
flask = "^3.1.0"
opentelemetry-instrumentation-flask = "^0.50b0"
opentelemetry-api = "^1.29.0"
opentelemetry-sdk = "^1.29.0"
opentelemetry-instrumentation-wsgi = "^0.50b0"
opentelemetry-semantic-conventions = "^0.50b0"
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.