Skip to content

Conversation

@satazor
Copy link
Contributor

@satazor satazor commented Mar 24, 2024

Which problem is this PR solving?

The constructor of the instrumentation runtime node does not mixin the default config. This means that if I pass an empty object, the eventLoopUtilizationMeasurementInterval will be undefined, which has the side-effect of the configured interval to be near 0.

Short description of the changes

Similarly to all other instrumentations, I've mixed in the default config to make sure default objects are always defined. Please note that the instrumentation file itself didn't have tests, so I haven't added any.

@satazor
Copy link
Contributor Author

satazor commented Mar 24, 2024

cc @d4nyll @pichlermarc @trentm

@codecov
Copy link

codecov bot commented Mar 24, 2024

Codecov Report

Merging #2036 (b25230d) into main (dfb2dff) will increase coverage by 0.00%.
Report is 15 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2036   +/-   ##
=======================================
  Coverage   90.97%   90.98%           
=======================================
  Files         146      145    -1     
  Lines        7492     7454   -38     
  Branches     1502     1488   -14     
=======================================
- Hits         6816     6782   -34     
+ Misses        676      672    -4     
Files Coverage Δ
...nstrumentation-runtime-node/src/instrumentation.ts 89.18% <100.00%> (ø)

... and 5 files with indirect coverage changes

Copy link
Member

@d4nyll d4nyll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @satazor

Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks 👍

@pichlermarc pichlermarc merged commit 146edfa into open-telemetry:main Mar 25, 2024
@dyladan dyladan mentioned this pull request Mar 25, 2024
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.

3 participants