Metric access for reporting tasks

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Metric access for reporting tasks

Omer Hadari
Hello,

I hope I am writing to the correct mailing list.
We use graphite in my organization, and recently started to use nifi.
We went on to write a simple reporting task for graphite, and I figured
it could be used by other people as well, so why not contribute it.
I was looking at other reporting tasks though (DataDog and Ambari), and
there seems to me that there is some code duplication in how they access
metrics. They both use very similar classes in order to to that:
org.apache.nifi.reporting.ambari.metrics.MetricsService
org.apache.nifi.reporting.ambari.metrics.MetricNames
org.apache.nifi.reporting.datadog.metrics.MetricsService
org.apache.nifi.reporting.datadog.metrics.MetricNames

They are not identical, but again - very similar. I think this
functionality can be easily exported to some other module, in order for
more reporting tasks that need to generally report the same metrics to be
written more easily.
My questions are:
a. Are more metric reporting tasks (like graphite) welcome
b. If the refactor I am suggesting is in order, will it belong in
nifi-commons or is a new module for reporting tasks in order?

I would be more than happy to implement any and all changes I have just
suggested by myself, and am simply asking these questions in order to best
fit into your conventions and workflow.

Thank you in advance!
Reply | Threaded
Open this post in threaded view
|

Re: Metric access for reporting tasks

Joe Witt
Omer

Is the right list and it's awesome you want to contribute.

Yes for sure such contribs are welcome.  Just need to be sure all libraries
used including transitive deps are fair game as far as licensing goes and
are properly accounted for.

As far as refactoring to avoid code duplication it could be helpful.  You
might want to just do a jira and PR to do yours in a nice and clean and
reusable way and once that is done and in then do another jira and PR to
clean up the others.

Thanks
Joe

On Sep 16, 2017 2:44 PM, "Omer Hadari" <[hidden email]> wrote:

> Hello,
>
> I hope I am writing to the correct mailing list.
> We use graphite in my organization, and recently started to use nifi.
> We went on to write a simple reporting task for graphite, and I figured
> it could be used by other people as well, so why not contribute it.
> I was looking at other reporting tasks though (DataDog and Ambari), and
> there seems to me that there is some code duplication in how they access
> metrics. They both use very similar classes in order to to that:
> org.apache.nifi.reporting.ambari.metrics.MetricsService
> org.apache.nifi.reporting.ambari.metrics.MetricNames
> org.apache.nifi.reporting.datadog.metrics.MetricsService
> org.apache.nifi.reporting.datadog.metrics.MetricNames
>
> They are not identical, but again - very similar. I think this
> functionality can be easily exported to some other module, in order for
> more reporting tasks that need to generally report the same metrics to be
> written more easily.
> My questions are:
> a. Are more metric reporting tasks (like graphite) welcome
> b. If the refactor I am suggesting is in order, will it belong in
> nifi-commons or is a new module for reporting tasks in order?
>
> I would be more than happy to implement any and all changes I have just
> suggested by myself, and am simply asking these questions in order to best
> fit into your conventions and workflow.
>
> Thank you in advance!
>
Reply | Threaded
Open this post in threaded view
|

Re: Metric access for reporting tasks

Andrew Hulbert-2
Hi Omer,

If you're interested in some help to implement, test, or review a
graphite/grafana metrics reporter please let me know! We have written a
very simple version and are interested in getting support into the main
codebase as well.

-Andrew

On 09/17/2017 05:57 PM, Joe Witt wrote:

> Omer
>
> Is the right list and it's awesome you want to contribute.
>
> Yes for sure such contribs are welcome.  Just need to be sure all libraries
> used including transitive deps are fair game as far as licensing goes and
> are properly accounted for.
>
> As far as refactoring to avoid code duplication it could be helpful.  You
> might want to just do a jira and PR to do yours in a nice and clean and
> reusable way and once that is done and in then do another jira and PR to
> clean up the others.
>
> Thanks
> Joe
>
> On Sep 16, 2017 2:44 PM, "Omer Hadari" <[hidden email]> wrote:
>
>> Hello,
>>
>> I hope I am writing to the correct mailing list.
>> We use graphite in my organization, and recently started to use nifi.
>> We went on to write a simple reporting task for graphite, and I figured
>> it could be used by other people as well, so why not contribute it.
>> I was looking at other reporting tasks though (DataDog and Ambari), and
>> there seems to me that there is some code duplication in how they access
>> metrics. They both use very similar classes in order to to that:
>> org.apache.nifi.reporting.ambari.metrics.MetricsService
>> org.apache.nifi.reporting.ambari.metrics.MetricNames
>> org.apache.nifi.reporting.datadog.metrics.MetricsService
>> org.apache.nifi.reporting.datadog.metrics.MetricNames
>>
>> They are not identical, but again - very similar. I think this
>> functionality can be easily exported to some other module, in order for
>> more reporting tasks that need to generally report the same metrics to be
>> written more easily.
>> My questions are:
>> a. Are more metric reporting tasks (like graphite) welcome
>> b. If the refactor I am suggesting is in order, will it belong in
>> nifi-commons or is a new module for reporting tasks in order?
>>
>> I would be more than happy to implement any and all changes I have just
>> suggested by myself, and am simply asking these questions in order to best
>> fit into your conventions and workflow.
>>
>> Thank you in advance!
>>

Reply | Threaded
Open this post in threaded view
|

Re: Metric access for reporting tasks

Omer Hadari
So I have created a generic metric reporting task, and implemented a
Graphite service for it (Thank you Bryan for the quick reviews and
responses!), and I am up to implementing the DataDog and Ambari reporting
tasks in the same manner as well. I think it's important for avoiding
confusion when implementing another reporting task, and for creating a
uniform UI for metric reporting (the same task, different implementations
of the controller service).
I don't think I can remove the old ones though (It will obviously break
flows that use them). What do you think is best practice here?
Personally I think implementing a "double" and deprecate the old ones in
some way is OK.

For reference here is the original ticket:
https://issues.apache.org/jira/browse/NIFI-4392
and here is the PR: https://github.com/apache/nifi/pull/2171

Thank you!

On Mon, Sep 18, 2017 at 6:07 PM, Andrew Hulbert <[hidden email]>
wrote:

> Hi Omer,
>
> If you're interested in some help to implement, test, or review a
> graphite/grafana metrics reporter please let me know! We have written a
> very simple version and are interested in getting support into the main
> codebase as well.
>
> -Andrew
>
>
> On 09/17/2017 05:57 PM, Joe Witt wrote:
>
>> Omer
>>
>> Is the right list and it's awesome you want to contribute.
>>
>> Yes for sure such contribs are welcome.  Just need to be sure all
>> libraries
>> used including transitive deps are fair game as far as licensing goes and
>> are properly accounted for.
>>
>> As far as refactoring to avoid code duplication it could be helpful.  You
>> might want to just do a jira and PR to do yours in a nice and clean and
>> reusable way and once that is done and in then do another jira and PR to
>> clean up the others.
>>
>> Thanks
>> Joe
>>
>> On Sep 16, 2017 2:44 PM, "Omer Hadari" <[hidden email]> wrote:
>>
>> Hello,
>>>
>>> I hope I am writing to the correct mailing list.
>>> We use graphite in my organization, and recently started to use nifi.
>>> We went on to write a simple reporting task for graphite, and I figured
>>> it could be used by other people as well, so why not contribute it.
>>> I was looking at other reporting tasks though (DataDog and Ambari), and
>>> there seems to me that there is some code duplication in how they access
>>> metrics. They both use very similar classes in order to to that:
>>> org.apache.nifi.reporting.ambari.metrics.MetricsService
>>> org.apache.nifi.reporting.ambari.metrics.MetricNames
>>> org.apache.nifi.reporting.datadog.metrics.MetricsService
>>> org.apache.nifi.reporting.datadog.metrics.MetricNames
>>>
>>> They are not identical, but again - very similar. I think this
>>> functionality can be easily exported to some other module, in order for
>>> more reporting tasks that need to generally report the same metrics to be
>>> written more easily.
>>> My questions are:
>>> a. Are more metric reporting tasks (like graphite) welcome
>>> b. If the refactor I am suggesting is in order, will it belong in
>>> nifi-commons or is a new module for reporting tasks in order?
>>>
>>> I would be more than happy to implement any and all changes I have just
>>> suggested by myself, and am simply asking these questions in order to
>>> best
>>> fit into your conventions and workflow.
>>>
>>> Thank you in advance!
>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: Metric access for reporting tasks

Bryan Bende
Omer,

I think adding the new versions that implement the new
MetricReporterService, and marking the old ones as deprecated makes
sense. They could potentially be removed on a major future release
like 2.0.0.

Were you envisioning the DataDogMetricReportService and
AmbariMetricReportingService living along side the
GraphiteMetricReportingService in nifi-metrics-reporting-task? or
would the DataDog and Ambari implementations live inside their
respective NARs and just depend on the MetricsReportingService API?

I haven't really looked at the dependencies to see if putting them all
in one NAR would cause any issues.

I have slight concerns over whether the Ambari one can be easily
converted to the new approach, obviously it would be good if we can,
but we need to ensure we port over the exact logic it is using.

Thanks,

Bryan


On Wed, Oct 11, 2017 at 8:40 AM, Omer Hadari <[hidden email]> wrote:

> So I have created a generic metric reporting task, and implemented a
> Graphite service for it (Thank you Bryan for the quick reviews and
> responses!), and I am up to implementing the DataDog and Ambari reporting
> tasks in the same manner as well. I think it's important for avoiding
> confusion when implementing another reporting task, and for creating a
> uniform UI for metric reporting (the same task, different implementations
> of the controller service).
> I don't think I can remove the old ones though (It will obviously break
> flows that use them). What do you think is best practice here?
> Personally I think implementing a "double" and deprecate the old ones in
> some way is OK.
>
> For reference here is the original ticket:
> https://issues.apache.org/jira/browse/NIFI-4392
> and here is the PR: https://github.com/apache/nifi/pull/2171
>
> Thank you!
>
> On Mon, Sep 18, 2017 at 6:07 PM, Andrew Hulbert <[hidden email]>
> wrote:
>
>> Hi Omer,
>>
>> If you're interested in some help to implement, test, or review a
>> graphite/grafana metrics reporter please let me know! We have written a
>> very simple version and are interested in getting support into the main
>> codebase as well.
>>
>> -Andrew
>>
>>
>> On 09/17/2017 05:57 PM, Joe Witt wrote:
>>
>>> Omer
>>>
>>> Is the right list and it's awesome you want to contribute.
>>>
>>> Yes for sure such contribs are welcome.  Just need to be sure all
>>> libraries
>>> used including transitive deps are fair game as far as licensing goes and
>>> are properly accounted for.
>>>
>>> As far as refactoring to avoid code duplication it could be helpful.  You
>>> might want to just do a jira and PR to do yours in a nice and clean and
>>> reusable way and once that is done and in then do another jira and PR to
>>> clean up the others.
>>>
>>> Thanks
>>> Joe
>>>
>>> On Sep 16, 2017 2:44 PM, "Omer Hadari" <[hidden email]> wrote:
>>>
>>> Hello,
>>>>
>>>> I hope I am writing to the correct mailing list.
>>>> We use graphite in my organization, and recently started to use nifi.
>>>> We went on to write a simple reporting task for graphite, and I figured
>>>> it could be used by other people as well, so why not contribute it.
>>>> I was looking at other reporting tasks though (DataDog and Ambari), and
>>>> there seems to me that there is some code duplication in how they access
>>>> metrics. They both use very similar classes in order to to that:
>>>> org.apache.nifi.reporting.ambari.metrics.MetricsService
>>>> org.apache.nifi.reporting.ambari.metrics.MetricNames
>>>> org.apache.nifi.reporting.datadog.metrics.MetricsService
>>>> org.apache.nifi.reporting.datadog.metrics.MetricNames
>>>>
>>>> They are not identical, but again - very similar. I think this
>>>> functionality can be easily exported to some other module, in order for
>>>> more reporting tasks that need to generally report the same metrics to be
>>>> written more easily.
>>>> My questions are:
>>>> a. Are more metric reporting tasks (like graphite) welcome
>>>> b. If the refactor I am suggesting is in order, will it belong in
>>>> nifi-commons or is a new module for reporting tasks in order?
>>>>
>>>> I would be more than happy to implement any and all changes I have just
>>>> suggested by myself, and am simply asking these questions in order to
>>>> best
>>>> fit into your conventions and workflow.
>>>>
>>>> Thank you in advance!
>>>>
>>>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: Metric access for reporting tasks

Omer Hadari
I looked at it and I think they could live under the same nar. That might
be preferred since we want each implementation to depend on the same
version of dropwizard-metrics, and including it in each nar is redundant
and might even cause problems (correct me if I am wrong here).

If you think these (or future) implementations might have conflicting
dependencies I guess it's also possible to separate each into it's own
submodule, or even separate specific problematic nars but keep most in the
same one. Anyway, I think the api should be extracted to it's own module so
that anyone who wants to implement their own service can do so without
including modules they don't need.

By the way, if it's OK of course, could you please add me to the jira so
that the issue can be assigned to me once opened? Thank you!

On Wed, 11 Oct 2017 at 17:56 Bryan Bende <[hidden email]> wrote:

> Omer,
>
> I think adding the new versions that implement the new
> MetricReporterService, and marking the old ones as deprecated makes
> sense. They could potentially be removed on a major future release
> like 2.0.0.
>
> Were you envisioning the DataDogMetricReportService and
> AmbariMetricReportingService living along side the
> GraphiteMetricReportingService in nifi-metrics-reporting-task? or
> would the DataDog and Ambari implementations live inside their
> respective NARs and just depend on the MetricsReportingService API?
>
> I haven't really looked at the dependencies to see if putting them all
> in one NAR would cause any issues.
>
> I have slight concerns over whether the Ambari one can be easily
> converted to the new approach, obviously it would be good if we can,
> but we need to ensure we port over the exact logic it is using.
>
> Thanks,
>
> Bryan
>
>
> On Wed, Oct 11, 2017 at 8:40 AM, Omer Hadari <[hidden email]>
> wrote:
> > So I have created a generic metric reporting task, and implemented a
> > Graphite service for it (Thank you Bryan for the quick reviews and
> > responses!), and I am up to implementing the DataDog and Ambari reporting
> > tasks in the same manner as well. I think it's important for avoiding
> > confusion when implementing another reporting task, and for creating a
> > uniform UI for metric reporting (the same task, different implementations
> > of the controller service).
> > I don't think I can remove the old ones though (It will obviously break
> > flows that use them). What do you think is best practice here?
> > Personally I think implementing a "double" and deprecate the old ones in
> > some way is OK.
> >
> > For reference here is the original ticket:
> > https://issues.apache.org/jira/browse/NIFI-4392
> > and here is the PR: https://github.com/apache/nifi/pull/2171
> >
> > Thank you!
> >
> > On Mon, Sep 18, 2017 at 6:07 PM, Andrew Hulbert <[hidden email]
> >
> > wrote:
> >
> >> Hi Omer,
> >>
> >> If you're interested in some help to implement, test, or review a
> >> graphite/grafana metrics reporter please let me know! We have written a
> >> very simple version and are interested in getting support into the main
> >> codebase as well.
> >>
> >> -Andrew
> >>
> >>
> >> On 09/17/2017 05:57 PM, Joe Witt wrote:
> >>
> >>> Omer
> >>>
> >>> Is the right list and it's awesome you want to contribute.
> >>>
> >>> Yes for sure such contribs are welcome.  Just need to be sure all
> >>> libraries
> >>> used including transitive deps are fair game as far as licensing goes
> and
> >>> are properly accounted for.
> >>>
> >>> As far as refactoring to avoid code duplication it could be helpful.
> You
> >>> might want to just do a jira and PR to do yours in a nice and clean and
> >>> reusable way and once that is done and in then do another jira and PR
> to
> >>> clean up the others.
> >>>
> >>> Thanks
> >>> Joe
> >>>
> >>> On Sep 16, 2017 2:44 PM, "Omer Hadari" <[hidden email]> wrote:
> >>>
> >>> Hello,
> >>>>
> >>>> I hope I am writing to the correct mailing list.
> >>>> We use graphite in my organization, and recently started to use nifi.
> >>>> We went on to write a simple reporting task for graphite, and I
> figured
> >>>> it could be used by other people as well, so why not contribute it.
> >>>> I was looking at other reporting tasks though (DataDog and Ambari),
> and
> >>>> there seems to me that there is some code duplication in how they
> access
> >>>> metrics. They both use very similar classes in order to to that:
> >>>> org.apache.nifi.reporting.ambari.metrics.MetricsService
> >>>> org.apache.nifi.reporting.ambari.metrics.MetricNames
> >>>> org.apache.nifi.reporting.datadog.metrics.MetricsService
> >>>> org.apache.nifi.reporting.datadog.metrics.MetricNames
> >>>>
> >>>> They are not identical, but again - very similar. I think this
> >>>> functionality can be easily exported to some other module, in order
> for
> >>>> more reporting tasks that need to generally report the same metrics
> to be
> >>>> written more easily.
> >>>> My questions are:
> >>>> a. Are more metric reporting tasks (like graphite) welcome
> >>>> b. If the refactor I am suggesting is in order, will it belong in
> >>>> nifi-commons or is a new module for reporting tasks in order?
> >>>>
> >>>> I would be more than happy to implement any and all changes I have
> just
> >>>> suggested by myself, and am simply asking these questions in order to
> >>>> best
> >>>> fit into your conventions and workflow.
> >>>>
> >>>> Thank you in advance!
> >>>>
> >>>>
> >>
>
Reply | Threaded
Open this post in threaded view
|

Re: Metric access for reporting tasks

Bryan Bende
I just added you to the contributors list in JIRA so you should be
able to assign things to yourself.

I think initially putting all the metric services into the same NAR
will probably be fine. If we add others in the future that bring in
any conflicting dependencies then we can re-evaluate.

The API is already separate in nifi-metrics-reporter-service-api right?

Someone could implement their own metrics service in another NAR by
having a NAR dependency on nifi-metrics-reporter-service-api-nar.


On Wed, Oct 11, 2017 at 1:23 PM, Omer Hadari <[hidden email]> wrote:

> I looked at it and I think they could live under the same nar. That might
> be preferred since we want each implementation to depend on the same
> version of dropwizard-metrics, and including it in each nar is redundant
> and might even cause problems (correct me if I am wrong here).
>
> If you think these (or future) implementations might have conflicting
> dependencies I guess it's also possible to separate each into it's own
> submodule, or even separate specific problematic nars but keep most in the
> same one. Anyway, I think the api should be extracted to it's own module so
> that anyone who wants to implement their own service can do so without
> including modules they don't need.
>
> By the way, if it's OK of course, could you please add me to the jira so
> that the issue can be assigned to me once opened? Thank you!
>
> On Wed, 11 Oct 2017 at 17:56 Bryan Bende <[hidden email]> wrote:
>
>> Omer,
>>
>> I think adding the new versions that implement the new
>> MetricReporterService, and marking the old ones as deprecated makes
>> sense. They could potentially be removed on a major future release
>> like 2.0.0.
>>
>> Were you envisioning the DataDogMetricReportService and
>> AmbariMetricReportingService living along side the
>> GraphiteMetricReportingService in nifi-metrics-reporting-task? or
>> would the DataDog and Ambari implementations live inside their
>> respective NARs and just depend on the MetricsReportingService API?
>>
>> I haven't really looked at the dependencies to see if putting them all
>> in one NAR would cause any issues.
>>
>> I have slight concerns over whether the Ambari one can be easily
>> converted to the new approach, obviously it would be good if we can,
>> but we need to ensure we port over the exact logic it is using.
>>
>> Thanks,
>>
>> Bryan
>>
>>
>> On Wed, Oct 11, 2017 at 8:40 AM, Omer Hadari <[hidden email]>
>> wrote:
>> > So I have created a generic metric reporting task, and implemented a
>> > Graphite service for it (Thank you Bryan for the quick reviews and
>> > responses!), and I am up to implementing the DataDog and Ambari reporting
>> > tasks in the same manner as well. I think it's important for avoiding
>> > confusion when implementing another reporting task, and for creating a
>> > uniform UI for metric reporting (the same task, different implementations
>> > of the controller service).
>> > I don't think I can remove the old ones though (It will obviously break
>> > flows that use them). What do you think is best practice here?
>> > Personally I think implementing a "double" and deprecate the old ones in
>> > some way is OK.
>> >
>> > For reference here is the original ticket:
>> > https://issues.apache.org/jira/browse/NIFI-4392
>> > and here is the PR: https://github.com/apache/nifi/pull/2171
>> >
>> > Thank you!
>> >
>> > On Mon, Sep 18, 2017 at 6:07 PM, Andrew Hulbert <[hidden email]
>> >
>> > wrote:
>> >
>> >> Hi Omer,
>> >>
>> >> If you're interested in some help to implement, test, or review a
>> >> graphite/grafana metrics reporter please let me know! We have written a
>> >> very simple version and are interested in getting support into the main
>> >> codebase as well.
>> >>
>> >> -Andrew
>> >>
>> >>
>> >> On 09/17/2017 05:57 PM, Joe Witt wrote:
>> >>
>> >>> Omer
>> >>>
>> >>> Is the right list and it's awesome you want to contribute.
>> >>>
>> >>> Yes for sure such contribs are welcome.  Just need to be sure all
>> >>> libraries
>> >>> used including transitive deps are fair game as far as licensing goes
>> and
>> >>> are properly accounted for.
>> >>>
>> >>> As far as refactoring to avoid code duplication it could be helpful.
>> You
>> >>> might want to just do a jira and PR to do yours in a nice and clean and
>> >>> reusable way and once that is done and in then do another jira and PR
>> to
>> >>> clean up the others.
>> >>>
>> >>> Thanks
>> >>> Joe
>> >>>
>> >>> On Sep 16, 2017 2:44 PM, "Omer Hadari" <[hidden email]> wrote:
>> >>>
>> >>> Hello,
>> >>>>
>> >>>> I hope I am writing to the correct mailing list.
>> >>>> We use graphite in my organization, and recently started to use nifi.
>> >>>> We went on to write a simple reporting task for graphite, and I
>> figured
>> >>>> it could be used by other people as well, so why not contribute it.
>> >>>> I was looking at other reporting tasks though (DataDog and Ambari),
>> and
>> >>>> there seems to me that there is some code duplication in how they
>> access
>> >>>> metrics. They both use very similar classes in order to to that:
>> >>>> org.apache.nifi.reporting.ambari.metrics.MetricsService
>> >>>> org.apache.nifi.reporting.ambari.metrics.MetricNames
>> >>>> org.apache.nifi.reporting.datadog.metrics.MetricsService
>> >>>> org.apache.nifi.reporting.datadog.metrics.MetricNames
>> >>>>
>> >>>> They are not identical, but again - very similar. I think this
>> >>>> functionality can be easily exported to some other module, in order
>> for
>> >>>> more reporting tasks that need to generally report the same metrics
>> to be
>> >>>> written more easily.
>> >>>> My questions are:
>> >>>> a. Are more metric reporting tasks (like graphite) welcome
>> >>>> b. If the refactor I am suggesting is in order, will it belong in
>> >>>> nifi-commons or is a new module for reporting tasks in order?
>> >>>>
>> >>>> I would be more than happy to implement any and all changes I have
>> just
>> >>>> suggested by myself, and am simply asking these questions in order to
>> >>>> best
>> >>>> fit into your conventions and workflow.
>> >>>>
>> >>>> Thank you in advance!
>> >>>>
>> >>>>
>> >>
>>
Reply | Threaded
Open this post in threaded view
|

Re: Metric access for reporting tasks

Omer Hadari
Yes it is separate, just read my e-mail again and I see I wasn't clear,
sorry about that.
Thank you for adding me to the list. I'll create a ticket for the refactor
and start working on it soon.
Also, what's the appropriate way to deprecate a
reporting-task/service/processor?


On Wed, Oct 11, 2017 at 8:48 PM, Bryan Bende <[hidden email]> wrote:

> I just added you to the contributors list in JIRA so you should be
> able to assign things to yourself.
>
> I think initially putting all the metric services into the same NAR
> will probably be fine. If we add others in the future that bring in
> any conflicting dependencies then we can re-evaluate.
>
> The API is already separate in nifi-metrics-reporter-service-api right?
>
> Someone could implement their own metrics service in another NAR by
> having a NAR dependency on nifi-metrics-reporter-service-api-nar.
>
>
> On Wed, Oct 11, 2017 at 1:23 PM, Omer Hadari <[hidden email]>
> wrote:
> > I looked at it and I think they could live under the same nar. That might
> > be preferred since we want each implementation to depend on the same
> > version of dropwizard-metrics, and including it in each nar is redundant
> > and might even cause problems (correct me if I am wrong here).
> >
> > If you think these (or future) implementations might have conflicting
> > dependencies I guess it's also possible to separate each into it's own
> > submodule, or even separate specific problematic nars but keep most in
> the
> > same one. Anyway, I think the api should be extracted to it's own module
> so
> > that anyone who wants to implement their own service can do so without
> > including modules they don't need.
> >
> > By the way, if it's OK of course, could you please add me to the jira so
> > that the issue can be assigned to me once opened? Thank you!
> >
> > On Wed, 11 Oct 2017 at 17:56 Bryan Bende <[hidden email]> wrote:
> >
> >> Omer,
> >>
> >> I think adding the new versions that implement the new
> >> MetricReporterService, and marking the old ones as deprecated makes
> >> sense. They could potentially be removed on a major future release
> >> like 2.0.0.
> >>
> >> Were you envisioning the DataDogMetricReportService and
> >> AmbariMetricReportingService living along side the
> >> GraphiteMetricReportingService in nifi-metrics-reporting-task? or
> >> would the DataDog and Ambari implementations live inside their
> >> respective NARs and just depend on the MetricsReportingService API?
> >>
> >> I haven't really looked at the dependencies to see if putting them all
> >> in one NAR would cause any issues.
> >>
> >> I have slight concerns over whether the Ambari one can be easily
> >> converted to the new approach, obviously it would be good if we can,
> >> but we need to ensure we port over the exact logic it is using.
> >>
> >> Thanks,
> >>
> >> Bryan
> >>
> >>
> >> On Wed, Oct 11, 2017 at 8:40 AM, Omer Hadari <[hidden email]>
> >> wrote:
> >> > So I have created a generic metric reporting task, and implemented a
> >> > Graphite service for it (Thank you Bryan for the quick reviews and
> >> > responses!), and I am up to implementing the DataDog and Ambari
> reporting
> >> > tasks in the same manner as well. I think it's important for avoiding
> >> > confusion when implementing another reporting task, and for creating a
> >> > uniform UI for metric reporting (the same task, different
> implementations
> >> > of the controller service).
> >> > I don't think I can remove the old ones though (It will obviously
> break
> >> > flows that use them). What do you think is best practice here?
> >> > Personally I think implementing a "double" and deprecate the old ones
> in
> >> > some way is OK.
> >> >
> >> > For reference here is the original ticket:
> >> > https://issues.apache.org/jira/browse/NIFI-4392
> >> > and here is the PR: https://github.com/apache/nifi/pull/2171
> >> >
> >> > Thank you!
> >> >
> >> > On Mon, Sep 18, 2017 at 6:07 PM, Andrew Hulbert <
> [hidden email]
> >> >
> >> > wrote:
> >> >
> >> >> Hi Omer,
> >> >>
> >> >> If you're interested in some help to implement, test, or review a
> >> >> graphite/grafana metrics reporter please let me know! We have
> written a
> >> >> very simple version and are interested in getting support into the
> main
> >> >> codebase as well.
> >> >>
> >> >> -Andrew
> >> >>
> >> >>
> >> >> On 09/17/2017 05:57 PM, Joe Witt wrote:
> >> >>
> >> >>> Omer
> >> >>>
> >> >>> Is the right list and it's awesome you want to contribute.
> >> >>>
> >> >>> Yes for sure such contribs are welcome.  Just need to be sure all
> >> >>> libraries
> >> >>> used including transitive deps are fair game as far as licensing
> goes
> >> and
> >> >>> are properly accounted for.
> >> >>>
> >> >>> As far as refactoring to avoid code duplication it could be helpful.
> >> You
> >> >>> might want to just do a jira and PR to do yours in a nice and clean
> and
> >> >>> reusable way and once that is done and in then do another jira and
> PR
> >> to
> >> >>> clean up the others.
> >> >>>
> >> >>> Thanks
> >> >>> Joe
> >> >>>
> >> >>> On Sep 16, 2017 2:44 PM, "Omer Hadari" <[hidden email]>
> wrote:
> >> >>>
> >> >>> Hello,
> >> >>>>
> >> >>>> I hope I am writing to the correct mailing list.
> >> >>>> We use graphite in my organization, and recently started to use
> nifi.
> >> >>>> We went on to write a simple reporting task for graphite, and I
> >> figured
> >> >>>> it could be used by other people as well, so why not contribute it.
> >> >>>> I was looking at other reporting tasks though (DataDog and Ambari),
> >> and
> >> >>>> there seems to me that there is some code duplication in how they
> >> access
> >> >>>> metrics. They both use very similar classes in order to to that:
> >> >>>> org.apache.nifi.reporting.ambari.metrics.MetricsService
> >> >>>> org.apache.nifi.reporting.ambari.metrics.MetricNames
> >> >>>> org.apache.nifi.reporting.datadog.metrics.MetricsService
> >> >>>> org.apache.nifi.reporting.datadog.metrics.MetricNames
> >> >>>>
> >> >>>> They are not identical, but again - very similar. I think this
> >> >>>> functionality can be easily exported to some other module, in order
> >> for
> >> >>>> more reporting tasks that need to generally report the same metrics
> >> to be
> >> >>>> written more easily.
> >> >>>> My questions are:
> >> >>>> a. Are more metric reporting tasks (like graphite) welcome
> >> >>>> b. If the refactor I am suggesting is in order, will it belong in
> >> >>>> nifi-commons or is a new module for reporting tasks in order?
> >> >>>>
> >> >>>> I would be more than happy to implement any and all changes I have
> >> just
> >> >>>> suggested by myself, and am simply asking these questions in order
> to
> >> >>>> best
> >> >>>> fit into your conventions and workflow.
> >> >>>>
> >> >>>> Thank you in advance!
> >> >>>>
> >> >>>>
> >> >>
> >>
>
Reply | Threaded
Open this post in threaded view
|

Re: Metric access for reporting tasks

Bryan Bende
There should be a DeprecationNotice annotation in the nifi-api module.

I believe the intent was to use this, and then later add some
visualization in the UI/docs to indicate what is deprecated.

Anyone else feel free to correct me here.

On Wed, Oct 11, 2017 at 1:54 PM, Omer Hadari <[hidden email]> wrote:

> Yes it is separate, just read my e-mail again and I see I wasn't clear,
> sorry about that.
> Thank you for adding me to the list. I'll create a ticket for the refactor
> and start working on it soon.
> Also, what's the appropriate way to deprecate a
> reporting-task/service/processor?
>
>
> On Wed, Oct 11, 2017 at 8:48 PM, Bryan Bende <[hidden email]> wrote:
>
>> I just added you to the contributors list in JIRA so you should be
>> able to assign things to yourself.
>>
>> I think initially putting all the metric services into the same NAR
>> will probably be fine. If we add others in the future that bring in
>> any conflicting dependencies then we can re-evaluate.
>>
>> The API is already separate in nifi-metrics-reporter-service-api right?
>>
>> Someone could implement their own metrics service in another NAR by
>> having a NAR dependency on nifi-metrics-reporter-service-api-nar.
>>
>>
>> On Wed, Oct 11, 2017 at 1:23 PM, Omer Hadari <[hidden email]>
>> wrote:
>> > I looked at it and I think they could live under the same nar. That might
>> > be preferred since we want each implementation to depend on the same
>> > version of dropwizard-metrics, and including it in each nar is redundant
>> > and might even cause problems (correct me if I am wrong here).
>> >
>> > If you think these (or future) implementations might have conflicting
>> > dependencies I guess it's also possible to separate each into it's own
>> > submodule, or even separate specific problematic nars but keep most in
>> the
>> > same one. Anyway, I think the api should be extracted to it's own module
>> so
>> > that anyone who wants to implement their own service can do so without
>> > including modules they don't need.
>> >
>> > By the way, if it's OK of course, could you please add me to the jira so
>> > that the issue can be assigned to me once opened? Thank you!
>> >
>> > On Wed, 11 Oct 2017 at 17:56 Bryan Bende <[hidden email]> wrote:
>> >
>> >> Omer,
>> >>
>> >> I think adding the new versions that implement the new
>> >> MetricReporterService, and marking the old ones as deprecated makes
>> >> sense. They could potentially be removed on a major future release
>> >> like 2.0.0.
>> >>
>> >> Were you envisioning the DataDogMetricReportService and
>> >> AmbariMetricReportingService living along side the
>> >> GraphiteMetricReportingService in nifi-metrics-reporting-task? or
>> >> would the DataDog and Ambari implementations live inside their
>> >> respective NARs and just depend on the MetricsReportingService API?
>> >>
>> >> I haven't really looked at the dependencies to see if putting them all
>> >> in one NAR would cause any issues.
>> >>
>> >> I have slight concerns over whether the Ambari one can be easily
>> >> converted to the new approach, obviously it would be good if we can,
>> >> but we need to ensure we port over the exact logic it is using.
>> >>
>> >> Thanks,
>> >>
>> >> Bryan
>> >>
>> >>
>> >> On Wed, Oct 11, 2017 at 8:40 AM, Omer Hadari <[hidden email]>
>> >> wrote:
>> >> > So I have created a generic metric reporting task, and implemented a
>> >> > Graphite service for it (Thank you Bryan for the quick reviews and
>> >> > responses!), and I am up to implementing the DataDog and Ambari
>> reporting
>> >> > tasks in the same manner as well. I think it's important for avoiding
>> >> > confusion when implementing another reporting task, and for creating a
>> >> > uniform UI for metric reporting (the same task, different
>> implementations
>> >> > of the controller service).
>> >> > I don't think I can remove the old ones though (It will obviously
>> break
>> >> > flows that use them). What do you think is best practice here?
>> >> > Personally I think implementing a "double" and deprecate the old ones
>> in
>> >> > some way is OK.
>> >> >
>> >> > For reference here is the original ticket:
>> >> > https://issues.apache.org/jira/browse/NIFI-4392
>> >> > and here is the PR: https://github.com/apache/nifi/pull/2171
>> >> >
>> >> > Thank you!
>> >> >
>> >> > On Mon, Sep 18, 2017 at 6:07 PM, Andrew Hulbert <
>> [hidden email]
>> >> >
>> >> > wrote:
>> >> >
>> >> >> Hi Omer,
>> >> >>
>> >> >> If you're interested in some help to implement, test, or review a
>> >> >> graphite/grafana metrics reporter please let me know! We have
>> written a
>> >> >> very simple version and are interested in getting support into the
>> main
>> >> >> codebase as well.
>> >> >>
>> >> >> -Andrew
>> >> >>
>> >> >>
>> >> >> On 09/17/2017 05:57 PM, Joe Witt wrote:
>> >> >>
>> >> >>> Omer
>> >> >>>
>> >> >>> Is the right list and it's awesome you want to contribute.
>> >> >>>
>> >> >>> Yes for sure such contribs are welcome.  Just need to be sure all
>> >> >>> libraries
>> >> >>> used including transitive deps are fair game as far as licensing
>> goes
>> >> and
>> >> >>> are properly accounted for.
>> >> >>>
>> >> >>> As far as refactoring to avoid code duplication it could be helpful.
>> >> You
>> >> >>> might want to just do a jira and PR to do yours in a nice and clean
>> and
>> >> >>> reusable way and once that is done and in then do another jira and
>> PR
>> >> to
>> >> >>> clean up the others.
>> >> >>>
>> >> >>> Thanks
>> >> >>> Joe
>> >> >>>
>> >> >>> On Sep 16, 2017 2:44 PM, "Omer Hadari" <[hidden email]>
>> wrote:
>> >> >>>
>> >> >>> Hello,
>> >> >>>>
>> >> >>>> I hope I am writing to the correct mailing list.
>> >> >>>> We use graphite in my organization, and recently started to use
>> nifi.
>> >> >>>> We went on to write a simple reporting task for graphite, and I
>> >> figured
>> >> >>>> it could be used by other people as well, so why not contribute it.
>> >> >>>> I was looking at other reporting tasks though (DataDog and Ambari),
>> >> and
>> >> >>>> there seems to me that there is some code duplication in how they
>> >> access
>> >> >>>> metrics. They both use very similar classes in order to to that:
>> >> >>>> org.apache.nifi.reporting.ambari.metrics.MetricsService
>> >> >>>> org.apache.nifi.reporting.ambari.metrics.MetricNames
>> >> >>>> org.apache.nifi.reporting.datadog.metrics.MetricsService
>> >> >>>> org.apache.nifi.reporting.datadog.metrics.MetricNames
>> >> >>>>
>> >> >>>> They are not identical, but again - very similar. I think this
>> >> >>>> functionality can be easily exported to some other module, in order
>> >> for
>> >> >>>> more reporting tasks that need to generally report the same metrics
>> >> to be
>> >> >>>> written more easily.
>> >> >>>> My questions are:
>> >> >>>> a. Are more metric reporting tasks (like graphite) welcome
>> >> >>>> b. If the refactor I am suggesting is in order, will it belong in
>> >> >>>> nifi-commons or is a new module for reporting tasks in order?
>> >> >>>>
>> >> >>>> I would be more than happy to implement any and all changes I have
>> >> just
>> >> >>>> suggested by myself, and am simply asking these questions in order
>> to
>> >> >>>> best
>> >> >>>> fit into your conventions and workflow.
>> >> >>>>
>> >> >>>> Thank you in advance!
>> >> >>>>
>> >> >>>>
>> >> >>
>> >>
>>