Skip to content
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): error hook #1963

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

raphael-theriault-swi
Copy link
Member

Which problem is this PR solving?

This adds a new error hook to the fs instrumentation to let users filter some errors from being reported as span events. This prevents trace data being polluted by irrelevant error events and failed spans for idiomatic filesystem access patterns where an error is expected, for instance opening a file without checking whether it exists then dealing with that case in the error handler.

Short description of the changes

  • Adds an optional errorHook option to the config that is called with the function name and error before reporting the error event. If the hook is present and returns false, the error event is not reported.
  • The error is still always propagated

Copy link

codecov bot commented Feb 26, 2024

Codecov Report

Merging #1963 (b99ec76) into main (038e0bf) will increase coverage by 0.04%.
Report is 5 commits behind head on main.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1963      +/-   ##
==========================================
+ Coverage   91.00%   91.04%   +0.04%     
==========================================
  Files         146      147       +1     
  Lines        7491     7530      +39     
  Branches     1502     1507       +5     
==========================================
+ Hits         6817     6856      +39     
  Misses        674      674              

see 2 files with indirect coverage changes

@raphael-theriault-swi
Copy link
Member Author

@open-telemetry/javascript-approvers would it be possible for anyone to take a look ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants