OpenTelemetry JDBC instrumentation library

[donbeave]

Hello!

This is the first preview of JDBC instrumentation as a library, which started from this discussion: #3309. It's not the final implementation yet, there are not any tests and documentation at the moment. I just want to confirm I'm going in the right direction. Most of this work is based on the code provided by https://github.com/opentracing-contrib/java-jdbc and utilizes the current JDBC javaagent implementation API classes, that's why I have moved them to the library module.

I have few questions here.

1. Maybe it's better to rename package for utilities classes, which now located in library module? My suggestion rename io.opentelemetry.javaagent.instrumentation.jdbc to io.opentelemetry.instrumentation.jdbc.
2. I didn't port Slow Query and Fast Query from the original OpenTracing Instrumentation for JDBC. Do I need to do it here or not? It looks like for me, if port this functionality, it must be ported to javaagent implementation as well. If so I think it will be better to use io.opentelemetry.instrumentation.api.config.Config to configure these features.
3. The current otel.library.name is io.opentelemetry.javaagent.jdbc. Maybe it's time to drop javaagent prefix and rename it to io.opentelemetry.jdbc.

Please let me know if anything needs to be changed or improved here.

[anuraaga]

Thanks @donbeave

Maybe it's better to rename package for utilities classes, which now located in library module?

Yup nothing in that module should have javaagent in the package name. For any helpers that need to be accessed from both library and javaagent code, move them to an internal subpackage

I didn't port Slow Query and Fast Query from the original OpenTracing Instrumentation for JDBC.

Let's avoid adding any new features, we'd probably want to go through opentelemetry specification for this if we want to

Maybe it's time to drop javaagent prefix and rename it to io.opentelemetry.jdbc

Sounds good (@trask et al maybe we should rename all our instrumentation names that currently include javaagent, even if we don't have library instrumentation for them yet)

[donbeave]

@marcingrzejszczak @anuraaga please let me know if anything else needs to be improved or fixed. Btw, there are two build steps are failed accept-pr and muzzle (:instrumentation:spring:spring-rabbit-1.0:javaagent), but both seems not related to my changes:

[anuraaga]

Just found one point about the driver version we report which I think is blocking, but other then that looks great

[trask]

there are two build steps are failed accept-pr and muzzle (:instrumentation:spring:spring-rabbit-1.0:javaagent), but both seems not related to my changes

👍 I opened #3417 to track/fix the muzzle failure. accept-pr is an aggregating build step, so fails when any other step fails. I'll hit retry on the build, good chance it will pick up different set of spring-rabbit versions to check.

[anuraaga]

Thanks!

[anuraaga]

@donbeave I've merged in #3414 to this branch so you may need to git pull if any other changes are needed

[donbeave]

@anuraaga Got it! Thank you!

[trask]

thanks @donbeave! this is a great addition! ❤️