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 <[email protected]> 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 <[email protected]> > 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 <[email protected] > > > > 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" <[email protected]> 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! > >>>> > >>>> > >> >
