CollectorRegistry should be just another type of MultiCollector

[zorba128]

Notice CollectorRegistry effectively is just yet another type (subclass) of MultiCollector.
It has same responsibility - to produce MetricsSnapshots, just method is named scrape() instead of collect().
Even the filtering api needs to match between one and another (see that ScrapeRequest leaked down into MultiCollector). Also it aggregates list of prometheusNames, its just not exposed.

I dont think this is coincidence...

With this change one could use registry at intermediate levels to compose simple collectors (like gauges, etc) to bigger collectors. And in the end composite of composites could be used as source for http metrics server.

marcin

[fstab]

This is a great point, thanks for bringing this up. I agree having Registry implements MultiCollector would be great for composing multiple registries.

Only caveat is that Registry would not override getPrometheusNames(), so potential name conflicts would only show at scrape time, not at registration time, and name filters would be applied after scraping, there won't be a way to skip a registry based on a name filter.

Anyway, still valueble to have Registry implements MultiCollector. Do you want to create a PR?

[zorba128]

1. if you get rid of this name conflict pre-check - those prometheus names will not be needed
2. why do you say filtering will not work? just simplify collector (multicollector) to single method that accepts filter - those all defaullts bring nothing but mess.
3. and then it really will be functional interface. now its not, because in order override it without breaking filtering/etc - you need to implement all three... you just hide it with defaults at cost of loosing filtering
4. I feel filtering is done at wrong place. Its not responsibility of caller to check wether given collector should be called or not. Its the collector who should simply follow the filter passed. Now it will not work, cause collector interface is complex, has default methods that effectively disable this feature, forcing you to do things like that. And notice - most collectors are implemented by your builders, rather than by direct implementation of MultiCollector interface. And builder can take care of that. If builder is for single-metrics - it just needs to check if name matches filter. If multicollector is an union of other collectors - just trust them and pass filter - they will immediately return empty result. Nothing will get constructed - this are just few nested calls on stacktrace. And adding empty MetricsSnapshots to MetricsSnapshots builder can be implemented so that its no-op.
5. And with this design you can have nice class FilteringCollector(underlting: MultiCollector) extends MultiCollector utility that can be applied over collector that one believes does not follow filtering conditions correctly. It would just call underlying,.collect() and post-filter. This is what you do now most of time. You could use it at top level (you actually do that in the registry - see the code) to make sure that if anyone does not follow spec, result still will be valid. But its better cause one can apply it at any aggregation level...