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-mysql2): instrument connection on 3.11.5 #2579

Merged

Conversation

pichlermarc
Copy link
Member

@pichlermarc pichlermarc commented Dec 4, 2024

Description:

[email protected] did include a refactoring that moved some of the functionality we instrument to a base class. This PR adapts to instrumenting the base class instead if one exists and the name of the base is known to us.

Fixes #2572

@github-actions github-actions bot added pkg:instrumentation-mysql2 pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found. labels Dec 4, 2024
Copy link

codecov bot commented Dec 4, 2024

Codecov Report

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

Project coverage is 90.75%. Comparing base (8bfa21f) to head (705a6a4).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../opentelemetry-instrumentation-mysql2/src/utils.ts 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2579      +/-   ##
==========================================
- Coverage   90.75%   90.75%   -0.01%     
==========================================
  Files         169      169              
  Lines        8018     8026       +8     
  Branches     1632     1635       +3     
==========================================
+ Hits         7277     7284       +7     
- Misses        741      742       +1     
Files with missing lines Coverage Δ
...etry-instrumentation-mysql2/src/instrumentation.ts 94.66% <100.00%> (ø)
.../opentelemetry-instrumentation-mysql2/src/utils.ts 93.18% <87.50%> (-1.27%) ⬇️

@pichlermarc pichlermarc marked this pull request as ready for review December 4, 2024 14:00
@pichlermarc pichlermarc requested a review from a team as a code owner December 4, 2024 14:00
@pichlermarc pichlermarc added bug Something isn't working priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect labels Dec 4, 2024

// [email protected] included a refactoring, where most code was moved out of the `Connection` class and into a shared base
// so we need to instrument that instead, see https://github.com/sidorares/node-mysql2/pull/3081
if (basePrototype?.constructor?.name === 'BaseConnection') {
Copy link
Contributor

Choose a reason for hiding this comment

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

This reminds me of #2522
What would you think about checking the presence of the functions in the base class' prototype?

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea, let's try that, one sec.

Copy link
Member Author

Choose a reason for hiding this comment

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

@david-luna doing that now (see latest commits)

@pichlermarc pichlermarc merged commit e674b1b into open-telemetry:main Dec 4, 2024
25 checks passed
@dyladan dyladan mentioned this pull request Dec 4, 2024
@pichlermarc pichlermarc deleted the fix/mysql-2-refactor branch December 4, 2024 16:14
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-mysql2 pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found. priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[instrumentation-mysql2] tests time out for [email protected]
2 participants