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.

Sign up for GitHub

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(instrumentation-mongodb): update mongodb to version without missing types #1036

Closed
wants to merge 1 commit into from
Closed

fix(instrumentation-mongodb): update mongodb to version without missing types #1036

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions plugins/node/opentelemetry-instrumentation-mongodb/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
"@types/node": "16.11.21",
"gts": "3.1.0",
"mocha": "7.2.0",
"mongodb": "3.6.11",
"mongodb": "4.6.0",
Copy link
Member

Choose a reason for hiding this comment

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

Having mongodb in dev deps only will result in missing types for consumers.
Adding a specific mongodb version as dependency seems not good.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree. thats why this is a draft. i'm trying to figure out other options. Unfortunately, I think we're going to have to stop trying to type check modules we're patching. causing too many dependency troubles.

Copy link
Member

Choose a reason for hiding this comment

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

For the patching part it's quite simple as the instrumentation base classes don't need/use the types at all.
The problem are the hooks offered by some instrumentations as they are APIs to end users. We could use unknown/any there and just document what is provided and user as to cast. This would even cover differences between versions.

Copy link
Member

Choose a reason for hiding this comment

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

This is deliberately set to version 3 as it is what we run in CI when invoking npm run test:

    "test": "nyc ts-mocha -p tsconfig.json --require '@opentelemetry/contrib-test-utils' 'test/**/mongodb-v3.test.ts'",

Since we can only run a single version, version 3 was choosen as it is more common

"nyc": "15.1.0",
"rimraf": "3.0.2",
"test-all-versions": "5.0.1",
Expand All @@ -67,7 +67,6 @@
},
"dependencies": {
"@opentelemetry/instrumentation": "^0.28.0",
"@opentelemetry/semantic-conventions": "^1.0.0",
"@types/mongodb": "3.6.20"
"@opentelemetry/semantic-conventions": "^1.0.0"
}
}