Skip to content

internal/shared/semconv: remove redundant HTTP method from http.route - #6937

Merged
dmathieu merged 6 commits into
open-telemetry:mainfrom
gaiaz-iusipov:http-route
Mar 26, 2025
Merged

internal/shared/semconv: remove redundant HTTP method from http.route#6937
dmathieu merged 6 commits into
open-telemetry:mainfrom
gaiaz-iusipov:http-route

Conversation

@gaiaz-iusipov

Copy link
Copy Markdown
Contributor

[http.Request.Pattern] may contain the name of the HTTP method, which is redundant in the value of the http.route attribute.

Related to #6193

@gaiaz-iusipov
gaiaz-iusipov requested review from a team, dashpole and dmathieu as code owners March 15, 2025 23:19
@github-actions
github-actions Bot requested a review from akats7 March 15, 2025 23:19
@codecov

codecov Bot commented Mar 15, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.8%. Comparing base (9760708) to head (7d08e8d).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #6937   +/-   ##
=====================================
  Coverage   75.8%   75.8%           
=====================================
  Files        207     207           
  Lines      19469   19494   +25     
=====================================
+ Hits       14758   14783   +25     
  Misses      4275    4275           
  Partials     436     436           
Files with missing lines Coverage Δ
...o-restful/otelrestful/internal/semconv/httpconv.go 86.9% <100.0%> (ø)
...ei/go-restful/otelrestful/internal/semconv/util.go 82.8% <100.0%> (+1.3%) ⬆️
...gin-gonic/gin/otelgin/internal/semconv/httpconv.go 86.9% <100.0%> (ø)
...com/gin-gonic/gin/otelgin/internal/semconv/util.go 82.8% <100.0%> (+1.3%) ⬆️
...m/gorilla/mux/otelmux/internal/semconv/httpconv.go 86.9% <100.0%> (ø)
...b.com/gorilla/mux/otelmux/internal/semconv/util.go 82.8% <100.0%> (+1.3%) ⬆️
...tptrace/otelhttptrace/internal/semconv/httpconv.go 86.9% <100.0%> (ø)
...p/httptrace/otelhttptrace/internal/semconv/util.go 82.8% <100.0%> (+1.3%) ⬆️
...ion/net/http/otelhttp/internal/semconv/httpconv.go 86.9% <100.0%> (ø)
...ntation/net/http/otelhttp/internal/semconv/util.go 82.8% <100.0%> (+1.3%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@akats7

akats7 commented Mar 17, 2025

Copy link
Copy Markdown
Contributor

This is a good call, however this also leads to the question what about hostnames, since 1.22 also adds support for matching on the hostname. Do we want to filter that out of the route as well?

@dmathieu

Copy link
Copy Markdown
Member

Could you please add new commits rather than force pushing ?

@gaiaz-iusipov

Copy link
Copy Markdown
Contributor Author

Could you please add new commits rather than force pushing ?

Sorry, from now on I will only push new commits.

@gaiaz-iusipov

Copy link
Copy Markdown
Contributor Author

This is a good call, however this also leads to the question what about hostnames, since 1.22 also adds support for matching on the hostname. Do we want to filter that out of the route as well?

Thank you, I have added a test case with a domain in the string.

@gaiaz-iusipov
gaiaz-iusipov requested a review from dmathieu March 17, 2025 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants