Skip to content

fix: remove setting http.route in http span attributes#2494

Merged
dyladan merged 3 commits intoopen-telemetry:mainfrom
open-o11y:remove-http-route-from-attributes
Sep 28, 2021
Merged

fix: remove setting http.route in http span attributes#2494
dyladan merged 3 commits intoopen-telemetry:mainfrom
open-o11y:remove-http-route-from-attributes

Conversation

@mustafain117
Copy link
Contributor

Which problem is this PR solving?

Short description of the changes

  • In the http instrumentation, the http.route attribute is a duplicated field (with http.target attribute). This PR removes setting of http.route attribute for http spans
  • This PR also adds a test to assert http.route is not set

@mustafain117 mustafain117 requested a review from a team September 23, 2021 22:04
@codecov
Copy link

codecov bot commented Sep 24, 2021

Codecov Report

Merging #2494 (543b245) into main (33935e7) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2494      +/-   ##
==========================================
- Coverage   93.20%   93.20%   -0.01%     
==========================================
  Files         137      137              
  Lines        5017     5016       -1     
  Branches     1060     1059       -1     
==========================================
- Hits         4676     4675       -1     
  Misses        341      341              
Impacted Files Coverage Δ
...es/opentelemetry-instrumentation-http/src/utils.ts 99.02% <0.00%> (-0.01%) ⬇️

@vmarchaud
Copy link
Member

@mustafain117 you'll need to rebase the PR so we can merge it

@vmarchaud vmarchaud added the bug Something isn't working label Sep 25, 2021
Copy link
Member

@dyladan dyladan left a comment

Choose a reason for hiding this comment

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

Looks good. Please fix the conflicts and this can merge.

Copy link
Member

@alolita alolita left a comment

Choose a reason for hiding this comment

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

+1

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove http.route from http span attributes

5 participants