-
Notifications
You must be signed in to change notification settings - Fork 805
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(instrumentation): make InstrumentationAbstract.init public #4418
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4418 +/- ##
==========================================
- Coverage 92.37% 90.81% -1.56%
==========================================
Files 304 87 -217
Lines 8682 1906 -6776
Branches 1831 405 -1426
==========================================
- Hits 8020 1731 -6289
+ Misses 662 175 -487 |
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.
Change from protected to public is technically expanding the API surface, but only minimally. I'm fine with it if the other maintainers are.
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.
I'm also okay with this change. 👍
Just one comment about the documentation of init()
experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
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, thanks for updating the JSdoc 🙂
@pichlermarc could you merge when you get a moment? thanks! |
…-telemetry#4418) * Make InstrumentationAbstract.init public * Update changelog * Update JSDoc --------- Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>
Which problem is this PR solving?
For the esbuild instrumentation plugin I'm attempting to rely on
.init()
at build time to get info about instrumentations, and in order for that to typecheck I need public access to this method.Fixes # (issue)
Short description of the changes
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Compile this package and use it in that plugin (locally).
In this repo:
npm run compile
cd experimental/packages/opentelemetry-instrumentation
npm pack
In my fork of the
opentelemetry-js-contrib
on theesbuild-plugin
branch (which is in the same directory asopentelemetry-js
)cd packages/esbuild-plugin-node
npm i ../../../opentelemetry-js/experimental/packages/opentelemetry-instrumentation-grpc/opentelemetry-instrumentation-0.47.0.tgz
Checklist: