markap14 commented on code in PR #89:
URL: https://github.com/apache/nifi-api/pull/89#discussion_r3313058358
##########
src/main/java/org/apache/nifi/controller/status/ProcessGroupStatus.java:
##########
@@ -293,6 +295,27 @@ public void
setProcessingPerformanceStatus(ProcessingPerformanceStatus processin
this.processingPerformanceStatus = processingPerformanceStatus;
}
+ /**
+ * Returns the snapshot of logging attributes (e.g. connector identity,
NiFi {@code processGroup*} keys) that apply to
+ * this Process Group at the time the status was captured. Consumers
should treat the returned map as immutable.
+ *
+ * @return the logging attributes for this Process Group, or an empty map
if none have been set
+ */
+ public Map<String, String> getLoggingAttributes() {
+ return loggingAttributes;
+ }
+
+ /**
+ * Sets the snapshot of logging attributes (e.g. connector identity, NiFi
{@code processGroup*} keys) that apply to
+ * this Process Group at the time the status was captured. The supplied
map is stored by reference; callers should
+ * provide an immutable snapshot (e.g. {@link Map#copyOf(Map)}). A {@code
null} value is treated as an empty map.
+ *
+ * @param loggingAttributes the logging attributes for this Process Group
+ */
+ public void setLoggingAttributes(final Map<String, String>
loggingAttributes) {
+ this.loggingAttributes = loggingAttributes == null ? Map.of() :
loggingAttributes;
+ }
Review Comment:
This feels like an area where we should make a defensive copy in `set` and a
`Collections.unmodifiableMap` (or another defensive copy) in `get`
##########
src/main/java/org/apache/nifi/controller/status/ProcessGroupStatus.java:
##########
@@ -317,6 +340,7 @@ public ProcessGroupStatus clone() {
clonedObj.bytesTransferred = bytesTransferred;
clonedObj.processingNanos = processingNanos;
clonedObj.processingPerformanceStatus = processingPerformanceStatus;
+ clonedObj.loggingAttributes = loggingAttributes;
Review Comment:
Here, I think we need a defensive copy as well
##########
src/main/java/org/apache/nifi/controller/status/ProcessGroupStatus.java:
##########
@@ -476,6 +500,10 @@ public static void merge(final ProcessGroupStatus target,
final ProcessGroupStat
}
target.setRegisteredFlowSnapshotMetadata(toMerge.getRegisteredFlowSnapshotMetadata());
+ if (!toMerge.getLoggingAttributes().isEmpty()) {
+ target.setLoggingAttributes(toMerge.getLoggingAttributes());
Review Comment:
If merging, should this be union or the intersection of the two maps maybe?
Just blindly overwriting the one with the other seems not ideal.
##########
src/main/java/org/apache/nifi/components/connector/AbstractConnector.java:
##########
@@ -96,6 +96,19 @@ protected final ConnectorInitializationContext
getInitializationContext() {
return initializationContext;
}
+ /**
+ * Sets custom logging attributes that the framework will include in the
SLF4J {@code MDC} for every log line emitted
+ * by this Connector and by any component running inside its managed flow.
Convenience wrapper for
+ * {@link ConnectorInitializationContext#setLoggingAttributes(Map)} that
subclasses can call once they have computed
+ * the attributes (for example, after configuration is applied). Calls
replace any previously set custom attributes.
+ *
+ * @param attributes the custom attributes to expose; must not be {@code
null}
+ * @see ConnectorInitializationContext#setLoggingAttributes(Map)
+ */
+ protected final void setLoggingAttributes(final Map<String, String>
attributes) {
+ getInitializationContext().setLoggingAttributes(attributes);
+ }
Review Comment:
Is it worth introducing this convenience wrapper just to avoid calling
`getInitializationContext()`? It would make sense if this were something that
we expect to call many times to keep code clean, but I can't envision this
really being called that frequently
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
