-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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(node): Add fsInstrumentation
#13291
Conversation
dev-packages/node-integration-tests/suites/fs-instrumentation/server.ts
Dismissed
Show dismissed
Hide dismissed
dev-packages/node-integration-tests/suites/fs-instrumentation/server.ts
Dismissed
Show dismissed
Hide dismissed
@@ -0,0 +1,11 @@ | |||
some-file.txt.copy |
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.
Can we just make this: some-file.txt.*
?
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.
and maybe move them out of the root of the package, into e.g. fixtures/
or something like this?
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.
nice!
Resolves #13057
This PR adds an integration to instrument
fs
API. We are using OTEL's@opentelemetry/instrumentation-fs
instrumentation under the hood.The integration creates spans with naming patterns of
fs.readFile
,fs.unlink
, and so on.Users can configure via option whether they want to include path arguments as attributes, and whether to include error messages as an attribute when an fs call fails.
I have found that the integration can slow down applications massively when there are a lot of fs calls, for example when running a dev server - so I put a disclaimer in the JS doc, and we should also put a similar disclaimer in the docs when we write them.