test(instrumentation-mysql2): Add test for mysql2/promise #2525

Karibash · 2024-11-13T07:32:26Z

close #650

Which problem is this PR solving?

Add test for mysql2/promise.

Short description of the changes

I added a test to confirm that @opentelemetry/instrumentation-mysql2 works with mysql2/promise.

linux-foundation-easycla · 2024-11-13T07:32:30Z

The committers listed above are authorized under a signed CLA.

✅ login: Karibash / name: Karibash (010b96d, 59b934b, c68038c)

github-actions · 2024-11-14T06:38:16Z

This package does not have an assigned component owner and is considered unmaintained. As such this package is in feature-freeze and this PR will be closed with 14 days unless a new owner or a sponsor (a member of @open-telemetry/javascript-approvers) for the feature is found. It is the responsibility of the author to find a sponsor for this feature.
Are you familiar with this package? Consider becoming a component owner.

github-actions · 2024-11-28T06:39:31Z

This issue was closed because no owner or sponsor has been found after 14 days

trentm · 2024-11-28T17:27:57Z

I'll sponsor.

trentm

Thanks!
One nit for a possible simplification.

trentm · 2024-11-28T17:33:15Z

plugins/node/opentelemetry-instrumentation-mysql2/test/mysql2-promise.test.ts

+    if (isPoolClusterEndIgnoreCallback()) {
+      await poolCluster.end();
+    } else {
+      await poolCluster.end();
+    }


I'm am guessing you could drop function isPoolClusterEndIgnoreCallback() below and simplify this block to:

Suggested change

if (isPoolClusterEndIgnoreCallback()) {

await poolCluster.end();

} else {

await poolCluster.end();

}

await poolCluster.end();

Fixed in the following commit. In addition, the ESLint error is also fixed.
ef5d5a6

plugins/node/opentelemetry-instrumentation-mysql2/test/mysql2-promise.test.ts

+const port = Number(process.env.MYSQL_PORT) || 33306;
+const database = process.env.MYSQL_DATABASE || 'test_db';
+const host = process.env.MYSQL_HOST || '127.0.0.1';
+const user = process.env.MYSQL_USER || 'otel';


plugins/node/opentelemetry-instrumentation-mysql2/test/mysql2-promise.test.ts

+    provider.addSpanProcessor(new SimpleSpanProcessor(memoryExporter));
+    rootConnection = await mysqlTypes.createConnection({
+      port,
+      user: 'root',


codecov · 2024-11-28T17:48:22Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.75%. Comparing base (5eb61d8) to head (d42bfc2).
Report is 12 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2525   +/-   ##
=======================================
  Coverage   90.75%   90.75%           
=======================================
  Files         169      169           
  Lines        8018     8018           
  Branches     1632     1632           
=======================================
  Hits         7277     7277           
  Misses        741      741

see 1 file with indirect coverage changes

trentm · 2024-11-28T18:12:54Z

@Karibash You'll need to run 'npm run lint:fix' to fix a style error in your added file.

trentm · 2024-11-28T19:02:23Z

@Karibash Then some of the "TAV" checks are failing. TAV stands for "test-all-versions". The TAV tests effectively run npm test for a number of version of the mysql2 dependency.

Looking at some of the TAV test runs above, it looks like:

the tests passed with [email protected]
the tests failed with [email protected]

Do you know what mysql2 version added support for mysql2/promises? We may want to get the tests in mysql2-promises.test.ts to be skipped in older version of mysql2. I can help with that if you need.

Some mysql2 release dates:

  '2.2.5': '2020-09-21T12:28:13.157Z',
  '2.3.0': '2021-08-05T13:47:23.743Z',
  '2.3.1': '2021-10-15T11:42:14.888Z',
  '2.3.2': '2021-10-16T06:08:46.680Z',
  '2.3.3': '2021-11-14T04:17:46.192Z',
  '3.0.0': '2023-01-12T04:51:01.542Z',

Perhaps [email protected] added promises support, or changed how its promises support works, such that this instrumentation works with it?

You should be able to test locally by running:

cd .../plugins/node/opentelemetry-instrumentation-mysql2
npm install --no-save [email protected]    # or some other version
RUN_MYSQL_TESTS=1 npm test

Karibash · 2024-11-29T02:17:26Z

@trentm
Thank you for your detailed explanation!
The reason the test was failing was that the Promise wrapper for PoolCluster did not exist in versions prior to v2.3.0.
In the following commit, I have modified the test so that the Promise tests for PoolCluster are only performed for v2.3.0 and later.
8877f75

…poolCluster

…ecuted from v2.3.0 or later

david-luna · 2025-01-24T12:53:34Z

@Karibash

#2662 already added some tests for mysql2/promise and conflicts with your PR. Do you think that's enough or do you wish to move this PR forward?

Karibash · 2025-01-24T14:44:04Z

@david-luna
If it has already been resolved, then it can't be helped. I'll close this PR. It's a bit disappointing that it was left unmerged for two months in the end 😢

Karibash requested a review from a team as a code owner November 13, 2024 07:32

Karibash force-pushed the feature/mysql2-promise branch from c848eb5 to 4b73476 Compare November 13, 2024 08:09

Karibash marked this pull request as draft November 13, 2024 11:26

Karibash force-pushed the feature/mysql2-promise branch from 4b73476 to 67004ba Compare November 13, 2024 15:20

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 Nov 13, 2024

Karibash force-pushed the feature/mysql2-promise branch from 67004ba to 104161c Compare November 13, 2024 15:21

Karibash changed the title ~~feat: mysql2/promise instrumentation~~ test(instrumentation-mysql2): Add test for mysql2/promise Nov 13, 2024

Karibash force-pushed the feature/mysql2-promise branch from 104161c to d42bfc2 Compare November 13, 2024 15:24

Karibash marked this pull request as ready for review November 13, 2024 15:24

github-actions bot added the pkg-status:unmaintained:autoclose-scheduled label Nov 14, 2024

github-actions bot closed this Nov 28, 2024

trentm reopened this Nov 28, 2024

trentm added the has:sponsor This package or feature has a sponsor that has volunteered to review PRs and respond to questions label Nov 28, 2024

trentm approved these changes Nov 28, 2024

View reviewed changes

github-advanced-security bot found potential problems Nov 28, 2024

View reviewed changes

Karibash added 3 commits December 11, 2024 13:41

test(instrumentation-mysql2): Add test for mysql2/promise

010b96d

chore(instrumentation-mysql2): Simplify the process of terminating a …

59b934b

…poolCluster

fix(instrumentation-mysql2): Make Promise test of PoolCluster be ex…

c68038c

…ecuted from v2.3.0 or later

Karibash force-pushed the feature/mysql2-promise branch from 8877f75 to c68038c Compare December 11, 2024 04:42

Karibash closed this Jan 24, 2025

Karibash deleted the feature/mysql2-promise branch January 24, 2025 14:44

test(instrumentation-mysql2): Add test for mysql2/promise #2525

test(instrumentation-mysql2): Add test for mysql2/promise #2525

Uh oh!

Conversation

Karibash commented Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which problem is this PR solving?

Short description of the changes

Uh oh!

linux-foundation-easycla bot commented Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 14, 2024

Uh oh!

github-actions bot commented Nov 28, 2024

Uh oh!

trentm commented Nov 28, 2024

Uh oh!

trentm left a comment

Choose a reason for hiding this comment

Uh oh!

trentm Nov 28, 2024

Choose a reason for hiding this comment

Uh oh!

Karibash Nov 29, 2024

Choose a reason for hiding this comment

Uh oh!

Check failure

Check failure

codecov bot commented Nov 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

trentm commented Nov 28, 2024

Uh oh!

trentm commented Nov 28, 2024

Uh oh!

Karibash commented Nov 29, 2024

Uh oh!

david-luna commented Jan 24, 2025

Uh oh!

Karibash commented Jan 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Karibash commented Nov 13, 2024 •

edited

Loading

linux-foundation-easycla bot commented Nov 13, 2024 •

edited

Loading

codecov bot commented Nov 28, 2024 •

edited

Loading