search

PromPHP / prometheus_client_php

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

feat: allow to disable metric sort #108

Closed simPod closed 1 year ago

simPod commented 1 year ago

This function does 50 % of a loading time (using inmemory adapter) so I'd like to be able to disable it.

simPod commented 1 year ago

disabling sort reduced the scrape duration by half

image
LKaemmerling commented 1 year ago

Hey @simPod,

good catch! I'm thinking of don't sort it per default. What do you think? Of course, this would be a bit different from what the other libs do (go/python) however it seems the sorting there is more efficient.

simPod commented 1 year ago

Yup but that would be BC. We can introduce this flag and flip the default value in next major? Your call.

LKaemmerling commented 1 year ago

Thank you!

javespi commented 1 year ago

Hello!

Yup but that would be BC. We can introduce this flag and flip the default value in next major? Your call.

I think it's still BC for any class which implements RegistryInterface. Use cases like mocks or wrappers.

After the latest minor release I am having this error:

PHP Fatal error:  Declaration of Javespi\MyCollectorRegistry::getMetricFamilySamples(): array must be compatible with Prometheus\RegistryInterface::getMetricFamilySamples(bool $sortMetrics = true): array

I think the only way of being BC is removing the type hint of the argument. I'll try to create a pull-request with a hotfix for this.

Edit: Reported as issue here https://github.com/PromPHP/prometheus_client_php/issues/117 Edit 2: Pull request to avoid BC here https://github.com/PromPHP/prometheus_client_php/pull/118