Skip to content
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

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

Merged

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