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

chore: fixing documentation for web tracer provider, fixing examples … #906

Merged
merged 3 commits into from
Mar 27, 2020

Conversation

obecny
Copy link
Member

@obecny obecny commented Mar 26, 2020

…for web, enable manager when registering

Which problem is this PR solving?

Short description of the changes

  1. The documentation for web was wrong when creating web tracer provider and changing the context manager.
  2. Also examples were outdated but also broken due to strange lint fixes.
  3. I have also added enabling the context manager when user passes a new one in register
    Without this the manager is being created but in fact it never works and it was quite difficult to finally figure out what was wrong. This way the simplified form of creating new context manager will be easier for a user
    for example this wasn't working - the zone was disabled, now it will be enabled
    provider.register({ contextManager: new ZoneContextManager(), });
    I have also added checking in constructor to throw an error if someone adds either contextManager or propagator into constructor instead of register - as it is currently shown in out documentation for web. This way user will now faster what is wrong.
  4. The last fix is in document-load plugin it adds parent to resources loaded during the document load
  5. One more fix: the user interaction plugin was broken after the context propagation changes

@obecny obecny self-assigned this Mar 26, 2020
@obecny obecny added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. bug Something isn't working Target:Browser labels Mar 26, 2020
@codecov-io
Copy link

codecov-io commented Mar 26, 2020

Codecov Report

Merging #906 into master will increase coverage by 0.40%.
The diff coverage is 95.65%.

@@            Coverage Diff             @@
##           master     #906      +/-   ##
==========================================
+ Coverage   94.08%   94.48%   +0.40%     
==========================================
  Files         248      248              
  Lines       10835    10997     +162     
  Branches     1055     1062       +7     
==========================================
+ Hits        10194    10391     +197     
+ Misses        641      606      -35     
Impacted Files Coverage Δ
...try-plugin-user-interaction/src/userInteraction.ts 92.78% <93.75%> (+4.95%) ⬆️
packages/opentelemetry-core/src/context/context.ts 94.11% <100.00%> (ø)
...telemetry-plugin-document-load/src/documentLoad.ts 98.03% <100.00%> (ø)
...ackages/opentelemetry-web/src/WebTracerProvider.ts 100.00% <100.00%> (ø)
...ntelemetry-tracing/test/MultiSpanProcessor.test.ts 97.43% <0.00%> (-2.57%) ⬇️
packages/opentelemetry-tracing/src/config.ts 100.00% <0.00%> (ø)
packages/opentelemetry-tracing/src/utility.ts 100.00% <0.00%> (ø)
packages/opentelemetry-plugin-https/src/utils.ts 100.00% <0.00%> (ø)
packages/opentelemetry-tracing/test/Span.test.ts 100.00% <0.00%> (ø)
... and 18 more

Copy link
Member

@mayurkale22 mayurkale22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks for fixing this.

@obecny obecny added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 27, 2020
@obecny
Copy link
Member Author

obecny commented Mar 27, 2020

added more fixes - point 5

@dyladan dyladan added the document Documentation-related label Mar 27, 2020
@dyladan dyladan merged commit 6afa63c into open-telemetry:master Mar 27, 2020
@obecny obecny deleted the web_fix_doc_zone branch July 8, 2020 12:13
dyladan pushed a commit to dyladan/opentelemetry-js that referenced this pull request Sep 9, 2022
…en-telemetry#906)

* chore: fixing documentation for web tracer provider, fixing examples for web, enable manager when registering

* chore: fixing span extraction from zone after context updates

* chore: bump
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023
martinkuba pushed a commit to martinkuba/opentelemetry-js that referenced this pull request Mar 13, 2024
martinkuba pushed a commit to martinkuba/opentelemetry-js that referenced this pull request Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working document Documentation-related size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation for Web is wrong - the provider is missing the context
5 participants