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

Align xhr span name with spec #1476

Merged
merged 2 commits into from
Aug 31, 2020
Merged

Align xhr span name with spec #1476

merged 2 commits into from
Aug 31, 2020

Conversation

johnbley
Copy link
Member

Which problem is this PR solving?

  • Span names from the xhr plugin are the url, while the OpenTelemetry spec says they should be lower cardinality. The fetch plugin uses low-cardinality HTTP {method} names; the two plugins should be in agreement about this.

Short description of the changes

  • Change xhr span names to HTTP {method} and adjust unit tests accordingly.

@codecov
Copy link

codecov bot commented Aug 28, 2020

Codecov Report

Merging #1476 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1476   +/-   ##
=======================================
  Coverage   93.99%   93.99%           
=======================================
  Files         153      153           
  Lines        4662     4663    +1     
  Branches      962      962           
=======================================
+ Hits         4382     4383    +1     
  Misses        280      280           
Impacted Files Coverage Δ
...s/opentelemetry-plugin-xml-http-request/src/xhr.ts 97.23% <100.00%> (+0.01%) ⬆️

@dyladan dyladan added the enhancement New feature or request label Aug 28, 2020
@dyladan
Copy link
Member

dyladan commented Aug 31, 2020

Why is the CLA check not returning?

@dyladan dyladan closed this Aug 31, 2020
@dyladan dyladan reopened this Aug 31, 2020
@dyladan
Copy link
Member

dyladan commented Aug 31, 2020

Closed and reopened to trigger the CLA check

@johnbley
Copy link
Member Author

Closed and reopened to trigger the CLA check

Have you tried turning it off and on again? 😄

@dyladan dyladan merged commit a7a6b7f into open-telemetry:master Aug 31, 2020
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants