-
Notifications
You must be signed in to change notification settings - Fork 94
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(spans): Improve span data http method extraction #2396
Conversation
Check `http.request.method` and `method` fields when trying to extract `span.action` from an http span.
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.
The code looks good!
Adding new spans to this test and running it should update a couple of snapshots and the tag should be available in there.
Added snapshot tests with d0de357 I also changed |
d0de357
to
2144d6a
Compare
@@ -200,15 +200,18 @@ pub(crate) fn extract_tags(span: &Span, config: &Config) -> BTreeMap<SpanTagKey, | |||
.and_then(|data| data.get("description.scrubbed")) | |||
.and_then(|value| value.as_str()); | |||
|
|||
// TODO(iker): we're relying on the existance of `http.method` | |||
// TODO(iker): we're relying on the existance of `http.method`/`http.response.method` |
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.
Can we remove this TODO now and just put in the doc string the places we actually look in (and maybe why? E.g. open telemetry scheme etc.), which in my opinion makes more sense.
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.
Yes I can amend this, good call.
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.
🚀
Check
http.request.method
andmethod
fields when trying to extractspan.action
from an http span.As a we start rolling out getsentry/team-sdks#19 to more SDKs, we updated the span data field for HTTP methods from
http.method
tohttp.request.method
. This is to match the new OpenTelemetry conventions.Not sure how to best test this, feedback appreciated!
#skip-changelog