[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/nifi/pull/2468


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[markap14]

Github user markap14 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r174577132
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-client-dto/src/main/java/org/apache/nifi/web/api/dto/diagnostics/RepositoryUsageDTO.java
 ---
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.nifi.web.api.dto.diagnostics;
+
+import javax.xml.bind.annotation.XmlType;
+
+import io.swagger.annotations.ApiModelProperty;
+
+@XmlType(name = "fileStoreUsage")
--- End diff --

Good catch -- renamed the class and forgot to rename the XmlType


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[markap14]

Github user markap14 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r174576129
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-client-dto/src/main/java/org/apache/nifi/web/api/entity/ProcessorDiagnosticsEntity.java
 ---
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.nifi.web.api.entity;
+
+import javax.xml.bind.annotation.XmlRootElement;
+
+import org.apache.nifi.web.api.dto.diagnostics.ProcessorDiagnosticsDTO;
+
+import io.swagger.annotations.ApiModelProperty;
+
+@XmlRootElement(name = "processorDiagnostics")
--- End diff --

Yup, will do.


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[markap14]

Github user markap14 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r174576032
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/StatusMerger.java
 ---
@@ -626,6 +634,132 @@ public static void merge(final 
SystemDiagnosticsSnapshotDTO target, final System
 updatePrettyPrintedFields(target);
 }
 
+public static void merge(final JVMDiagnosticsSnapshotDTO target, final 
JVMDiagnosticsSnapshotDTO toMerge, final long numMillis) {
+if (target == null || toMerge == null) {
+return;
+}
+
+if (toMerge.getControllerDiagnostics() == null) {
+target.setControllerDiagnostics(null);
+} else {
+merge(target.getControllerDiagnostics(), 
toMerge.getControllerDiagnostics());
+}
+
+if (toMerge.getFlowDiagnosticsDto() == null) {
+target.setFlowDiagnosticsDto(null);
+} else {
+merge(target.getFlowDiagnosticsDto(), 
toMerge.getFlowDiagnosticsDto());
+}
+
+if (toMerge.getSystemDiagnosticsDto() == null) {
+target.setSystemDiagnosticsDto(null);
+} else {
+merge(target.getSystemDiagnosticsDto(), 
toMerge.getSystemDiagnosticsDto(), numMillis);
+}
+}
+
+private static void merge(final JVMControllerDiagnosticsSnapshotDTO 
target, final JVMControllerDiagnosticsSnapshotDTO toMerge) {
+if (toMerge == null || target == null) {
+return;
+}
+
+
target.setMaxEventDrivenThreads(add(target.getMaxEventDrivenThreads(), 
toMerge.getMaxEventDrivenThreads()));
+
target.setMaxTimerDrivenThreads(add(target.getMaxTimerDrivenThreads(), 
toMerge.getMaxTimerDrivenThreads()));
--- End diff --

I think so - can imagine it being helpful to see how many threads are 
available across the cluster.


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[markap14]

Github user markap14 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r174575890
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardProcessorNode.java
 ---
@@ -1349,10 +1404,12 @@ private void initiateStart(final 
ScheduledExecutorService taskScheduler, final l
 LOG.debug("Successfully invoked @OnScheduled 
methods of {} but scheduled state is no longer STARTING so will stop processor 
now", processor);
 
 // can only happen if stopProcessor was called 
before service was transitioned to RUNNING state
+activateThread();
 try {
 
ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnUnscheduled.class, 
processor, processContext);
-} finally {
--- End diff --

It shouldn't really matter -- the point of 
quietlyInvokeMethodsWithAnnotation is that the method will not throw any 
Exceptions. So whether it's in a finally block or not isn't really a big deal.


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[mcgilman]

Github user mcgilman commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r174137522
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardProcessorNode.java
 ---
@@ -1366,10 +1423,12 @@ private void initiateStart(final 
ScheduledExecutorService taskScheduler, final l
 
 // If processor's task completed Exceptionally, then we 
want to retry initiating the start (if Processor is still scheduled to run).
 try (final NarCloseable nc = 
NarCloseable.withComponentNarLoader(processor.getClass(), 
processor.getIdentifier())) {
+activateThread();
 try {
 
ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnUnscheduled.class, 
processor, processContext);
-} finally {
--- End diff --

Same as above.


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[mcgilman]

Github user mcgilman commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r174141958
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-client-dto/src/main/java/org/apache/nifi/web/api/dto/diagnostics/RepositoryUsageDTO.java
 ---
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.nifi.web.api.dto.diagnostics;
+
+import javax.xml.bind.annotation.XmlType;
+
+import io.swagger.annotations.ApiModelProperty;
+
+@XmlType(name = "fileStoreUsage")
--- End diff --

nit-pick... update name to be consistent with the class name.


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[mcgilman]

Github user mcgilman commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r174141284
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-client-dto/src/main/java/org/apache/nifi/web/api/entity/ProcessorDiagnosticsEntity.java
 ---
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.nifi.web.api.entity;
+
+import javax.xml.bind.annotation.XmlRootElement;
+
+import org.apache.nifi.web.api.dto.diagnostics.ProcessorDiagnosticsDTO;
+
+import io.swagger.annotations.ApiModelProperty;
+
+@XmlRootElement(name = "processorDiagnostics")
--- End diff --

We use processorDiagnostics for the DTO name, can we update this one to be 
processorDiagnosticsEntity to be more consistent with other entities. I believe 
this name is important for folks consuming the swagger.json like nipy-api.


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[mcgilman]

Github user mcgilman commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r174139485
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/manager/StatusMerger.java
 ---
@@ -626,6 +634,132 @@ public static void merge(final 
SystemDiagnosticsSnapshotDTO target, final System
 updatePrettyPrintedFields(target);
 }
 
+public static void merge(final JVMDiagnosticsSnapshotDTO target, final 
JVMDiagnosticsSnapshotDTO toMerge, final long numMillis) {
+if (target == null || toMerge == null) {
+return;
+}
+
+if (toMerge.getControllerDiagnostics() == null) {
+target.setControllerDiagnostics(null);
+} else {
+merge(target.getControllerDiagnostics(), 
toMerge.getControllerDiagnostics());
+}
+
+if (toMerge.getFlowDiagnosticsDto() == null) {
+target.setFlowDiagnosticsDto(null);
+} else {
+merge(target.getFlowDiagnosticsDto(), 
toMerge.getFlowDiagnosticsDto());
+}
+
+if (toMerge.getSystemDiagnosticsDto() == null) {
+target.setSystemDiagnosticsDto(null);
+} else {
+merge(target.getSystemDiagnosticsDto(), 
toMerge.getSystemDiagnosticsDto(), numMillis);
+}
+}
+
+private static void merge(final JVMControllerDiagnosticsSnapshotDTO 
target, final JVMControllerDiagnosticsSnapshotDTO toMerge) {
+if (toMerge == null || target == null) {
+return;
+}
+
+
target.setMaxEventDrivenThreads(add(target.getMaxEventDrivenThreads(), 
toMerge.getMaxEventDrivenThreads()));
+
target.setMaxTimerDrivenThreads(add(target.getMaxTimerDrivenThreads(), 
toMerge.getMaxTimerDrivenThreads()));
--- End diff --

Do we want to add these maxThread fields?


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[mcgilman]

Github user mcgilman commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r174137385
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardProcessorNode.java
 ---
@@ -1349,10 +1404,12 @@ private void initiateStart(final 
ScheduledExecutorService taskScheduler, final l
 LOG.debug("Successfully invoked @OnScheduled 
methods of {} but scheduled state is no longer STARTING so will stop processor 
now", processor);
 
 // can only happen if stopProcessor was called 
before service was transitioned to RUNNING state
+activateThread();
 try {
 
ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnUnscheduled.class, 
processor, processContext);
-} finally {
--- End diff --

Not sure which approach is correct, but this appears to be a change in 
behavior. If OnUnscheduled throws, OnStopped will not be invoked anymore. Do we 
want this new behavior, or do this finally block contain a nested try/finally 
with deactiveThread() in the nested finally? This would maintain the same 
behavior.


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[markap14]

Github user markap14 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r173906446
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java
 ---
@@ -4506,6 +4515,123 @@ public ComponentHistoryDTO 
getComponentHistory(final String componentId) {
 return history;
 }
 
+private ControllerServiceEntity createControllerServiceEntity(final 
String serviceId, final NiFiUser user) {
+final ControllerServiceNode serviceNode = 
controllerServiceDAO.getControllerService(serviceId);
+return createControllerServiceEntity(serviceNode, 
Collections.emptySet(), user);
+}
+
+@Override
+public ProcessorDiagnosticsEntity getProcessorDiagnostics(final String 
id) {
+final ProcessorNode processor = processorDAO.getProcessor(id);
+final ProcessorStatus processorStatus = 
controllerFacade.getProcessorStatus(id);
+
+// Generate Processor Diagnostics
+final NiFiUser user = NiFiUserUtils.getNiFiUser();
+final ProcessorDiagnosticsDTO dto = 
controllerFacade.getProcessorDiagnostics(processor, processorStatus, 
bulletinRepository, serviceId -> createControllerServiceEntity(serviceId, 
user));
+
+// Filter anything out of diagnostics that the user is not 
authorized to see.
+final List jvmDiagnosticsSnaphots = new 
ArrayList<>();
+final JVMDiagnosticsDTO jvmDiagnostics = dto.getJvmDiagnostics();
+jvmDiagnosticsSnaphots.add(jvmDiagnostics.getAggregateSnapshot());
+
+// filter controller-related information
+final boolean canReadController = 
authorizableLookup.getController().isAuthorized(authorizer, RequestAction.READ, 
user);
+if (!canReadController) {
+for (final JVMDiagnosticsSnapshotDTO snapshot : 
jvmDiagnosticsSnaphots) {
+snapshot.setMaxEventDrivenThreads(null);
+snapshot.setMaxTimerDrivenThreads(null);
+snapshot.setBundlesLoaded(null);
+}
+}
+
+// filter system diagnostics information
+final boolean canReadSystem = 
authorizableLookup.getSystem().isAuthorized(authorizer, RequestAction.READ, 
user);
+if (!canReadSystem) {
+for (final JVMDiagnosticsSnapshotDTO snapshot : 
jvmDiagnosticsSnaphots) {
+snapshot.setContentRepositoryStorageUsage(null);
+snapshot.setCpuCores(null);
+snapshot.setCpuLoadAverage(null);
+snapshot.setFlowFileRepositoryStorageUsage(null);
+snapshot.setMaxHeap(null);
+snapshot.setMaxHeapBytes(null);
+snapshot.setProvenanceRepositoryStorageUsage(null);
+snapshot.setPhysicalMemory(null);
+snapshot.setPhysicalMemoryBytes(null);
+snapshot.setGarbageCollectionDiagnostics(null);
+}
+}
+
+// filter connections
+final Predicate connectionAuthorized = 
connectionDiagnostics -> {
+final String connectionId = 
connectionDiagnostics.getConnection().getId();
+return 
authorizableLookup.getConnection(connectionId).getAuthorizable().isAuthorized(authorizer,
 RequestAction.READ, user);
+};
+
+// Function that can be used to remove the Source or Destination 
of a ConnectionDTO, if the user is not authorized.
+final Function 
filterSourceDestination = connectionDiagnostics -> {
--- End diff --

Good call. That would mean that the second Function there is not really 
needed. Will address.


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[markap14]

Github user markap14 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r173902565
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java
 ---
@@ -4506,6 +4515,123 @@ public ComponentHistoryDTO 
getComponentHistory(final String componentId) {
 return history;
 }
 
+private ControllerServiceEntity createControllerServiceEntity(final 
String serviceId, final NiFiUser user) {
+final ControllerServiceNode serviceNode = 
controllerServiceDAO.getControllerService(serviceId);
+return createControllerServiceEntity(serviceNode, 
Collections.emptySet(), user);
+}
+
+@Override
+public ProcessorDiagnosticsEntity getProcessorDiagnostics(final String 
id) {
+final ProcessorNode processor = processorDAO.getProcessor(id);
+final ProcessorStatus processorStatus = 
controllerFacade.getProcessorStatus(id);
+
+// Generate Processor Diagnostics
+final NiFiUser user = NiFiUserUtils.getNiFiUser();
+final ProcessorDiagnosticsDTO dto = 
controllerFacade.getProcessorDiagnostics(processor, processorStatus, 
bulletinRepository, serviceId -> createControllerServiceEntity(serviceId, 
user));
+
+// Filter anything out of diagnostics that the user is not 
authorized to see.
+final List jvmDiagnosticsSnaphots = new 
ArrayList<>();
+final JVMDiagnosticsDTO jvmDiagnostics = dto.getJvmDiagnostics();
+jvmDiagnosticsSnaphots.add(jvmDiagnostics.getAggregateSnapshot());
+
+// filter controller-related information
+final boolean canReadController = 
authorizableLookup.getController().isAuthorized(authorizer, RequestAction.READ, 
user);
+if (!canReadController) {
+for (final JVMDiagnosticsSnapshotDTO snapshot : 
jvmDiagnosticsSnaphots) {
+snapshot.setMaxEventDrivenThreads(null);
+snapshot.setMaxTimerDrivenThreads(null);
+snapshot.setBundlesLoaded(null);
+}
+}
+
+// filter system diagnostics information
+final boolean canReadSystem = 
authorizableLookup.getSystem().isAuthorized(authorizer, RequestAction.READ, 
user);
+if (!canReadSystem) {
+for (final JVMDiagnosticsSnapshotDTO snapshot : 
jvmDiagnosticsSnaphots) {
--- End diff --

That's a good call. Will update that.


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[markap14]

Github user markap14 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r173902301
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java
 ---
@@ -4506,6 +4515,123 @@ public ComponentHistoryDTO 
getComponentHistory(final String componentId) {
 return history;
 }
 
+private ControllerServiceEntity createControllerServiceEntity(final 
String serviceId, final NiFiUser user) {
+final ControllerServiceNode serviceNode = 
controllerServiceDAO.getControllerService(serviceId);
+return createControllerServiceEntity(serviceNode, 
Collections.emptySet(), user);
+}
+
+@Override
+public ProcessorDiagnosticsEntity getProcessorDiagnostics(final String 
id) {
+final ProcessorNode processor = processorDAO.getProcessor(id);
+final ProcessorStatus processorStatus = 
controllerFacade.getProcessorStatus(id);
+
+// Generate Processor Diagnostics
+final NiFiUser user = NiFiUserUtils.getNiFiUser();
+final ProcessorDiagnosticsDTO dto = 
controllerFacade.getProcessorDiagnostics(processor, processorStatus, 
bulletinRepository, serviceId -> createControllerServiceEntity(serviceId, 
user));
+
+// Filter anything out of diagnostics that the user is not 
authorized to see.
+final List jvmDiagnosticsSnaphots = new 
ArrayList<>();
+final JVMDiagnosticsDTO jvmDiagnostics = dto.getJvmDiagnostics();
+jvmDiagnosticsSnaphots.add(jvmDiagnostics.getAggregateSnapshot());
+
+// filter controller-related information
+final boolean canReadController = 
authorizableLookup.getController().isAuthorized(authorizer, RequestAction.READ, 
user);
+if (!canReadController) {
+for (final JVMDiagnosticsSnapshotDTO snapshot : 
jvmDiagnosticsSnaphots) {
--- End diff --

I don't think that active event & timer driven threads need to be filtered 
because they are available to anyone with access to /flow. I also feel like 
uptime should be available. I could see an argument for the elected primary & 
coordinator not being included, but I went back and forth on that a bit 
personally, because currently that info is not exposed anywhere except if you 
have that permissions. It seemed quite benign to me to include this, but If you 
think they should be removed I am okay with it.


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[markap14]

Github user markap14 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r173900631
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/EntityFactory.java
 ---
@@ -77,6 +79,27 @@
 
 public final class EntityFactory {
 
+public ProcessorDiagnosticsEntity 
createProcessorDiagnosticsEntity(final ProcessorDiagnosticsDTO dto, final 
RevisionDTO revision, final PermissionsDTO processorPermissions,
+final ProcessorStatusDTO status, final List 
bulletins) {
+final ProcessorDiagnosticsEntity entity = new 
ProcessorDiagnosticsEntity();
+entity.setRevision(revision);
+if (dto != null) {
+entity.setPermissions(processorPermissions);
+entity.setId(dto.getProcessor().getId());
+if (processorPermissions != null && 
processorPermissions.getCanRead()) {
+entity.setComponent(dto);
+entity.setBulletins(bulletins);
+}
+}
+
+entity.setBulletins(bulletins);
+return entity;
+}
+
+private void pairDownDiagnostics(final ProcessorDiagnosticsDTO dto, 
final PermissionsDTO controllerPermissions) {
--- End diff --

Whoops, yes, good call.


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[mcgilman]

Github user mcgilman commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r173508849
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/EntityFactory.java
 ---
@@ -77,6 +79,27 @@
 
 public final class EntityFactory {
 
+public ProcessorDiagnosticsEntity 
createProcessorDiagnosticsEntity(final ProcessorDiagnosticsDTO dto, final 
RevisionDTO revision, final PermissionsDTO processorPermissions,
+final ProcessorStatusDTO status, final List 
bulletins) {
+final ProcessorDiagnosticsEntity entity = new 
ProcessorDiagnosticsEntity();
+entity.setRevision(revision);
+if (dto != null) {
+entity.setPermissions(processorPermissions);
+entity.setId(dto.getProcessor().getId());
+if (processorPermissions != null && 
processorPermissions.getCanRead()) {
+entity.setComponent(dto);
+entity.setBulletins(bulletins);
+}
+}
+
+entity.setBulletins(bulletins);
+return entity;
+}
+
+private void pairDownDiagnostics(final ProcessorDiagnosticsDTO dto, 
final PermissionsDTO controllerPermissions) {
--- End diff --

This is method OBE since the permission-based filtering happens in the 
service facade?


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[mcgilman]

Github user mcgilman commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r173512958
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java
 ---
@@ -4506,6 +4515,123 @@ public ComponentHistoryDTO 
getComponentHistory(final String componentId) {
 return history;
 }
 
+private ControllerServiceEntity createControllerServiceEntity(final 
String serviceId, final NiFiUser user) {
+final ControllerServiceNode serviceNode = 
controllerServiceDAO.getControllerService(serviceId);
+return createControllerServiceEntity(serviceNode, 
Collections.emptySet(), user);
+}
+
+@Override
+public ProcessorDiagnosticsEntity getProcessorDiagnostics(final String 
id) {
+final ProcessorNode processor = processorDAO.getProcessor(id);
+final ProcessorStatus processorStatus = 
controllerFacade.getProcessorStatus(id);
+
+// Generate Processor Diagnostics
+final NiFiUser user = NiFiUserUtils.getNiFiUser();
+final ProcessorDiagnosticsDTO dto = 
controllerFacade.getProcessorDiagnostics(processor, processorStatus, 
bulletinRepository, serviceId -> createControllerServiceEntity(serviceId, 
user));
+
+// Filter anything out of diagnostics that the user is not 
authorized to see.
+final List jvmDiagnosticsSnaphots = new 
ArrayList<>();
+final JVMDiagnosticsDTO jvmDiagnostics = dto.getJvmDiagnostics();
+jvmDiagnosticsSnaphots.add(jvmDiagnostics.getAggregateSnapshot());
+
+// filter controller-related information
+final boolean canReadController = 
authorizableLookup.getController().isAuthorized(authorizer, RequestAction.READ, 
user);
+if (!canReadController) {
+for (final JVMDiagnosticsSnapshotDTO snapshot : 
jvmDiagnosticsSnaphots) {
+snapshot.setMaxEventDrivenThreads(null);
+snapshot.setMaxTimerDrivenThreads(null);
+snapshot.setBundlesLoaded(null);
+}
+}
+
+// filter system diagnostics information
+final boolean canReadSystem = 
authorizableLookup.getSystem().isAuthorized(authorizer, RequestAction.READ, 
user);
+if (!canReadSystem) {
+for (final JVMDiagnosticsSnapshotDTO snapshot : 
jvmDiagnosticsSnaphots) {
+snapshot.setContentRepositoryStorageUsage(null);
+snapshot.setCpuCores(null);
+snapshot.setCpuLoadAverage(null);
+snapshot.setFlowFileRepositoryStorageUsage(null);
+snapshot.setMaxHeap(null);
+snapshot.setMaxHeapBytes(null);
+snapshot.setProvenanceRepositoryStorageUsage(null);
+snapshot.setPhysicalMemory(null);
+snapshot.setPhysicalMemoryBytes(null);
+snapshot.setGarbageCollectionDiagnostics(null);
+}
+}
+
+// filter connections
+final Predicate connectionAuthorized = 
connectionDiagnostics -> {
+final String connectionId = 
connectionDiagnostics.getConnection().getId();
+return 
authorizableLookup.getConnection(connectionId).getAuthorizable().isAuthorized(authorizer,
 RequestAction.READ, user);
+};
+
+// Function that can be used to remove the Source or Destination 
of a ConnectionDTO, if the user is not authorized.
+final Function 
filterSourceDestination = connectionDiagnostics -> {
--- End diff --

Currently, the permissions of the connection dto are based on source and 
destination.


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[mcgilman]

Github user mcgilman commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r173511759
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java
 ---
@@ -4506,6 +4515,123 @@ public ComponentHistoryDTO 
getComponentHistory(final String componentId) {
 return history;
 }
 
+private ControllerServiceEntity createControllerServiceEntity(final 
String serviceId, final NiFiUser user) {
+final ControllerServiceNode serviceNode = 
controllerServiceDAO.getControllerService(serviceId);
+return createControllerServiceEntity(serviceNode, 
Collections.emptySet(), user);
+}
+
+@Override
+public ProcessorDiagnosticsEntity getProcessorDiagnostics(final String 
id) {
+final ProcessorNode processor = processorDAO.getProcessor(id);
+final ProcessorStatus processorStatus = 
controllerFacade.getProcessorStatus(id);
+
+// Generate Processor Diagnostics
+final NiFiUser user = NiFiUserUtils.getNiFiUser();
+final ProcessorDiagnosticsDTO dto = 
controllerFacade.getProcessorDiagnostics(processor, processorStatus, 
bulletinRepository, serviceId -> createControllerServiceEntity(serviceId, 
user));
+
+// Filter anything out of diagnostics that the user is not 
authorized to see.
+final List jvmDiagnosticsSnaphots = new 
ArrayList<>();
+final JVMDiagnosticsDTO jvmDiagnostics = dto.getJvmDiagnostics();
+jvmDiagnosticsSnaphots.add(jvmDiagnostics.getAggregateSnapshot());
+
+// filter controller-related information
+final boolean canReadController = 
authorizableLookup.getController().isAuthorized(authorizer, RequestAction.READ, 
user);
+if (!canReadController) {
+for (final JVMDiagnosticsSnapshotDTO snapshot : 
jvmDiagnosticsSnaphots) {
--- End diff --

Thoughts about active event/timer driven threads, cluster details like 
primary and coordinator, and uptime? Should these also be filtered out here?


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[mcgilman]

Github user mcgilman commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r173511910
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java
 ---
@@ -4506,6 +4515,123 @@ public ComponentHistoryDTO 
getComponentHistory(final String componentId) {
 return history;
 }
 
+private ControllerServiceEntity createControllerServiceEntity(final 
String serviceId, final NiFiUser user) {
+final ControllerServiceNode serviceNode = 
controllerServiceDAO.getControllerService(serviceId);
+return createControllerServiceEntity(serviceNode, 
Collections.emptySet(), user);
+}
+
+@Override
+public ProcessorDiagnosticsEntity getProcessorDiagnostics(final String 
id) {
+final ProcessorNode processor = processorDAO.getProcessor(id);
+final ProcessorStatus processorStatus = 
controllerFacade.getProcessorStatus(id);
+
+// Generate Processor Diagnostics
+final NiFiUser user = NiFiUserUtils.getNiFiUser();
+final ProcessorDiagnosticsDTO dto = 
controllerFacade.getProcessorDiagnostics(processor, processorStatus, 
bulletinRepository, serviceId -> createControllerServiceEntity(serviceId, 
user));
+
+// Filter anything out of diagnostics that the user is not 
authorized to see.
+final List jvmDiagnosticsSnaphots = new 
ArrayList<>();
+final JVMDiagnosticsDTO jvmDiagnostics = dto.getJvmDiagnostics();
+jvmDiagnosticsSnaphots.add(jvmDiagnostics.getAggregateSnapshot());
+
+// filter controller-related information
+final boolean canReadController = 
authorizableLookup.getController().isAuthorized(authorizer, RequestAction.READ, 
user);
+if (!canReadController) {
+for (final JVMDiagnosticsSnapshotDTO snapshot : 
jvmDiagnosticsSnaphots) {
+snapshot.setMaxEventDrivenThreads(null);
+snapshot.setMaxTimerDrivenThreads(null);
+snapshot.setBundlesLoaded(null);
+}
+}
+
+// filter system diagnostics information
+final boolean canReadSystem = 
authorizableLookup.getSystem().isAuthorized(authorizer, RequestAction.READ, 
user);
+if (!canReadSystem) {
+for (final JVMDiagnosticsSnapshotDTO snapshot : 
jvmDiagnosticsSnaphots) {
--- End diff --

What about open and max file descriptors?


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[mcgilman]

Github user mcgilman commented on a diff in the pull request:

https://github.com/apache/nifi/pull/2468#discussion_r173513519
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java
 ---
@@ -3116,6 +3135,312 @@ public ResourceDTO createResourceDto(final Resource 
resource) {
 return dto;
 }
 
+/**
+ * Creates a ProcessorDiagnosticsDTO from the given Processor and 
status information with some additional supporting information
+ *
+ * @param procNode the processor to create diagnostics for
+ * @param procStatus the status of given processor
+ * @param bulletinRepo the bulletin repository
+ * @param flowController flowController
+ * @param serviceEntityFactory function for creating a 
ControllerServiceEntity from a given ID
+ * @return ProcessorDiagnosticsDTO for the given Processor
+ */
+public ProcessorDiagnosticsDTO createProcessorDiagnosticsDto(final 
ProcessorNode procNode, final ProcessorStatus procStatus, final 
BulletinRepository bulletinRepo,
+final FlowController flowController, final Function serviceEntityFactory) {
+
+final ProcessorDiagnosticsDTO procDiagnostics = new 
ProcessorDiagnosticsDTO();
+
+
procDiagnostics.setClassLoaderDiagnostics(createClassLoaderDiagnosticsDto(procNode));
+
procDiagnostics.setIncomingConnections(procNode.getIncomingConnections().stream()
+.map(this::createConnectionDiagnosticsDto)
+.collect(Collectors.toSet()));
+
procDiagnostics.setOutgoingConnections(procNode.getConnections().stream()
+.map(this::createConnectionDiagnosticsDto)
+.collect(Collectors.toSet()));
+
procDiagnostics.setJvmDiagnostics(createJvmDiagnosticsDto(flowController));
+procDiagnostics.setProcessor(createProcessorDto(procNode));
+
procDiagnostics.setProcessorStatus(createProcessorStatusDto(procStatus));
+procDiagnostics.setThreadDumps(createThreadDumpDtos(procNode));
--- End diff --

Should thread dumps and classloader diagnostics also require system 
permissions?


---

[GitHub] nifi pull request #2468: NIFI-4849: Implemented REST Endpoint and associated...

[markap14]

GitHub user markap14 opened a pull request:

https://github.com/apache/nifi/pull/2468

NIFI-4849: Implemented REST Endpoint and associated backend code to g…

…enerate a Diagnostics Report for a Processor

Thank you for submitting a contribution to Apache NiFi.

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- where  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 master)?

- [ ] Is your initial contribution a single, squashed commit?

### For code changes:
- [ ] Have you ensured that the full suite of tests is executed via mvn 
-Pcontrib-check clean install at the root nifi folder?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)? 
- [ ] If applicable, have you updated the LICENSE file, including the main 
LICENSE file under nifi-assembly?
- [ ] If applicable, have you updated the NOTICE file, including the main 
NOTICE file found under nifi-assembly?
- [ ] If adding new Properties, have you added .displayName in addition to 
.name (programmatic access) for each of the new properties?

### 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 travis-ci for build 
issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/markap14/nifi NIFI-4849

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi/pull/2468.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2468


commit 9258cdd81c854d0e1a130a67f02913dc2c56dd16
Author: Mark Payne 
Date:   2018-02-02T17:16:36Z

NIFI-4849: Implemented REST Endpoint and associated backend code to 
generate a Diagnostics Report for a Processor




---