PromPHP / prometheus_client_php

Prometheus instrumentation library for PHP applications
https://prometheus.io/docs/concepts/metric_types/
Apache License 2.0
423 stars 93 forks source link

Do not collect default metrics in constructor #62

Open Radiergummi opened 3 years ago

Radiergummi commented 3 years ago

Currently, the collector registry can be instructed to collect the default metrics by passing the $registerDefaultMetrics flag to the constructor:

$registry = new CollectorRegistry($adapter, registerDefaultMetrics: true);

The registry will register the default metrics right away by calling a private member method.

Looks like a good idea at first, but producing side-effects in a constructor is considered an anti-pattern for a reason :) (Edit: Sounds kind of condescending, isn't meant that way!) Case in point, when using dependency injection, it may be necessary to type-hint the collector registry as a constructor dependency for, say, a Symfony Console Command. Commands will generally be constructed very early in the application lifecycle, as the console component requires actual instances to parse CLI input.
This means that an application will create instances of all registered commands, inject the collector registry somewhere, which attempts to collect the default metrics... which breaks down if the connection to, say, Redis, can't be established at that point.

This may seem fine for production environments, but it also breaks stuff like artisan discover in Laravel applications if any command requires the Prometheus client as a dependency, both locally or in a CI (where Redis isn't available).


What I'd like to ask for would be to open that registerDefaultMetrics method to the outside, and make it public; that way, implementations could choose the point at which they desire to collect those metrics.