-
Notifications
You must be signed in to change notification settings - Fork 651
Update http Semconv to v1.20.0 #4320
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
Update http Semconv to v1.20.0 #4320
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4320 +/- ##
=======================================
+ Coverage 80.8% 81.1% +0.2%
=======================================
Files 150 150
Lines 10374 10759 +385
=======================================
+ Hits 8388 8731 +343
- Misses 1844 1872 +28
- Partials 142 156 +14
|
Nice! :) |
From SIG meeting:
|
So they aren't lost, this was the results of running the example from the sig meeting:
From this The server is missing:
And the Client is missing:
Note these may be optional or not accessible. |
I think we are compliant:
|
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 that handling of net.protocol.name
and net.protocol.version
should be changed.
References:
- https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/trace/semantic_conventions/http.md
- https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/metrics/semantic_conventions/http-metrics.md
Besides I only left some nit comments regarding the comments.
instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go
Outdated
Show resolved
Hide resolved
instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go
Outdated
Show resolved
Hide resolved
instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go
Outdated
Show resolved
Hide resolved
instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go
Outdated
Show resolved
Hide resolved
instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go
Outdated
Show resolved
Hide resolved
instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go
Outdated
Show resolved
Hide resolved
instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go
Outdated
Show resolved
Hide resolved
instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go
Outdated
Show resolved
Hide resolved
instrumentation/net/http/otelhttp/internal/semconvutil/httpconv.go
Outdated
Show resolved
Hide resolved
@MadVikingGod Any update on this PR? |
This updates all http instrumentation (anything using
internal/shared/semconvutil
) to v1.20.0.In addition, I did an audit of the currently recommended semantic conventions for HTTP and put a comment of what is included and what is not to hopefully be easier to reference on the next update.
This doesn't go to 1.21 as that included a new set of logic around the
OTEL_SEMCONV_STABILITY_OPT_IN
environment variable. That will require a release using 1.20.0 for this to work correctly.