Hello, Pavel.

> there should be an obvious public API to retrieve metrics

What do you mean by «retrieve»?
I don’t create pure Java API for metric intentionally.
We have MetricExporterSPI for this purpose.
It very simple from my point of view.
What use cases for `Ignite.monitoring()` API exists?

```
public interface MetricExporterSpi extends IgniteSpi {
    public void setMetricRegistry(ReadOnlyMetricRegistry registry);
    public void setExportFilter(Predicate<MetricRegistry> filter);
}
```

```
public interface ReadOnlyMetricRegistry extends Iterable<MetricRegistry> {
    public void addMetricRegistryCreationListener(Consumer<MetricRegistry> 
lsnr);
    public void addMetricRegistryRemoveListener(Consumer<MetricRegistry> lsnr);
}
```

```
public class MetricRegistry implements Iterable<Metric> {
    public String name();
    @Nullable public <M extends Metric> M findMetric(String name);
}
```

> 16 янв. 2020 г., в 20:32, Pavel Tupitsyn <ptupit...@apache.org> написал(а):
> 
> Agree with Alexey, there should be an obvious public API to retrieve metrics
> 
> On Thu, Jan 16, 2020 at 8:23 PM Alexey Goncharuk <alexey.goncha...@gmail.com>
> wrote:
> 
>> That's an option, I agree. Yet for me, from the usability point of view, it
>> would be quite strange - to get an instance of the monitoring interface, I
>> would need to do
>> JmxMetricExporterSpi metrics =
>> (JmxMetricExporterSpi)ignite.configuration().getMetricExporterSpi()[0];
>> which is too verbose and fragile if someone changes configuration (say,
>> inserts another SPI to the first position).
>> 
>> чт, 16 янв. 2020 г. в 20:14, Николай Ижиков <nizhi...@apache.org>:
>> 
>>> May be we should enable JMX exporter, by default?
>>> This would provide the same value for the user as `IgniteMonitoring` you
>>> propose.
>>> 
>>>> 16 янв. 2020 г., в 20:06, Alexey Goncharuk <alexey.goncha...@gmail.com
>>> 
>>> написал(а):
>>>> 
>>>> I think it would make sense to make something like a new
>> IgniteMonitoring
>>>> facade on Ignite interface and add an ability to obtain a metric value
>>>> through this facade, not through an exporter. This would be a
>>>> straightforward replacement for all old metrics interfaces.
>>>> 
>>>> чт, 16 янв. 2020 г. в 17:13, Николай Ижиков <nizhi...@apache.org>:
>>>> 
>>>>> Ticket to export MetricRegistry to the public API created -
>>>>> https://issues.apache.org/jira/browse/IGNITE-12552
>>>>> 
>>>>>> 16 янв. 2020 г., в 16:58, Николай Ижиков <nizhikov....@gmail.com>
>>>>> написал(а):
>>>>>> 
>>>>>> Do you propose to keep these interfaces forever?
>>>>>> 
>>>>>>> 16 янв. 2020 г., в 16:52, Alexey Goncharuk <
>>> alexey.goncha...@gmail.com>
>>>>> написал(а):
>>>>>>> 
>>>>>>> Let's say I am upgrading from 2.7 to 2.8. Given that I figured out
>> to
>>>>>>> obtain an instance of the JMX exporter SPI, how should I map the old
>>>>>>> 'interface + method name' to the new metric name? I think
>> deprecation
>>> of
>>>>>>> the public API may be a bit harsh in this case.
>>>>>>> 
>>>>>>> чт, 16 янв. 2020 г. в 16:44, Николай Ижиков <nizhi...@apache.org>:
>>>>>>> 
>>>>>>>> Hello, Alexey.
>>>>>>>> 
>>>>>>>>> * DataRegionMetric public interface is deprecated, however, the
>>>>>>>> suggested replacement class GridMetricsManager is internal and
>> cannot
>>>>> be
>>>>>>>> acquired by a user. This makes impossible for the user to fix their
>>>>> code to
>>>>>>>> not use deprecated API
>>>>>>>> 
>>>>>>>> May be it’s not clear text here.
>>>>>>>> My point here, that user should obtain metrics values via
>> configured
>>>>>>>> metric exporters and don’t use *Metric interfaces.
>>>>>>>> 
>>>>>>>>> * In ReadOnlyMetricsRegistry there is a Consumer<MetricRegistry>,
>>> but
>>>>>>>> MetricRegistry is again an internal class.
>>>>>>>> 
>>>>>>>> This is a real issue.
>>>>>>>> We should expose MetricRegistry interface to the public API and
>> keep
>>>>>>>> internal implementation.
>>>>>>>> 
>>>>>>>> I will create ticket and fix it, shortly.
>>>>>>>> 
>>>>>>>> 
>>>>>>>>> 16 янв. 2020 г., в 16:39, Alexey Goncharuk <
>>>>> alexey.goncha...@gmail.com>
>>>>>>>> написал(а):
>>>>>>>>> 
>>>>>>>>> Igniters, Nikolay,
>>>>>>>>> 
>>>>>>>>> I was adding a new metric in the scope of the ticket [1] and was
>>>>>>>> surprised by a few things:
>>>>>>>>> * DataRegionMetric public interface is deprecated, however, the
>>>>>>>> suggested replacement class GridMetricsManager is internal and
>> cannot
>>>>> be
>>>>>>>> acquired by a user. This makes impossible for the user to fix their
>>>>> code to
>>>>>>>> not use deprecated API
>>>>>>>>> * In ReadOnlyMetricsRegistry there is a Consumer<MetricRegistry>,
>>> but
>>>>>>>> MetricRegistry is again an internal class. This will cause the user
>>>>> code
>>>>>>>> compilation to break when MetricRegistry is changed
>>>>>>>>> 
>>>>>>>>> These things violate the public-private API separation and need to
>>> be
>>>>>>>> fixed prior 2.8 is released. Let's discuss what changes need to be
>>>>> made to
>>>>>>>> the API or perhaps move incomplete IEP-35 interfaces to the private
>>>>> package
>>>>>>>> and remove deprecations until the API is ready.
>>>>>>>>> 
>>>>>>>>> --AG
>>>>>>>> 
>>>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>> 
>>> 
>> 

Reply via email to