[🐛 BUG]: The metric plugin does not work correctly with namespace

[agratushniy]

No duplicates 🥲.

• [X] I have searched for a similar issue in our bug tracker and didn't find any solutions.

What happened?

At the time of declaring the metric, it is possible to specify the namespace. The problem is that this namespace is not taken into account in the plugin, and the collector is registered only by name. And this problem makes it impossible to create two metrics with the same name in different namespaces.

Version (rr --version)

$ rr --version
rr version 2023.2.2 (build time: 2023-08-10T16:38:29+0000, go1.21.0), OS: linux, arch: amd64

How to reproduce the issue?

$metrics->declare(
    'foo',
    Collector::histogram()->withHelp('Help name ns1->foo')->withNamespace('ns1')
 );

$metrics->declare(
    'foo',
    Collector::histogram()->withHelp('Help name ns2->foo')->withNamespace('ns2')
 );
       

After executing the code described above, only one metric ns1 -> foo will be registered in the collector. Also, when specifying a metric value, it is not possible to specify a namespace

$metrics->observe('foo', 1);

ref: https://github.com/roadrunner-php/issues/issues/23

[rustatian]

Hey @agratushniy 👋🏻 Thanks for the report 👍🏻 I found the issue, will try to fix it until the next bugfix release.

[rustatian]

@agratushniy Could you please update your example and provide a complete worker which reproduces this problem?

[rustatian]

This fix is currently postponed for the version 2024.1 (and I'm still not sure if we need to change the behavior). It contains breaking changes in metrics behavior. If you check the logs, you'll see the error message for the second declare call with the same name.

[rustatian]

The problem is that this namespace is not taken into account in the plugin, and the collector is registered only by name.

This statement is not correct. RR registers the metric name with the provided namespace (namespace is considered in the collector). But it saves the collector, using the name as a key in an associated array.

[agratushniy]

@agratushniy Could you please update your example and provide a complete worker which reproduces this problem?

is an example still required ?

[rustatian]

@agratushniy Could you please update your example and provide a complete worker which reproduces this problem?

is an example still required ?

Not anymore.

[Volonda]

We use Spiral\RoadRunner\Metrics\MetricsInterface::declare() to declare metrics in php DI CompilerPassInterface (before php runtime). If we have n php process (service in .rr.cli.yml) it will try to register metric n-times. Everything works, but error will be written to log.

This code in Spiral\RoadRunner\Metrics\Metrics::declare() hide exception to save php process alive

} catch (ServiceException $e) {
    if (\str_contains($e->getMessage(), 'tried to register existing collector')) {
        // suppress duplicate metric error
        return;
    }
}

but we will see error in log: ERROR metrics metric with provided name already exist {"name": "xxx", "type": "xxx", "namespace": ""}

Is there way to hide this error? Maybe it will be good idea to add idempotency for declare()?

[rustatian]

Hey @Volonda :wave: You don't need to hide or suppress the message. Just use our lock plugin to have mutable access to the declaration of the metric. For example:

// Acquire lock with ttl - 10 seconds
$id = $lock->lock('pdf:create', ttl: 10); // id will be false for all workers except one

// if id is not false - you have the lock, do the declaration

Or, you can use any existing in PHP lock (mutex) libraries as well.

[Volonda]

thank you for idea!

[rustatian]

Hey @Volonda 👋 Has that advice helped you? Have you resolved your problem?