Pavel, 

As I said before - I don’t see any real use-cases for your example.

That’s why

> I don’t create pure Java API for metric intentionally.

What real-world use-case you are keeping in mind?
Why do you want to do it in that way?

The API you’re talking about is very simple and straightforward.
It’s an easy thing to create, but this will open to the user's wrong usage 
pattern.
From my point of view, to get metrics values one should use MetricExporterSPI.

> 17 янв. 2020 г., в 09:40, Pavel Tupitsyn <ptupit...@apache.org> написал(а):
> 
> Николай, can you provide a full example - how do I get a metric value in my
> code as an Ignite user?
> 
> var ignite = Ignition.ignite();
> var totalAllocatedPages = ???;
> 
> On Fri, Jan 17, 2020 at 12:47 AM Николай Ижиков <nizhi...@apache.org> wrote:
> 
>> 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