feat: add sequelize instrumentation #2396
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2396 +/- ##
=======================================
Coverage ? 89.11%
=======================================
Files ? 190
Lines ? 9326
Branches ? 1923
=======================================
Hits ? 8311
Misses ? 1015
Partials ? 0
🚀 New features to boost your workflow:
|
|
@seemk - please update the PR with owners according to the contributing guidelines (similar situation as with #2187) if you're still planning to get this instrumentation added. Thanks! 🙂 |
Updated |
| "repository": "open-telemetry/opentelemetry-js-contrib", | ||
| "scripts": { | ||
| "clean": "rimraf build/*", | ||
| "setup:dev": "nx run-many -t compile -p @opentelemetry/instrumentation-sequelize", |
There was a problem hiding this comment.
| "setup:dev": "nx run-many -t compile -p @opentelemetry/instrumentation-sequelize", | |
| "compile:with-dependencies": "nx run-many -t compile -p @opentelemetry/instrumentation-sequelize", |
| "prewatch": "npm run precompile", | ||
| "prepublishOnly": "npm run compile", | ||
| "tdd": "npm run test -- --watch-extensions ts --watch", | ||
| "test": "nyc mocha --require '@opentelemetry/contrib-test-utils' 'test/**/*.test.ts'", |
There was a problem hiding this comment.
Suggestions to align with the latest changes in the packages regarding testing.
| "test": "nyc mocha --require '@opentelemetry/contrib-test-utils' 'test/**/*.test.ts'", | |
| "test": "nyc --no-clean mocha --require '@opentelemetry/contrib-test-utils' 'test/**/*.test.ts'", | |
| "test:with-services-env": "cross-env NODE_OPTIONS='-r dotenv/config' DOTENV_CONFIG_PATH=../../test/test-services.env npm test", | |
| "test-merge-coverage": "nyc merge .nyc_output coverage/coverage-final.json", | |
| "test-services:start": "cd ../.. && npm run test-services:start postgres", | |
| "test-services:stop": "cd ../.. && npm run test-services:stop postgres", |
There was a problem hiding this comment.
No containers need to be started for these tests, so I don't think these lines are necessary
| }); | ||
|
|
||
| describe('postgres', () => { | ||
| const DB_SYSTEM = 'postgres'; |
There was a problem hiding this comment.
The CI controls some of the connection configs via en vars. Here is an example
There was a problem hiding this comment.
In this case these are test specific local strings to validate attributes, no actual connections need to be made.
And added |
|
With this PR, we can finally replace Excellent work, @seemk ! 👏🏻 🎉 |
8 participants
Add
sequelize instrumentationoriginally created by Aspecto.Contains minor changes compared to the original: updated to latest semantic conventions and instrumentation package, moved the tests to
assert.