Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds MySQL JDBC conformance alongside PostgreSQL and MariaDB using the shared database runner and JDBC workloads.
Changes:
- Adds a pinned MySQL backend, schema, dispatch, classification, and tests.
- Adds Connector/J to both OpenTelemetry instrumentation launchers.
- Adds MySQL conformance configurations, generated coverage, and documentation.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
tools/database/runner/tests/test_database_session.py |
Tests MySQL backend dispatch. |
tools/database/runner/tests/test_database_model.py |
Verifies the MySQL span type. |
tools/database/runner/tests/test_database_coverage.py |
Tests MySQL span classification. |
tools/database/runner/tests/test_database_backends.py |
Covers MySQL lifecycle, image, schema, and errors. |
tools/database/runner/src/database_conformance/mysql.sql |
Defines the shared MySQL schema. |
tools/database/runner/src/database_conformance/_mysql.py |
Implements the MySQL container backend. |
tools/database/runner/src/database_conformance/_coverage.py |
Adds MySQL vendor span classification. |
tools/database/runner/src/database_conformance/_container.py |
Supports backend-specific startup timeouts. |
tools/database/runner/src/database_conformance/__init__.py |
Registers the MySQL backend. |
tools/database/runner/README.md |
Documents MySQL runner support. |
scenarios/database/README.md |
Documents MySQL scenarios and schema. |
scenarios/database/java/shared/jdbc/opentelemetry-library/build.gradle.kts |
Adds Connector/J to the library launcher. |
scenarios/database/java/shared/jdbc/opentelemetry-javaagent/build.gradle.kts |
Adds Connector/J to the agent launcher. |
scenarios/database/java/README.md |
Documents Java MySQL coverage. |
scenarios/database/java/mysql/jdbc/opentelemetry-library/database.yaml |
Selects MySQL for library tests. |
scenarios/database/java/mysql/jdbc/opentelemetry-library/data.json |
Records library coverage. |
scenarios/database/java/mysql/jdbc/opentelemetry-library/conformance.yaml |
Defines library conformance expectations. |
scenarios/database/java/mysql/jdbc/opentelemetry-javaagent/database.yaml |
Selects MySQL for agent tests. |
scenarios/database/java/mysql/jdbc/opentelemetry-javaagent/data.json |
Records agent coverage. |
scenarios/database/java/mysql/jdbc/opentelemetry-javaagent/conformance.yaml |
Defines agent conformance expectations. |
scenarios/database/java/gradle/libs.versions.toml |
Pins Connector/J 26.7.0. |
.github/renovate.json5 |
Enables MySQL image updates. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
trask
force-pushed
the
trask-mysql-jdbc-conformance
branch
from
August 30, 2026 22:44
9c63755 to
4fb452c
Compare
…dispatch test Review finding: test_database_session_dispatches_mysql cannot fail if the MySQL backend is never registered. monkeypatch.setitem inserts the "mysql" key into database_conformance._BACKENDS whether or not it is already there, so _backend_name's membership check and the dispatch that follows both succeed against the stub alone. Deleting "mysql": MySQL from _BACKENDS in __init__.py would leave every test in tools/database/runner/tests green while every real MySQL run fails with SpecError. Nothing else in the suite asserts that registration: test_database_backends.py imports MySQL directly, and test_database_model.py and test_database_coverage.py exercise only classify_span. Fix: assert the real registration in this test before patching it, for example import MySQL and assert database_conformance._BACKENDS["mysql"] is MySQL ahead of the monkeypatch.setitem call. Analysis: pytest's monkeypatch.setitem writes the key unconditionally and records notset for the undo when the key was absent, so the stub the test installs also supplies the registry entry the test means to check. Reading the real entry before the patch is applied is the only point where the production registry is still visible, so the assertion goes there. It compares against the MySQL class rather than merely checking membership, which also catches an entry wired to the wrong backend. Upsides: removing or misdirecting "mysql": MySQL in database_conformance._BACKENDS now fails a test instead of failing only a real container run, and the test's name and its coverage agree. Downsides: the test imports the private module database_conformance._mysql, which tools/database/runner/tests/test_database_backends.py already does, and it reads the private _BACKENDS mapping, which the surrounding tests already do. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hes the wait strategy Review finding: The new per-backend startup timeout is never verified by a test. BackendSpec gains startup_timeout_seconds and start() now feeds it to ExecWaitStrategy.with_startup_timeout, but the only assertion any test makes about the wait strategy is `assert container.wait_strategy is not None` in test_starts_initializes_publishes_and_removes_database. A regression that dropped self._spec.startup_timeout_seconds and went back to the shared 60-second default would keep the whole suite green, and MySQL, the one backend that needs the longer window, would start failing intermittently only in real container runs. Fix: assert the applied timeout in test_database_backends.py, reading it from the StubContainer's recorded wait strategy, and parametrize it so MySQL expects 120 seconds while PostgreSQL and MariaDB expect the 60-second default. Analysis: testcontainers stores the startup timeout privately and exposes no getter, so the recorded strategy object cannot be read back. The test substitutes a subclass of ExecWaitStrategy in the _container module, the same seam install_stub already uses for DockerContainer, and records what start() passes to the public with_startup_timeout. It clears OTEL_CONFORMANCE_DATABASE_STARTUP_TIMEOUT so an ambient override cannot mask the per-backend value. Restoring the previous shared-default call in _container.py makes the MySQL case fail with 60 against an expected 120, so the test does catch the regression it names. Upsides: the per-backend startup timeout is now covered for all three backends, and MySQL's longer window is pinned at the value the backend declares instead of being visible only in a real container run. Downsides: the test depends on ExecWaitStrategy.with_startup_timeout staying the method start() calls, so a testcontainers API change would need it updated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pull request dashboard statusWaiting on the author · refreshed 2026-09-17 06:53 UTC Move out of draft to request review. Status above doesn't look right?
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds MySQL to the database conformance runner and measures the shared JDBC workload through both the OpenTelemetry Java agent and
opentelemetry-jdbc.mysql:9.7.2-oraclelinux9by digest and bootstraps the shared empty schema.db.mysql.clientand commits generated coverage for both packages.