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

Adjust Apache/Drill tracing to cover the shared superclass from Apache/Calcite that the Drill class delegates to #3424

Merged
merged 1 commit into from Mar 3, 2022

Conversation

mcculls
Copy link
Contributor

@mcculls mcculls commented Mar 3, 2022

DrillPreparedStatementImpl extends AvaticaPreparedStatement and while it does override most of its methods it doesn’t override executeQuery - this means we need to move up to trace AvaticaPreparedStatement to catch that call

Tracing the super-class is safe here because DrillPreparedStatementImpl ends up calling the AvaticaPreparedStatement methods to do the actual work of talking to the database - the DrillPreparedStatementImpl sub-class just adds some pre/post-condition checks that we don’t need to cover with tracing, as well as some non-JDBC methods specific to Apache/Drill

This is slightly complicated by the fact that drill-jdbc-all-1.15.0.jar refactors its copy of AvaticaPreparedStatement under the oadd prefix, so we need to add it twice - once with and once without the prefix

…e/Calcite that the Drill class delegates to.

Also include the shaded package name used in the 'drill-jdbc-all' uber-jar
@mcculls mcculls added inst: others All other instrumentations inst: jdbc JDBC instrumentation labels Mar 3, 2022
@mcculls mcculls requested a review from a team as a code owner March 3, 2022 11:18
@mcculls mcculls requested a review from eric-ming2 March 3, 2022 11:34
Copy link
Contributor

@eric-ming2 eric-ming2 left a comment

Choose a reason for hiding this comment

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

Nice catch, this is complicated stuff!

@mcculls mcculls merged commit d8ff93d into master Mar 3, 2022
@mcculls mcculls deleted the mcculls/drill-jdbc-all branch March 3, 2022 13:51
@github-actions github-actions bot added this to the 0.97.0 milestone Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inst: jdbc JDBC instrumentation inst: others All other instrumentations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants