exceptionfactory commented on code in PR #7124: URL: https://github.com/apache/nifi/pull/7124#discussion_r1165799800
########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/JmxMetricsResource.java: ########## @@ -0,0 +1,129 @@ +/* + * 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; + +import io.swagger.annotations.Api; +import io.swagger.annotations.ApiOperation; +import io.swagger.annotations.ApiParam; +import io.swagger.annotations.ApiResponse; +import io.swagger.annotations.ApiResponses; +import io.swagger.annotations.Authorization; +import org.apache.nifi.authorization.Authorizer; +import org.apache.nifi.authorization.RequestAction; +import org.apache.nifi.authorization.resource.Authorizable; +import org.apache.nifi.authorization.user.NiFiUserUtils; +import org.apache.nifi.web.NiFiServiceFacade; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsCollector; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsFilter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResult; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResultConverter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsWriter; + +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import javax.ws.rs.core.StreamingOutput; +import java.util.Collection; + +/** + * RESTful endpoint for JMX metrics. + */ +@Path("/jmx-metrics") +@Api( + value = "/jmx-metrics", + description = "Endpoint for accessing the JMX metrics." +) +public class JmxMetricsResource extends ApplicationResource { + private static final String JMX_METRICS_NIFI_PROPERTY = "nifi.jmx.metrics.blacklisting.filter"; + private NiFiServiceFacade serviceFacade; + private Authorizer authorizer; + + /** + * Retrieves the JMX metrics. + * + * @return A jmxMetricsResult list. + */ + @GET + @Consumes(MediaType.WILDCARD) + @Produces(MediaType.WILDCARD) Review Comment: This should be changed from `WILDCARD` to `APPLICATION_JSON`. ```suggestion @Produces(MediaType.APPLICATION_JSON) ``` ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/metrics/jmx/JmxMetricsWriter.java: ########## @@ -0,0 +1,46 @@ +/* + * 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.metrics.jmx; + +import com.fasterxml.jackson.databind.MapperFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.json.JsonMapper; + +import java.io.IOException; +import java.io.OutputStream; +import java.util.Collection; + +public class JmxMetricsWriter { + private final static ObjectMapper MAPPER = JsonMapper.builder() + .disable(MapperFeature.CAN_OVERRIDE_ACCESS_MODIFIERS) + .build(); + private final JmxMetricsFilter metricsFilter; + + public JmxMetricsWriter(final JmxMetricsFilter metricsFilter) { + this.metricsFilter = metricsFilter; + } + + public void write(final OutputStream outputStream, final Collection<JmxMetricsResult> results) { + final Collection<JmxMetricsResult> filteredResults = metricsFilter.filter(results); + + try { + MAPPER.writerWithDefaultPrettyPrinter().writeValue(outputStream, filteredResults); + } catch (IOException e) { + throw new RuntimeException(e); Review Comment: An `UncheckedIOException` should be used, with a message. ```suggestion throw new UncheckedIOException("JSON serialization failed", e); ``` ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/metrics/jmx/JmxMetricsResultConverter.java: ########## @@ -0,0 +1,63 @@ +/* + * 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.metrics.jmx; + +import javax.management.openmbean.CompositeData; +import javax.management.openmbean.TabularData; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +public class JmxMetricsResultConverter { Review Comment: With this being a separate class, having a dedicated unit test would be helpful to illustrate expected behavior. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/JmxMetricsResource.java: ########## @@ -0,0 +1,129 @@ +/* + * 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; + +import io.swagger.annotations.Api; +import io.swagger.annotations.ApiOperation; +import io.swagger.annotations.ApiParam; +import io.swagger.annotations.ApiResponse; +import io.swagger.annotations.ApiResponses; +import io.swagger.annotations.Authorization; +import org.apache.nifi.authorization.Authorizer; +import org.apache.nifi.authorization.RequestAction; +import org.apache.nifi.authorization.resource.Authorizable; +import org.apache.nifi.authorization.user.NiFiUserUtils; +import org.apache.nifi.web.NiFiServiceFacade; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsCollector; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsFilter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResult; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResultConverter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsWriter; + +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import javax.ws.rs.core.StreamingOutput; +import java.util.Collection; + +/** + * RESTful endpoint for JMX metrics. + */ +@Path("/jmx-metrics") +@Api( + value = "/jmx-metrics", + description = "Endpoint for accessing the JMX metrics." +) +public class JmxMetricsResource extends ApplicationResource { + private static final String JMX_METRICS_NIFI_PROPERTY = "nifi.jmx.metrics.blacklisting.filter"; + private NiFiServiceFacade serviceFacade; + private Authorizer authorizer; + + /** + * Retrieves the JMX metrics. + * + * @return A jmxMetricsResult list. + */ + @GET + @Consumes(MediaType.WILDCARD) + @Produces(MediaType.WILDCARD) + @ApiOperation( + value = "Gets all allowed JMX metrics", + response = StreamingOutput.class, Review Comment: The `response` class should indicate the actual object definition, not the wrapper class. It looks like the implementation should be adjusted to return a wrapping entity class. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/metrics/jmx/JmxMetricsCollector.java: ########## @@ -0,0 +1,74 @@ +/* + * 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.metrics.jmx; + +import javax.management.AttributeNotFoundException; +import javax.management.InstanceNotFoundException; +import javax.management.IntrospectionException; +import javax.management.MBeanAttributeInfo; +import javax.management.MBeanException; +import javax.management.MBeanInfo; +import javax.management.MalformedObjectNameException; +import javax.management.ObjectInstance; +import javax.management.ObjectName; +import javax.management.ReflectionException; +import javax.management.RuntimeMBeanException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Set; + +public class JmxMetricsCollector { + private final static String PATTERN_FOR_ALL_OBJECT_NAMES = "*:*"; + private final JmxMetricsResultConverter resultConverter; + + public JmxMetricsCollector(final JmxMetricsResultConverter metricsResultConverter) { + this.resultConverter = metricsResultConverter; + } + + public Collection<JmxMetricsResult> getBeanMetrics() { + final javax.management.MBeanServer mBeanServer = java.lang.management.ManagementFactory.getPlatformMBeanServer(); Review Comment: The full class names should be replaced with import statements. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/metrics/jmx/JmxMetricsCollector.java: ########## @@ -0,0 +1,74 @@ +/* + * 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.metrics.jmx; + +import javax.management.AttributeNotFoundException; +import javax.management.InstanceNotFoundException; +import javax.management.IntrospectionException; +import javax.management.MBeanAttributeInfo; +import javax.management.MBeanException; +import javax.management.MBeanInfo; +import javax.management.MalformedObjectNameException; +import javax.management.ObjectInstance; +import javax.management.ObjectName; +import javax.management.ReflectionException; +import javax.management.RuntimeMBeanException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Set; + +public class JmxMetricsCollector { + private final static String PATTERN_FOR_ALL_OBJECT_NAMES = "*:*"; + private final JmxMetricsResultConverter resultConverter; + + public JmxMetricsCollector(final JmxMetricsResultConverter metricsResultConverter) { + this.resultConverter = metricsResultConverter; + } + + public Collection<JmxMetricsResult> getBeanMetrics() { + final javax.management.MBeanServer mBeanServer = java.lang.management.ManagementFactory.getPlatformMBeanServer(); + final Set<ObjectInstance> instances; + try { + instances = mBeanServer.queryMBeans(new ObjectName(PATTERN_FOR_ALL_OBJECT_NAMES), null); + } catch (MalformedObjectNameException e) { + throw new RuntimeException(e); Review Comment: All exceptions should include a message providing a summary reason. ########## nifi-docs/src/main/asciidoc/administration-guide.adoc: ########## @@ -4824,3 +4824,32 @@ nifi.diagnostics.on.shutdown.max.directory.size=10 MB ``` In the case of a lengthy diagnostic, NiFi may terminate before the command execution ends. In this case, the `graceful.shutdown.seconds` property should be set to a higher value in the `bootstrap.conf` configuration file. + +[[jmx_bean_metrics]] +== JMX Bean metrics + +It is possible to get JMX metrics data calling `<your_nifi_url>/nifi-api/jmx-metrics` with 'Flow' 'Read' and 'SystemDiagnostics' 'Read' permissions. + +The information available depends on the registered MBeans and usually related to performance indicators such as request/response rate, latency, request size, request counts or tool specific ones like commit rate or sync/join time for Kafka. + +Listing of certain MBeans can be prohibited by a list of regex expressions, separated by a `;` and can be configured by a NiFi application property. Leaving the property empty means all the MBeans may appear in the result list. Default value is to display all MBeans. Review Comment: Instead of supporting multiple expressions, it should be sufficient to define one regular expression. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/JmxMetricsResource.java: ########## @@ -0,0 +1,129 @@ +/* + * 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; + +import io.swagger.annotations.Api; +import io.swagger.annotations.ApiOperation; +import io.swagger.annotations.ApiParam; +import io.swagger.annotations.ApiResponse; +import io.swagger.annotations.ApiResponses; +import io.swagger.annotations.Authorization; +import org.apache.nifi.authorization.Authorizer; +import org.apache.nifi.authorization.RequestAction; +import org.apache.nifi.authorization.resource.Authorizable; +import org.apache.nifi.authorization.user.NiFiUserUtils; +import org.apache.nifi.web.NiFiServiceFacade; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsCollector; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsFilter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResult; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResultConverter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsWriter; + +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import javax.ws.rs.core.StreamingOutput; +import java.util.Collection; + +/** + * RESTful endpoint for JMX metrics. + */ +@Path("/jmx-metrics") +@Api( + value = "/jmx-metrics", + description = "Endpoint for accessing the JMX metrics." +) +public class JmxMetricsResource extends ApplicationResource { + private static final String JMX_METRICS_NIFI_PROPERTY = "nifi.jmx.metrics.blacklisting.filter"; + private NiFiServiceFacade serviceFacade; + private Authorizer authorizer; + + /** + * Retrieves the JMX metrics. + * + * @return A jmxMetricsResult list. + */ + @GET + @Consumes(MediaType.WILDCARD) + @Produces(MediaType.WILDCARD) + @ApiOperation( + value = "Gets all allowed JMX metrics", + response = StreamingOutput.class, + authorizations = { + @Authorization(value = "Read - /flow"), + @Authorization(value = "Read - /system-diagnostics") + } + ) + @ApiResponses( + value = { + @ApiResponse(code = 400, message = "NiFi was unable to complete the request because it was invalid. The request should not be retried without modification."), + @ApiResponse(code = 401, message = "Client could not be authenticated."), + @ApiResponse(code = 403, message = "Client is not authorized to make this request."), + @ApiResponse(code = 404, message = "The specified resource could not be found."), + @ApiResponse(code = 409, message = "The request was valid but NiFi was not in the appropriate state to process it. Retrying the same request later may be successful.") + } + ) + public Response getJmxMetrics( + @ApiParam( + value = "Regular Expression Pattern to be applied against the ObjectName") + @QueryParam("beanNameFilter") final String beanNameFilter + + ) { + authorizeJmxMetrics(); + + final String blackListingFilter = getProperties().getProperty(JMX_METRICS_NIFI_PROPERTY); Review Comment: The variable should be renamed, but access to the NiFi Properties should be pushed down to the service level, not in the REST Resource method. ```suggestion final String blockedFilterPattern = getProperties().getProperty(JMX_METRICS_NIFI_PROPERTY); ``` ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/JmxMetricsResource.java: ########## @@ -0,0 +1,129 @@ +/* + * 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; + +import io.swagger.annotations.Api; +import io.swagger.annotations.ApiOperation; +import io.swagger.annotations.ApiParam; +import io.swagger.annotations.ApiResponse; +import io.swagger.annotations.ApiResponses; +import io.swagger.annotations.Authorization; +import org.apache.nifi.authorization.Authorizer; +import org.apache.nifi.authorization.RequestAction; +import org.apache.nifi.authorization.resource.Authorizable; +import org.apache.nifi.authorization.user.NiFiUserUtils; +import org.apache.nifi.web.NiFiServiceFacade; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsCollector; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsFilter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResult; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResultConverter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsWriter; + +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import javax.ws.rs.core.StreamingOutput; +import java.util.Collection; + +/** + * RESTful endpoint for JMX metrics. + */ +@Path("/jmx-metrics") +@Api( + value = "/jmx-metrics", + description = "Endpoint for accessing the JMX metrics." +) +public class JmxMetricsResource extends ApplicationResource { + private static final String JMX_METRICS_NIFI_PROPERTY = "nifi.jmx.metrics.blacklisting.filter"; + private NiFiServiceFacade serviceFacade; + private Authorizer authorizer; + + /** + * Retrieves the JMX metrics. + * + * @return A jmxMetricsResult list. + */ + @GET + @Consumes(MediaType.WILDCARD) + @Produces(MediaType.WILDCARD) + @ApiOperation( + value = "Gets all allowed JMX metrics", Review Comment: Recommend adjusting the wording: ```suggestion value = "Get all available JMX metrics", ``` ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/metrics/jmx/JmxMetricsResultConverter.java: ########## @@ -0,0 +1,63 @@ +/* + * 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.metrics.jmx; + +import javax.management.openmbean.CompositeData; +import javax.management.openmbean.TabularData; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +public class JmxMetricsResultConverter { + private static final String COMPOSITE_DATA_KEY = "CompositeData%s"; + + public Object convert(final Object attributeValue) { + if (attributeValue instanceof CompositeData[]) { + final Map<String, Object> values = new HashMap<>(); + for (int i = 0; i < ((CompositeData[]) attributeValue).length; i++) { + final Map<String, Object> subValues = new HashMap<>(); + convertCompositeData(((CompositeData[]) attributeValue)[i], subValues); + values.put(String.format(COMPOSITE_DATA_KEY, i), subValues); + } + return values; + } else if (attributeValue instanceof CompositeData) { + final Map<String, Object> values = new HashMap<>(); + convertCompositeData(((CompositeData) attributeValue), values); + return values; + } else if (attributeValue instanceof TabularData) { Review Comment: What should happen in the value of other attribute value types? The current implementation does not have an `else` condition. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/metrics/jmx/JmxMetricsResultConverter.java: ########## @@ -0,0 +1,63 @@ +/* + * 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.metrics.jmx; + +import javax.management.openmbean.CompositeData; +import javax.management.openmbean.TabularData; +import java.util.HashMap; Review Comment: Recommend using `LinkedHashMap` instead of `HashMap` to ensure deterministic ordering. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/JmxMetricsResource.java: ########## @@ -0,0 +1,129 @@ +/* + * 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; + +import io.swagger.annotations.Api; +import io.swagger.annotations.ApiOperation; +import io.swagger.annotations.ApiParam; +import io.swagger.annotations.ApiResponse; +import io.swagger.annotations.ApiResponses; +import io.swagger.annotations.Authorization; +import org.apache.nifi.authorization.Authorizer; +import org.apache.nifi.authorization.RequestAction; +import org.apache.nifi.authorization.resource.Authorizable; +import org.apache.nifi.authorization.user.NiFiUserUtils; +import org.apache.nifi.web.NiFiServiceFacade; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsCollector; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsFilter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResult; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResultConverter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsWriter; + +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import javax.ws.rs.core.StreamingOutput; +import java.util.Collection; + +/** + * RESTful endpoint for JMX metrics. + */ +@Path("/jmx-metrics") +@Api( + value = "/jmx-metrics", + description = "Endpoint for accessing the JMX metrics." +) +public class JmxMetricsResource extends ApplicationResource { + private static final String JMX_METRICS_NIFI_PROPERTY = "nifi.jmx.metrics.blacklisting.filter"; + private NiFiServiceFacade serviceFacade; + private Authorizer authorizer; + + /** + * Retrieves the JMX metrics. + * + * @return A jmxMetricsResult list. + */ + @GET + @Consumes(MediaType.WILDCARD) + @Produces(MediaType.WILDCARD) + @ApiOperation( + value = "Gets all allowed JMX metrics", + response = StreamingOutput.class, + authorizations = { + @Authorization(value = "Read - /flow"), + @Authorization(value = "Read - /system-diagnostics") Review Comment: The value should be `/system` instead of `/system-diagnostics`. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/metrics/jmx/JmxMetricsWriter.java: ########## @@ -0,0 +1,46 @@ +/* + * 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.metrics.jmx; + +import com.fasterxml.jackson.databind.MapperFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.json.JsonMapper; + +import java.io.IOException; +import java.io.OutputStream; +import java.util.Collection; + +public class JmxMetricsWriter { Review Comment: It seems like this class is specific to the output format, so it could be moved into a method in the REST Resource. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/metrics/jmx/JmxMetricsResult.java: ########## @@ -0,0 +1,46 @@ +/* + * 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.metrics.jmx; + +public class JmxMetricsResult { + private String beanName; + private String attributeName; + private Object attributeValue; + + public JmxMetricsResult() { + } Review Comment: Is this no-arg constructor necessary? As it stands, the object properties cannot be set outside of the constructor. This might be necessary for reading JSON, but it should not be necessary for writing. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/metrics/jmx/JmxMetricsResultConverter.java: ########## @@ -0,0 +1,63 @@ +/* + * 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.metrics.jmx; + +import javax.management.openmbean.CompositeData; +import javax.management.openmbean.TabularData; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +public class JmxMetricsResultConverter { + private static final String COMPOSITE_DATA_KEY = "CompositeData%s"; + + public Object convert(final Object attributeValue) { + if (attributeValue instanceof CompositeData[]) { + final Map<String, Object> values = new HashMap<>(); + for (int i = 0; i < ((CompositeData[]) attributeValue).length; i++) { Review Comment: Recommend casting and declaring `CompositeData[]` on a separate line to make this easier to read. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/metrics/jmx/JmxMetricsFilter.java: ########## @@ -0,0 +1,60 @@ +/* + * 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.metrics.jmx; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.Collection; +import java.util.regex.Pattern; +import java.util.regex.PatternSyntaxException; +import java.util.stream.Collectors; + +public class JmxMetricsFilter { + private static final Logger LOGGER = LoggerFactory.getLogger(JmxMetricsFilter.class); + private final static String MATCH_NOTHING = "~^"; + private final static String MATCH_ALL = ""; + private final static String SEPARATOR = ";\\s?"; + private final static String REPLACEMENT = "|"; + private final Pattern blackListingFilter; Review Comment: Recommend renaming this variable to `blockedNameFilter`. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/JmxMetricsResource.java: ########## @@ -0,0 +1,129 @@ +/* + * 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; + +import io.swagger.annotations.Api; +import io.swagger.annotations.ApiOperation; +import io.swagger.annotations.ApiParam; +import io.swagger.annotations.ApiResponse; +import io.swagger.annotations.ApiResponses; +import io.swagger.annotations.Authorization; +import org.apache.nifi.authorization.Authorizer; +import org.apache.nifi.authorization.RequestAction; +import org.apache.nifi.authorization.resource.Authorizable; +import org.apache.nifi.authorization.user.NiFiUserUtils; +import org.apache.nifi.web.NiFiServiceFacade; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsCollector; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsFilter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResult; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResultConverter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsWriter; + +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import javax.ws.rs.core.StreamingOutput; +import java.util.Collection; + +/** + * RESTful endpoint for JMX metrics. + */ +@Path("/jmx-metrics") +@Api( + value = "/jmx-metrics", + description = "Endpoint for accessing the JMX metrics." +) +public class JmxMetricsResource extends ApplicationResource { + private static final String JMX_METRICS_NIFI_PROPERTY = "nifi.jmx.metrics.blacklisting.filter"; + private NiFiServiceFacade serviceFacade; + private Authorizer authorizer; + + /** + * Retrieves the JMX metrics. + * + * @return A jmxMetricsResult list. + */ + @GET + @Consumes(MediaType.WILDCARD) + @Produces(MediaType.WILDCARD) + @ApiOperation( + value = "Gets all allowed JMX metrics", + response = StreamingOutput.class, + authorizations = { + @Authorization(value = "Read - /flow"), + @Authorization(value = "Read - /system-diagnostics") + } + ) + @ApiResponses( + value = { + @ApiResponse(code = 400, message = "NiFi was unable to complete the request because it was invalid. The request should not be retried without modification."), + @ApiResponse(code = 401, message = "Client could not be authenticated."), + @ApiResponse(code = 403, message = "Client is not authorized to make this request."), + @ApiResponse(code = 404, message = "The specified resource could not be found."), + @ApiResponse(code = 409, message = "The request was valid but NiFi was not in the appropriate state to process it. Retrying the same request later may be successful.") + } + ) + public Response getJmxMetrics( + @ApiParam( + value = "Regular Expression Pattern to be applied against the ObjectName") + @QueryParam("beanNameFilter") final String beanNameFilter + + ) { + authorizeJmxMetrics(); + + final String blackListingFilter = getProperties().getProperty(JMX_METRICS_NIFI_PROPERTY); + final JmxMetricsResultConverter metricsResultConverter = new JmxMetricsResultConverter(); + final JmxMetricsCollector jmxMetricsCollector = new JmxMetricsCollector(metricsResultConverter); + + final Collection<JmxMetricsResult> results = jmxMetricsCollector.getBeanMetrics(); Review Comment: Although the implementation is fairly straightforward, in general there should be some decoupling between REST Resource methods and implementations. Recommend introducing a `JmxMetricsService` interface to encapsulate the collection and filtering, and provide the implementation through property wiring. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/metrics/jmx/JmxMetricsCollector.java: ########## @@ -0,0 +1,74 @@ +/* + * 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.metrics.jmx; + +import javax.management.AttributeNotFoundException; +import javax.management.InstanceNotFoundException; +import javax.management.IntrospectionException; +import javax.management.MBeanAttributeInfo; +import javax.management.MBeanException; +import javax.management.MBeanInfo; +import javax.management.MalformedObjectNameException; +import javax.management.ObjectInstance; +import javax.management.ObjectName; +import javax.management.ReflectionException; +import javax.management.RuntimeMBeanException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Set; + +public class JmxMetricsCollector { + private final static String PATTERN_FOR_ALL_OBJECT_NAMES = "*:*"; + private final JmxMetricsResultConverter resultConverter; + + public JmxMetricsCollector(final JmxMetricsResultConverter metricsResultConverter) { + this.resultConverter = metricsResultConverter; + } + + public Collection<JmxMetricsResult> getBeanMetrics() { + final javax.management.MBeanServer mBeanServer = java.lang.management.ManagementFactory.getPlatformMBeanServer(); + final Set<ObjectInstance> instances; + try { + instances = mBeanServer.queryMBeans(new ObjectName(PATTERN_FOR_ALL_OBJECT_NAMES), null); + } catch (MalformedObjectNameException e) { + throw new RuntimeException(e); + } + + final Collection<JmxMetricsResult> results = new ArrayList<>(); + for (final ObjectInstance instance : instances) { + final MBeanInfo info; + try { + info = mBeanServer.getMBeanInfo(instance.getObjectName()); + } catch (InstanceNotFoundException | ReflectionException | IntrospectionException e) { + continue; + } + + for (MBeanAttributeInfo attribute : info.getAttributes()) { + try { + final String beanName = instance.getObjectName().getCanonicalName(); + final String attributeName = attribute.getName(); + final Object attributeValue = resultConverter.convert(mBeanServer.getAttribute(instance.getObjectName(), attribute.getName())); + + results.add(new JmxMetricsResult(beanName, attributeName, attributeValue)); + } catch (MBeanException | RuntimeMBeanException | ReflectionException | InstanceNotFoundException | AttributeNotFoundException e) { + //Empty or invalid attributes should not stop the loop. Review Comment: Recommend introducing a trace log at this point for potential troubleshooting, instead of hiding the exception. ########## nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/JmxMetricsResource.java: ########## @@ -0,0 +1,129 @@ +/* + * 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; + +import io.swagger.annotations.Api; +import io.swagger.annotations.ApiOperation; +import io.swagger.annotations.ApiParam; +import io.swagger.annotations.ApiResponse; +import io.swagger.annotations.ApiResponses; +import io.swagger.annotations.Authorization; +import org.apache.nifi.authorization.Authorizer; +import org.apache.nifi.authorization.RequestAction; +import org.apache.nifi.authorization.resource.Authorizable; +import org.apache.nifi.authorization.user.NiFiUserUtils; +import org.apache.nifi.web.NiFiServiceFacade; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsCollector; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsFilter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResult; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsResultConverter; +import org.apache.nifi.web.api.metrics.jmx.JmxMetricsWriter; + +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.QueryParam; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import javax.ws.rs.core.StreamingOutput; +import java.util.Collection; + +/** + * RESTful endpoint for JMX metrics. + */ +@Path("/jmx-metrics") +@Api( + value = "/jmx-metrics", + description = "Endpoint for accessing the JMX metrics." +) +public class JmxMetricsResource extends ApplicationResource { + private static final String JMX_METRICS_NIFI_PROPERTY = "nifi.jmx.metrics.blacklisting.filter"; Review Comment: As mentioned elsewhere, this property should be injected at the Service level, not at the Resource level. -- 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: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
