Skip to content
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

MINIFICPP-2501 Add processorStatuses C2 metric node to FlowInformation #1909

Closed
wants to merge 9 commits into from

Conversation

lordgamez
Copy link
Contributor

https://issues.apache.org/jira/browse/MINIFICPP-2501


Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.

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 MINIFICPP-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?

For code changes:

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE file?
  • If applicable, have you updated the NOTICE file?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.

@lordgamez lordgamez force-pushed the MINIFICPP-2501 branch 3 times, most recently from 7f2991b to efa4f0c Compare December 14, 2024 14:47
@lordgamez lordgamez marked this pull request as ready for review December 14, 2024 21:04
@martinzink
Copy link
Member

LGTM, tried it over C2, and now it looks the same as the java agent. One thing we might want to move the FlowInfo::components::ProcessorName::running into this FlowInfo::processorStatuses struct

@lordgamez
Copy link
Contributor Author

LGTM, tried it over C2, and now it looks the same as the java agent. One thing we might want to move the FlowInfo::components::ProcessorName::running into this FlowInfo::processorStatuses struct

Thanks for checking it out! It's a good point to move the running field to the processorStatuses node as no other fields are present there and the components node only refers to processors. I moved the running field to that node in 8a5bc25

C2.md Outdated Show resolved Hide resolved
libminifi/src/core/ProcessorMetrics.cpp Outdated Show resolved Hide resolved
libminifi/test/integration/C2MetricsTest.cpp Outdated Show resolved Hide resolved
libminifi/test/integration/C2MetricsTest.cpp Outdated Show resolved Hide resolved
libminifi/test/integration/C2MetricsTest.cpp Outdated Show resolved Hide resolved
libminifi/test/integration/C2MetricsTest.cpp Outdated Show resolved Hide resolved
C2.md Outdated

### Processor Metric Response Nodes

Each processor can have its own metrics. These metric nodes can be configured in the minifi.properties by requesting metrics in the \<ProcessorType\>Metric format, for example GetTCPMetrics to request metrics for the GetTCP processors. Besides configuring processor metrics directly, they can also be configured using regular expressions with the `processorMetrics/` prefix. For example `processorMetrics/Get.*Metrics` will match all processor metrics that start with Get.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: \<ProcessorType\>Metric -> \<ProcessorType\>Metrics

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in 3cb95da

'minifi_incoming_flow_files', 'minifi_incoming_bytes', 'minifi_bytes_read', 'minifi_bytes_written',
'minifi_processing_nanos'], metric_class, labels) and \
self.verify_metrics_larger_than_zero(['minifi_onTrigger_invocations', 'minifi_transferred_flow_files', 'minifi_transferred_to_success',
'minifi_transferred_bytes', 'minifi_processing_nanos'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'minifi_processing_nanos' is in both checks -- is that intentional / useful?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in 3cb95da

@szaszm
Copy link
Member

szaszm commented Jan 21, 2025

related: apache/nifi#9651
we should change the format to match the format here, i.e. include "runState" as an enum (JSON string with well defined values)

@lordgamez
Copy link
Contributor Author

related: apache/nifi#9651 we should change the format to match the format here, i.e. include "runState" as an enum (JSON string with well defined values)

Updated to use "runningState" in C2 flowInfo with possible values "RUNNING" and "STOPPED" in c34e908

@szaszm szaszm closed this in 07b9641 Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants