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

feat(Node-Tracer): initialize plugin loader #252

Merged
merged 3 commits into from Sep 19, 2019

Conversation

@mayurkale22
Copy link
Contributor

commented Sep 10, 2019

Which problem is this PR solving?

Short description of the changes

  • With Node-Tracer will load all the default supported plugins.
@codecov-io

This comment has been minimized.

Copy link

commented Sep 11, 2019

Codecov Report

Merging #252 into master will decrease coverage by 0.05%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #252      +/-   ##
==========================================
- Coverage   98.91%   98.85%   -0.06%     
==========================================
  Files          66       67       +1     
  Lines        2582     2536      -46     
  Branches      171      172       +1     
==========================================
- Hits         2554     2507      -47     
- Misses         28       29       +1
Impacted Files Coverage Δ
packages/opentelemetry-node-tracer/src/config.ts 100% <100%> (ø)
...ckages/opentelemetry-node-tracer/src/NodeTracer.ts 100% <100%> (ø) ⬆️
.../opentelemetry-node-tracer/test/NodeTracer.test.ts 100% <100%> (ø) ⬆️
...pentelemetry-basic-tracer/src/NoopSpanProcessor.ts 66.66% <0%> (-33.34%) ⬇️
...kages/opentelemetry-basic-tracer/test/Span.test.ts 100% <0%> (ø) ⬆️
packages/opentelemetry-basic-tracer/src/config.ts 100% <0%> (ø) ⬆️
...pe-async-hooks/test/AsyncHooksScopeManager.test.ts 100% <0%> (ø) ⬆️
...sic-tracer/test/export/SimpleSpanProcessor.test.ts 100% <0%> (ø) ⬆️
...metry-basic-tracer/test/MultiSpanProcessor.test.ts 100% <0%> (ø) ⬆️
...ic-tracer/test/export/InMemorySpanExporter.test.ts 100% <0%> (ø) ⬆️
... and 2 more
@mayurkale22

This comment has been minimized.

Copy link
Contributor Author

commented Sep 16, 2019

Ping, for more reviews.

@mayurkale22 mayurkale22 force-pushed the mayurkale22:pluginloader branch from c211de7 to 1b400c1 Sep 16, 2019


afterEach(() => {
// clear require cache
Object.keys(require.cache).forEach(key => delete require.cache[key]);

This comment has been minimized.

Copy link
@markwolff

markwolff Sep 17, 2019

Member

Does this "un-require" everything? What would be in the cache at this point? Since (I think?) only 1 test is requiring in your test packages.

This comment has been minimized.

Copy link
@mayurkale22

mayurkale22 Sep 17, 2019

Author Contributor

Yes, it will clean the everything from the cache. Do you suggest to delete a particular module from the cache instead of everything?

@OlivierAlbertini
Copy link
Member

left a comment

LGTM. I'm agree with draffensperger's comment

@mayurkale22 mayurkale22 force-pushed the mayurkale22:pluginloader branch from 1b400c1 to 0fdd581 Sep 17, 2019

mayurkale22 added 3 commits Sep 8, 2019

@mayurkale22 mayurkale22 force-pushed the mayurkale22:pluginloader branch from 0fdd581 to 8d431ca Sep 19, 2019

@mayurkale22 mayurkale22 merged commit 03ef951 into open-telemetry:master Sep 19, 2019

8 checks passed

ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: node10 Your tests passed on CircleCI!
Details
ci/circleci: node11 Your tests passed on CircleCI!
Details
ci/circleci: node12 Your tests passed on CircleCI!
Details
ci/circleci: node12-browsers Your tests passed on CircleCI!
Details
ci/circleci: node8 Your tests passed on CircleCI!
Details
cla/linuxfoundation mayurkale22 authorized
Details

@mayurkale22 mayurkale22 deleted the mayurkale22:pluginloader branch Sep 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.