Skip to content

NIFI-15979 Add Connector reporting-task visibility#89

Merged
exceptionfactory merged 6 commits into
apache:mainfrom
kevdoran:NIFI-15979-connector-logging
Jun 2, 2026
Merged

NIFI-15979 Add Connector reporting-task visibility#89
exceptionfactory merged 6 commits into
apache:mainfrom
kevdoran:NIFI-15979-connector-logging

Conversation

@kevdoran
Copy link
Copy Markdown
Contributor

@kevdoran kevdoran commented May 27, 2026

Summary

NIFI-15979

These API changes relate to NIP-30

  • EventAccess.getConnectorStatuses(): new default method returning an empty collection; implementations override to expose connector-managed Process Groups to reporting tasks.

Pure API additions with default impls keep existing code compiling unchanged.

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • Pull Request refers to a feature branch with one commit containing changes

@kevdoran kevdoran requested a review from markap14 May 27, 2026 16:18
- ConnectorInitializationContext.setLoggingAttributes(Map): new default method
  (throws UOE) for connectors to surface custom MDC context.
- AbstractConnector.setLoggingAttributes(Map): protected helper forwarding to
  the init context.
- ProcessGroupStatus.loggingAttributes: snapshot map (default Map.of()) plus
  accessors; propagated through clone() and merge().
- EventAccess.getConnectorStatuses(): new default method returning an empty
  collection; implementations override to expose connector-managed Process
  Groups to reporting tasks.

Pure API additions with default impls keep existing code compiling unchanged.
@kevdoran kevdoran force-pushed the NIFI-15979-connector-logging branch from ffd0870 to ff83ed2 Compare May 27, 2026 16:23
@kevdoran kevdoran requested a review from exceptionfactory May 27, 2026 16:25
Comment thread src/main/java/org/apache/nifi/controller/status/ProcessGroupStatus.java Outdated
Comment thread src/main/java/org/apache/nifi/controller/status/ProcessGroupStatus.java Outdated
Comment thread src/main/java/org/apache/nifi/controller/status/ProcessGroupStatus.java Outdated
Comment thread src/main/java/org/apache/nifi/components/connector/AbstractConnector.java Outdated
@kevdoran
Copy link
Copy Markdown
Contributor Author

Thanks for the review @markap14! I accepted all suggestions.

Comment thread src/main/java/org/apache/nifi/reporting/EventAccess.java Outdated
Copy link
Copy Markdown
Contributor

@exceptionfactory exceptionfactory left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together @kevdoran. The changes look good for Connector Status. The addition of an open-ended way to set logging attributes is more concerning, and does not seem to align with the existing goal of framework-managed attributes. Instead of an open-ended Map of attributes, it seems better to extend the MDC set of attributes at the framework level, based on declared Connector properties.

if (initializationContext == null) {
throw new IllegalStateException("Connector has not been initialized");
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This change appears unnecessary

* @param attributes the custom attributes to expose on logs from this Connector and its managed flow; must not be {@code null}
* @throws UnsupportedOperationException if the runtime implementation does not support custom logging attributes
*/
default void setLoggingAttributes(Map<String, String> attributes) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This method is very open-ended, and moves in a different direction from the current framework-controlled Logging Attributes in a Process Group. Rather than exposing it as a point of extension, it seems better to maintain a strategy of declared properties, which the framework handles for inclusion.

kevdoran and others added 2 commits June 1, 2026 10:20
The blank line between the null-check and the return statement in
getInitializationContext() was inadvertently removed in an earlier
revision of this branch and not restored when the surrounding helper
method was dropped. Addresses review feedback on PR apache#89.

Co-authored-by: Cursor <cursoragent@cursor.com>
@kevdoran kevdoran changed the title NIFI-15979 Add Connector logging MDC and reporting-task visibility NIFI-15979 Add Connector reporting-task visibility Jun 2, 2026
Per review feedback, remove the loggingAttributes snapshot field, its
accessors, and the associated clone()/merge() plumbing from
ProcessGroupStatus. The remaining ConnectorStatus type and
EventAccess.getConnectorStatuses() provide connector identity to
reporting consumers without adding a logging-attributes map to the
Process Group status.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown
Contributor

@exceptionfactory exceptionfactory left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes and adjusting the approach @kevdoran, the latest version looks good. Glad to merge pending concurrence from @markap14.

@exceptionfactory exceptionfactory merged commit 86f28b3 into apache:main Jun 2, 2026
1 check passed
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.

3 participants