From 4bfe51bb342ac3edcb8c3a6d43aa621167427436 Mon Sep 17 00:00:00 2001 From: Vasilii Alferov Date: Sat, 9 Aug 2025 16:52:59 +0100 Subject: [PATCH 1/3] ZOOKEEPER-4741: Modernize PrometheusMetricsProvider with Jetty and new client In production environments under heavy load, the existing PrometheusMetricsProvider can introduce high latency. This is largely due to its use of the outdated `io.prometheus.client` (simpleclient 0.x) library and its basic, built-in HTTPServer, which has limitations in server configuration and threading. This commit modernizes the entire component to address these performance and maintainability issues by: 1. **Upgrading to the `io.prometheus.metrics` (client_java 1.x) library.** This aligns the provider with the current standard for Prometheus instrumentation in Java and ensures future compatibility. 2. **Replacing the legacy server and threading model with an embedded Jetty server.** The previous implementation used a custom thread pool for a metrics processing task queue, which is now obsolete in the new client library. This has been replaced with a robust Jetty server, which uses its own configurable thread pool to handle exporter servlet requests directly. This change improves stability and simplifies the threading model, resolving the latency issues under load. These changes make the PrometheusMetricsProvider more stable, performant, maintainable, and easier to configure for production use. --- .../zookeeper-prometheus-metrics/pom.xml | 49 +- .../prometheus/PrometheusMetricsProvider.java | 843 +++++++----------- .../prometheus/PrometheusRegistryDumper.java | 160 ++++ .../metrics/prometheus/ExportJvmInfoTest.java | 5 +- .../PrometheusHttpsMetricsProviderTest.java | 98 +- .../PrometheusMetricsProviderConfigTest.java | 5 +- .../PrometheusMetricsProviderTest.java | 338 +++---- .../prometheus/PrometheusMetricsTestBase.java | 31 +- 8 files changed, 772 insertions(+), 757 deletions(-) create mode 100644 zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusRegistryDumper.java diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/pom.xml b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/pom.xml index 4b13fa6adfb..ddc4fe22e27 100644 --- a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/pom.xml +++ b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/pom.xml @@ -32,7 +32,8 @@ ZooKeeper Prometheus.io Metrics Provider implementation - 0.9.0 + 1.3.10 + 9.4.57.v20241219 @@ -46,28 +47,56 @@ io.prometheus - simpleclient + prometheus-metrics-core ${prometheus.version} io.prometheus - simpleclient_hotspot + prometheus-metrics-instrumentation-jvm ${prometheus.version} io.prometheus - simpleclient_servlet + prometheus-metrics-exporter-servlet-javax ${prometheus.version} - org.eclipse.jetty - jetty-server - provided + io.prometheus + prometheus-metrics-exporter-httpserver + ${prometheus.version} + + + io.prometheus + prometheus-metrics-exposition-formats + ${prometheus.version} + + + io.prometheus + prometheus-metrics-config + ${prometheus.version} + + + org.eclipse.jetty + jetty-server + ${jetty.version} + + + org.eclipse.jetty + jetty-servlet + ${jetty.version} + + + + org.eclipse.jetty + jetty-util + ${jetty.version} + - org.eclipse.jetty - jetty-servlet - provided + javax.servlet + javax.servlet-api + 3.1.0 + provided ch.qos.logback diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java index 76739787c8b..5202822a03a 100644 --- a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java +++ b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java @@ -7,7 +7,7 @@ * "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 + * 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, @@ -18,25 +18,16 @@ package org.apache.zookeeper.metrics.prometheus; -import io.prometheus.client.Collector; -import io.prometheus.client.CollectorRegistry; -import io.prometheus.client.exporter.MetricsServlet; -import io.prometheus.client.hotspot.DefaultExports; +import io.prometheus.metrics.core.metrics.GaugeWithCallback; +import io.prometheus.metrics.exporter.servlet.javax.PrometheusMetricsServlet; +import io.prometheus.metrics.instrumentation.jvm.JvmMetrics; +import io.prometheus.metrics.model.registry.PrometheusRegistry; import java.io.IOException; -import java.util.Enumeration; +import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.Properties; -import java.util.concurrent.BlockingQueue; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.RejectedExecutionHandler; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiConsumer; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -46,699 +37,495 @@ import org.apache.zookeeper.metrics.Gauge; import org.apache.zookeeper.metrics.GaugeSet; import org.apache.zookeeper.metrics.MetricsContext; +import org.apache.zookeeper.metrics.MetricsContext.DetailLevel; import org.apache.zookeeper.metrics.MetricsProvider; import org.apache.zookeeper.metrics.MetricsProviderLifeCycleException; import org.apache.zookeeper.metrics.Summary; import org.apache.zookeeper.metrics.SummarySet; -import org.apache.zookeeper.server.RateLogger; -import org.eclipse.jetty.security.ConstraintMapping; -import org.eclipse.jetty.security.ConstraintSecurityHandler; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; -import org.eclipse.jetty.util.security.Constraint; import org.eclipse.jetty.util.ssl.KeyStoreScanner; import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * A Metrics Provider implementation based on https://prometheus.io. - * + * This implementation uses prometheus-metrics-core interfaces and exposes metrics via an embedded Jetty server * @since 3.6.0 */ public class PrometheusMetricsProvider implements MetricsProvider { private static final Logger LOG = LoggerFactory.getLogger(PrometheusMetricsProvider.class); private static final String LABEL = "key"; - private static final String[] LABELS = {LABEL}; - - /** - * Number of worker threads for reporting Prometheus summary metrics. - * Default value is 1. - * If the number is less than 1, the main thread will be used. - */ - static final String NUM_WORKER_THREADS = "numWorkerThreads"; - - /** - * The max queue size for Prometheus summary metrics reporting task. - * Default value is 10000. - */ - static final String MAX_QUEUE_SIZE = "maxQueueSize"; - - /** - * The timeout in ms for Prometheus worker threads shutdown. - * Default value is 1000ms. - */ - static final String WORKER_SHUTDOWN_TIMEOUT_MS = "workerShutdownTimeoutMs"; - /** - * We are using the 'defaultRegistry'. - *

- * When you are running ZooKeeper (server or client) together with other - * libraries every metrics will be expected as a single view. - *

- */ - private final CollectorRegistry collectorRegistry = CollectorRegistry.defaultRegistry; - private final RateLogger rateLogger = new RateLogger(LOG, 60 * 1000); - private String host = "0.0.0.0"; + private final PrometheusRegistry registry = PrometheusRegistry.defaultRegistry; private int httpPort = -1; private int httpsPort = -1; private boolean exportJvmInfo = true; - private Server server; - private final MetricsServletImpl servlet = new MetricsServletImpl(); private final Context rootContext = new Context(); - private int numWorkerThreads = 1; - private int maxQueueSize = 10000; - private long workerShutdownTimeoutMs = 1000; - private Optional executorOptional = Optional.empty(); + private PrometheusRegistryDumper dumper; + private CustomPrometheusMetricsServlet servlet; - // Constants for SSL configuration - public static final int SCAN_INTERVAL = 60 * 10; // 10 minutes + private Server server; + private int numWorkerThreads; + private String host; + + // SSL Configuration fields + private String keyStorePath; + private String keyStorePassword; + private String keyStoreType; + private String trustStorePath; + private String trustStorePassword; + private String trustStoreType; + private boolean needClientAuth = true; // Secure default + private boolean wantClientAuth = true; // Secure default + + // Constants for configuration + public static final String NUM_WORKER_THREADS = "numWorkerThreads"; public static final String SSL_KEYSTORE_LOCATION = "ssl.keyStore.location"; public static final String SSL_KEYSTORE_PASSWORD = "ssl.keyStore.password"; public static final String SSL_KEYSTORE_TYPE = "ssl.keyStore.type"; public static final String SSL_TRUSTSTORE_LOCATION = "ssl.trustStore.location"; public static final String SSL_TRUSTSTORE_PASSWORD = "ssl.trustStore.password"; public static final String SSL_TRUSTSTORE_TYPE = "ssl.trustStore.type"; - public static final String SSL_X509_CN = "ssl.x509.cn"; - public static final String SSL_X509_REGEX_CN = "ssl.x509.cn.regex"; public static final String SSL_NEED_CLIENT_AUTH = "ssl.need.client.auth"; public static final String SSL_WANT_CLIENT_AUTH = "ssl.want.client.auth"; + public static final int SCAN_INTERVAL = 60 * 10; // 10 minutes - private String keyStorePath; - private String keyStorePassword; - private String keyStoreType; - private String trustStorePath; - private String trustStorePassword; - private String trustStoreType; - private boolean needClientAuth = true; - private boolean wantClientAuth = true; + /** + * Custom servlet to disable the TRACE method for security reasons. + */ + private static class CustomPrometheusMetricsServlet extends PrometheusMetricsServlet { + public CustomPrometheusMetricsServlet(PrometheusRegistry registry) { + super(registry); + } + + @Override + protected void doTrace(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + resp.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED); + } + } @Override public void configure(Properties configuration) throws MetricsProviderLifeCycleException { - LOG.info("Initializing metrics, configuration: {}", configuration); + LOG.info("Initializing Prometheus metrics with Jetty, configuration: {}", configuration); + this.host = configuration.getProperty("httpHost", "0.0.0.0"); - if (configuration.containsKey("httpsPort")) { - this.httpsPort = Integer.parseInt(configuration.getProperty("httpsPort")); + this.httpPort = Integer.parseInt(configuration.getProperty("httpPort", "-1")); + this.httpsPort = Integer.parseInt(configuration.getProperty("httpsPort", "-1")); + this.exportJvmInfo = Boolean.parseBoolean(configuration.getProperty("exportJvmInfo", "true")); + this.numWorkerThreads = Integer.parseInt(configuration.getProperty(NUM_WORKER_THREADS, "10")); + + // If httpsPort is specified, parse all SSL properties + if (this.httpsPort != -1) { this.keyStorePath = configuration.getProperty(SSL_KEYSTORE_LOCATION); this.keyStorePassword = configuration.getProperty(SSL_KEYSTORE_PASSWORD); - this.keyStoreType = configuration.getProperty(SSL_KEYSTORE_TYPE); + this.keyStoreType = configuration.getProperty(SSL_KEYSTORE_TYPE, "PKCS12"); this.trustStorePath = configuration.getProperty(SSL_TRUSTSTORE_LOCATION); this.trustStorePassword = configuration.getProperty(SSL_TRUSTSTORE_PASSWORD); - this.trustStoreType = configuration.getProperty(SSL_TRUSTSTORE_TYPE); + this.trustStoreType = configuration.getProperty(SSL_TRUSTSTORE_TYPE, "PKCS12"); this.needClientAuth = Boolean.parseBoolean(configuration.getProperty(SSL_NEED_CLIENT_AUTH, "true")); this.wantClientAuth = Boolean.parseBoolean(configuration.getProperty(SSL_WANT_CLIENT_AUTH, "true")); - //check if httpPort is also configured - this.httpPort = Integer.parseInt(configuration.getProperty("httpPort", "-1")); - } else { - // Use the default HTTP port (7000) or the configured port if HTTPS is not set. - this.httpPort = Integer.parseInt(configuration.getProperty("httpPort", "7000")); } - this.exportJvmInfo = Boolean.parseBoolean(configuration.getProperty("exportJvmInfo", "true")); - this.numWorkerThreads = Integer.parseInt( - configuration.getProperty(NUM_WORKER_THREADS, "1")); - this.maxQueueSize = Integer.parseInt( - configuration.getProperty(MAX_QUEUE_SIZE, "10000")); - this.workerShutdownTimeoutMs = Long.parseLong( - configuration.getProperty(WORKER_SHUTDOWN_TIMEOUT_MS, "1000")); + + // Validate that at least one port is configured. + if (httpPort == -1 && httpsPort == -1) { + throw new MetricsProviderLifeCycleException( + "Either httpPort or httpsPort must be configured for Prometheus exporter."); + } + + this.dumper = new PrometheusRegistryDumper(this.registry); + this.servlet = new CustomPrometheusMetricsServlet(this.registry); } @Override public void start() throws MetricsProviderLifeCycleException { - this.executorOptional = createExecutor(); + // Register JVM metrics if enabled + if (exportJvmInfo) { + JvmMetrics.builder().register(this.registry); + } try { - LOG.info("Starting /metrics endpoint at HTTP port: {}, HTTPS port: {}, exportJvmInfo: {}", - httpPort >= 0 ? httpPort : "disabled", - httpsPort >= 0 ? httpsPort : "disabled", - exportJvmInfo); - if (exportJvmInfo) { - DefaultExports.initialize(); - } - server = new Server(); - ServerConnector httpConnector = null; - ServerConnector httpsConnector = null; - if (httpPort >= 0) { - httpConnector = new ServerConnector(server); - httpConnector.setHost(host); - httpConnector.setPort(httpPort); + LOG.info("Starting Prometheus Jetty server..."); + + // QueuedThreadPool needs a minimum of 4 threads for stable operation + QueuedThreadPool threadPool = new QueuedThreadPool(Math.max(this.numWorkerThreads + 3, 4)); + threadPool.setReservedThreads(0); + threadPool.setName("prometheus-jetty-server"); + + this.server = new Server(threadPool); + + // Define number of acceptors and selectors for connectors + int acceptors = 1; + int selectors = 1; + + // Configure HTTP connector if enabled + if (this.httpPort != -1) { + ServerConnector httpConnector = new ServerConnector(server, acceptors, selectors); + httpConnector.setPort(this.httpPort); + httpConnector.setHost(this.host); server.addConnector(httpConnector); } - if (httpsPort >= 0) { - SslContextFactory sslServerContextFactory = new SslContextFactory.Server(); - configureSslContextFactory(sslServerContextFactory); - KeyStoreScanner keystoreScanner = new KeyStoreScanner(sslServerContextFactory); + + // Configure HTTPS connector if enabled + if (this.httpsPort != -1) { + SslContextFactory.Server sslContextFactory = createSslContextFactory(); + KeyStoreScanner keystoreScanner = new KeyStoreScanner(sslContextFactory); keystoreScanner.setScanInterval(SCAN_INTERVAL); server.addBean(keystoreScanner); - httpsConnector = new ServerConnector(server, sslServerContextFactory); - httpsConnector.setHost(host); - httpsConnector.setPort(httpsPort); - server.addConnector(httpsConnector); + server.addConnector(createSslConnector(server, acceptors, selectors, sslContextFactory)); } + + // Set up the servlet context handler ServletContextHandler context = new ServletContextHandler(); context.setContextPath("/"); - constrainTraceMethod(context); server.setHandler(context); context.addServlet(new ServletHolder(servlet), "/metrics"); + server.start(); - if (httpPort == 0) { - LOG.info("Bound /metrics endpoint to HTTP port: {}", httpConnector.getLocalPort()); - } - if (httpsPort == 0) { - LOG.info("Bound /metrics endpoint to HTTPS port: {}", httpsConnector.getLocalPort()); - } - } catch (Exception err) { - LOG.error("Cannot start /metrics server", err); - if (server != null) { - try { - server.stop(); - } catch (Exception suppressed) { - err.addSuppressed(suppressed); - } finally { - server = null; - } - } - throw new MetricsProviderLifeCycleException(err); + + LOG.info("Prometheus metrics provider with Jetty started. HTTP port: {}, HTTPS port: {}", + httpPort != -1 ? httpPort : "disabled", httpsPort != -1 ? httpsPort : "disabled"); + + } catch (Exception e) { + LOG.error("Failed to start Prometheus Jetty server", e); + // Ensure server is stopped on startup failure + stop(); + throw new MetricsProviderLifeCycleException("Failed to start Prometheus Jetty server", e); } } - @SuppressWarnings("deprecation") - private void configureSslContextFactory(SslContextFactory sslServerContextFactory) { - if (keyStorePath != null) { - sslServerContextFactory.setKeyStorePath(keyStorePath); - } else { - LOG.error("KeyStore configuration is incomplete keyStorePath: {}", keyStorePath); - throw new IllegalStateException("KeyStore configuration is incomplete keyStorePath: " + keyStorePath); - } - if (keyStorePassword != null) { - sslServerContextFactory.setKeyStorePassword(keyStorePassword); - } else { - LOG.error("keyStorePassword configuration is incomplete "); - throw new IllegalStateException("keyStorePassword configuration is incomplete "); - } - if (keyStoreType != null) { - sslServerContextFactory.setKeyStoreType(keyStoreType); + /** + * Creates and configures the SslContextFactory for the server. + * + * @return A configured SslContextFactory.Server instance. + */ + private SslContextFactory.Server createSslContextFactory() { + SslContextFactory.Server sslContextFactory = new SslContextFactory.Server(); + + // Validate and set KeyStore properties + if (this.keyStorePath == null || this.keyStorePath.isEmpty()) { + throw new IllegalArgumentException("SSL/TLS is enabled, but '" + SSL_KEYSTORE_LOCATION + "' is not set."); } - if (trustStorePath != null) { - sslServerContextFactory.setTrustStorePath(trustStorePath); - } else { - LOG.error("TrustStore configuration is incomplete trustStorePath: {}", trustStorePath); - throw new IllegalStateException("TrustStore configuration is incomplete trustStorePath: " + trustStorePath); + sslContextFactory.setKeyStorePath(this.keyStorePath); + sslContextFactory.setKeyStorePassword(this.keyStorePassword); + if (this.keyStoreType != null) { + sslContextFactory.setKeyStoreType(this.keyStoreType); } - if (trustStorePassword != null) { - sslServerContextFactory.setTrustStorePassword(trustStorePassword); - } else { - LOG.error("trustStorePassword configuration is incomplete"); - throw new IllegalStateException("trustStorePassword configuration is incomplete"); + + // Validate and set TrustStore properties (often needed for client auth) + if (this.needClientAuth && (this.trustStorePath == null || this.trustStorePath.isEmpty())) { + throw new IllegalArgumentException( + "'" + SSL_NEED_CLIENT_AUTH + "' is true, but '" + SSL_TRUSTSTORE_LOCATION + "' is not set."); } - if (trustStoreType != null) { - sslServerContextFactory.setTrustStoreType(trustStoreType); + if (this.trustStorePath != null) { + sslContextFactory.setTrustStorePath(this.trustStorePath); + sslContextFactory.setTrustStorePassword(this.trustStorePassword); + if (this.trustStoreType != null) { + sslContextFactory.setTrustStoreType(this.trustStoreType); + } } - sslServerContextFactory - .setNeedClientAuth(needClientAuth); - sslServerContextFactory - .setWantClientAuth(wantClientAuth); - } - // for tests - MetricsServletImpl getServlet() { - return servlet; + sslContextFactory.setNeedClientAuth(this.needClientAuth); + sslContextFactory.setWantClientAuth(this.wantClientAuth); + + return sslContextFactory; } - @Override - public MetricsContext getRootContext() { - return rootContext; + /** + * Creates and configures an SSL/TLS connector for the Jetty server. + * + * @param server + * The server instance. + * @param acceptors + * The number of acceptor threads. + * @param selectors + * The number of selector threads. + * @param sslContextFactory + * The pre-configured SslContextFactory. + * + * @return A configured ServerConnector for HTTPS. + */ + private ServerConnector createSslConnector(Server server, int acceptors, int selectors, + SslContextFactory.Server sslContextFactory) { + ServerConnector sslConnector = new ServerConnector(server, acceptors, selectors, sslContextFactory); + sslConnector.setPort(this.httpsPort); + sslConnector.setHost(this.host); + return sslConnector; } @Override public void stop() { - shutdownExecutor(); if (server != null) { try { + LOG.info("Stopping Prometheus Jetty server."); server.stop(); } catch (Exception err) { - LOG.error("Cannot safely stop Jetty server", err); + LOG.error("Cannot safely stop Prometheus Jetty server", err); } finally { server = null; } } + registry.clear(); } /** - * Dump all values to the 4lw interface and to the Admin server. - *

- * This method is not expected to be used to serve metrics to Prometheus. We - * are using the MetricsServlet provided by Prometheus for that, leaving the - * real representation to the Prometheus Java client. - *

- * - * @param sink the receiver of data (4lw interface, Admin server or tests) + * Returns a Prometheus servlet for integration with existing web applications. This is primarily used for testing + * purposes. */ - @Override - public void dump(BiConsumer sink) { - sampleGauges(); - Enumeration samplesFamilies = collectorRegistry.metricFamilySamples(); - while (samplesFamilies.hasMoreElements()) { - Collector.MetricFamilySamples samples = samplesFamilies.nextElement(); - samples.samples.forEach(sample -> { - String key = buildKeyForDump(sample); - sink.accept(key, sample.value); - }); - } + public PrometheusMetricsServlet getServlet() { + return this.servlet; } - private static String buildKeyForDump(Collector.MetricFamilySamples.Sample sample) { - StringBuilder keyBuilder = new StringBuilder(); - keyBuilder.append(sample.name); - if (sample.labelNames.size() > 0) { - keyBuilder.append('{'); - for (int i = 0; i < sample.labelNames.size(); ++i) { - if (i > 0) { - keyBuilder.append(','); - } - keyBuilder.append(sample.labelNames.get(i)); - keyBuilder.append("=\""); - keyBuilder.append(sample.labelValues.get(i)); - keyBuilder.append('"'); - } - keyBuilder.append('}'); - } - return keyBuilder.toString(); + @Override + public MetricsContext getRootContext() { + return rootContext; } - /** - * Update Gauges. In ZooKeeper Metrics API Gauges are callbacks served by - * internal components and the value is not held by Prometheus structures. - */ - private void sampleGauges() { - rootContext.gauges.values() - .forEach(PrometheusGaugeWrapper::sample); - - rootContext.gaugeSets.values() - .forEach(PrometheusLabelledGaugeWrapper::sample); + @Override + public void dump(BiConsumer sink) { + dumper.dump().forEach(sink); } @Override public void resetAllValues() { - // not supported on Prometheus + // The new prometheus client does not support resetting metric values. + LOG.debug("resetAllValues is a no-op for PrometheusMetricsProvider"); } /** - * Add constraint to a given context to disallow TRACE method. - * @param ctxHandler the context to modify + * Inner class implementing the MetricsContext interface. It handles the creation and registration of different + * metric types. */ - private void constrainTraceMethod(ServletContextHandler ctxHandler) { - Constraint c = new Constraint(); - c.setAuthenticate(true); - - ConstraintMapping cmt = new ConstraintMapping(); - cmt.setConstraint(c); - cmt.setMethod("TRACE"); - cmt.setPathSpec("/*"); - - ConstraintSecurityHandler securityHandler = new ConstraintSecurityHandler(); - securityHandler.setConstraintMappings(new ConstraintMapping[] {cmt}); - - ctxHandler.setSecurityHandler(securityHandler); - } - private class Context implements MetricsContext { - private final ConcurrentMap gauges = new ConcurrentHashMap<>(); - private final ConcurrentMap gaugeSets = new ConcurrentHashMap<>(); - private final ConcurrentMap counters = new ConcurrentHashMap<>(); - private final ConcurrentMap counterSets = new ConcurrentHashMap<>(); - private final ConcurrentMap basicSummaries = new ConcurrentHashMap<>(); - private final ConcurrentMap summaries = new ConcurrentHashMap<>(); - private final ConcurrentMap basicSummarySets = new ConcurrentHashMap<>(); - private final ConcurrentMap summarySets = new ConcurrentHashMap<>(); + private final ConcurrentMap counters = + new ConcurrentHashMap<>(); + private final ConcurrentMap counterSets = + new ConcurrentHashMap<>(); + private final ConcurrentMap registeredGauges = + new ConcurrentHashMap<>(); + private final ConcurrentMap basicSummaries = + new ConcurrentHashMap<>(); + private final ConcurrentMap advancedSummaries = + new ConcurrentHashMap<>(); + private final ConcurrentMap basicSummarySets = + new ConcurrentHashMap<>(); + private final ConcurrentMap advancedSummarySets = + new ConcurrentHashMap<>(); @Override public MetricsContext getContext(String name) { - // no hierarchy yet + // This provider uses a flat namespace, so sub-contexts are not needed. return this; } @Override public Counter getCounter(String name) { - return counters.computeIfAbsent(name, PrometheusCounter::new); + return counters.computeIfAbsent(name, key -> { + io.prometheus.metrics.core.metrics.Counter prometheusCounter = + io.prometheus.metrics.core.metrics.Counter + .builder().name(key).help(key + " counter").register(registry); + return new PrometheusCounterWrapper(prometheusCounter); + }); } @Override - public CounterSet getCounterSet(final String name) { - Objects.requireNonNull(name, "Cannot register a CounterSet with null name"); - return counterSets.computeIfAbsent(name, PrometheusLabelledCounter::new); + public CounterSet getCounterSet(String name) { + return counterSets.computeIfAbsent(name, key -> { + Objects.requireNonNull(name, "Cannot register a CounterSet with null name"); + io.prometheus.metrics.core.metrics.Counter prometheusCounter = + io.prometheus.metrics.core.metrics.Counter + .builder().name(key).help(key + " counter set").labelNames(LABEL).register(registry); + return new PrometheusLabelledCounterWrapper(prometheusCounter); + }); } - /** - * Gauges may go up and down, in ZooKeeper they are a way to export - * internal values with a callback. - * - * @param name the name of the gauge - * @param gauge the callback - */ @Override - public void registerGauge(String name, Gauge gauge) { - Objects.requireNonNull(name); - gauges.compute(name, (id, prev) -> - new PrometheusGaugeWrapper(id, gauge, prev != null ? prev.inner : null)); + public void registerGaugeSet(final String name, final GaugeSet gaugeSet) { + Objects.requireNonNull(name, "Cannot register a GaugeSet with null name"); + Objects.requireNonNull(gaugeSet, "Cannot register a null GaugeSet for " + name); + + GaugeWithCallback oldGauge = registeredGauges.get(name); + if (oldGauge != null) { + registry.unregister(oldGauge); + } + + GaugeWithCallback newGauge = GaugeWithCallback.builder().name(name).help(name).labelNames(LABEL) + .callback(callback -> { + Map values = gaugeSet.values(); + if (values != null) { + for (Map.Entry value : values.entrySet()) { + if (value.getKey() == null) { + throw new IllegalArgumentException("GaugeSet key cannot be null."); + } + callback.call(value.getValue().doubleValue(), value.getKey()); + } + } + }).register(registry); + registeredGauges.put(name, newGauge); } @Override - public void unregisterGauge(String name) { - PrometheusGaugeWrapper existing = gauges.remove(name); - if (existing != null) { - existing.unregister(); + public void registerGauge(String name, Gauge gauge) { + if (name == null) { + throw new IllegalArgumentException("Gauge name cannot be null."); + } + if (gauge == null) { + throw new IllegalArgumentException("Cannot register a null Gauge for " + name); } + + GaugeWithCallback oldGauge = registeredGauges.get(name); + if (oldGauge != null) { + registry.unregister(oldGauge); + } + + GaugeWithCallback newGauge = GaugeWithCallback.builder().name(name).help(name).callback(callback -> { + Number value = gauge.get(); + if (value == null) { + throw new IllegalArgumentException("Gauge " + name + " cannot return a null value."); + } + callback.call(value.doubleValue()); + }).register(registry); + registeredGauges.put(name, newGauge); } @Override - public void registerGaugeSet(final String name, final GaugeSet gaugeSet) { - Objects.requireNonNull(name, "Cannot register a GaugeSet with null name"); - Objects.requireNonNull(gaugeSet, "Cannot register a null GaugeSet for " + name); - - gaugeSets.compute(name, (id, prev) -> - new PrometheusLabelledGaugeWrapper(name, gaugeSet, prev != null ? prev.inner : null)); + public void unregisterGauge(String name) { + GaugeWithCallback gauge = registeredGauges.remove(name); + if (gauge != null) { + registry.unregister(gauge); + } } @Override public void unregisterGaugeSet(final String name) { Objects.requireNonNull(name, "Cannot unregister GaugeSet with null name"); + unregisterGauge(name); + } + + private io.prometheus.metrics.core.metrics.Summary createPrometheusSummary(String name, DetailLevel detailLevel, + String... labelNames) { + io.prometheus.metrics.core.metrics.Summary.Builder builder = io.prometheus.metrics.core.metrics.Summary + .builder().name(name).help(name + " summary").quantile(0.5, 0.05); // Median - final PrometheusLabelledGaugeWrapper existing = gaugeSets.remove(name); - if (existing != null) { - existing.unregister(); + if (detailLevel == DetailLevel.ADVANCED) { + builder.quantile(0.95, 0.05) // 95th percentile + .quantile(0.99, 0.05); // 99th percentile } + + if (labelNames.length > 0) { + builder.labelNames(labelNames); + } + return builder.register(registry); } @Override public Summary getSummary(String name, DetailLevel detailLevel) { - if (detailLevel == DetailLevel.BASIC) { - return basicSummaries.computeIfAbsent(name, (n) -> { - if (summaries.containsKey(n)) { - throw new IllegalArgumentException("Already registered a non basic summary as " + n); - } - return new PrometheusSummary(name, detailLevel); - }); - } else { - return summaries.computeIfAbsent(name, (n) -> { - if (basicSummaries.containsKey(n)) { - throw new IllegalArgumentException("Already registered a basic summary as " + n); - } - return new PrometheusSummary(name, detailLevel); - }); - } + ConcurrentMap map = detailLevel == DetailLevel.BASIC ? basicSummaries + : advancedSummaries; + return map.computeIfAbsent(name, key -> { + if ((detailLevel == DetailLevel.BASIC && advancedSummaries.containsKey(key)) + || (detailLevel == DetailLevel.ADVANCED && basicSummaries.containsKey(key))) { + throw new IllegalArgumentException( + "Already registered a summary as " + key + " with a different detail level"); + } + io.prometheus.metrics.core.metrics.Summary prometheusSummary = createPrometheusSummary(key, + detailLevel); + return new PrometheusSummaryWrapper(prometheusSummary); + }); } @Override public SummarySet getSummarySet(String name, DetailLevel detailLevel) { - if (detailLevel == DetailLevel.BASIC) { - return basicSummarySets.computeIfAbsent(name, (n) -> { - if (summarySets.containsKey(n)) { - throw new IllegalArgumentException("Already registered a non basic summary set as " + n); - } - return new PrometheusLabelledSummary(name, detailLevel); - }); - } else { - return summarySets.computeIfAbsent(name, (n) -> { - if (basicSummarySets.containsKey(n)) { - throw new IllegalArgumentException("Already registered a basic summary set as " + n); - } - return new PrometheusLabelledSummary(name, detailLevel); - }); - } + ConcurrentMap map = detailLevel == DetailLevel.BASIC + ? basicSummarySets : advancedSummarySets; + return map.computeIfAbsent(name, key -> { + if ((detailLevel == DetailLevel.BASIC && advancedSummarySets.containsKey(key)) + || (detailLevel == DetailLevel.ADVANCED && basicSummarySets.containsKey(key))) { + throw new IllegalArgumentException( + "Already registered a summary set as " + key + " with a different detail level"); + } + io.prometheus.metrics.core.metrics.Summary prometheusSummary = createPrometheusSummary(key, detailLevel, + LABEL); + return new PrometheusLabelledSummaryWrapper(prometheusSummary); + }); } - } - private class PrometheusCounter implements Counter { + // --- Wrapper classes to adapt Prometheus metrics to ZooKeeper's metric interfaces --- - private final io.prometheus.client.Counter inner; - private final String name; + private static class PrometheusCounterWrapper implements Counter { + private final io.prometheus.metrics.core.metrics.Counter prometheusCounter; - public PrometheusCounter(String name) { - this.name = name; - this.inner = io.prometheus.client.Counter - .build(name, name) - .register(collectorRegistry); + public PrometheusCounterWrapper(io.prometheus.metrics.core.metrics.Counter prometheusCounter) { + this.prometheusCounter = prometheusCounter; } @Override public void add(long delta) { try { - inner.inc(delta); - } catch (IllegalArgumentException err) { - LOG.error("invalid delta {} for metric {}", delta, name, err); + this.prometheusCounter.inc(delta); + } catch (final IllegalArgumentException e) { + LOG.error("invalid delta {} for metric {}", delta, prometheusCounter.getPrometheusName(), e); } } @Override public long get() { - // this method is used only for tests - // Prometheus returns a "double" - // it is safe to fine to a long - // we are never setting non-integer values - return (long) inner.get(); + return (long) this.prometheusCounter.get(); } - } - private class PrometheusLabelledCounter implements CounterSet { - private final String name; - private final io.prometheus.client.Counter inner; + private static class PrometheusLabelledCounterWrapper implements CounterSet { + private final io.prometheus.metrics.core.metrics.Counter prometheusCounter; - public PrometheusLabelledCounter(final String name) { - this.name = name; - this.inner = io.prometheus.client.Counter - .build(name, name) - .labelNames(LABELS) - .register(collectorRegistry); + public PrometheusLabelledCounterWrapper(io.prometheus.metrics.core.metrics.Counter prometheusCounter) { + this.prometheusCounter = prometheusCounter; } @Override - public void add(final String key, final long delta) { + public void add(String key, long delta) { try { - inner.labels(key).inc(delta); + this.prometheusCounter.labelValues(key).inc(delta); } catch (final IllegalArgumentException e) { - LOG.error("invalid delta {} for metric {} with key {}", delta, name, key, e); - } - } - } - - private class PrometheusGaugeWrapper { - - private final io.prometheus.client.Gauge inner; - private final Gauge gauge; - private final String name; - - public PrometheusGaugeWrapper(String name, Gauge gauge, io.prometheus.client.Gauge prev) { - this.name = name; - this.gauge = gauge; - this.inner = prev != null ? prev - : io.prometheus.client.Gauge - .build(name, name) - .register(collectorRegistry); - } - - /** - * Call the callback and update Prometheus Gauge. This method is called - * when the server is polling for a value. - */ - private void sample() { - Number value = gauge.get(); - this.inner.set(value != null ? value.doubleValue() : 0); - } - - private void unregister() { - collectorRegistry.unregister(inner); - } - } - - /** - * Prometheus implementation of GaugeSet interface. It wraps the GaugeSet object and - * uses the callback API to update the Prometheus Gauge. - */ - private class PrometheusLabelledGaugeWrapper { - private final GaugeSet gaugeSet; - private final io.prometheus.client.Gauge inner; - - private PrometheusLabelledGaugeWrapper(final String name, - final GaugeSet gaugeSet, - final io.prometheus.client.Gauge prev) { - this.gaugeSet = gaugeSet; - this.inner = prev != null ? prev : - io.prometheus.client.Gauge - .build(name, name) - .labelNames(LABELS) - .register(collectorRegistry); - } - - /** - * Call the callback provided by the GaugeSet and update Prometheus Gauge. - * This method is called when the server is polling for a value. - */ - private void sample() { - gaugeSet.values().forEach((key, value) -> - this.inner.labels(key).set(value != null ? value.doubleValue() : 0)); - } - - private void unregister() { - collectorRegistry.unregister(inner); - } - } - - private class PrometheusSummary implements Summary { - - private final io.prometheus.client.Summary inner; - private final String name; - - public PrometheusSummary(String name, MetricsContext.DetailLevel level) { - this.name = name; - if (level == MetricsContext.DetailLevel.ADVANCED) { - this.inner = io.prometheus.client.Summary - .build(name, name) - .quantile(0.5, 0.05) // Add 50th percentile (= median) with 5% tolerated error - .quantile(0.9, 0.01) // Add 90th percentile with 1% tolerated error - .quantile(0.99, 0.001) // Add 99th percentile with 0.1% tolerated error - .register(collectorRegistry); - } else { - this.inner = io.prometheus.client.Summary - .build(name, name) - .quantile(0.5, 0.05) // Add 50th percentile (= median) with 5% tolerated error - .register(collectorRegistry); + LOG.error("invalid delta {} for metric {} with key {}", delta, prometheusCounter.getPrometheusName(), + key, e); } } @Override - public void add(long delta) { - reportMetrics(() -> observe(delta)); - } - - private void observe(final long delta) { - try { - inner.observe(delta); - } catch (final IllegalArgumentException err) { - LOG.error("invalid delta {} for metric {}", delta, name, err); - } + public void inc(String key) { + add(key, 1); } } - private class PrometheusLabelledSummary implements SummarySet { - - private final io.prometheus.client.Summary inner; - private final String name; - - public PrometheusLabelledSummary(String name, MetricsContext.DetailLevel level) { - this.name = name; - if (level == MetricsContext.DetailLevel.ADVANCED) { - this.inner = io.prometheus.client.Summary - .build(name, name) - .labelNames(LABELS) - .quantile(0.5, 0.05) // Add 50th percentile (= median) with 5% tolerated error - .quantile(0.9, 0.01) // Add 90th percentile with 1% tolerated error - .quantile(0.99, 0.001) // Add 99th percentile with 0.1% tolerated error - .register(collectorRegistry); - } else { - this.inner = io.prometheus.client.Summary - .build(name, name) - .labelNames(LABELS) - .quantile(0.5, 0.05) // Add 50th percentile (= median) with 5% tolerated error - .register(collectorRegistry); - } - } - - @Override - public void add(String key, long value) { - reportMetrics(() -> observe(key, value)); - } + private static class PrometheusSummaryWrapper implements Summary { + private final io.prometheus.metrics.core.metrics.Summary prometheusSummary; - private void observe(final String key, final long value) { - try { - inner.labels(key).observe(value); - } catch (final IllegalArgumentException err) { - LOG.error("invalid value {} for metric {} with key {}", value, name, key, err); - } + public PrometheusSummaryWrapper(io.prometheus.metrics.core.metrics.Summary prometheusSummary) { + this.prometheusSummary = prometheusSummary; } - } - - class MetricsServletImpl extends MetricsServlet { - @Override - protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { - // little trick: update the Gauges before serving data - // from Prometheus CollectorRegistry - sampleGauges(); - // serve data using Prometheus built in client. - super.doGet(req, resp); + public void add(long value) { + this.prometheusSummary.observe(value); } } - private Optional createExecutor() { - if (numWorkerThreads < 1) { - LOG.info("Executor service was not created as numWorkerThreads {} is less than 1", numWorkerThreads); - return Optional.empty(); - } - - final BlockingQueue queue = new LinkedBlockingQueue<>(maxQueueSize); - final ThreadPoolExecutor executor = new ThreadPoolExecutor(numWorkerThreads, - numWorkerThreads, - 0L, - TimeUnit.MILLISECONDS, - queue, - new PrometheusWorkerThreadFactory(), - new PrometheusRejectedExecutionHandler()); - LOG.info("Executor service was created with numWorkerThreads {} and maxQueueSize {}", - numWorkerThreads, - maxQueueSize); - return Optional.of(executor); - } + private static class PrometheusLabelledSummaryWrapper implements SummarySet { + private final io.prometheus.metrics.core.metrics.Summary prometheusSummary; - private void shutdownExecutor() { - if (executorOptional.isPresent()) { - LOG.info("Shutdown executor service with timeout {}", workerShutdownTimeoutMs); - final ExecutorService executor = executorOptional.get(); - executor.shutdown(); - try { - if (!executor.awaitTermination(workerShutdownTimeoutMs, TimeUnit.MILLISECONDS)) { - LOG.error("Not all the Prometheus worker threads terminated properly after {} timeout", - workerShutdownTimeoutMs); - executor.shutdownNow(); - } - } catch (final Exception e) { - LOG.error("Error occurred while terminating Prometheus worker threads", e); - executor.shutdownNow(); - } + public PrometheusLabelledSummaryWrapper(io.prometheus.metrics.core.metrics.Summary prometheusSummary) { + this.prometheusSummary = prometheusSummary; } - } - - private static class PrometheusWorkerThreadFactory implements ThreadFactory { - private static final AtomicInteger workerCounter = new AtomicInteger(1); @Override - public Thread newThread(final Runnable runnable) { - final String threadName = "PrometheusMetricsProviderWorker-" + workerCounter.getAndIncrement(); - final Thread thread = new Thread(runnable, threadName); - thread.setDaemon(true); - return thread; - } - } - - private class PrometheusRejectedExecutionHandler implements RejectedExecutionHandler { - private final String queueExceededMessage = "Prometheus metrics queue size exceeded the max " + maxQueueSize; - - @Override - public void rejectedExecution(final Runnable r, final ThreadPoolExecutor e) { - rateLogger.rateLimitLog(queueExceededMessage); - } - } - - private void reportMetrics(final Runnable task) { - if (executorOptional.isPresent()) { - executorOptional.get().submit(task); - } else { - task.run(); + public void add(String key, long value) { + this.prometheusSummary.labelValues(key).observe(value); } } } diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusRegistryDumper.java b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusRegistryDumper.java new file mode 100644 index 00000000000..146f761595e --- /dev/null +++ b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusRegistryDumper.java @@ -0,0 +1,160 @@ +/* + * 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.zookeeper.metrics.prometheus; + +import io.prometheus.metrics.model.registry.PrometheusRegistry; +import io.prometheus.metrics.model.snapshots.CounterSnapshot; +import io.prometheus.metrics.model.snapshots.GaugeSnapshot; +import io.prometheus.metrics.model.snapshots.Labels; +import io.prometheus.metrics.model.snapshots.MetricSnapshot; +import io.prometheus.metrics.model.snapshots.MetricSnapshots; +import io.prometheus.metrics.model.snapshots.Quantile; +import io.prometheus.metrics.model.snapshots.SummarySnapshot; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +/** + * Handles the logic of converting a PrometheusRegistry scrape result into a sequence of key-value pairs. + */ +public class PrometheusRegistryDumper { + + private final PrometheusRegistry registry; + + public PrometheusRegistryDumper(PrometheusRegistry registry) { + this.registry = registry; + } + + /** + * Dumps all metrics from the PrometheusRegistry into a key-value map. + * + * @return a map containing all the metrics + */ + public Map dump() { + Map allMetrics = new LinkedHashMap<>(); + MetricSnapshots metricSnapshots = registry.scrape(); + for (MetricSnapshot snapshot : metricSnapshots) { + Map convertedMetrics = null; + if (snapshot instanceof CounterSnapshot) { + convertedMetrics = convert((CounterSnapshot) snapshot); + } else if (snapshot instanceof GaugeSnapshot) { + convertedMetrics = convert((GaugeSnapshot) snapshot); + } else if (snapshot instanceof SummarySnapshot) { + convertedMetrics = convert((SummarySnapshot) snapshot); + } + + if (convertedMetrics != null) { + allMetrics.putAll(convertedMetrics); + } + } + return allMetrics; + } + + private Map convert(CounterSnapshot snapshot) { + Map result = new LinkedHashMap<>(); + String metricName = snapshot.getMetadata().getName(); + for (CounterSnapshot.CounterDataPointSnapshot dataPoint : snapshot.getDataPoints()) { + result.put(buildKeyForDump(metricName, dataPoint.getLabels()), dataPoint.getValue()); + } + return result; + } + + private Map convert(GaugeSnapshot snapshot) { + Map result = new LinkedHashMap<>(); + String metricName = snapshot.getMetadata().getName(); + for (GaugeSnapshot.GaugeDataPointSnapshot dataPoint : snapshot.getDataPoints()) { + result.put(buildKeyForDump(metricName, dataPoint.getLabels()), dataPoint.getValue()); + } + return result; + } + + private Map convert(SummarySnapshot snapshot) { + Map result = new LinkedHashMap<>(); + String metricName = snapshot.getMetadata().getName(); + for (SummarySnapshot.SummaryDataPointSnapshot dataPoint : snapshot.getDataPoints()) { + double count = dataPoint.getCount(); + double sum = dataPoint.getSum(); + double avg = (count == 0) ? 0 : sum / count; + + // Add metrics in the requested order with prefixes + result.put(buildKeyForDump(metricName + "_avg", dataPoint.getLabels()), avg); + // Note: Prometheus Summary does not provide min/max, so they are omitted. + result.put(buildKeyForDump(metricName + "_count", dataPoint.getLabels()), count); + result.put(buildKeyForDump(metricName + "_sum", dataPoint.getLabels()), sum); + + // A summary is considered "advanced" if it has more than one quantile configured. + boolean isAdvanced = dataPoint.getQuantiles().size() > 1; + + if (isAdvanced) { + List quantiles = new ArrayList<>(); + dataPoint.getQuantiles().forEach(quantiles::add); + quantiles.sort(Comparator.comparingDouble(Quantile::getQuantile)); + + for (Quantile quantile : quantiles) { + String quantileValue = String.valueOf(quantile.getQuantile()); + switch (quantileValue) { + default: + break; + case "0.5": + result.put(buildKeyForDump(metricName, dataPoint.getLabels().add("quantile", quantileValue)), + quantile.getValue()); + break; + case "0.95": + result.put(buildKeyForDump(metricName, dataPoint.getLabels().add("quantile", quantileValue)), + quantile.getValue()); + break; + case "0.99": + result.put(buildKeyForDump(metricName, dataPoint.getLabels().add("quantile", quantileValue)), + quantile.getValue()); + break; + } + } + } + } + return result; + } + + /** + * Builds a string key for a given metric and its labels, in a format suitable for the dump output. + * + * @param metricName + * the name of the metric + * @param labels + * the labels associated with the metric + * + * @return a formatted string key + */ + private String buildKeyForDump(String metricName, Labels labels) { + StringBuilder sb = new StringBuilder(); + sb.append(metricName); + if (labels.size() > 0) { + sb.append("{"); + for (int i = 0; i < labels.size(); i++) { + if (i > 0) { + sb.append(","); + } + sb.append(labels.getName(i)).append("=\"").append(labels.getValue(i)).append("\""); + } + sb.append("}"); + } + return sb.toString(); + } +} diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/ExportJvmInfoTest.java b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/ExportJvmInfoTest.java index 5d51f9c1e54..a923348f758 100644 --- a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/ExportJvmInfoTest.java +++ b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/ExportJvmInfoTest.java @@ -23,8 +23,7 @@ import org.junit.jupiter.api.Test; /** - * Tests about Prometheus Metrics Provider. Please note that we are not testing - * Prometheus but our integration. + * Tests about Prometheus Metrics Provider. Please note that we are not testing Prometheus but our integration. */ public class ExportJvmInfoTest extends PrometheusMetricsTestBase { @@ -46,7 +45,7 @@ private void runTest(boolean exportJvmInfo) throws Exception { configuration.setProperty("exportJvmInfo", exportJvmInfo + ""); provider.configure(configuration); provider.start(); - boolean[] found = {false}; + boolean[] found = { false }; provider.dump((k, v) -> { found[0] = found[0] || k.contains("heap"); }); diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusHttpsMetricsProviderTest.java b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusHttpsMetricsProviderTest.java index 26cdb4c23c5..c091934709f 100644 --- a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusHttpsMetricsProviderTest.java +++ b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusHttpsMetricsProviderTest.java @@ -7,7 +7,7 @@ * "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 + * 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, @@ -20,6 +20,7 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; import java.io.BufferedReader; import java.io.FileInputStream; import java.io.IOException; @@ -37,8 +38,7 @@ import org.junit.jupiter.api.Test; /** - * Tests about Prometheus Metrics Provider. Please note that we are not testing - * Prometheus but only our integration. + * Tests about Prometheus Metrics Provider. Please note that we are not testing Prometheus but only our integration. */ public class PrometheusHttpsMetricsProviderTest extends PrometheusMetricsTestBase { @@ -75,7 +75,14 @@ void testHttpResponse() throws Exception { configuration.setProperty("httpPort", String.valueOf(httpPort)); initializeProviderWithCustomConfig(configuration); simulateMetricIncrement(); - validateMetricResponse(callHttpServlet("http://" + httpHost + ":" + httpPort + "/metrics")); + String metricsUrl = String.format("http://%s:%d/metrics", httpHost, httpPort); + + HttpURLConnection conn = callAndGetResponse(metricsUrl, "GET"); + validateMetricResponse(readResponse(conn)); + + conn = callAndGetResponse(metricsUrl, "TRACE"); + assertEquals(HttpURLConnection.HTTP_BAD_METHOD, conn.getResponseCode()); + conn.disconnect(); } @Test @@ -84,7 +91,14 @@ void testHttpsResponse() throws Exception { configuration.setProperty("httpsPort", String.valueOf(httpsPort)); initializeProviderWithCustomConfig(configuration); simulateMetricIncrement(); - validateMetricResponse(callHttpsServlet("https://" + httpHost + ":" + httpsPort + "/metrics")); + String metricsUrl = String.format("https://%s:%d/metrics", httpHost, httpsPort); + + HttpURLConnection conn = callAndGetResponse(metricsUrl, "GET"); + validateMetricResponse(readResponse(conn)); + + conn = callAndGetResponse(metricsUrl, "TRACE"); + assertEquals(HttpURLConnection.HTTP_BAD_METHOD, conn.getResponseCode()); + conn.disconnect(); } @Test @@ -94,44 +108,48 @@ void testHttpAndHttpsResponse() throws Exception { configuration.setProperty("httpPort", String.valueOf(httpPort)); initializeProviderWithCustomConfig(configuration); simulateMetricIncrement(); - validateMetricResponse(callHttpServlet("http://" + httpHost + ":" + httpPort + "/metrics")); - validateMetricResponse(callHttpsServlet("https://" + httpHost + ":" + httpsPort + "/metrics")); - } - private String callHttpsServlet(String urlString) throws Exception { - // Load and configure the SSL context from the keystore and truststore - KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); - try (FileInputStream keystoreStream = new FileInputStream(testDataPath + "/ssl/client_keystore.jks")) { - keyStore.load(keystoreStream, "testpass".toCharArray()); - } + HttpURLConnection conn = callAndGetResponse(String.format("https://%s:%d/metrics", httpHost, httpsPort), "GET"); + validateMetricResponse(readResponse(conn)); + } - KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType()); - try (FileInputStream trustStoreStream = new FileInputStream(testDataPath + "/ssl/client_truststore.jks")) { - trustStore.load(trustStoreStream, "testpass".toCharArray()); - } + private HttpURLConnection callAndGetResponse(String urlString, String method) throws Exception { + URL url = new URL(urlString); + HttpURLConnection conn; - SSLContext sslContext = SSLContext.getInstance("TLS"); - KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); - keyManagerFactory.init(keyStore, "testpass".toCharArray()); - TrustManagerFactory trustManagerFactory = TrustManagerFactory - .getInstance(TrustManagerFactory.getDefaultAlgorithm()); - trustManagerFactory.init(trustStore); - sslContext.init(keyManagerFactory.getKeyManagers(), trustManagerFactory.getTrustManagers(), - new java.security.SecureRandom()); + if (url.getProtocol().equalsIgnoreCase("https")) { + // Re-use the existing SSL setup logic. + KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); + try (FileInputStream keystoreStream = new FileInputStream(testDataPath + "/ssl/client_keystore.jks")) { + keyStore.load(keystoreStream, "testpass".toCharArray()); + } - HttpsURLConnection.setDefaultSSLSocketFactory(sslContext.getSocketFactory()); - URL url = new URL(urlString); - HttpsURLConnection connection = (HttpsURLConnection) url.openConnection(); - connection.setRequestMethod("GET"); + KeyStore trustStore = KeyStore.getInstance(KeyStore.getDefaultType()); + try (FileInputStream trustStoreStream = new FileInputStream(testDataPath + "/ssl/client_truststore.jks")) { + trustStore.load(trustStoreStream, "testpass".toCharArray()); + } - return readResponse(connection); - } + SSLContext sslContext = SSLContext.getInstance("TLS"); + KeyManagerFactory keyManagerFactory = KeyManagerFactory + .getInstance(KeyManagerFactory.getDefaultAlgorithm()); + keyManagerFactory.init(keyStore, "testpass".toCharArray()); + TrustManagerFactory trustManagerFactory = TrustManagerFactory + .getInstance(TrustManagerFactory.getDefaultAlgorithm()); + trustManagerFactory.init(trustStore); + sslContext.init(keyManagerFactory.getKeyManagers(), trustManagerFactory.getTrustManagers(), + new java.security.SecureRandom()); + + HttpsURLConnection httpsConn = (HttpsURLConnection) url.openConnection(); + httpsConn.setSSLSocketFactory(sslContext.getSocketFactory()); + httpsConn.setHostnameVerifier((hostname, session) -> true); + conn = httpsConn; + } else { + conn = (HttpURLConnection) url.openConnection(); + } - private String callHttpServlet(String urlString) throws IOException { - URL url = new URL(urlString); - HttpURLConnection connection = (HttpURLConnection) url.openConnection(); - connection.setRequestMethod("GET"); - return readResponse(connection); + conn.setRequestMethod(method); + conn.connect(); + return conn; } private String readResponse(HttpURLConnection connection) throws IOException { @@ -155,7 +173,7 @@ public void simulateMetricIncrement() { } private void validateMetricResponse(String response) throws IOException { - assertThat(response, containsString("# TYPE cc counter")); - assertThat(response, containsString("cc 10.0")); + assertThat(response, containsString("# TYPE cc_total counter")); + assertThat(response, containsString("cc_total 10.0")); } -} \ No newline at end of file +} diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderConfigTest.java b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderConfigTest.java index aa9cd2c7170..c151854b05a 100644 --- a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderConfigTest.java +++ b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderConfigTest.java @@ -88,15 +88,14 @@ public void testValidHttpsAndHttpConfig() throws MetricsProviderLifeCycleExcepti provider.start(); } - @Test public void testInvalidSslConfig() throws MetricsProviderLifeCycleException { assertThrows(MetricsProviderLifeCycleException.class, () -> { PrometheusMetricsProvider provider = new PrometheusMetricsProvider(); Properties configuration = new Properties(); String testDataPath = System.getProperty("test.data.dir", "src/test/resources/data"); - configuration.setProperty("httpsPort", "0"); - //keystore missing + configuration.setProperty("httpsPort", "50514"); + // keystore missing configuration.setProperty("ssl.keyStore.password", "testpass"); configuration.setProperty("ssl.trustStore.location", testDataPath + "/ssl/server_truststore.jks"); configuration.setProperty("ssl.trustStore.password", "testpass"); diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderTest.java b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderTest.java index 59115e31d90..2d36b001763 100644 --- a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderTest.java +++ b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProviderTest.java @@ -26,13 +26,10 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.PrintWriter; -import java.io.StringWriter; -import java.lang.reflect.Field; -import java.net.HttpURLConnection; -import java.net.URL; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -42,6 +39,9 @@ import java.util.Properties; import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.ServletException; +import javax.servlet.ServletOutputStream; +import javax.servlet.WriteListener; +import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.zookeeper.metrics.Counter; @@ -52,16 +52,13 @@ import org.apache.zookeeper.metrics.Summary; import org.apache.zookeeper.metrics.SummarySet; import org.apache.zookeeper.server.util.QuotaMetricsUtils; -import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.ServerConnector; import org.hamcrest.CoreMatchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; /** - * Tests about Prometheus Metrics Provider. Please note that we are not testing - * Prometheus but only our integration. + * Tests about Prometheus Metrics Provider. Please note that we are not testing Prometheus but only our integration. */ public class PrometheusMetricsProviderTest extends PrometheusMetricsTestBase { @@ -72,7 +69,7 @@ public class PrometheusMetricsProviderTest extends PrometheusMetricsTestBase { public void setup() throws Exception { provider = new PrometheusMetricsProvider(); Properties configuration = new Properties(); - configuration.setProperty("numWorkerThreads", "0"); // sync behavior for test + configuration.setProperty("numWorkerThreads", "1"); // sync behavior for test configuration.setProperty("httpHost", "127.0.0.1"); // local host for test configuration.setProperty("httpPort", "0"); // ephemeral port configuration.setProperty("exportJvmInfo", "false"); @@ -91,13 +88,12 @@ public void tearDown() { public void testCounters() throws Exception { Counter counter = provider.getRootContext().getCounter("cc"); counter.add(10); - int[] count = {0}; + int[] count = { 0 }; provider.dump((k, v) -> { assertEquals("cc", k); assertEquals(10, ((Number) v).intValue()); count[0]++; - } - ); + }); assertEquals(1, count[0]); count[0] = 0; @@ -108,16 +104,15 @@ public void testCounters() throws Exception { assertEquals("cc", k); assertEquals(10, ((Number) v).intValue()); count[0]++; - } - ); + }); assertEquals(1, count[0]); // we always must get the same object assertSame(counter, provider.getRootContext().getCounter("cc")); String res = callServlet(); - assertThat(res, CoreMatchers.containsString("# TYPE cc counter")); - assertThat(res, CoreMatchers.containsString("cc 10.0")); + assertThat(res, CoreMatchers.containsString("# TYPE cc_total counter")); + assertThat(res, CoreMatchers.containsString("cc_total 10.0")); } @Test @@ -125,7 +120,7 @@ public void testCounterSet_single() throws Exception { // create and register a CounterSet final String name = QuotaMetricsUtils.QUOTA_EXCEEDED_ERROR_PER_NAMESPACE; final CounterSet counterSet = provider.getRootContext().getCounterSet(name); - final String[] keys = {"ns1", "ns2"}; + final String[] keys = { "ns1", "ns2" }; final int count = 3; // update the CounterSet multiple times @@ -142,10 +137,10 @@ public void testCounterSet_single() throws Exception { validateWithDump(expectedMetricsMap); // validate with servlet call - final List expectedNames = Collections.singletonList(String.format("# TYPE %s count", name)); + final List expectedNames = Collections.singletonList(String.format("# TYPE %s_total counter", name)); final List expectedMetrics = new ArrayList<>(); for (final String key : keys) { - expectedMetrics.add(String.format("%s{key=\"%s\",} %s", name, key, count * 3.0)); + expectedMetrics.add(String.format("%s_total{key=\"%s\"} %s", name, key, count * 3.0)); } validateWithServletCall(expectedNames, expectedMetrics); @@ -157,9 +152,9 @@ public void testCounterSet_single() throws Exception { public void testCounterSet_multiple() throws Exception { final String name = QuotaMetricsUtils.QUOTA_EXCEEDED_ERROR_PER_NAMESPACE; - final String[] names = new String[]{name + "_1", name + "_2"}; - final String[] keys = new String[]{"ns21", "ns22"}; - final int[] counts = new int[] {3, 5}; + final String[] names = new String[] { name + "_1", name + "_2" }; + final String[] keys = new String[] { "ns21", "ns22" }; + final int[] counts = new int[] { 3, 5 }; final int length = names.length; final CounterSet[] counterSets = new CounterSet[length]; @@ -187,16 +182,15 @@ public void testCounterSet_multiple() throws Exception { final List expectedNames = new ArrayList<>(); final List expectedMetrics = new ArrayList<>(); for (int i = 0; i < length; i++) { - expectedNames.add(String.format("# TYPE %s count", names[i])); - expectedMetrics.add(String.format("%s{key=\"%s\",} %s", names[i], keys[i], counts[i] * 1.0)); + expectedNames.add(String.format("# TYPE %s_total counter", names[i])); + expectedMetrics.add(String.format("%s_total{key=\"%s\"} %s", names[i], keys[i], counts[i] * 1.0)); } validateWithServletCall(expectedNames, expectedMetrics); } @Test public void testCounterSet_registerWithNullName() { - assertThrows(NullPointerException.class, - () -> provider.getRootContext().getCounterSet(null)); + assertThrows(NullPointerException.class, () -> provider.getRootContext().getCounterSet(null)); } @Test @@ -222,8 +216,8 @@ public void testCounterSet_nullKey() { @Test public void testGauge() throws Exception { - int[] values = {78, -89}; - int[] callCounts = {0, 0}; + int[] values = { 78, -89 }; + int[] callCounts = { 0, 0 }; Gauge gauge0 = () -> { callCounts[0]++; return values[0]; @@ -234,13 +228,12 @@ public void testGauge() throws Exception { }; provider.getRootContext().registerGauge("gg", gauge0); - int[] count = {0}; + int[] count = { 0 }; provider.dump((k, v) -> { assertEquals("gg", k); assertEquals(values[0], ((Number) v).intValue()); count[0]++; - } - ); + }); assertEquals(1, callCounts[0]); assertEquals(0, callCounts[1]); assertEquals(1, count[0]); @@ -252,8 +245,7 @@ public void testGauge() throws Exception { provider.getRootContext().unregisterGauge("gg"); provider.dump((k, v) -> { count[0]++; - } - ); + }); assertEquals(2, callCounts[0]); assertEquals(0, callCounts[1]); assertEquals(0, count[0]); @@ -266,8 +258,7 @@ public void testGauge() throws Exception { assertEquals("gg", k); assertEquals(values[1], ((Number) v).intValue()); count[0]++; - } - ); + }); assertEquals(2, callCounts[0]); assertEquals(1, callCounts[1]); assertEquals(1, count[0]); @@ -285,8 +276,7 @@ public void testGauge() throws Exception { provider.dump((k, v) -> { count[0]++; - } - ); + }); assertEquals(1, count[0]); assertEquals(3, callCounts[0]); assertEquals(2, callCounts[1]); @@ -294,41 +284,37 @@ public void testGauge() throws Exception { @Test public void testBasicSummary() throws Exception { - Summary summary = provider.getRootContext() - .getSummary("cc", MetricsContext.DetailLevel.BASIC); + Summary summary = provider.getRootContext().getSummary("cc", MetricsContext.DetailLevel.BASIC); summary.add(10); summary.add(10); - int[] count = {0}; + int[] count = { 0 }; provider.dump((k, v) -> { count[0]++; int value = ((Number) v).intValue(); switch (k) { - case "cc{quantile=\"0.5\"}": - assertEquals(10, value); - break; - case "cc_count": - assertEquals(2, value); - break; - case "cc_sum": - assertEquals(20, value); - break; - default: - fail("unespected key " + k); - break; + case "cc_avg": + assertEquals(10, value); + break; + case "cc_count": + assertEquals(2, value); + break; + case "cc_sum": + assertEquals(20, value); + break; + default: + fail("unespected key " + k); + break; } - } - ); + }); assertEquals(3, count[0]); count[0] = 0; // we always must get the same object - assertSame(summary, provider.getRootContext() - .getSummary("cc", MetricsContext.DetailLevel.BASIC)); + assertSame(summary, provider.getRootContext().getSummary("cc", MetricsContext.DetailLevel.BASIC)); try { - provider.getRootContext() - .getSummary("cc", MetricsContext.DetailLevel.ADVANCED); + provider.getRootContext().getSummary("cc", MetricsContext.DetailLevel.ADVANCED); fail("Can't get the same summary with a different DetailLevel"); } catch (IllegalArgumentException err) { assertThat(err.getMessage(), containsString("Already registered")); @@ -337,53 +323,55 @@ public void testBasicSummary() throws Exception { String res = callServlet(); assertThat(res, containsString("# TYPE cc summary")); assertThat(res, CoreMatchers.containsString("cc_sum 20.0")); - assertThat(res, CoreMatchers.containsString("cc_count 2.0")); - assertThat(res, CoreMatchers.containsString("cc{quantile=\"0.5\",} 10.0")); + assertThat(res, CoreMatchers.containsString("cc_count 2")); + assertThat(res, CoreMatchers.containsString("cc{quantile=\"0.5\"} 10.0")); } @Test public void testAdvancedSummary() throws Exception { - Summary summary = provider.getRootContext() - .getSummary("cc", MetricsContext.DetailLevel.ADVANCED); + Summary summary = provider.getRootContext().getSummary("cc", MetricsContext.DetailLevel.ADVANCED); summary.add(10); summary.add(10); - int[] count = {0}; + int[] count = { 0 }; provider.dump((k, v) -> { count[0]++; int value = ((Number) v).intValue(); switch (k) { - case "cc{quantile=\"0.5\"}": - assertEquals(10, value); - break; - case "cc{quantile=\"0.9\"}": - assertEquals(10, value); - break; - case "cc{quantile=\"0.99\"}": - assertEquals(10, value); - break; - case "cc_count": - assertEquals(2, value); - break; - case "cc_sum": - assertEquals(20, value); - break; - default: - fail("unespected key " + k); - break; + case "cc{quantile=\"0.5\"}": + assertEquals(10, value); + break; + case "cc{quantile=\"0.9\"}": + assertEquals(10, value); + break; + case "cc{quantile=\"0.95\"}": + assertEquals(10, value); + break; + case "cc{quantile=\"0.99\"}": + assertEquals(10, value); + break; + case "cc_count": + assertEquals(2, value); + break; + case "cc_sum": + assertEquals(20, value); + break; + case "cc_avg": + assertEquals(10, value); + break; + default: + fail("unespected key " + k); + break; } - } - ); - assertEquals(5, count[0]); + }); + assertEquals(6, count[0]); count[0] = 0; // we always must get the same object - assertSame(summary, provider.getRootContext() - .getSummary("cc", MetricsContext.DetailLevel.ADVANCED)); + assertSame(summary, provider.getRootContext().getSummary("cc", MetricsContext.DetailLevel.ADVANCED)); try { - provider.getRootContext() - .getSummary("cc", MetricsContext.DetailLevel.BASIC); + provider.getRootContext().getSummary("cc", MetricsContext.DetailLevel.BASIC); fail("Can't get the same summary with a different DetailLevel"); } catch (IllegalArgumentException err) { assertThat(err.getMessage(), containsString("Already registered")); @@ -392,27 +380,35 @@ public void testAdvancedSummary() throws Exception { String res = callServlet(); assertThat(res, containsString("# TYPE cc summary")); assertThat(res, CoreMatchers.containsString("cc_sum 20.0")); - assertThat(res, CoreMatchers.containsString("cc_count 2.0")); - assertThat(res, CoreMatchers.containsString("cc{quantile=\"0.5\",} 10.0")); - assertThat(res, CoreMatchers.containsString("cc{quantile=\"0.9\",} 10.0")); - assertThat(res, CoreMatchers.containsString("cc{quantile=\"0.99\",} 10.0")); + assertThat(res, CoreMatchers.containsString("cc_count 2")); + assertThat(res, CoreMatchers.containsString("cc{quantile=\"0.5\"} 10.0")); + assertThat(res, CoreMatchers.containsString("cc{quantile=\"0.95\"} 10.0")); + assertThat(res, CoreMatchers.containsString("cc{quantile=\"0.99\"} 10.0")); } /** - * Using TRACE method to visit metrics provider, the response should be 403 forbidden. + * Using the TRACE method to visit the metrics provider, the response should be 405 Method Not Allowed. This unit + * test replaces the old test which was tightly coupled to the Jetty server implementation. */ @Test - public void testTraceCall() throws IOException, IllegalAccessException, NoSuchFieldException { - Field privateServerField = provider.getClass().getDeclaredField("server"); - privateServerField.setAccessible(true); - Server server = (Server) privateServerField.get(provider); - int port = ((ServerConnector) server.getConnectors()[0]).getLocalPort(); - - String metricsUrl = String.format(URL_FORMAT, port); - HttpURLConnection conn = (HttpURLConnection) new URL(metricsUrl).openConnection(); - conn.setRequestMethod("TRACE"); - conn.connect(); - assertEquals(HttpURLConnection.HTTP_FORBIDDEN, conn.getResponseCode()); + public void testTraceCall() throws ServletException, IOException { + final HttpServlet servlet = provider.getServlet(); + final HttpServletRequest request = mock(HttpServletRequest.class); + final HttpServletResponse response = mock(HttpServletResponse.class); + + // Configure the mock to return an empty enumeration for headers. + when(request.getHeaderNames()).thenReturn(Collections.enumeration(Collections.emptyList())); + when(request.getMethod()).thenReturn("TRACE"); + + final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + final ServletOutputStream servletOutputStream = createMockServletOutputStream(outputStream); + + when(response.getOutputStream()).thenReturn(servletOutputStream); + + servlet.service(request, response); + + // Verify that the servlet set the response status to 405 Method Not Allowed. + verify(response).sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED); } @Test @@ -428,8 +424,8 @@ public void testSummary_asyncAndExceedMaxQueueSize() throws Exception { metricsProvider = new PrometheusMetricsProvider(); metricsProvider.configure(config); metricsProvider.start(); - final Summary summary = - metricsProvider.getRootContext().getSummary("cc", MetricsContext.DetailLevel.ADVANCED); + final Summary summary = metricsProvider.getRootContext().getSummary("cc", + MetricsContext.DetailLevel.ADVANCED); // make sure no error is thrown for (int i = 0; i < 10; i++) { @@ -437,7 +433,7 @@ public void testSummary_asyncAndExceedMaxQueueSize() throws Exception { } } finally { if (metricsProvider != null) { - metricsProvider.stop(); + metricsProvider.stop(); } } } @@ -445,12 +441,11 @@ public void testSummary_asyncAndExceedMaxQueueSize() throws Exception { @Test public void testSummarySet() throws Exception { final String name = "ss"; - final String[] keys = {"ns1", "ns2"}; + final String[] keys = { "ns1", "ns2" }; final double count = 3.0; // create and register a SummarySet - final SummarySet summarySet = provider.getRootContext() - .getSummarySet(name, MetricsContext.DetailLevel.BASIC); + final SummarySet summarySet = provider.getRootContext().getSummarySet(name, MetricsContext.DetailLevel.BASIC); // update the SummarySet multiple times for (int i = 0; i < count; i++) { @@ -460,7 +455,7 @@ public void testSummarySet() throws Exception { // validate with dump call final Map expectedMetricsMap = new HashMap<>(); for (final String key : keys) { - expectedMetricsMap.put(String.format("%s{key=\"%s\",quantile=\"0.5\"}", name, key), 1.0); + expectedMetricsMap.put(String.format("%s_avg{key=\"%s\"}", name, key), 1.0); expectedMetricsMap.put(String.format("%s_count{key=\"%s\"}", name, key), count); expectedMetricsMap.put(String.format("%s_sum{key=\"%s\"}", name, key), count); } @@ -470,43 +465,84 @@ public void testSummarySet() throws Exception { final List expectedNames = Collections.singletonList(String.format("# TYPE %s summary", name)); final List expectedMetrics = new ArrayList<>(); for (final String key : keys) { - expectedMetrics.add(String.format("%s{key=\"%s\",quantile=\"0.5\",} %s", name, key, 1.0)); - expectedMetrics.add(String.format("%s_count{key=\"%s\",} %s", name, key, count)); - expectedMetrics.add(String.format("%s_sum{key=\"%s\",} %s", name, key, count)); + expectedMetrics.add(String.format("%s{key=\"%s\",quantile=\"0.5\"} %s", name, key, 1.0)); + expectedMetrics.add(String.format("%s_count{key=\"%s\"} %d", name, key, (int) count)); + expectedMetrics.add(String.format("%s_sum{key=\"%s\"} %s", name, key, count)); } validateWithServletCall(expectedNames, expectedMetrics); // validate registering with same name, no overwriting - assertSame(summarySet, provider.getRootContext() - .getSummarySet(name, MetricsContext.DetailLevel.BASIC)); + assertSame(summarySet, provider.getRootContext().getSummarySet(name, MetricsContext.DetailLevel.BASIC)); // validate registering with different DetailLevel, not allowed try { - provider.getRootContext() - .getSummarySet(name, MetricsContext.DetailLevel.ADVANCED); + provider.getRootContext().getSummarySet(name, MetricsContext.DetailLevel.ADVANCED); fail("Can't get the same summarySet with a different DetailLevel"); } catch (final IllegalArgumentException e) { assertThat(e.getMessage(), containsString("Already registered")); } } + /** + * Helper method to create a mock ServletOutputStream that writes to a provided ByteArrayOutputStream. + */ + private ServletOutputStream createMockServletOutputStream(final ByteArrayOutputStream outputStream) { + return new ServletOutputStream() { + @Override + public boolean isReady() { + return true; + } + + @Override + public void setWriteListener(WriteListener writeListener) { + } + + @Override + public void write(int b) throws IOException { + outputStream.write(b); + } + }; + } + + /** + * A utility method to simulate a GET request to the PrometheusMetricsServlet and return the response body as a + * String using Mockito mocks for the base servlet interfaces. + *

+ * This method demonstrates how to test a servlet by mocking the request and response objects and capturing the + * output. + *

+ * + * @return The content of the servlet's response body. + * + * @throws ServletException + * if a servlet-related error occurs. + * @throws IOException + * if an I/O error occurs. + */ private String callServlet() throws ServletException, IOException { - // we are not performing an HTTP request - // but we are calling directly the servlet - StringWriter writer = new StringWriter(); - HttpServletResponse response = mock(HttpServletResponse.class); - when(response.getWriter()).thenReturn(new PrintWriter(writer)); - HttpServletRequest req = mock(HttpServletRequest.class); - provider.getServlet().doGet(req, response); - String res = writer.toString(); - return res; + final HttpServlet servlet = provider.getServlet(); + final HttpServletRequest request = mock(HttpServletRequest.class); + final HttpServletResponse response = mock(HttpServletResponse.class); + + final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + final ServletOutputStream servletOutputStream = createMockServletOutputStream(outputStream); + + // Configure the mock response to return our custom output stream. + when(response.getOutputStream()).thenReturn(servletOutputStream); + when(request.getMethod()).thenReturn("GET"); + servlet.service(request, response); + + // Verify that the servlet set the HTTP status code to OK. + verify(response).setStatus(HttpServletResponse.SC_OK); + + return outputStream.toString(); } @Test public void testGaugeSet_singleGaugeSet() throws Exception { final String name = QuotaMetricsUtils.QUOTA_BYTES_LIMIT_PER_NAMESPACE; - final Number[] values = {10.0, 100.0}; - final String[] keys = {"ns11", "ns12"}; + final Number[] values = { 10.0, 100.0 }; + final String[] keys = { "ns11", "ns12" }; final Map metricsMap = new HashMap<>(); for (int i = 0; i < values.length; i++) { metricsMap.put(keys[i], values[i]); @@ -528,7 +564,7 @@ public void testGaugeSet_singleGaugeSet() throws Exception { final List expectedNames = Collections.singletonList(String.format("# TYPE %s gauge", name)); final List expectedMetrics = new ArrayList<>(); for (int i = 0; i < values.length; i++) { - expectedMetrics.add(String.format("%s{key=\"%s\",} %s", name, keys[i], values[i])); + expectedMetrics.add(String.format("%s{key=\"%s\"} %s", name, keys[i], values[i])); } validateWithServletCall(expectedNames, expectedMetrics); assertEquals(2, callCount.get()); @@ -548,13 +584,11 @@ public void testGaugeSet_singleGaugeSet() throws Exception { @Test public void testGaugeSet_multipleGaugeSets() throws Exception { - final String[] names = new String[] { - QuotaMetricsUtils.QUOTA_COUNT_LIMIT_PER_NAMESPACE, - QuotaMetricsUtils.QUOTA_COUNT_USAGE_PER_NAMESPACE - }; + final String[] names = new String[] { QuotaMetricsUtils.QUOTA_COUNT_LIMIT_PER_NAMESPACE, + QuotaMetricsUtils.QUOTA_COUNT_USAGE_PER_NAMESPACE }; - final Number[] values = new Number[] {20.0, 200.0}; - final String[] keys = new String[]{"ns21", "ns22"}; + final Number[] values = new Number[] { 20.0, 200.0 }; + final String[] keys = new String[] { "ns21", "ns22" }; final int count = names.length; final AtomicInteger[] callCounts = new AtomicInteger[count]; @@ -581,7 +615,7 @@ public void testGaugeSet_multipleGaugeSets() throws Exception { final List expectedMetrics = new ArrayList<>(); for (int i = 0; i < count; i++) { expectedNames.add(String.format("# TYPE %s gauge", names[i])); - expectedMetrics.add(String.format("%s{key=\"%s\",} %s", names[i], keys[i], values[i])); + expectedMetrics.add(String.format("%s{key=\"%s\"} %s", names[i], keys[i], values[i])); } validateWithServletCall(expectedNames, expectedMetrics); for (int i = 0; i < count; i++) { @@ -609,14 +643,12 @@ public void testGaugeSet_multipleGaugeSets() throws Exception { @Test public void testGaugeSet_overwriteRegister() { - final String[] names = new String[] { - QuotaMetricsUtils.QUOTA_COUNT_LIMIT_PER_NAMESPACE, - QuotaMetricsUtils.QUOTA_COUNT_USAGE_PER_NAMESPACE - }; + final String[] names = new String[] { QuotaMetricsUtils.QUOTA_COUNT_LIMIT_PER_NAMESPACE, + QuotaMetricsUtils.QUOTA_COUNT_USAGE_PER_NAMESPACE }; final int count = names.length; - final Number[] values = new Number[]{30.0, 300.0}; - final String[] keys = new String[] {"ns31", "ns32"}; + final Number[] values = new Number[] { 30.0, 300.0 }; + final String[] keys = new String[] { "ns31", "ns32" }; final AtomicInteger[] callCounts = new AtomicInteger[count]; // create and register the GaugeSets @@ -656,22 +688,18 @@ public void testGaugeSet_nullKey() { @Test public void testGaugeSet_registerWithNullGaugeSet() { - assertThrows(NullPointerException.class, - () -> provider.getRootContext().registerGaugeSet("name", null)); + assertThrows(NullPointerException.class, () -> provider.getRootContext().registerGaugeSet("name", null)); - assertThrows(NullPointerException.class, - () -> provider.getRootContext().registerGaugeSet(null, HashMap::new)); + assertThrows(NullPointerException.class, () -> provider.getRootContext().registerGaugeSet(null, HashMap::new)); } @Test public void testGaugeSet_unregisterNull() { - assertThrows(NullPointerException.class, - () -> provider.getRootContext().unregisterGaugeSet(null)); + assertThrows(NullPointerException.class, () -> provider.getRootContext().unregisterGaugeSet(null)); } - private void createAndRegisterGaugeSet(final String name, - final Map metricsMap, - final AtomicInteger callCount) { + private void createAndRegisterGaugeSet(final String name, final Map metricsMap, + final AtomicInteger callCount) { final GaugeSet gaugeSet = () -> { callCount.addAndGet(1); return metricsMap; @@ -686,8 +714,8 @@ private void validateWithDump(final Map expectedMetrics) { expectedMetrics.forEach((key, value) -> assertEquals(value, returnedMetrics.get(key))); } - private void validateWithServletCall(final List expectedNames, - final List expectedMetrics) throws Exception { + private void validateWithServletCall(final List expectedNames, final List expectedMetrics) + throws Exception { final String response = callServlet(); if (expectedNames.isEmpty() && expectedMetrics.isEmpty()) { assertTrue(response.isEmpty()); diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsTestBase.java b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsTestBase.java index 9ed4995fbd0..efd8f38a8a5 100644 --- a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsTestBase.java +++ b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsTestBase.java @@ -18,32 +18,27 @@ package org.apache.zookeeper.metrics.prometheus; -import io.prometheus.client.CollectorRegistry; -import io.prometheus.client.hotspot.DefaultExports; +import io.prometheus.metrics.instrumentation.jvm.JvmMetrics; +import io.prometheus.metrics.model.registry.PrometheusRegistry; import java.lang.reflect.Field; +import java.util.Set; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; /** * The base test for prometheus metrics unit tests. */ public abstract class PrometheusMetricsTestBase { - @BeforeEach - void setUp() throws Exception { - CollectorRegistry.defaultRegistry.clear(); - resetDefaultExportsInitializedFlag(); - } - @AfterEach void tearDown() throws Exception { - CollectorRegistry.defaultRegistry.clear(); - resetDefaultExportsInitializedFlag(); - } - - protected void resetDefaultExportsInitializedFlag() throws Exception { - Field initializedField = DefaultExports.class.getDeclaredField("initialized"); - initializedField.setAccessible(true); - initializedField.set(null, false); + PrometheusRegistry.defaultRegistry.clear(); + // JvmMetrics uses a static Set to track which registries it has been + // registered with. We need to clear this set via reflection to allow + // re-initialization in subsequent tests. + Field registeredField = JvmMetrics.class.getDeclaredField("REGISTERED"); + registeredField.setAccessible(true); + @SuppressWarnings("unchecked") + Set registeredSet = (Set) registeredField.get(null); + registeredSet.clear(); } -} +} \ No newline at end of file From a69b82485e116a4339986e9a42a4106c33fb3c2a Mon Sep 17 00:00:00 2001 From: Vasilii Alferov Date: Mon, 11 Aug 2025 14:12:28 +0100 Subject: [PATCH 2/3] Re-import JKS keystore fixtures with PKCS12 format. This resolves CI pipeline failures due to java.io.IOException: DerInputStream.getLength(): lengthTag=109, too big --- .../resources/data/ssl/client_keystore.jks | Bin 2246 -> 2591 bytes .../resources/data/ssl/client_truststore.jks | Bin 986 -> 1218 bytes .../resources/data/ssl/server_keystore.jks | Bin 2276 -> 2615 bytes .../resources/data/ssl/server_truststore.jks | Bin 958 -> 1186 bytes 4 files changed, 0 insertions(+), 0 deletions(-) diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/client_keystore.jks b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/client_keystore.jks index 8636f41dd088b564573843725f639cc0bd01e400..56c0bdb7b8b07b266530cf977716547b9aa6e6a1 100644 GIT binary patch literal 2591 zcmY+Ec{CJ?7stmKV@&pDjIBX=_GKoCEDa&DjBSYQyBU-%lPyb;L4zmC5>u9kAtbvj z$yOq=Gsv2Ro^@n;OsOPx1*CB zvjiB@iJe2g%iQ;3$+0xKnYJ^Klcd%&4da?NL$wrbf!p0*{LwFTKG@*?G`;9A91V zn8qJ!o?9la)lXjv`k73Dr+a>MO^?V>;~3Eqnkql}1nR@v><_kEH)pT&vfFDGN%moSFk&+2A>~}+scDU(kF*mT zl>6rd<`;)$kTOU2UbbKY*X^@8jpRGbYZwKlHTXCU1N)J-x=E>!Z|yf2*qn&Q7xUE4jkHa5-TAtJ4GvayxT7zH%*jCEEB6Q7fVn{NP<{NM~0}cs_%;jak30-cyzH`hn>B zqKEfe$a(%FxcSpk?rnwbfgjB@tqiJ}Y>Z12FKY8QzRXH&1d}gDu-nTFaa)UGR>F{4;MXy0N}Wf~gttiNuIDc9(riZ4Yp`^8b=q%7IOJAPd5mSG@Op&or3X=;ZgS;<48JXsmUslE4*vMQ zoTgR!Xzgmcx$^oQ$8>1epv8}>3q#PRSp$(}3#eiHgwwKb1X)+^CHaqE&b@qLCfv>V z$H93wJw{D4djWChg|idsvFUsD_t@l}9D_R>>RJPzhdKWoS5*of`K5cgdQuKs+Aq$+ zlBSlPpJ8ZGJk7?%e6oGS)Wuum>cvZUdcaYXOM$e;usGSR@#WA zaKD|@j3Ktl*O7O=BT*Te_Pc4et2%eFB3HuQg3j3XOy~6W*bswag=SrvmLEMGABILo z&EK#=#$e&xxPTR$Q5{7>n7W^N>W)E4nb7+4E_i))m@Yh$=yOnhc{nxOO z(TwzKBHOOwJe)Xse*pg%7Tx%e#|y-w=R-$Xlhd{vm2ASVWxE0oll1L<9uDbuDNO#< zQ}}4vou863F0LBO_gmkqNtFxwu_QBjjBaQJ?yb{eE{jv zF>X01dCgg%aK+wgJNK+Vf(|$`Ymqhvb~H2!*#Yh(tn`$hxL2}M9o&CtePhK`E%ElK4o=rdr1s=D2}7C#DREpwudSLjodJIWi((i}vLrL9hbZfV^kf-6-1sZ0jve z^DTe)@C!AUC_CsU6A}FRsVExU*0${J6qaiqfFyX}=G~^Y(u&1{y9>Ie#hhvmg#vUY zPEwE`7%wp9Ky3!}VWJbOzGpNYkrA021%c=nJ`=w|OA8@GI4;Q-h5+-aTlQlWHo389 zLGKfzNL5xYRG|o!RZ|?=cof$!#zz^S&9x_tl{05W-21rhs7t!8U4u_pky9){#o{p$d_cQps&%XMKwI#lfR;{Ld=~JFc0AlE)dipsZIakm7Rf^vGNb`YZjc%%;hhW)|g>^k6f9RWwiRRV2IfmS! z&loMITui!@?B*)Uc+z3;+f=R5715`mK#s7cd4tnem?kGzyrBZ;1%A0}Pz$y(n$)|< zhkC`oo;WyCu1$XInwES`DzY3{D}=JorQ6zi9)Aw2n=t?L5SG7FyepU@FM-MUay(Yq zDPk3EHn!fF@OsiZ>gcP$m1*WCmngEBmrxKdN+n;DE+mFcz}(`hr5_S+HxGBU(c{sJGixe@>X literal 2246 zcmchY`8N~{7sqGDm|>7*EJ?-KnJ{BlmLxmPFk}yn8T-zYrNW@>FC|+UYnHK_>_j5j zo+Q%9lC4KFRAdeB)Omk+{(|?1`@{X5d(OS*d++()WBM^2004j(0sIpXypJcrFNBe% zJJ$WqgaE-nSr9i6$P56Jz^eh=eaZM?tLR1@vYi&FxLAuS8oA+V zvDW@YZhTxW#c~$_^g1gQCwV9U{hBK{^cEW%m$dGfRNAbtbBZkUC{m`bDOfdrBl4YJ zHo^lQi%}EO-U_7}1t+2{{l17dtwRUJ>h=t0l1+*ZpLyV;$%*I^!t!nn_IWd(doR8WXm1H^0jFQ z7gBf5PO<-d?J3EQ@S;%tXZ5)*T)7!MPYfiF2AsR#_3K^6$%57SMZFF(T z$66%_dNR~?N;;jJuK(I0@iLr_u2i<~`Y`o9bk}&;pvQ!mLsq$atGDyG!~(x_W4^_% z3Q=w7L7B{wn3KtF2^9jWW@<`>8N<^xaqf$}8yu}Za;&Z6MQYzo8icz(YkfQp)2t$v;uZG8L$dimPKbF3(8n4VA0iSN{Lun_4cKdI-&AYNHQE8~t-s#e=*0J$f zy@1S7F}+R>Ssp^Bw77Ot#1)vCh2wToru-sau6{CNDR;O2zAp`*qOV(agX>jRVlhax zJT9YDA4Cos+3hS-wbF~lCQg+($BSG2vX(<$(4JkQ z?6-1{^0d+Vi8)+*POP_&^xmivFIg2vlEvM0|P zEY@`>=`h`PY);{xYG7ua*k(Ci3Y|KJhK8?^_4y#i-IZ=;bYPu>4&D=4sj(lj1f3KV>vTUMxP4u+c zUt^onK_n)VatkQ?`Wwr@Wf&d#rLYwoj<~+U=h12`QL(K=3wNcTIsdCdHLyuGviBlw z?l{Sd*(|l83g*VBn?t|H4@eui4!$V(SJkX zBRYY51N1b1jta4BFNZ5v=O8f$m^~FBi{gi?vAng|1NZg#+HEv*H#H+VPvG^-AIO}hU3vmr0pt(_;zZn$H@~6@>Bpl7fa9aj~!J)qqCdeJlhGP9q*x(>zD>Nsg z9V20aL-hW5HA+N)Ls1!}j8a9bqcoHi?N4(mq8UI5t@J4tCEBdl@to#1fDy zkz#W6;MMXdjmrIiKKZx(S=2P)s@o!(-=yf1*+g-F;U014!OEal{NO@qD(u6kMdS96 zS;e&@Ybv`R<#M2)sLG1RiSP9$!nd>>`jZC~sFI*rGSpM?#R;|NM$m{v3hUF?N&&v6 zNz*yJYw9-P+`hyA2#!QY7j(FzDougfD{Hx|G5E=$n;Epbznt&1Cc z_^8~^cX=t*uBi)^Z{u+;4yi(VRZBR*qqsZhZ61y40VOdfcXAuP0k*Lm`15jN)KJIM z?#cL(qA73d1n~_wO5Jh{u>)asAJ(N2sOf&{(-0Ru3C1a}4rDuzgg_YDCD0ic2eXas@;WH5pRU@(FNTm}g$hDe6@ z4FLxRpn?QVFoFb00s#Opf&@Ya2`Yw2hW8Bt2LUiC1_~;MNQUAH z(oJg(!GJ>dbrg{f`I6<{0s{cUP=JC22(*l9k-guwP&3obJKG=afNWZF<8ZUzSF=-) z-{vZhko7ZdO2MJzm`VWiydvcsgybY8VWQ20)NPiNKSFkzU%VCY~(+~pS9S5s@)@wl*xWZ@e#yj%R@F&q=28S`_LqSU*O z+#6nQnEKA@^Ml5I8dn-2?FQ!Orbu`aHbewTq58sH78#W)+s=cUAlL>(x?2&Gmx%4% zk)oc)o$b5bD=JA^7HEziOjlgo`tlo8)0!A~`5dzEX_gP~Ke;qPy5Ug`0vN;W%qga~ zWSWl_lY+uD5;6Pl?yT8zwCaF zcm}wgkkbH6ofl=h6gHJ3z!*GKXcp`|#aEEEoTMudB}6g&eJz0H7CS4I%2 zDjIU97;FhaB-!^k0e_c*`S>5LghOR2bydT_q8(nJa?ov;S5~1CrizC5At6?gvCuI7 zx4;<9OhUM}GSsa-kf5mQHt+Dvc=>;CC)<|vW5lxSmP;JNpqD^*A{tyn&kLaH8786r zLnc~i#u{`Euy!I{rRm!snplNcPTKa44zG3*<8%nZpC@=x3oBMI;l>mB=1@;l|IzJz zW&c&F{|VR6lm}|z-6fH%_YYl_r$U@6XwKUCTAJIzO{EdO0iNbBpO2+NS=HR~KaBL2 z^9|6S{=Wdxh+BOee5;T|qK*f@^Jv zv@t7xBuL{MBX3+y;=Q|Y+s4BrBFh_3+*RBF9&0s{etpoC~VXaE2J literal 986 zcmezO_TO6u1_mY|W(3o0#i>PQsYO6;-$rA%^@w z>wp|)VYXnf8x0i1d5ugAObpBnEetFTO`|0EjSPVT#)igF0eU;AiBSnTNElfen41{+ z84Q{jxtN+585vH*N~^M%?llXT-CLNzom_PEw*7B@)gAI$r%r5WU=Iyj$sKIK^YrJr z>xXADfB!Y@fS=5r>$+Wgr_v~2Mk75hd&0SQ`H1|%+ ziT?>g{dMc-tUeykKJP;Hv7dKX)s)ZvzS8^bO_aL%i9NZm?^^5Mdh>mK5ufvWS$pYOYM5p$f)5-qh)XeG{i>>BlKWjNP%g};LC+yB1V$i1?5 zcEsmR)w~a_1ON5UyqCqQVAyA`98;yL>oV`6jskD5eUIGcd9U8h4vSRnu8>&yPewPn zX0?NCO%&Jeh_GK*PAoL$xgBTtcgnk65i5(ft(tR1+t=pIP7h5jd6}NIQ+6-Dxe|M(Q>)os+h26m@47HFW?0s{U>i diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/server_keystore.jks b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/server_keystore.jks index d524c6268a563a3a371d26812554b7ecf92a3458..2cfdbfc6d15619d85f56c986621687734860eecf 100644 GIT binary patch literal 2615 zcmY+EcQ_k*7sn+c_KJ~Qt0A~{6B6ap3fBl76d|QmqgJb^QF}C0u2ziJo(Woe)^1T- zDQdN9^-?uT#Hv-}b>HWG?|tqczw>*Z&-a}3oWDLuBAW^jz=$NWu7O$QqHs}roB(D3 zg~&PsBC>uy!{bOIlk2}CCMyt;$>I!~p1o^e_Ww|+RFz`|URe#X*MYcdi3;3G7~>n2 za&X!-H0BORq-~CDE=?CPIbMuF$1&;!)=+e_9uHq~y6f~szDpJ{INk{_+4>G4rK88T zqg;-2 zTNcu{V!e8i!ld2Dj>;eWxuDFSeQUx<%`x^$SXA~A`THYupFGMo$wcpJVhqk63kzpA z84-KLRr_o(I6>W+JKFJdupsLAHGZJj^6F%x8okhpZd2#%=gpZw#@sa%Y)_M0waIz- zjpggeay&#`)c`gXoD%o+K(sQcQF1iKOfRYGWa2^G;VYSrtgCugQuodoQP}F-E_)es z=?1nC_#bN!>qe`H@}=gARCS2;cn7PX>C_N^0Q~rt(Ruc~zM;7p=}Zy8%i}G9BXN%R=7l*ur$$TZ z$90HbieK<>L8<3JHN}t0%yAK%bGg^~LVTXGsjd!s%OuOb8unSuhubCvlxfu8 zhNoxEa}`1i1rtUBk5yR?>9#@g{Y ziv%;CWU?vDUM8b=>>!44WHgy%-Qrsekh^4F`=wt}O|iW{`Qdwuyj}aOYh`ruymiBl z6X$PU9&rF%HuYS)R9Xrb)+*@kb=<&cCdTd4N!QMewhGHhZ5UHovp}^SgZnT}>3im{ zo4P>Km?lN3XNzhhoDETZ%(k>45=M#h9bil#yfg82Cs`*G*pk!XiilD{5+V6jQ<>oFZ6rn|ivw_D>OC>dpm34E`mo(n2l(`vW3x~Iy ze{29;4Q#bDp!xd2ikhocr%C4&KRjnStRENbth*@1-&C-s1ei5vuD-FzT}Ob^MZ%0@ zDAv7L{>QtseH54^$&bo*dP3u5l7uryet40I3vE(u>iW3#D-ByEDIp;_?WE(W)@pz* z5kg^+!hfrhl^?|=#=+pt;K<;`@bIsS6#pj_K=Feh=C1c0;iwB3v^olN0j;WnMxM#@ z&fiVU;4`1%&(L)sfZ^;j`X>SW%dp_T44WQtGiBAcA=e_-+oBySoHEVR#%D!fsO)-rK7v!>1Z%0(jBJG%3qJeG%;ws7 zWEJ=axZEQ;CkJ7y)gaZbC6dl^%Bkbb70j!s6CYBVONFP0Gscp1M7g0!?-tam1yb+l z6f74aE_`ppJlcYh-aptMf3nz3 z*QJOo%-9-xVeRu0f%&SCH~X$#@T;l^;Qkj3_Qb!xm=V*9NoF z1c;s!u#7dp!CT+o1Jp?ZE#}lsI}&a$PzInDi)E#Ld+D!4wY!{W*FQSj;T8AkiRz7T zmeXn&g}MF3_e}jRh3$fzE8~ci)Xb}Ey6hoFMtesWuytQdH@yz;KkWJ3vNlX~0a5dt zDqN32H330cctT6)=%dL6KPc#2#jaaz(*SQ=K)GgXjls_wcQGtic5^-Xq*=i@Uw}k&Q5erH0Zrm-lIK+-*CyN%x<2{u6oC zrkY0dX8EnPE(Od56-&cn(fZTcit+3@rEgUW>+LASi!gqrfREpOoxJ<^Z?bFXwDeN( z+G)q0kqmeH?x_@XqEd~y^cUdqk{B1OyQ2ln!bg34r}hLE{pvfIr~RHLGd#-gLNYMu z6D~so>sGRxN~7d9+k}wZZ7by^r}ZDo;Ac#iQod@L$Q0+C-5A}&il(=n7x~%Id30n5 zUTQPUR5k>^#hF@C`K1*6jReKpuY?PE52Z^SoEsuYHKr!{ldr|DE|cM>5t$v#1hG&z za=#VZlH6MT_G{UEkkKVZs}Wmq(KwUtA=#X&V{m4Dl*Dt943d4+Br8p#z#u3OCu@*w zb>7a}HiUjzF2&%JWW`hSwh&Q=gD~~ui)dcmW1~)>l&w1u)y1p2dNR48hHFK|YcI}g zF0`>!=g)+4w%C1!YJyX14wif7)*lr0>fjhvV=Q7;drsIs#dSfq&uhrFsl%`PqTjvb z8s^9?cXBVSwT%AS9n4~^_>FSM*gp-=#}61B&?pHC{zEPsHq|ntJ+N%r>)C`c|Fcyz z9bXCHmpfF2aKf1p-cRdYjOG;O~s4O0SARe~vAYW4ERKi(}NFCoQ| ztY9X2E=Hgb69b5Uq%fPqg$8AoiB*$6lFo~|H(JjD;?K@4Y~8cS&wSS`_twL~Ds(!+ LDig>Eh@$)lE|}qV6;5bSVDG5 zgs~^G6h(!?NFhW#bo^dCf5Gp?_2T?o_i^0!abEXvUVF?vCI|$A92EFZ!Gk;lLp=iz zWct&;$Lv8M76?Fw?E&0y4tY2j29|^Hg25~x7#X&RrRO^0RBjBOV!euWf-06fi<@VW;!n3%0wLLTE)O+?z*e$ZjGB*dVl-D z#eBd-HjTh?z!$wPU=}w|vcP0VaRs?fzo?JD$Hae&d~_k0X*dZobhoEhZnS0g=TWm& zN)(FevHh56Xl+7(pS%=&*|jzT&dYx-%f(xH@^YQ5B?4;IN{yZ~T<|mg4))ibXB~UY z(K_hN=@OLspi~z{gnyjLSpCNq=nq!f&7=Bl8S{~&=0bZ1x@yAO7#1Gpx&-R@oG@}C zKHXYeiHOq(-T7e4jEv7-dfxs}YMxx^^cEE*|6!{-;rvG_t`*ALcpJ9y`)$5rZ+zCC zk4B6YWvkt%?XEV4^TCo)(6Hkb7P~5ZxEKC;e^p&Iv(tHtdbHY3JrTyL>vQFGIU|*d zD{j>nznX&ffE@W15Jcd2IJJRu5U#70-?inGVHJcZRq8D~=QS{ieTFFu5gF2onJKVc zsYjivJJlg@Dd+kSWn+IYO*MVxy;$8@h1oa=Pn7!YnCud#H-rwo4C-mRT5NCKa>%vv zBt80YaaZwUszY+@XZ1Ogu;ACu+ya-77MHj_w}uj&9>>fL5;moivE@E2*hTidch*k6M3D}9 z`J-@{#kH+xM$eRcuyW}A0)Ke8!xyJ!BmE!TKk=_Lca_zB1MlhAK2_lRhGRETZ~}J{ z8p=H^v!NzzrqkhdT1Tp@p9tPej-JsL{zc)8C^_9P{~W#NxQxy4%Mv@InvsT*#FLQl zPl-dd0&*rKM6&*L9w(;A{@R`9FZUl%%?*s<l1x5?~3IRr{AE_uYm@C7s$hED=ZR z$bxS6Tk6CnB-|~8xtE-F+dX7NP;*d9KFU<|BWIk~6pf?t!49WFQT6mW>%X25 z4fD6APEFMJe57@IcJ!Opo9G>v+!7oxju3@AC!mB4Xt@?u$q3t7&ku@!H)~s& z=}cfNCfzU+N39BWMZZpoAtYmI+8pN3ta~{qQ#L1=3*W{x*4LjmHtKwsKDjjF(4^>o zH`VN&X2zCH^xJcq?6Fj&F737qqCAXjE*I*p6{KH#BMLTZRPlK`BU{socvOg4Xpr2( zn;CP*veIz9yHr7NV0{ODI$F#WbmrY%2AOJ_Luosa+m2qK4iNt&Ba5?=8lVXRLGuAJ z^d3Nl#1=y#UTjCt+?TyS=|M6<+AE-)Cu0s{Q7D1Z+Rb%eqB5iX>VKtoc9e{f&~ zmJi@Lq-^}`htwgM80?AV1(1h~m7nd;NuS^dEYCsS9v}{W)3QB zMIKytAfWtkhd*fzh@!bvZ~zWa$DRZMYstaJfRe#T z5I_cVK*(S)h;ja?Buwp@x^4cg05^n3VCOsi9i(IfMy{9fB#zY)PeV8WoO9cQHI(Ru z?mPG|`1A14p7sWWo z5QA2Bd{ns7PHg>v;c4|**YBGXUxZKf;J?o5xk3|y#EAFbOBM6|!d&V0`I(fMw2=Q; zMm%>3-=kz|m0|xM6}Y6v}sg_a8a27fBStev?_-m8n=p1#slB%?slPc**R( zBoCtRWl~UZ5-j#$LI2m_gFkm*Fc>7l7IkjQXIVwm=9Tv8#EtctW{y6xNV?TZg>!+S zI#E-eAxF&nWK5W+IdRmB{^m8U|Giv=phf39uT@{?n$@=3%lP~K6&6u!y1s;Kq@z01n@fynFNkICQRn_M@$%yb)^`1nJ(a`FQFld86r7l=s>b>N7L;upSh^BM)GX9%Vg r<7if)2G~%`grZclC!b$4$rl%iYqB47iW08)7YV`TvW{*o%WnP)0leK{ diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/server_truststore.jks b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/server_truststore.jks index faa3228368eafcfea74aa091b4b164d26d1852a9..33d022ef0308da65d1d627b5e4ca4693028d9809 100644 GIT binary patch literal 1186 zcmV;T1YP?uf&`ud0Ru3C1Xl(LDuzgg_YDCD0ic2eNCbieL@Tyc`wqS=+ z0tB&7YZtKRkJIxiY>YsP`)LH<#!#AbS*fxgtaq*HPPaR)`T4!ap440ENmI+IV;83E}K%1H(>w$31df_ znXq^NhTVZB8KK+fy=r9+L==1Nb2$qdOBgMVD`G>)|B=aglak0Re-1!%%oL=0rkxH@ zTgViMtY{k$3;K>ch>62KE(o*VYD+>!lWAF_v*$XEexL^9g~6v2Nt7rsTS*?+Mf=*G z3BMO58y@W(i}!A)nfAu>IolDhj25A4eISX8*9vs{Bopi^H0KkR@BGZO_{s{6r_7!<%37E%5Pdkn$=(2U@f$wb%M0 ztx@KiY$-l5umom`mpzYTxkbNnw`M3Do#tlf$cv17CZrGt>UAm$j{A*Va zAr%wIIcbD(q6vfaD0W0x zUbk+w-rQtmQoF-N=9qhFWg427_Q^I*;ND-nS^$&-Dqd6)I1Lh7IdaubqmA&3pZymN zyo;Gph_-o@ndu~OZ_>B3%_TQ9>$@5_IML*?o*r7ZY?_v%j3h3o|W2>YqkvZOrDap!3Fu!#zw+@j#JxD%{PRTh3uc;c@^tKqf} z-oMk_I4~H|DXiCYEWOo{Q2Nh2`|vxdTG>c!gmz^7k}jd#NKS1t7kbLx4g z7Ie?xYrEA6Y3YlJN(1+6!wx_Y=`fb{(+6YMt0xfj`UTjj5v1M1fq^Jep4FPZUDWqx z1VY}uF`KME^Cmfxs!wE4lLq&>QSfREy2@Lbjv|Gqn37(|tuQ_?AutIB1uG5%0vZJX z1Qf#om0zQa%_Q~Z&Dy!@lgF0~ig^SS^+(|Y^jUq}q&`M#AH_R7WH~__0s{etpkmA? AlmGw# literal 958 zcmezO_TO6u1_mY|W(3o0$vK&+c_lz@-22D)K3z(T0 znV4AWQ_7tTc-c6$+C196^D;7WvoaXu8j2VQu`!3TunBWV=9d;Z=a=S{6jd4u83=$x zIfOYuqQNDJC8>sj2K*o~c3}>%%FL2VLjeOmkPw$JJ4ndCDBY0TfDptpmX7?qHNgprkjxrvdV z!Jvtei>Zl`k)fkXq~T1#)ScUu(&Xlyxp{H(s@VUZKFMlvJ3L6EXjAenw zX097&12=s+<+H-?Z|G*8yj2c`l1ipeGydm>ZB6~qH{-*p1zvN*_eFwRMNzob${Y( zx6gXRtmw<~=pv?&H<3+aYwaS#;N>hNUgsn-5$KRkCM{KfHU- zrA1-I>fc_;E}eAb@{8#03ww(-nV1GO6bZ{&LSr0}alXZ@+&OZQ)GJ+tt7c4&vnyOdSypERcJRt#wm-nlyZc3am` zwFs63`J$7j^1~ZvdQac=bk6mdfWqj|x6alTS&CEM?8@8AD^ z!Jc&O)RSVI+ua%DJ7aBgr!2PF^0=FAZs5du>ARj^7s^f!&z#Mr&Bpd@%h@>o^49=P CnQXTJ From 121a1cb0ca994b60efd698716fb033dcc9f55956 Mon Sep 17 00:00:00 2001 From: Vasilii Alferov Date: Thu, 21 Aug 2025 10:26:28 +0100 Subject: [PATCH 3/3] Skip gauge metric when callback returns NULL --- .../metrics/prometheus/PrometheusMetricsProvider.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java index 5202822a03a..1fbb34169c7 100644 --- a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java +++ b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java @@ -385,10 +385,9 @@ public void registerGauge(String name, Gauge gauge) { GaugeWithCallback newGauge = GaugeWithCallback.builder().name(name).help(name).callback(callback -> { Number value = gauge.get(); - if (value == null) { - throw new IllegalArgumentException("Gauge " + name + " cannot return a null value."); + if (value != null) { + callback.call(value.doubleValue()); } - callback.call(value.doubleValue()); }).register(registry); registeredGauges.put(name, newGauge); }