opentelemetry-js
opentelemetry-js copied to clipboard
Browser identification resource attributes
Which problem is this PR solving?
For all the traces that are generated from browser ,we need to have the browser identifier attributes like platform, brands, mobile, and user_agent. Instead of adding these attributes to individual spans, it is better if we have them as part of resource attributes which will be common for all the traces. This PR is adding these attributes at WebTracerProvider level.
Short description of the changes
At WebTracerProvider add these browser identifier attributes to the config and pass it to the trace provider constructor.
Please delete options that are not relevant.
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ x ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ x ] This change requires a documentation update
How Has This Been Tested?
The change is tested on a sample app that uses the otel instrumentation libraries for browser and confirming that the resource attributes include the ones added here
Checklist:
- [x] Followed the style guidelines of this project
- [x] Unit tests have been added
- [ ] Documentation has been updated
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: Abinet18 (573f0c8400bf0a6ded0bd940c4ae2b187eaba6cf, feb6d7cc13917494b1757e4c0f3e1abc90c7e21e)
Thanks for reviewing. I have signed the CLA. And regarding the BrowserDetector, I don't see it being used anywhere except in the tests. Is it possible to use it in the WebTraceProvider to initialize the browser resource attributes ? what is your thought on how to do this ?
@Abinet18 we currently didn't automatically detect resources in the web packages. The detector has to be invoked manually to detect browser resource attributes. Ideally, we can create a package @opentelemetry/sdk-web that is analogous to the @opentelemetry/sdk-node that set up common platform attributes.
As the first step, I'd encourage adding support to detect the new resource attributes in the browser resource detector.
I have done as you advised, brought the attribute addition to the Browser detector. I also wanted to use SemanticResourceAttributes for the attribute names but these names are not yet included there ?
Codecov Report
Merging #3201 (4ad3c46) into main (32cb123) will decrease coverage by
0.91%. The diff coverage is100.00%.
Additional details and impacted files
@@ Coverage Diff @@
## main #3201 +/- ##
==========================================
- Coverage 93.28% 92.36% -0.92%
==========================================
Files 201 195 -6
Lines 6579 6118 -461
Branches 1379 1291 -88
==========================================
- Hits 6137 5651 -486
- Misses 442 467 +25
| Impacted Files | Coverage Δ | |
|---|---|---|
| ...lemetry-resources/src/detectors/BrowserDetector.ts | 100.00% <100.00%> (ø) |
|
| packages/opentelemetry-resources/src/types.ts | 100.00% <100.00%> (ø) |
|
| .../src/platform/browser/export/BatchSpanProcessor.ts | 47.61% <100.00%> (+2.61%) |
:arrow_up: |
| packages/opentelemetry-sdk-trace-web/src/utils.ts | 65.62% <0.00%> (-29.38%) |
:arrow_down: |
| ...-trace-base/src/platform/node/RandomIdGenerator.ts | 87.50% <0.00%> (-6.25%) |
:arrow_down: |
| ...emetry-instrumentation-xml-http-request/src/xhr.ts | ||
| ...s/opentelemetry-instrumentation-fetch/src/fetch.ts | ||
| ...-instrumentation-fetch/src/enums/AttributeNames.ts | ||
| ...mentation-xml-http-request/src/enums/EventNames.ts | ||
| ...ation-xml-http-request/src/enums/AttributeNames.ts | ||
| ... and 3 more |