-
Notifications
You must be signed in to change notification settings - Fork 225
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: add support for tedious v17 #3901
Conversation
Co-authored-by: Trent Mick <trent.mick@elastic.co>
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.
I think also want this change:
diff --git a/test/instrumentation/modules/tedious.test.js b/test/instrumentation/modules/tedious.test.js
index 085e0dca..5a342339 100644
--- a/test/instrumentation/modules/tedious.test.js
+++ b/test/instrumentation/modules/tedious.test.js
@@ -24,6 +24,7 @@ const tediousVer =
require('../../../node_modules/tedious/package.json').version;
const semver = require('semver');
if (
+ (semver.gte(tediousVer, '17.0.0') && semver.lt(process.version, '18.0.0')) ||
(semver.gte(tediousVer, '16.0.0') && semver.lt(process.version, '16.0.0')) ||
(semver.gte(tediousVer, '15.0.0') && semver.lt(process.version, '14.0.0')) ||
(semver.gte(tediousVer, '12.0.0') && semver.lt(process.version, '12.3.0')) ||
or similar. However, tedious@17.0.0 hasn't yet made any changes that actually break on node v16.
@david-luna Did you see this comment? #3901 (review) |
sorry no, I was distracted the the mongodb issue in OTel. Your propose change makes sense, I'm going to apply it now. |
…m-agent-nodejs into dluna/feat-support-tedious-17
Co-authored-by: Trent Mick <trent.mick@elastic.co>
v17 release does not have major changes but just drops node versions support.
Checklist