opentelemetry-js
opentelemetry-js copied to clipboard
fix(instrumentation-xhr): http.url attribute should be absolute
Which problem is this PR solving?
As per the specification http.url attribute should be "Full HTTP request URL in the form scheme://host[:port]/path?query[#fragment]." Currently if a XHR request is done with a relative URL, then the http.url attribute is also relative
Also added a test that checks this in fetch instrumentation
Short description of the changes
Added relative -> abs conversion via parseUrl
Type of change
Please delete options that are not relevant.
- Bug fix (non-breaking change which fixes an issue)
How Has This Been Tested?
- Unit tests
Checklist:
- Followed the style guidelines of this project
- Unit tests have been added
- Documentation has been updated
Codecov Report
Merging #3200 (3d0556a) into main (b9f5561) will decrease coverage by
0.01%. The diff coverage isn/a.
Additional details and impacted files
@@ Coverage Diff @@
## main #3200 +/- ##
==========================================
- Coverage 93.29% 93.28% -0.02%
==========================================
Files 247 247
Lines 7352 7352
Branches 1512 1512
==========================================
- Hits 6859 6858 -1
- Misses 493 494 +1
| Impacted Files | Coverage Δ | |
|---|---|---|
| ...emetry-instrumentation-xml-http-request/src/xhr.ts | 97.58% <ø> (ø) |
|
| ...-trace-base/src/platform/node/RandomIdGenerator.ts | 87.50% <0.00%> (-6.25%) |
:arrow_down: |
Lack of changes in fetch instrumentation actually brings up a small but significant difference in the ignoreUrls config option of XHR and fetch instrumentations
In fetch instrumentation already parsed URL is passed to _createSpan
https://github.com/open-telemetry/opentelemetry-js/blob/a5abee69119cc41d9d34f6beb5c1826eef1ac0dd/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts#L304
In both instrumentations, ignoreUrls check is in _createSpan method, which means in fetch absolute url is always used while in xhr it depends on what URL was passed to XHR.open call. Therefore when using ignoreUrls you need to either have separate configs for these instrumentations (which sucks), or make sure your regexp works with both (unintuitive and undocumented) and have strings in both absolute and relative versions (
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.