Metrics naming formalization.

[Mikhail Petrov]

Hello, Igniters.

I faced the following problem with the metrics - JmxMetricExporter fails
to export discovery metrics (ticket - [1]).

The main reason: JMX exporter assumes that each metric name must start
with the name of the registry it belongs to, but discovery metrics do
not obey this naming convection.

I propose to formalize the names of the metrics included in the metrics
registry so that they always start with the name of the register. And
add corresponding assertions (ticket - [2]).
It helps to make the metric naming consistent and fixes the mentioned
above issue.

The following metric names are proposed to be changed:

JoinedNodes -> io.discovery.JoinedNodes
LeftNodes -> io.discovery.LeftNodes
FailedNodes -> io.discovery.FailedNodes
PendingMessagesRegistered -> io.discovery.PendingMessagesRegistered
CommunicationErrors -> io.discovery.CommunicationErrors
CurrentTopologyVersion -> io.discovery.CurrentTopologyVersion

Any objections?


[1] - https://issues.apache.org/jira/browse/IGNITE-14376
[2] - https://issues.apache.org/jira/browse/IGNITE-14428

[agura]

Hi, Mikhail!

I've looked at the problem and the fix. The root cause of the problem
is MetricsRegistry design which allows to register metrics in two
ways:

- using a family of methods like MetricRegistre.register(..,
XxxSupplier, ...) and MetricRegistry.xxxMetric. These methods firstly
create metrics and take into account registry's name. These methods
provide the right way for registering metrics in MetricRegistry.
- using the method MetricRegistry.register(Metric metric). This method
takes as a parameter an already created metric which was created out
of the metric registry context. This method is the wrong (roughly
speaking) way for registering metrics in MetricRegistry.

Using the right approach for metrics registering is a guaranteed way
to avoid problems like described in [1]. Moreover, this approach does
not require any metrics naming formalization because it hides internal
naming details from an user.

So, IMHO, the issue [2] ideally should be re-fixed in a right way.

Also, ideally, we should limit the visibility scope of  metric classes
constructors and do not allow instantiate these classes directly.

[1] https://issues.apache.org/jira/browse/IGNITE-14376
[2] https://issues.apache.org/jira/browse/IGNITE-14428

On Fri, Apr 9, 2021 at 1:25 PM Mikhail Petrov <[hidden email]> wrote:

>
> Hello, Igniters.
>
> I faced the following problem with the metrics - JmxMetricExporter fails
> to export discovery metrics (ticket - [1]).
>
> The main reason: JMX exporter assumes that each metric name must start
> with the name of the registry it belongs to, but discovery metrics do
> not obey this naming convection.
>
> I propose to formalize the names of the metrics included in the metrics
> registry so that they always start with the name of the register. And
> add corresponding assertions (ticket - [2]).
> It helps to make the metric naming consistent and fixes the mentioned
> above issue.
>
> The following metric names are proposed to be changed:
>
> JoinedNodes -> io.discovery.JoinedNodes
> LeftNodes -> io.discovery.LeftNodes
> FailedNodes -> io.discovery.FailedNodes
> PendingMessagesRegistered -> io.discovery.PendingMessagesRegistered
> CommunicationErrors -> io.discovery.CommunicationErrors
> CurrentTopologyVersion -> io.discovery.CurrentTopologyVersion
>
> Any objections?
>
>
> [1] - https://issues.apache.org/jira/browse/IGNITE-14376
> [2] - https://issues.apache.org/jira/browse/IGNITE-14428
>