-
Notifications
You must be signed in to change notification settings - Fork 534
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-fs): require parent span #1335
Conversation
e408510
to
70c2dd9
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1335 +/- ##
=======================================
Coverage 96.08% 96.08%
=======================================
Files 14 14
Lines 893 893
Branches 191 191
=======================================
Hits 858 858
Misses 35 35 |
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.
Thanks for working on this
I added a few comments about code style. They are all non-blocker and optional, so feel free to resolve them if you think otherwise.
Thank you for the comments @blumamir! They make a lot of sense to me -- the naming around the test generator function is very wonky. I'll address these the coming week. |
70c2dd9
to
11632bc
Compare
Done. Feel free to merge if the tests pass 🚀 EDIT: not sure why the tests don't pass -- the linter seems to be unhappy about a file that isn't changed by this PR? |
11632bc
to
6d5681d
Compare
Which problem is this PR solving?
requireParentSpan
option, in order to only trace calls to the fs module that happen in the context of a broader action. Applications might, for example, read some files during boot, which would rarely make for useful traces.Short description of the changes
requireParentSpan
option, which defaults tofalse
. If set totrue
, calls to the fs module will not be instrumented unless the current active context has a span, so the fs instrumentation will never create a trace of its own.