Skip to content

Conversation

@david-luna
Copy link
Contributor

@david-luna david-luna commented Jun 16, 2025

Which problem is this PR solving?

Short description of the changes

  • add docker compose file & .env file for testing
  • add script to spin up services for testing
  • move MySQL log config command from workflow to tests

@github-actions github-actions bot added pkg:instrumentation-mysql pkg:instrumentation-mysql2 pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found. labels Jun 16, 2025
@david-luna david-luna changed the title Add docker compose and test serivces scripts chore: Add docker compose and test services scripts Jun 16, 2025
@codecov
Copy link

codecov bot commented Jun 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.82%. Comparing base (50af7ec) to head (d049606).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2886      +/-   ##
==========================================
+ Coverage   89.77%   89.82%   +0.04%     
==========================================
  Files         187      187              
  Lines        9147     9147              
  Branches     1884     1884              
==========================================
+ Hits         8212     8216       +4     
+ Misses        935      931       -4     

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Contributor

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.

@david-luna david-luna marked this pull request as ready for review June 18, 2025 10:16
@david-luna david-luna requested a review from a team as a code owner June 18, 2025 10:16
- uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node }}
- name: Set MySQL variables
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note for reviewer: this is required only for testing @opentelemetry/instrumentation-mysql and can be executed as a query using the client. Hence is removed from there and added in plugins/node/opentelemetry-instrumentation-mysql/test/mysql.test.ts

- uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node }}
- name: Set MySQL variables
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note for reviewer: same here

const shouldTest = process.env.RUN_MYSQL_TESTS;

before(async function () {
const connection = createConnection({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note for reviewer: enabling here the general_log setting instead of doing it in the workflows

user: process.env.POSTGRES_USER || 'postgres',
password: process.env.POSTGRES_PASSWORD || 'postgres',
database: process.env.POSTGRES_DB || 'postgres',
database: process.env.POSTGRES_DB || 'otel_pg_database',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note for reviewer: align the value with the one in workflow services

Copy link
Member

@raphael-theriault-swi raphael-theriault-swi left a comment

Choose a reason for hiding this comment

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

Love this, small nit tho.

Copy link
Contributor

@trentm trentm left a comment

Choose a reason for hiding this comment

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

Should do the cross-env thing. Else I just have nits. Thanks for picking this up!

If you only want to test a sigle package (e.g. the `instrumentation-mongodb`) you can `cd` into it and run the tests after you started the services.

```sh
npm run test-services:start # starts services in Docker
Copy link
Contributor

Choose a reason for hiding this comment

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

This could optionally be npm run test-services:start mongodb but probably don't need to bother mentioning this. Easier to keep it simple.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm planning to do a follow up PR to add scripts for each package which will use this feature. I'll remember to update this description.

@pichlermarc pichlermarc added has:sponsor This package or feature has a sponsor that has volunteered to review PRs and respond to questions and removed pkg-status:unmaintained:autoclose-scheduled labels Jun 25, 2025
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.

thank you for working on this! 🙂

@david-luna david-luna merged commit 2aef158 into open-telemetry:main Jun 26, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

has:sponsor This package or feature has a sponsor that has volunteered to review PRs and respond to questions pkg:instrumentation-mysql pkg:instrumentation-mysql2 pkg:instrumentation-pg 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.

5 participants