Inconsistent datasource metrics configuration between MP Metrics and Micrometer

[mjiderhamn]

Describe the bug

The datasource documentation says

By default, this metric collection mechanism gets turned on for all data sources if a metrics extension is present and metrics for the Agroal extension are enabled.

and the all config options documentation indicate that

quarkus.datasource.jdbc.enable-metrics - If unspecified, collecting metrics will be enabled by default if a metrics extension is active.

However, when using the Micrometer extension quarkus-micrometer it is necessary to explicitly set quarkus.datasource.jdbc.enable-metrics to true, in addition to quarkus.datasource.metrics.enabled=true.

The reason seems to be that quarkus-micrometer does not declare to have the io.quarkus.metrics capability, the same way that the SmallRye metrics extension quarkus-smallrye-metrics does.

Expected behavior

Using the Micrometer extension quarkus-micrometer, Agroal datasource metrics should be collected after setting quarkus.datasource.metrics.enabled=true, with quarkus.datasource.jdbc.enable-metrics left unset.

Actual behavior

quarkus.datasource.jdbc.enable-metrics needs to be explicitly set to true.

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

[quarkus-bot[bot]]

/cc @ebullient, @jmartisk

[ebullient]

The help text is wrong, I think. We have started to be more conservative with the types of metrics that are automatically turned on: they aren't free. JDBC metrics (in particular) can be expensive.

Metrics support in the runtime is not capability-based, it uses a build item instead (to ensure single provider). This changed in the 1.8/1.9 timeframe (when the micrometer extension was introduced): https://quarkus.io/guides/writing-extensions#extension-metrics

[ebullient]

The problem lies in DataSources.applyNewConfiguration:

        // metrics
        if (dataSourceJdbcBuildTimeConfig.enableMetrics.isPresent()) {
            dataSourceConfiguration.metricsEnabled(dataSourceJdbcBuildTimeConfig.enableMetrics.get());
        } else {
            // if the enable-metrics property is unspecified, treat it as true if MP Metrics are being exposed
            dataSourceConfiguration.metricsEnabled(dataSourcesBuildTimeConfig.metricsEnabled && mpMetricsPresent);
        }

I don't remember why MP Metrics is handled differently.

[mjiderhamn]

The problem lies in DataSources.applyNewConfiguration ... I don't remember why MP Metrics is handled differently.

Yes, and as far as I could tell mpMetricsPresent was set based on the io.quarkus.metrics capability.

[jmartisk]

So the problem is that with SmallRye, metrics are enabled by default for all data sources, but disabled with Micrometer. The javadoc implies that they are enabled in both cases. If we're not going to change the behavior, the javadoc should be changed, but I think it would make sense to unify this. If the global setting quarkus.datasource.metrics.enabled is true, and that needs to be set explicitly (!), why should we not enable them for all data sources by default? Most microservice apps only have one data source anyway.

[wind57]

I find it awkward that we can't do it per named datasource, we have to enable for all and disable for some:

quarkus:
  datasource:
    metrics:
      enabled: true
    jdbc:
      enable-metrics: true

    datasource-A:
        enable-metrics: true
    datasource-B:
        enable-metrics: false

[geoand]

Do we still care about this one?

[hajdukda]

Thank you for explanation, was wondering why metrics are missing.

[Ruudieboy]

I agree the documentation is a bit confusing.

[geoand]

@jmartisk @brunobat is there something we should do here to clear the confusion once and for all?

[brunobat]

I'm creating a PR and will also include the opentelemety-jdbc lib as a dependency so we don't need to declare it explicitly.

[brunobat]

I could not reproduce your issue anymore. quarkus.datasource.metrics.enabled=true is enough to activate the agroal metrics related with the JDBC connection. You can also see that the capability is being registered when quarkus-micrometer is present, here: https://github.com/quarkusio/quarkus/blob/58b40e7ed2baa526bc77421ab5db5f6407a36ff4/extensions/micrometer/runtime/pom.xml#L161

Saying this, the documentation is outdated, please see the related PR: https://github.com/quarkusio/quarkus/pull/47976 We cannot remove the old quarkus.datasource.jdbc.enable-metrics prop for now but I marked it as deprecated.

The PR also includes a setup simplification for tracing.