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: prefer use of global TracerProvider/MeterProvider #2127

Merged
merged 8 commits into from
Apr 23, 2021

Conversation

Flarna
Copy link
Member

@Flarna Flarna commented Apr 20, 2021

Remove setting of TracerProvider in samples where it is actually not needed as the global TracerProvider should be preferred on default.

I found it also inconsistent to set a TracerProvider but no MeterProvider.

Remove setting of TracerProvider in samples where it is actually not needed as the
global TracerProvider should be prefered on default.
@codecov
Copy link

codecov bot commented Apr 20, 2021

Codecov Report

Merging #2127 (c9e4062) into main (23ba4bf) will increase coverage by 1.11%.
The diff coverage is n/a.

❗ Current head c9e4062 differs from pull request most recent head 9766d51. Consider uploading reports for the commit 9766d51 to get more accurate results

@@            Coverage Diff             @@
##             main    #2127      +/-   ##
==========================================
+ Coverage   92.76%   93.87%   +1.11%     
==========================================
  Files         140       70      -70     
  Lines        4987     3085    -1902     
  Branches     1029      676     -353     
==========================================
- Hits         4626     2896    -1730     
+ Misses        361      189     -172     
Impacted Files Coverage Δ
packages/opentelemetry-sdk-node/src/sdk.ts 78.12% <ø> (ø)
...s/opentelemetry-core/src/platform/node/sdk-info.ts 0.00% <0.00%> (-100.00%) ⬇️
...pentelemetry-core/src/platform/node/performance.ts 0.00% <0.00%> (-100.00%) ⬇️
...emetry-api-metrics/src/platform/node/globalThis.ts 0.00% <0.00%> (-100.00%) ⬇️
...kages/opentelemetry-node/src/NodeTracerProvider.ts 95.83% <0.00%> (-4.17%) ⬇️
...atform/node/instrumentationNodeModuleDefinition.ts
...ry-exporter-prometheus/src/PrometheusSerializer.ts
packages/opentelemetry-tracing/src/config.ts
...ource-detector-aws/src/detectors/AwsEc2Detector.ts
...ges/opentelemetry-metrics/src/export/Controller.ts
... and 65 more

Copy link
Member

@dyladan dyladan left a comment

Choose a reason for hiding this comment

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

Thanks this is something I've been meaning to do.

@dyladan dyladan added the document Documentation-related label Apr 21, 2021
Copy link
Member

@obecny obecny left a comment

Choose a reason for hiding this comment

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

lgtm,
Are you up to make the same in contrib, so we have unified examples ?

@Flarna
Copy link
Member Author

Flarna commented Apr 22, 2021

I added one more commit to align sequence of provider.register() and registerInstrumenations().

Are you up to make the same in contrib, so we have unified examples ?

Sure, will do once this landed.

@vmarchaud vmarchaud merged commit f077df3 into open-telemetry:main Apr 23, 2021
@Flarna Flarna deleted the prefer-global-providers branch April 23, 2021 13:04
@dyladan dyladan mentioned this pull request Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
document Documentation-related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants