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 ClassCastException in JDBC instrumentation #6088

Merged
merged 4 commits into from
Jun 16, 2022

Conversation

laurit
Copy link
Contributor

@laurit laurit commented May 24, 2022

Resolves #6015
If jdbc instrumentation is active in both parent and child class loader it is possible to read DbInfo that belongs to the wrong class loader from

This pr attempts to fix this by moving DbInfo to boot loader in javaagent so that it wouldn't matter where the DbInfo is create or used.

@laurit laurit requested a review from a team May 24, 2022 10:59
@laurit laurit changed the title Move DbInfo to boot loader Fix ClassCastException in JDBC instrumentation May 24, 2022
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

how painful would it be to write a test that covers this?

plugins {
id("otel.javaagent-bootstrap")
}

Copy link
Member

Choose a reason for hiding this comment

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

I think this file would be a good place to document why DbInfo needs to be in the bootstrap class loader, maybe walking through an example of how the wrong DbInfo class can get picked up without this

@laurit laurit force-pushed the jdbc-class-cast branch from fd463e3 to a88e04f Compare June 16, 2022 12:04
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

thx ❤️

@trask trask merged commit b4a1e2e into open-telemetry:main Jun 16, 2022
@laurit laurit deleted the jdbc-class-cast branch July 6, 2023 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JDBC instrumentation: classloader issue
2 participants