quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.55k stars 2.62k forks source link

Micrometer Metrics does not measure http_server metrics if vertx metrics are disabled #42368

Open andreas-eberle opened 1 month ago

andreas-eberle commented 1 month ago

Describe the bug

When we disable the vertx metrics for the Micrometer Metrics by setting quarkus.micrometer.binder.vertx.enabled=false, then the http_server metrics are no longer logged, although we use quarkus-rest. There is also a property to specifically enable the http-server metrics: quarkus.micrometer.binder.http-server.enabled. But as soon as the vertx metrics are disabled, this flag has no effect.

I think this is caused by the following line of code: https://github.com/quarkusio/quarkus/blob/main/extensions/micrometer/deployment/src/main/java/io/quarkus/micrometer/deployment/binder/HttpBinderProcessor.java#L45

There, both flags, quarkus.micrometer.binder.vertx.enabled and quarkus.micrometer.binder.http-server.enabled are checked and the http server metrics seem to only be created if both are true. This is not reflected in the documentation and it is unclear to me why this would be the case.

image

In general, I would think that I don't need the vertx metrics but still want to see the http_server metrics when I use quarkus-rest. I'm using vertx under the hood, but I might not be interested in those metrics.

By the way, the http_client metrics still work when vertx metrics are disabled.

I could create a PR to remove the vertx check for the http_server metrics. But I don't know if that is required due to some other reasons I don't understand yet. If you agree this should still work without the vertx metrics enabled, I would create a PR.

Expected behavior

http server metrics should be recorded, even if vertx is disabled. If that is not possible, the docs need to highlight this.

Actual behavior

As soon as the vertx metrics are disabled, the http server metrics no longer work while the http client metrics still work.

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

3.13.0

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

No response

Additional information

No response

quarkus-bot[bot] commented 1 month ago

/cc @brunobat (micrometer), @ebullient (metrics,micrometer), @jmartisk (metrics)

brunobat commented 3 weeks ago

Hi @andreas-eberle what you describe makes sense to me. We would accept a PR but first we need to agree on a behaviour. quarkus.micrometer.binder.vertx.enabled is probable obsolete or should also be applied to the vertx client. What do you think @ebullient?

ebullient commented 3 weeks ago

The "enabled" flag is a build-time switch. IIRC, that switch prevents registration of MetricsOptions with Vert.x (i.e. it removes that capability entirely, http instrumentation is related to/based on vertx hooks).

My fuzzy memory also seems to think that the RestClient did not use Vert.x at that time.

Either we keep that meaning (and quarkus.micrometer.binder.vertx.enabled toggles whether or not any vertx instrumentation is present), in which case it should be applied to the vertx client, too, or we change that meaning and always instrument vertx, and apply it to *vertx* meters only.