Support jOOQ 3.15 and later

[kousalik]

Describe the bug
Class CreateTableColumnStep has been removed from the jOOQ implementation in this commit in February '22.
Using the MetricsDSLContext with a version without that class causes a runtime exception.

Environment

• Micrometer version: 1.10.6
• jOOQ version: 3.18.3
• OS: any
• Java version: any

To Reproduce
How to reproduce the bug:
Attempt to use MetricsDSLContext implementation with one of the latest jOOQ releases. Fails at runtime due to java.lang.ClassNotFoundException: org.jooq.CreateTableColumnStep

Expected behavior
Normal function.

[jonatan-ivanov]

Right now we compile against JOOQ 3.14.x, we should look into upgrading it, thank you for reporting it. I think the only immediate workaround I can give you is downgrading.

[diastremskii]

Hey! 👋
Another issue here is that jOOQ dropped support for Java 8 some time ago. Since there is already multi-release jar present, the updated version could be moved to Java 11, or both versions can be kept, the one for jOOQ 3.14.x as a default one and the one for jOOQ 3.17.x as the Java 11 one.
I've made the changes to support jOOQ 3.17.x and pushed them here, they pass the tests (if you change the baseline version to 11), but I'm not exactly sure what's the best way to include them in multi-release jar. Would be great if someone could help with that!

[shakuzen]

@diastremskii thank you for the offer and starting work on it. I can understand how that sounds like an appealing solution and it may work in practice, but since the API of the class will be different between the multi-releases, it sounds like it goes against the design of the multi-release jar feature. JEP 238 has the following to say about it:

Every version of the library should offer the same API; investigation is required to determine whether this should be strict backwards compatibility where the API is exactly the same (byte code signature equality), or whether this can be relaxed to some degree without necessarily enabling the introduction of new enhancements that would blur the notion of one unit of release. This may imply, at a minimum, that a public class present in a release-specific directory should also be present in the root, though it need not be present in an earlier release directory. The run-time system will not verify this property, but tooling can and should detect such API compatibility issues, and a library method may also be provided to perform such varification (for example on java.util.jar.JarFile).

Given that, I'm not sure we should try to go that route. It would be more ideal if we can offer instrumentation that isn't affected by so many unrelated changes in jOOQ and therefore is compatible across a much wider range of versions.

[shakuzen]

Looking at the existing instrumentation, our pain is in implementing DSLContext. Under the hood, the instrumentation is done with a package-private JooqExecuteListener. I wonder if it would be an acceptable compromise to users if we made the JooqExecuteListener public and deprecated/removed the MetricsDSLContext. Then users would need to configure the JooqExecuteListener themselves, but I think it would be much more compatible across versions. Could any jOOQ users provide feedback on this? It was previously brought up in #2686 (comment) by @mdindoffer.

This would lose the ability to add tags for a specific query like this example in the class JavaDoc. We would need to find another way to offer similar functionality if this is important to users.

jooq.tag("name", "selectAllAuthors").select(asterisk()).from("author").fetch();

[ferdinand-swoboda]

Could any jOOQ users provide feedback on this?

FWIW we internally maintain a PicnicMetricsDslContext implements DslContext that is effectively a MetricsDslContext compatible with newer jOOQ versions. Exposing the JooqExecuteListener would certainly be handy since adding that to our standardised jOOQ configuration is trivial.
However, we have some non-critical usages of context.tag(...) that might turn out as blockers for relying entirely on Micrometer's JooqExecuteListener.

Are there (significant) downsides to exposing JooqExecuteListener?
It opens up the API but it does not prevent further development/iterations on MetricsDslContext AFAICS.

[shakuzen]

Thanks for quickly giving us feedback on this, @ferdinand-swoboda.

Are there (significant) downsides to exposing JooqExecuteListener?

I don't think so. It's more a matter of it wasn't needed and no one asked for it before. It would also create two ways of instrumenting compared to now where using the MetricsDslContext is the only way.

It opens up the API but it does not prevent further development/iterations on MetricsDslContext AFAICS.

The goal would be to deprecate and eventually remove our MetricsDslContext because we do not have a good way to keep this compatible with the latest versions given the Java baseline requirement of free support jOOQ versions. Aside from that issue, it requires maintaining a lot of code unrelated to instrumentation due to the nature of implementing DslContext.

If you are maintaining your own DslContext implementation that could use JooqExecuteListener if we opened it up, then it sounds like that will work for you as you can have the tag methods in your DslContext implementation. I worry, though, if we are dooming jOOQ users to all have to maintain their own DslContext implementation and how onerous that is.

[ferdinand-swoboda]

If you are maintaining your own DslContext implementation that could use JooqExecuteListener if we opened it up, then it sounds like that will work for you as you can have the tag methods in your DslContext implementation.

👍🏼

I worry, though, if we are dooming jOOQ users to all have to maintain their own DslContext implementation and how onerous that is.

This issue currently prevents users from upgrading jOOQ beyond v3.15 (not sure if it's even patched anymore, current is 3.19) which is IMHO worse than having to maintain a separate, lightweight adapter class. It surely ain't great but our internal equivalent class needs to be touched at most once per year and IIUC it's only needed if a jOOQ user is also interested in assigning tags(..).

I suspect if an alternative integration path in/outside jOOQ opens up (see e.g. jOOQ/jOOQ#15053) some degree of downstream work is unavoidable, whether one uses JooqExecuteListener or not.

[shakuzen]

I ended up going with a different approach in #4727 of extending the DefaultDSLContext instead of implementing all the methods in DSLContext. That should be more compatible and sustainable going forward.

[shakuzen]

Everyone who was affected by this, please try out the changes in 1.13.0-SNAPSHOT or the 1.13.0-M1 milestone planned for next week. Let us know if you experience any issues.

[ferdinand-swoboda]

Everyone who was affected by this, please try out the changes in 1.13.0-SNAPSHOT or the 1.13.0-M1 milestone planned for next week. Let us know if you experience any issues.

Apologies for not testing this earlier; this causes a NoSuchMethodError when upgrading to v1.13.

[shakuzen]

Thanks for letting us know about the issue. We cannot compile against higher than jooq 3.14 due to the minimum Java version required. However, we are trying to test compatibility of our instrumentation with newer versions of jooq in a sample project that runs with Java 17+ and the latest jooq (3.19.8 right now). See https://github.com/micrometer-metrics/micrometer/blob/a3aeb9b707d4c7d27f4bbd46dede48c7bb56abec/samples/micrometer-samples-jooq/src/test/java/io/micrometer/samples/jooq/MetricsDSLContextTest.java

I'm wondering why the sample isn't failing. Regardless, I'm not sure what we can do about this since TableLike and the corresponding method that uses it is not there in DSLContext in jooq 3.14.

[jonatan-ivanov]

I think the real solution for these issues would be instrumenting JOOQ in JOOQ: jOOQ/jOOQ#15053. For this to happen I think Lukas (maintainer of JOOQ) needs masses of users requesting this, so I think a 👍🏼 and/or a comment on the issue might help a little.