Skip to content

Switch the semconv migration default to new#6899

Merged
MrAlias merged 19 commits intoopen-telemetry:mainfrom
dmathieu:semconv-default-dup
Apr 8, 2025
Merged

Switch the semconv migration default to new#6899
MrAlias merged 19 commits intoopen-telemetry:mainfrom
dmathieu:semconv-default-dup

Conversation

@dmathieu
Copy link
Copy Markdown
Member

@dmathieu dmathieu commented Mar 7, 2025

This switches the semantic conventions migration default from the old version to be the new one, and keeps the possibility to use http/dup to get both.

Note: we previously tested and documented an old config.
That worked because it used the default. However, that behavior is not documented in the semconv migration spec, and allowing to keep using the old values only when the next release will entirely drop their support looks like a bad idea.

This also removes the v1.20.0_test.go files. Those tests rely on the global server/client, but only tests the old behavior.
We can't run the old behavior only anymore, so running those would require a non-trivial refactoring that seems unneeded since this entire code will be removed in the next release.

Closes #6897

@dmathieu dmathieu force-pushed the semconv-default-dup branch from 8c76268 to 71c225f Compare March 7, 2025 09:23
@dmathieu dmathieu force-pushed the semconv-default-dup branch from ce8ba58 to e8d0ac8 Compare March 7, 2025 09:54
@dmathieu dmathieu force-pushed the semconv-default-dup branch from e8d0ac8 to e334be7 Compare March 7, 2025 09:56
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 7, 2025

Codecov Report

Attention: Patch coverage is 95.75758% with 7 lines in your changes missing coverage. Please review.

Project coverage is 80.7%. Comparing base (56eeab7) to head (c1b8859).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...tp/httptrace/otelhttptrace/internal/semconv/env.go 93.9% 2 Missing ⚠️
...entation/net/http/otelhttp/internal/semconv/env.go 93.9% 2 Missing ⚠️
...lei/go-restful/otelrestful/internal/semconv/env.go 96.9% 1 Missing ⚠️
....com/gin-gonic/gin/otelgin/internal/semconv/env.go 96.9% 1 Missing ⚠️
...ub.com/gorilla/mux/otelmux/internal/semconv/env.go 96.9% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #6899     +/-   ##
=======================================
- Coverage   81.0%   80.7%   -0.3%     
=======================================
  Files        205     205             
  Lines      18098   18083     -15     
=======================================
- Hits       14667   14604     -63     
- Misses      3000    3060     +60     
+ Partials     431     419     -12     
Files with missing lines Coverage Δ
...lei/go-restful/otelrestful/internal/semconv/env.go 90.9% <96.9%> (+1.8%) ⬆️
....com/gin-gonic/gin/otelgin/internal/semconv/env.go 94.4% <96.9%> (+1.9%) ⬆️
...ub.com/gorilla/mux/otelmux/internal/semconv/env.go 92.3% <96.9%> (+1.8%) ⬆️
...tp/httptrace/otelhttptrace/internal/semconv/env.go 89.5% <93.9%> (+1.8%) ⬆️
...entation/net/http/otelhttp/internal/semconv/env.go 89.5% <93.9%> (+1.8%) ⬆️

... and 12 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dmathieu dmathieu marked this pull request as ready for review March 7, 2025 10:27
@dmathieu dmathieu requested a review from a team as a code owner March 7, 2025 10:27
Comment thread CHANGELOG.md
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Copy link
Copy Markdown

@AyseGokmen AyseGokmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new convention http.client.request.duration.seconds might still be recording the metrics in milliseconds and with the histogram buckets that would fit the milliseconds use case. Can we please update them as well when switching the default to the new convention?

@dmathieu
Copy link
Copy Markdown
Member Author

Thank you. I have extracted your report into #6941

Comment thread CHANGELOG.md
@dmathieu dmathieu requested a review from XSAM March 26, 2025 10:01
@MrAlias
Copy link
Copy Markdown
Contributor

MrAlias commented Apr 8, 2025

@XSAM PTAL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

Switch from opt-in to new semconv to being opt-out of semconv for all HTTP instrumentation

6 participants