chore(sdk-node): migrate to the new sdk-trace package#6828
Conversation
This adds a new 'trace' export, which re-exports the sdk-trace package. This is meant as a replacement for the existing 'node' and 'tracing' re-exports of the sdk-trace-node and sdk-trace-base packages, which will soon be deprecated.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6828 +/- ##
==========================================
+ Coverage 95.51% 95.52% +0.01%
==========================================
Files 382 384 +2
Lines 12941 12974 +33
Branches 2967 2976 +9
==========================================
+ Hits 12360 12394 +34
+ Misses 581 580 -1
🚀 New features to boost your workflow:
|
| } from '@opentelemetry/configuration'; | ||
| import type { SpanLimits } from '@opentelemetry/sdk-trace'; | ||
|
|
||
| export function createSpanLimitsFromConfig( |
There was a problem hiding this comment.
Note: This continues the create<SDK Thing>FromConfig() pattern started in #6785
I've also started moving "FromConfig"-specific things to this file, from the overly large "utils.ts".
I think it'll be helpful for maint and testing to separate the "FromEnv" handling and the "FromConfig" handling. Over a number of PRs I'll move/re-implement things in utils.ts over to "create-from-config.ts" and "create-from-env.ts".
|
|
||
| const DEFAULT_RATIO = 1; | ||
|
|
||
| export function createSamplerFromEnv(): Sampler | undefined { |
There was a problem hiding this comment.
Note: Adapted from buildSamplerFromEnv in sdk-trace-base:
| return ratio; | ||
| } | ||
|
|
||
| export function createSpanLimitsFromEnv(): SpanLimits | undefined { |
There was a problem hiding this comment.
Note: Adapted from reconfigureLimits in sdk-trace-base:
| export * as node from '@opentelemetry/sdk-trace-node'; | ||
| /** | ||
| * @deprecated Use `trace` re-export from `@opentelemetry/sdk-trace` instead. | ||
| */ | ||
| export * as tracing from '@opentelemetry/sdk-trace-base'; |
There was a problem hiding this comment.
Note: These existing re-exports are the only reason sdk-node cannot yet drop its dep on sdk-trace-node and sdk-trace-base. We'll drop these with SDK 3.0.
There was a problem hiding this comment.
I see you opened #6824. If you're feeling adventurous, we could also try to remove it and see if anybody is unhappy with that. I'd be willing to bet that almost nobody is using these re-exports.
We don't need to do it in this PR though, let's keep this low-friction for now.
| this._tracerProviderConfig = { | ||
| tracerConfig: tracerProviderConfig, | ||
| spanProcessors, | ||
| }; |
There was a problem hiding this comment.
Note: All of this handling of input config options for span processors is moved (and cleaned up) to and if-block in .start() below.
| if (configuration.sampler) { | ||
| tracerProviderConfig.sampler = configuration.sampler; | ||
| } | ||
| if (configuration.spanLimits) { | ||
| tracerProviderConfig.spanLimits = configuration.spanLimits; | ||
| } | ||
| if (configuration.idGenerator) { | ||
| tracerProviderConfig.idGenerator = configuration.idGenerator; | ||
| } |
There was a problem hiding this comment.
Note: This tracerProviderConfig was never used.
pichlermarc
left a comment
There was a problem hiding this comment.
Overall looks good, thanks for working on this 🙌
(just one comment about the re-exports; non-blocking)
This deprecates the existing 'node' and 'tracing' re-exports of the sdk-trace-node and sdk-trace-base packages, which themselves will soon be deprecated.
We decided to not add a replacement re-export of the sdk-trace package.
The recommendation is now that users should export directly from the other packages.
See #6824 and #5461 for discussion.
Refs: #6775
This is the first of some changes to migrate all usages of
sdk-trace-base,sdk-trace-node, andsdk-trace-webin the core and contrib repos over tosdk-trace. The usage in sdk-node was, I think, the big one. So it was helpful to do it in a separate PR.There should be no functional change other than the added
tracere-export (see src/index.ts).