-
Notifications
You must be signed in to change notification settings - Fork 977
feat(instrumentation): add createInstrumentation factory function
#6163
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(instrumentation): add createInstrumentation factory function
#6163
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6163 +/- ##
==========================================
+ Coverage 95.40% 95.43% +0.02%
==========================================
Files 317 317
Lines 9439 9521 +82
Branches 2187 2197 +10
==========================================
+ Hits 9005 9086 +81
- Misses 434 435 +1
🚀 New features to boost your workflow:
|
…/opentelemetry-js into dluna-instrumentation-factory
|
There is still work to do but is ready to be reviewed. IMHO we should decide 1st if the approach is the right one and then I can do the browser version and apply it to the rest of instrumentations in this repository |
Which problem is this PR solving?
this PR add a factory function to create instrumentations. the factory function has a single param to get an object that implements the
InstrumentationDelegateinterface. The factory function wraps the delegate and takes care of the enabling or disabling the instrumentation (interaction with RITM and ITM).This approach removes the inheritance from
InstrumentationAbstract -> InstrumentationBaseavoiding any initialisation issue in the constructor as it happens in #1989.with this change we will have two ways of creating instrumentations:
InstrumentationBasecreateInstrumentationfunction with a delegateIMHO the path should be to sunset the former in favour of the later so we avoid the init issue in all instrumentations and also simplifies a bit the instrumentation code.
Fixes #1989
Short description of the changes
createInstrumentationin instrumentation package for nodeHttpInstrumentationDelegateclass and exportcreateHttpInstrumentationfunction in http instrumentationcreateInstrumentationin instrumentation package for browserType of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
http-delegate-*.test.tsfilesChecklist: