Skip to content

fix(instrumentation): only patch core modules if enabled - #2993

Merged
vmarchaud merged 3 commits into
open-telemetry:mainfrom
santigimeno:santi/instrumentation_fix
May 28, 2022
Merged

fix(instrumentation): only patch core modules if enabled#2993
vmarchaud merged 3 commits into
open-telemetry:mainfrom
santigimeno:santi/instrumentation_fix

Conversation

@santigimeno

Copy link
Copy Markdown
Contributor

Which problem is this PR solving?

Otherwise, disabling the instrumentation module before actually
requiring the to-be-instrumented module doesn't work, and the module is
patched.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Existing tests pass.

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

@santigimeno
santigimeno requested a review from a team May 25, 2022 13:54
@codecov

codecov Bot commented May 25, 2022

Copy link
Copy Markdown

Codecov Report

Merging #2993 (e8c65da) into main (eda0b09) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2993      +/-   ##
==========================================
+ Coverage   92.71%   92.77%   +0.06%     
==========================================
  Files         183      183              
  Lines        6134     6135       +1     
  Branches     1299     1300       +1     
==========================================
+ Hits         5687     5692       +5     
+ Misses        447      443       -4     
Impacted Files Coverage Δ
...strumentation/src/platform/node/instrumentation.ts 63.21% <100.00%> (+5.07%) ⬆️

Otherwise, disabling the instrumentation module before actually
requiring the to-be-instrumented module doesn't work, and the module is
patched.
@santigimeno
santigimeno force-pushed the santi/instrumentation_fix branch from 353acc9 to 68e1caa Compare May 26, 2022 07:48
@linux-foundation-easycla

linux-foundation-easycla Bot commented May 26, 2022

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: santigimeno / name: Santiago Gimeno (68e1caa)

@dyladan

dyladan commented May 27, 2022

Copy link
Copy Markdown
Member

Lint failure not caused by this pull request

@vmarchaud
vmarchaud merged commit edf6157 into open-telemetry:main May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants