-
Notifications
You must be signed in to change notification settings - Fork 945
feat(otlp-transformer)!: introduce separate entry-points for OTLP serializers #5263
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
base: main
Are you sure you want to change the base?
feat(otlp-transformer)!: introduce separate entry-points for OTLP serializers #5263
Conversation
ec2720d to
176b776
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5263 +/- ##
==========================================
- Coverage 95.15% 95.14% -0.02%
==========================================
Files 316 316
Lines 9166 9166
Branches 2023 2023
==========================================
- Hits 8722 8721 -1
- Misses 444 445 +1
🚀 New features to boost your workflow:
|
176b776 to
2e74b8a
Compare
2a53f1b to
8c1c6be
Compare
8c1c6be to
7aca149
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good other than the legacy typesVersions thing
Co-authored-by: Godfrey Chan <godfreykfc@gmail.com>
|
Hello. Any idea when this might be merged and released please? I suspect it might unblock using the package in nextJS middleware, which runs on Edge runtime, if it's enough for #4987. |
I'm still waiting on @open-telemetry/javascript-approvers reviews. Once I have an approving review this can get merged. If that happens before Feb 17, we'll release it then. |
aecf93b to
a28cb38
Compare
| "types": "./build/src/trace/json/index.d.ts", | ||
| "default": "./build/src/trace/json/index.js" | ||
| }, | ||
| "./experimental/logs": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I'm not aware off all the history behind logs. I know the API is still experimental. Is that the reason why the word appears in the export path?
@opentelemetry/otlp-transformer package is already experimental so IMHO it feels redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would unblock us from marking this package as stable before logs becomes stable.
I suppose this will become dependent on the outcome of open-telemetry/opentelemetry.io#8208
Which problem is this PR solving?
This is the second-to-last breaking PR for
@opentelemetry/otlp-transformerbefore marking the package as stable (#4582). This solves some issues with rollup, that warns even for code that is dead in the end-user's app. See #5096, #4987.This PR adds entry points for each signal, each signal gets an entry point for
protobufandjsonto address #5096 and #4987The new entrypoint structure is:
-
/signal,/signal/protobuf,signal/json- experimental signals are prefixed by
experimental/to allow us marking the package stable before the experimental signals are - the spec moves quicker than we do, so I expect there to always be one that is not stableCloses #5216
Closes #4584
Fixes #5096
Partially addresses #4987
Type of change
How Has This Been Tested?