Collector vs MultiCollector

[zorba128]

trafficstars

Hi

I'm just upgrading my project to new version of client, and have feeling separation between Collector and MultiCollector interfaces brings unnecessary complexity to the code. If you look into registry implementation, amount of code actually doubles unnecessarily due to separate handling paths.

I see it that Collector is just a special case of more generic MultiCollector. Moreover - its existence is only for convenience of implementers, it has no additional logic over MultiCollector.

Shouldn't it be that:

• have Collector interface that returns multiple metrics (actually just rename MultiCollector)
• have abstract class SingleCollector extends Collector that just makes implementation simpler by allowing to implement MetricSnapshot collectSingle(), and wraps result as one-row MetricSnapshots

marcin

[fstab]

The benefit of the current Collector interface is that it's a functional interface, i.e. you can simply register a Lambda with a PrometheusRegistry:

registry.register(() -> CounterSnapshot.builder()
        .name("my_count")
        .dataPoint(CounterDataPointSnapshot.builder()
                .labels(Labels.of("key", "value"))
                .value(3.0)
                .build())
        .build());

The tradeoff is: We need two separate functional interfaces: Collector returning a single Snapshot and MultiCollector returning multiple Snapshots.

I agree that the internal implementation of PrometheusRegistry could be simplified if we just had only the MultiCollector interface. However, then you would need to implement this for every collector, even if the collector just returns a single metric.

These trade-offs are always hard to get right, but I figured making it easy to register a Lambda is worth the increased internal complexity in PrometheusRegistry.

[zorba128]

But MultiCollector is also functional interface, yet more powerful.

It has one method: collect(filter, scrapeRequest): MetricsSnapshots

and code you provided should simply be calling CounterSnapshots.builder rather than CounterSnapshot.builder. But I don't like this code - its like manual implementation of Collector/MultiCollector - what is the point over just creating and registering Counter? And default counter implementation (counter builder) can take care of filtering and everything...

However, then you would need to implement this for every collector, even if the collector just returns a single metric

But note the builder does that. You can make Gauge.builder().build() return MultiCollector without any other changes - noone will notice - client code will not change at all...