-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NIFI-14077 Add ProcessGroup Performance Metrics to Prometheus #9577
base: main
Are you sure you want to change the base?
Conversation
…TAL_TASK_DURATION to ProcessGroups, add allable values to the documentation of the flow metrics API.
I suspect the macos system test is flakey so I am rerunning it on my fork https://github.com/esecules/nifi/actions/runs/12285436758?pr=3 EDIT: |
You're welcome, I have recently noticed a couple flaky tests in the system-tests workflow. |
Looks like the retry failed still 😔, however it's passing in my fork and my machine (MacBook). Do these tests run in parallel and might interfere with each other? Or the tests might be running on an underpowered server and the 60 second timeout isn't sufficient for GitHub Actions? Github's macos runners do only have 3 cores. Ubuntu runners have 4 cores. I am assuming apache/nifi uses the default runners. |
Looks like system tests passed on retry! Thanks for all the retries. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for proposing these changes @esecules. I'm not complete sure about introducing the additional gauges because the processing performance information can be disabled. However, it looks like the null handling accounts for that fact, so this approach may be sufficient. I noted a few minor recommendations.
description = "The producer for flow file metrics. Each producer may have its own output format. " + | ||
"Allowed values: [prometheus, json]", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of indicating the allowed values in the description, it should be possible to provide schema
that indicates allowed values.
@@ -275,5 +275,35 @@ public NiFiMetricsRegistry() { | |||
.help("Provenance repository free space in bytes") | |||
.labelNames("instance", "component_type", "component_name", "component_id", "parent_id", "repo_identifier") | |||
.register(registry)); | |||
|
|||
nameToGaugeMap.put("PROCESSING_PERF_CPU_MILLIS", Gauge.build() | |||
.name("nifi_processing_cpu_duration") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although it is longer, including performance
in the name would provide better alignment with the source of the information.
.name("nifi_processing_cpu_duration") | |
.name("nifi_processing_performance_cpu_duration") |
|
||
nameToGaugeMap.put("PROCESSING_PERF_CPU_MILLIS", Gauge.build() | ||
.name("nifi_processing_cpu_duration") | ||
.help("Estimated cpu time (in milliseconds) used by this component") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.help("Estimated cpu time (in milliseconds) used by this component") | |
.help("Estimated CPU time (in milliseconds) used by this component") |
|
||
nameToGaugeMap.put("PROCESSING_PERF_GC_MILLIS", Gauge.build() | ||
.name("nifi_processing_gc_duration") | ||
.help("Estimated gc time (in milliseconds) used by this component") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.help("Estimated gc time (in milliseconds) used by this component") | |
.help("Estimated garbage collection time (in milliseconds) used by this component") |
I'll check if I already added test coverage for when the performance feature is disabled and I'll add it if it's missing. |
Summary
NIFI-14077
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000
NIFI-00000
Pull Request Formatting
main
branchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
mvn clean install -P contrib-check
Licensing
(no new deps)
LICENSE
andNOTICE
filesDocumentation