ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexey Goncharuk <alexey.goncha...@gmail.com>
Subject Re: Internal classes are exposed in public API
Date Thu, 16 Jan 2020 17:22:48 GMT
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, Николай Ижиков <nizhikov@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.goncharuk@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, Николай Ижиков <nizhikov@apache.org>:
> >
> >> Ticket to export MetricRegistry to the public API created -
> >> https://issues.apache.org/jira/browse/IGNITE-12552
> >>
> >>> 16 янв. 2020 г., в 16:58, Николай Ижиков <nizhikov.dev@gmail.com>
> >> написал(а):
> >>>
> >>> Do you propose to keep these interfaces forever?
> >>>
> >>>> 16 янв. 2020 г., в 16:52, Alexey Goncharuk <
> alexey.goncharuk@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, Николай Ижиков <nizhikov@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.goncharuk@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
> >>>>>
> >>>>>
> >>>
> >>
> >>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message