Adding a label match for "path" causes NPE in

cut-to-the-cheese commented 5 years ago

Setting up vertx metrics options like this causes NPE whenever you try to hit the http server:

new MicrometerMetricsOptions()
      .setPrometheusOptions(new VertxPrometheusOptions()
        .setEnabled(true))
      .setLabels(EnumSet.of( Label.HTTP_METHOD, Label.HTTP_PATH, Label.REMOTE, Label.LOCAL, Label.EB_ADDRESS))
      .addLabelMatch(new Match()
        .setDomain(MetricsDomain.HTTP_SERVER)
        .setValue(Constants.HTTP_SERVER_METRICS_FOR_URL_REGEX)
        .setLabel(Label.HTTP_PATH.toString())
        .setType(MatchType.REGEX))
      .setEnabled(true);

The issue is pretty easily reproducible. In PrometheusMetricsITest's shouldStartEmbeddedServer test, I have added the same LabelMatch and the test does fail: https://github.com/vert-x3/vertx-micrometer-metrics/commit/9b35c7775d71ffc427de1b10cdff33a7a2fa3c69?diff=split

The stack trace I have from my application is bit different however. It originates from VertxNetServerMetrics from where some of the internal metrics are being reported:

"stackTrace":"java.lang.NullPointerException: null\n\tat java.util.Objects.requireNonNull(Objects.java:203)\n\tat io.micrometer.core.instrument.ImmutableTag.<init>(ImmutableTag.java:30)\n\tat io.micrometer.core.instrument.Tag.of(Tag.java:29)\n\tat io.vertx.micrometer.impl.Labels.toTags(Labels.java:49)\n\tat io.vertx.micrometer.impl.meters.Summaries.get(Summaries.java:47)\n\tat io.vertx.micrometer.impl.VertxNetServerMetrics$Instance.bytesRead(VertxNetServerMetrics.java:77)\n\tat io.vertx.micrometer.impl.VertxNetServerMetrics$Instance.bytesRead(VertxNetServerMetrics.java:56)\n\tat

I have tried excluding metric categories as well but no luck: .setDisabledMetricsCategories(EnumSet.of(MetricsDomain.DATAGRAM_SOCKET, MetricsDomain.NAMED_POOLS, MetricsDomain.NET_CLIENT, MetricsDomain.NET_SERVER))

cut-to-the-cheese commented 5 years ago

This does seem to work: https://github.com/jahanzebbaber/vertx-micrometer-metrics/commit/938a4038f7d1d6df283621f2cb31a6d60669d894 Happy to raise a PR if this is indeed an acceptable fix.

jotak commented 5 years ago

Hi @jahanzebbaber , thanks for reporting

Happy to raise a PR if this is indeed an acceptable fix.

Yes please, a PR is welcome

cut-to-the-cheese commented 5 years ago

Raised a PR with a brand new branch: https://github.com/vert-x3/vertx-micrometer-metrics/pull/76

jotak commented 5 years ago

I'm closing, PR was merged. Thank you @jahanzebbaber

vert-x3 / vertx-micrometer-metrics

Adding a label match for "path" causes NPE in #75