Use global Collector to handle multiple reporters in the JVM

[mimaison]

This allows starting multiple reporter instances in the same JVM. All reporters share a global Collector, PrometheusCollector, which aggregates the metrics from all reporter instances.

This allows multiple reporters to register metrics with the same names (but different labels). This happen when multiple instances of a Kafka client (producer, consumer, admin) are started in the same JVM.

This fixes #43

[k-wall]

In the block above you are telling the reader how to enable the reporter for Connect/Streams and then in the block below, you show how to supply configuration. So, lines 54, 56, 58 and 60 seem unnecessary/repetitious - as you will show that again a few lines below.

[mimaison]

The example above shows how to enable the reporter. I added this sentence to say that you also need to add the prefixes when setting reporter-specific configurations for example prometheus.metrics.reporter.allowlist.

[k-wall]

Got it. I misunderstood. "auto.include.jmx.reporter" is property understood by Kafka itself. I had mistakenly thought it was being consumed by our reporter.

[k-wall]

Wider point, I don't find the admin. and producer. and consumer. prefixes particularly intuitive. If I had to guess what they did, my immediate thought would be that they control an aspect related to the ordinary Kafka Admin, Producer and Consumer clients, not something particular to Streams or Connect. I'm not really too familiar with either Streams or Connect, so maybe it is obvious to someone working in that space.

[mimaison]

Streams and Connect can be seen as "regular" applications and for all of their interactions with Kafka they use internal consumer, producer or admin clients. Both Streams and Connect use these prefixes to pass configurations to all the internal clients they use.

[k-wall]

ok. so it should be familiar to those working with Streams/Connect.

[k-wall]

Code smell - single pattern without a private constructor.
I wonder if you shifted the responsibility for the singleton to a separate class (CollectorHolder)

[mimaison]

The only reason I did not add a private constructor is for tests. It's useful to get unique instances instead of the global collector so tests are independent. We can add a package-only constructor.

[mimaison]

I've added a package only constructor in 79dd195

[OwenCorrigan76]

I have tested this with the Streams WordCount app mentioned in the issue with the following config in Streams:

metric.reporters=io.strimzi.kafka.metrics.KafkaPrometheusMetricsReporter
auto.include.jmx.reporter=false
admin.metric.reporters=io.strimzi.kafka.metrics.KafkaPrometheusMetricsReporter
admin.auto.include.jmx.reporter=false
producer.metric.reporters=io.strimzi.kafka.metrics.KafkaPrometheusMetricsReporter
producer.auto.include.jmx.reporter=false
consumer.metric.reporters=io.strimzi.kafka.metrics.KafkaPrometheusMetricsReporter
consumer.auto.include.jmx.reporter=false
prometheus.metrics.reporter.listener = http://localhost:8080

The error outlined in #43 An Exception occurred while scraping metrics: java.lang.IllegalStateException: kafka_consumer_app_info_commit_id: duplicate metric name is gone

[k-wall]

lgtm