-
Notifications
You must be signed in to change notification settings - Fork 47
Conversation
Codecov Report
@@ Coverage Diff @@
## main #147 +/- ##
==========================================
+ Coverage 94.67% 95.19% +0.52%
==========================================
Files 42 45 +3
Lines 582 645 +63
Branches 94 102 +8
==========================================
+ Hits 551 614 +63
Misses 31 31
Continue to review full report at Codecov.
|
Is this the exact implementation from core or have you made any notable changes?
Can you create issues for those components at least so we have something to track? |
The only new change introduced in this PR
Sure, will do that. |
Are there already plans how to handle this move to API in
|
@dyladan 's recommendation was to move this initially and then update wherever it is used. Encourage people to use the api method and issue depreciation notice to core component. But we will continue to keep maintaining the same changes in two places until the next major version. |
Even though the core is GA we could just make a major bump, its supposed to be a internal package for instrumentations/sdks not something public facing right ?
EDIT: What i'm saying is that i believe we could do this, however i think its overkill for just moving this implementation |
I think the first two steps are fine for now. Maybe add a doc deprecation of There are a few other issues open to move more from core to API. Maybe core gets anyway empty at some point in time... |
Right now having all stable SDK packages have the same version makes it a lot easier for end users. Also, our main repo is not really well set up to have packages independently versioned. I think publishing a minor API, updating usage, and adding a deprecation notice in the core package is sufficient for now.
|
Follow up based on 29/12 call,
TraceState
implementation should be part of API for various reasons (Should be able to use in creating in custom propagator only by taking dependency on@opentelemetry/api
, library instrumentation, specification). There is already an interfaceTraceState
exported so introducing the factory functioncreateTraceState
that returns newTraceState
implementation.It is worth noting that there are other components that are supposed to be in api/sdk but are part of
opentelemetry-core
package butTraceState
is something I needed at the moment.