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(instrumentation-knex): Support better-sqlite3 errors #2650

Merged
merged 13 commits into from
Feb 6, 2025

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
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