Add semconv v1.24.0 to otelhttp
#5092
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MadVikingGod
requested review from
Aneurysm9,
dmathieu and
a team
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5092 +/- ##
=======================================
+ Coverage 61.2% 61.8% +0.5%
=======================================
Files 185 190 +5
Lines 11207 11444 +237
=======================================
+ Hits 6865 7076 +211
- Misses 4142 4163 +21
- Partials 200 205 +5
|
Removed Metrics Placeholders
dashpole
reviewed
Feb 21, 2024
| @@ -0,0 +1,22 @@ | |||
| # Migration Plan | |||
There was a problem hiding this comment.
when using both, which metric names do they get, or are the metric names the same?
There was a problem hiding this comment.
This could go one of two ways:
- Because the Metrics Semconv, and our package is not stable we just eat this breaking change. Then in the both we follow whatever we set for the instrumentation's Semantic Version (which should be 1.24.0 for both).
- We add an abstraction back in that makes the creation of the metrics (from the
MeterProvider) and what names are used as part of this new package. This is basically what I've tried to remove in this package as much as possible.
I would very much not want to build an abstraction on this code path; it's what made it so hard to upgrade and will eventually leave us with an API surface that no longer makes sense, e.g., HTTPFlavor was something we eventually had to drop.
dashpole
previously approved these changes
Feb 29, 2024
carrbs
reviewed
Mar 15, 2024
| if attr, ok := methodLookup[strings.ToUpper(method)]; ok { | ||
| attrs[0] = attr | ||
| } else { | ||
| // If the Original methos is not a standard HTTP method fallback to GET |
4 tasks
This was referenced Apr 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change adds semantic convention v1.24.0 to
otlehttpand the compatibility mode that was required in the v1.21.0 semantic convention.Scope
This is the first in a series of patches to fully finish out this feature. The goal is to show how the environment variable can be supported and how it can be used.
What was excluded to aid in reviewability, all can be added after:
HTTPServerinterface and supplied by new types.Benchmarks
A benchmark was added to aid in comparing. I've added the raw data to this gist.
First I wanted to make sure that this didn't introduce performance regressions to users using the default/old attributes:
Next I wanted to make sure the new way of attributes were not significantly different. I assumed that
http/dupwill take more time, because it has to deal with twice as many attributes, but I wanted to allocations to stay the same.Testing
I utilized the semantic convention checker, from PR4701, but modified, set to
http/dup, and used two configs one for v1.20 and v1.24. I can put this onto a branch if someone would like to recreate this work.