-
Notifications
You must be signed in to change notification settings - Fork 897
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
Remove obsolete http.status_text
from semantic conventions
#972
Remove obsolete http.status_text
from semantic conventions
#972
Conversation
@@ -44,7 +44,7 @@ In the case of an HTTP redirect, the request should normally be considered succe | |||
unless the client aborts following redirects due to hitting some limit (redirect loop). | |||
If following a (chain of) redirect(s) successfully, the status should be set according to the result of the final HTTP request. | |||
|
|||
Don't set the span status description if the reason can be inferred from `http.status_code` and `http.status_text`. | |||
Don't set the span status description if the reason can be inferred from `http.status_code`. |
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.
I think we can leave this out completely.
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.
I think it's fine to mention that one should not set the reason phrase as status message.
I'm totally in favor of this change. Can we remove it from the HTTP metrics semantic convention at the same time, or should I open a separate issue for that? |
I think we should remove it in this PR actually. |
@justinfoote I've already done that, see https://github.com/open-telemetry/opentelemetry-specification/pull/972/files#diff-0a939af5f14b4209f713354478b76841. 🙂 |
😅 Sorry I missed that. Looks good to me then. |
Sufficient approvals on here and wide consensus on the issue to remove it, merging. |
Fixes #950.