Fix multi-layer JDBC driver call depth calculation

[marcobjorge]

The #2756 request introduced a check on the call depth of the JDBC instrumentation that limits tracing with multi-layered drivers. For example, with Apache Calcite, the Calcite JDBC driver is caught on the call stack and none of the underlying connections are traced.

For a better support of different use cases the check should be on the concrete class being instrumented rather than the generic JDBC interface.

Something like:

## this
CallDepthThreadLocalMap.getCallDepth(statement.class).getAndIncrement() > 0
## instead of this
CallDepthThreadLocalMap.getCallDepth(Statement.class).getAndIncrement() > 0

[iNikem]

On the one hand, this is by design: we do want to suppress nested instrumentations that would result in nested spans with same semantic conventions, e.g. nested HTTP or DB client spans. On the other hand, there is #3965 to give end-users an option to disable this suppression. Even after implementing #3965 this specific use-case will remain unsolved, because it uses different way of suppression.

All in all, I think this is a valid request, but we should solve using usual semantic suppression, not by the specific change suggested.

[marcobjorge]

Although I do understand, however even if I exclude the top-level JDBC driver from instrumentation I don't get the underlying drivers to be instrumented because of this secondary suppression mechanism which leads me to believe this is incorrect behaviour.

I think regardless of #3965 that this particular situation should be addressed anyway and I understand my proposal for fix might not be the best one though.

[marcobjorge]

@iNikem , would you please give me some details on your vision and I'll do my best to submit a PR aligned with that vision.

[iNikem]

@marcobjorge I took a closer look and my assumption was wrong. We already implement semantic suppression there and the code you referenced explicitly says why is it needed in addition to the semantic suppression.

I have some doubts that your suggestion is the best final solution, but it seems like a good enough step in the right direction. So if that works for you, go ahead and submit a PR for this :)

@mateuszrzeszutek WDYT?

[mateuszrzeszutek]

Yeah, I think the proposed solution is a step in a good direction - you'd still have to turn off the semantic suppression though.
Another option is to implement an IgnoredTypesConfigurer that excludes known wrapper JDBC types' implementations (like Apache Calcite) from instrumentation.

[marcobjorge]

See pull/5004.

[marcobjorge]

In my tests was just not excluding ALL Calcite java.sql.{Driver,Connection,Statement,PreparedStatement} in the classpath and the javaagent instrumentation was picking up something it shouldn't and therefore instrumenting the top level driver instead of the underlying drivers.

This was caused by my incorrect usage of the javaagent - the JDBC driver call depth calculation is correct and nothing needs to be fixed.

[trask]

@marcobjorge thanks for the insights! it would definitely be nice if the javaagent can capture the "best" telemetry out-of-the-box without additional configuration. I've opened #5012 to track this further.