NIFI-9455 Add aggregated predictions to Prometheus#5582
NIFI-9455 Add aggregated predictions to Prometheus#5582timeabarna wants to merge 2 commits intoapache:mainfrom
Conversation
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for the improvements @timeabarna! The filtering parameters are similar to the current PR #5571. Could you take a look at that PR and consider how the changes here would align?
PR #5571 implements similar sample name filtering, and include includes a collector registry filter, it also leverages the existing CollectorRegistry.filteredMetricFamilySamples() method to select one or more sample names. I would like to incorporate those changes to improve filtering for the existing Prometheus producer output before introducing the new JSON producer output if possible, so it would be helpful to get your feedback on that PR before moving forward on this one.
...nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/FlowResource.java
Outdated
Show resolved
Hide resolved
...nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/FlowResource.java
Outdated
Show resolved
Hide resolved
|
This PR is pending on #5673 |
9522f24 to
7ec7ba0
Compare
|
Thanks @exceptionfactory for your help. This PR should be the final of the consolidated PRs, can you please help reviewing it? |
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for updating this pull request and implementing the features @timeabarna. The implementation looks straightforward, just a few small notes regarding the potential for declaring and reusing some static variables.
...fi-prometheus-utils/src/main/java/org/apache/nifi/prometheus/util/PrometheusMetricsUtil.java
Show resolved
Hide resolved
...fi-prometheus-utils/src/main/java/org/apache/nifi/prometheus/util/PrometheusMetricsUtil.java
Outdated
Show resolved
Hide resolved
...ework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java
Outdated
Show resolved
Hide resolved
...ework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java
Outdated
Show resolved
Hide resolved
...rting-task/src/test/java/org/apache/nifi/reporting/prometheus/TestPrometheusMetricsUtil.java
Outdated
Show resolved
Hide resolved
...rting-task/src/test/java/org/apache/nifi/reporting/prometheus/TestPrometheusMetricsUtil.java
Outdated
Show resolved
Hide resolved
Adding backpressure now Prediction metrics
…mments for task duration calculation
|
Thanks @exceptionfactory for your review, I've modified the code based on your recommendations. |
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for making the adjustments @timeabarna! The changes look good and work as expected at runtime. +1 Merging.
This closes apache#5582 Signed-off-by: David Handermann <exceptionfactory@apache.org>
This closes apache#5582 Signed-off-by: David Handermann <exceptionfactory@apache.org>
https://issues.apache.org/jira/browse/NIFI-9455
Description of PR
Adding root level aggregated prediction metrics
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically
main)?Is your initial contribution a single, squashed commit? Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not
squashor use--forcewhen pushing to allow for clean monitoring of changes.For code changes:
mvn -Pcontrib-check clean installat the rootnififolder?LICENSEfile, including the mainLICENSEfile undernifi-assembly?NOTICEfile, including the mainNOTICEfile found undernifi-assembly?.displayNamein addition to .name (programmatic access) for each of the new properties?For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.