Metrics - unify the naming of the config properties and the default behavior

[mkouba]

It seems that most extensions expose a boolean config property with the metrics.enabled suffix which is false by default. However, there are few exceptions. We should probably unify the naming of the properties and the default behavior.

Extension	Property	Default behavior
❓ Agroal - Database connection pool	quarkus.datasource.jdbc.enable-metrics	If unspecified, collecting metrics will be enabled by default if a metrics extension is active.
❓ Additional named datasources	quarkus.datasource."datasource-name".jdbc.enable-metrics	If unspecified, collecting metrics will be enabled by default if a metrics extension is active.
Datasource configuration	quarkus.datasource.metrics.enabled	Disabled
Hibernate ORM	quarkus.hibernate-orm.metrics.enabled	Disabled
Jaeger	quarkus.jaeger.metrics.enabled	Disabled
MongoDB client	quarkus.mongodb.metrics.enabled	Disabled
❓ Neo4j connection pool	quarkus.neo4j.pool.metrics-enabled	Disabled
Logging	quarkus.log.metrics.enabled	Disabled
RESTEasy Server	quarkus.resteasy.metrics.enabled	Disabled
❓ SmallRye GraphQL	quarkus.smallrye-graphql.metrics.enabled	By default this will be enabled if the metrics extension is added.
SmallRye Reactive Messaging	quarkus.reactive-messaging.metrics.enabled	Disabled

[quarkus-bot]

/cc @ebullient, @jmartisk

[ebullient]

Some of the micrometer meter binders (e.g. kafka) should be included in this list
I think vertx/http being on by default is semi-expected .. but if we're going to look at the whole list.. we should add a few rows

[jmartisk]

Gah, this will be quite hard but I'll try to recall each case and what reasons we had to make it this way...
Make me a bucket of coffee for that.

So I'll assume that *.metrics.enabled is the "consistent" property naming to enable metrics, and "disabled" the "consistent" default.

Agroal:

• Regarding the default value:
  • quarkus.datasource.jdbc.enable-metrics and quarkus.datasource."datasource-name".jdbc.enable-metrics are per-datasource properties that control whether metrics are collected for that individual data source. And these default to true only if quarkus.datasource.metrics.enabled (which is the global setting) is true. So we basically only enable metrics of each individual data source by default, not data source metrics in general. That doesn't sound very inconsistent to me - if you enable data source metrics in general, you probably want them for all data sources, and don't want to enable each one separately.
  • IIUC the default of enable-metrics is true if a metrics extension is active AND the global setting (quarkus.datasource.metrics.enabled) is true as well, perhaps we should change the description?
• Regarding the name:
  • quarkus.datasource.metrics.enabled satisfies the consistency requirement. .jdbc.enable-metrics properties don't, but they have a slightly different meaning, so they should probably have a different name to avoid confusion (?). I think renaming them to .jdbc.metrics.enabled would be even more confusing because then we would have quarkus.datasource.metrics.enabled (global) and quarkus.datasource.jdbc.metrics.enabled (specific to the default datasource), which is terrible if you ask me.

Neo4j:

• I think it is named metrics-enabled rather than metrics.enabled simply because when I was adding the metrics support, this property already existed (it had served only for enabling Neo4j 'internal' metrics, now it also enables their export over SmallRye/Micrometer) and I didn't want to break compatibility. We could possibly rename it.

SmallRye GraphQL:

• Agree that the default should probably be 'disabled' which it isn't right now

I hope I summarized it correctly. I remember quite lengthy discussions about this (especially Agroal) and hoped it was already over ;)

[mkouba]

Agroal:

Is quarkus.datasource.jdbc.enable-metrics really a per-datasource property? I miss the datasource name...

Neo4j:

+1 to rename the property. 2.0 is a good opportunity to break things like these ;-).

SmallRye GraphQL:

+1 to change the default behavior.

[ebullient]

quarkus.datasource."datasource-name".jdbc.enable-metrics is per data source, quarkus.datasource.jdbc.enable-metrics
is overall. I would ask @Sanne about defaults. metrics can be expensive...

[jmartisk]

quarkus.datasource.jdbc.enable-metrics applies to the default (unnamed) data source. Named data sources have quarkus.datasource.NAME.jdbc.enable-metrics. This works the same as all other jdbc.* properties.

quarkus.datasource.metrics.enabled is the global setting.

[mkouba]

quarkus.datasource.jdbc.enable-metrics applies to the default (unnamed) data source. Named data sources have quarkus.datasource.NAME.jdbc.enable-metrics. This works the same as all other jdbc.* properties.

quarkus.datasource.metrics.enabled is the global setting.

Ok, in that case I think that we don't need to change the Agroal properties and behavior.

[brunobat]

@mkouba @ebullient do you have an idea of the properties still needing naming and behaviour updates?