opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

Browser identification resource attributes

Open Abinet18 opened this issue 3 years ago • 5 comments
trafficstars

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

Abinet18 avatar Aug 25 '22 17:08 Abinet18

CLA Signed

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 avatar Aug 26 '22 16:08 Abinet18

@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.

legendecas avatar Aug 29 '22 07:08 legendecas

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 ?

Abinet18 avatar Aug 30 '22 16:08 Abinet18

Codecov Report

Merging #3201 (4ad3c46) into main (32cb123) will decrease coverage by 0.91%. The diff coverage is 100.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

codecov[bot] avatar Aug 31 '22 02:08 codecov[bot]