Skip to content

Conversation

deejay1
Copy link
Contributor

@deejay1 deejay1 commented Jan 16, 2025

Which problem is this PR solving?

Resolves #2297

Short description of the changes

Make sure better-sqlite3 is supported by not assuming how the error constructor is set up. Adds test accordingly.

This is a rebase of #2298 by @AbhiPrasad with included review changes.

@deejay1 deejay1 requested a review from a team as a code owner January 16, 2025 11:53
@deejay1 deejay1 changed the title Knex sqlite3 v2 fix(instrumentation-knex): Support better-sqlite3 errors Jan 16, 2025
@github-actions github-actions bot added pkg:instrumentation-knex pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found. labels Jan 16, 2025
@pichlermarc pichlermarc added the bug Something isn't working label Jan 16, 2025
Copy link

codecov bot commented Jan 20, 2025

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.96%. Comparing base (5ecbd03) to head (2b3cea2).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...de/opentelemetry-instrumentation-knex/src/utils.ts 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2650      +/-   ##
==========================================
- Coverage   90.96%   90.96%   -0.01%     
==========================================
  Files         172      172              
  Lines        8137     8134       -3     
  Branches     1649     1649              
==========================================
- Hits         7402     7399       -3     
  Misses        735      735              
Files with missing lines Coverage Δ
...emetry-instrumentation-knex/src/instrumentation.ts 98.79% <100.00%> (ø)
...de/opentelemetry-instrumentation-knex/src/utils.ts 89.47% <80.00%> (-0.78%) ⬇️

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.

approving since this was already approved at #2298

@deejay1
Copy link
Contributor Author

deejay1 commented Feb 3, 2025

@pichlermarc can you merge this maybe?

@swnia
Copy link

swnia commented Feb 6, 2025

@pichlermarc or @trentm is there something blocking this getting merged and released?

@pichlermarc pichlermarc merged commit 4560d14 into open-telemetry:main Feb 6, 2025
25 checks passed
@dyladan dyladan mentioned this pull request Feb 6, 2025
AbhiPrasad added a commit to getsentry/sentry-javascript that referenced this pull request Feb 24, 2025
The main thing this change does is to upgrade all the packages to use
https://github.com/open-telemetry/opentelemetry-js/releases/tag/experimental%2Fv0.57.1

Other changes include:
- Knex instrumentation bug fix:
open-telemetry/opentelemetry-js-contrib#2650
- Fastify deprecation:
open-telemetry/opentelemetry-js-contrib#2651
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:instrumentation-knex pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

instrumentation-knex doesn't handle errors raised by better-sqlite3
4 participants