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

fix(sdk-trace-web): remove browsing context only api dependency on utils #2714

Closed
wants to merge 2 commits into from

Conversation

legendecas
Copy link
Member

Which problem is this PR solving?

sdk-trace-web should be compatible with web environments like web worker.

Short description of the changes

  1. Added guidance of Web API usages.
  2. Removed window references in sdk-trace-web/src.
  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update

How Has This Been Tested?

Existing tests with karma should continue to work. Worker integration tests should be introduced with #2659 or new karma configurations (for a full run of existing test suites on workers).

https://github.com/open-telemetry/opentelemetry-js/tree/main/selenium-tests is referencing experimental packages and it is not linked to the versions in the local repository, so testing with it is quite awkward. I'll continue to split it into two parts in #2659. However, if it is located in the experimental/ subdirectory, it can not be linked to the latest un-published version of sdk-trace-web, and we can't get it passed without the changes in this PR :(. So I'm submitting this patch and hope to land it first to continue the setups in #2659.

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

@legendecas legendecas requested a review from a team January 13, 2022 10:03
@codecov
Copy link

codecov bot commented Jan 13, 2022

Codecov Report

Merging #2714 (d3eb2f8) into main (21fc8b5) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2714   +/-   ##
=======================================
  Coverage   93.04%   93.04%           
=======================================
  Files         155      155           
  Lines        5378     5378           
  Branches     1137     1137           
=======================================
  Hits         5004     5004           
  Misses        374      374           
Impacted Files Coverage Δ
packages/opentelemetry-sdk-trace-web/src/utils.ts 95.00% <100.00%> (ø)

@vmarchaud vmarchaud added the bug Something isn't working label Jan 15, 2022
@vmarchaud vmarchaud requested a review from a team January 17, 2022 10:22
@legendecas
Copy link
Member Author

Closed in favor of #2719.

@legendecas legendecas closed this Jan 19, 2022
@legendecas legendecas deleted the trace-web-worker branch April 15, 2022 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants