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 > > >>>>> > > >>>>> > > >>> > > >> > > >> > > > > >
