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..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 @@ -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,494 @@ 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) { + 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 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 8636f41dd08..56c0bdb7b8b 100644 Binary files a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/client_keystore.jks and b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/client_keystore.jks differ diff --git a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/client_truststore.jks b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/client_truststore.jks index 3e5893d0b6b..23fc75232fd 100644 Binary files a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/client_truststore.jks and b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/client_truststore.jks differ 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 d524c6268a5..2cfdbfc6d15 100644 Binary files a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/server_keystore.jks and b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/server_keystore.jks differ 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 faa3228368e..33d022ef030 100644 Binary files a/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/server_truststore.jks and b/zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/test/resources/data/ssl/server_truststore.jks differ