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

fix: Updated shimmer to allow registering instrumentation for different versions of the same module #1799

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

bizob2828
Copy link
Member

Description

We never had a test that tried to instrument the same package on different versions. This was a regression in v10 when we moved indicators of if instrumentation ran from module to the instrumentation hook. I also took the time to update an error message to indicate if an instrumentation hook was skipped because it failed or already ran. I also updated shimmer to include a few missing test cases. Also this PR will set us up for shifting registering instrumentation based on versions from the hook itself to shimmer(future change).

Links

Closes #1798

@mrickard mrickard self-assigned this Oct 6, 2023
@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Merging #1799 (b762bd9) into main (9ca78ae) will decrease coverage by 0.02%.
Report is 4 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1799      +/-   ##
==========================================
- Coverage   96.84%   96.83%   -0.02%     
==========================================
  Files         198      199       +1     
  Lines       38755    38959     +204     
==========================================
+ Hits        37531    37724     +193     
- Misses       1224     1235      +11     
Flag Coverage Δ
integration-tests-16.x 78.92% <93.45%> (+0.06%) ⬆️
integration-tests-18.x 79.17% <93.45%> (+0.05%) ⬆️
integration-tests-20.x 79.19% <93.45%> (+0.06%) ⬆️
unit-tests-16.x 91.43% <91.23%> (+0.02%) ⬆️
unit-tests-18.x 91.41% <91.23%> (+0.02%) ⬆️
unit-tests-20.x 91.41% <91.23%> (+0.02%) ⬆️
versioned-tests-16.x 73.06% <86.85%> (-2.55%) ⬇️
versioned-tests-18.x 73.06% <86.85%> (-2.55%) ⬇️
versioned-tests-20.x 72.97% <77.57%> (-2.66%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
api.js 96.14% <100.00%> (+<0.01%) ⬆️
lib/instrumentation/@elastic/elasticsearch.js 100.00% <100.00%> (ø)
lib/instrumentations.js 100.00% <100.00%> (ø)
lib/shim/datastore-shim.js 99.41% <100.00%> (+<0.01%) ⬆️
lib/shimmer.js 94.46% <100.00%> (+2.11%) ⬆️

... and 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bizob2828 bizob2828 force-pushed the allow-inst-multiple-versions branch from 20a80eb to 794fed4 Compare October 6, 2023 17:38
@bizob2828 bizob2828 force-pushed the allow-inst-multiple-versions branch from 794fed4 to b762bd9 Compare October 6, 2023 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Unable to instrument multiple version of the same module
2 participants