You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by ch...@apache.org on 2023/05/04 03:47:09 UTC

[bookkeeper] branch master updated: Fix some metrics generated by prometheus client without type info (#3927)

This is an automated email from the ASF dual-hosted git repository.

chenhang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 16282e71df Fix some metrics generated by prometheus client without type info (#3927)
16282e71df is described below

commit 16282e71dfa88c5b29356a263a2a78669329bdb5
Author: Hang Chen <ch...@apache.org>
AuthorDate: Thu May 4 11:47:03 2023 +0800

    Fix some metrics generated by prometheus client without type info (#3927)
    
    ### Motivation
    When we get the metrics output from Prometheus, we found the metrics generated by the Prometheus client without type info, which will lead to some monitor integration systems can't parse the metrics
    ```
    jvm_gc_collection_seconds_count{gc="G1 Young Generation"} 4.0
    jvm_gc_collection_seconds_sum{gc="G1 Young Generation"} 2.287
    jvm_gc_collection_seconds_count{gc="G1 Old Generation"} 0.0
    jvm_gc_collection_seconds_sum{gc="G1 Old Generation"} 0.0
    jvm_memory_direct_bytes_max{} 2.147483648E9
    jvm_threads_current{} 136.0
    jvm_threads_daemon{} 18.0
    jvm_threads_peak{} 137.0
    jvm_threads_started_total{} 415.0
    jvm_threads_deadlocked{} 0.0
    jvm_threads_deadlocked_monitor{} 0.0
    process_cpu_seconds_total{} 740.860392
    process_start_time_seconds{} 1.681901598385E9
    process_open_fds{} 484.0
    process_max_fds{} 10240.0
    log4j2_appender_total{level="debug"} 0.0
    log4j2_appender_total{level="warn"} 20.0
    log4j2_appender_total{level="trace"} 0.0
    log4j2_appender_total{level="error"} 3.0
    log4j2_appender_total{level="fatal"} 0.0
    log4j2_appender_total{level="info"} 317.0
    caffeine_cache_hit_total{cache="bookies-racks-exists"} 0.0
    caffeine_cache_hit_total{cache="bookies-racks-children"} 0.0
    caffeine_cache_hit_total{cache="bookies-racks-data"} 2.0
    caffeine_cache_miss_total{cache="bookies-racks-exists"} 0.0
    caffeine_cache_miss_total{cache="bookies-racks-children"} 0.0
    caffeine_cache_miss_total{cache="bookies-racks-data"} 2.0
    caffeine_cache_requests_total{cache="bookies-racks-exists"} 0.0
    caffeine_cache_requests_total{cache="bookies-racks-children"} 0.0
    caffeine_cache_requests_total{cache="bookies-racks-data"} 4.0
    caffeine_cache_eviction_total{cache="bookies-racks-exists"} 0.0
    caffeine_cache_eviction_total{cache="bookies-racks-children"} 0.0
    ```
    
    ### Changes
    Write the metrics' type info to the output.
---
 .../stats/prometheus/PrometheusTextFormat.java     | 27 +++++++++++
 .../stats/prometheus/PrometheusTextFormatTest.java | 39 ---------------
 .../stats/prometheus/TestPrometheusFormatter.java  | 56 ++++++++++++++++++++++
 3 files changed, 83 insertions(+), 39 deletions(-)

diff --git a/stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusTextFormat.java b/stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusTextFormat.java
index a3bf919f1f..20fc1f5a49 100644
--- a/stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusTextFormat.java
+++ b/stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusTextFormat.java
@@ -178,6 +178,9 @@ public class PrometheusTextFormat {
         Enumeration<MetricFamilySamples> metricFamilySamples = registry.metricFamilySamples();
         while (metricFamilySamples.hasMoreElements()) {
             MetricFamilySamples metricFamily = metricFamilySamples.nextElement();
+            // Write type of metric
+            w.append("# TYPE ").append(metricFamily.name).append(getTypeNameSuffix(metricFamily.type)).append(' ')
+                    .append(getTypeStr(metricFamily.type)).write('\n');
 
             for (int i = 0; i < metricFamily.samples.size(); i++) {
                 Sample sample = metricFamily.samples.get(i);
@@ -200,6 +203,30 @@ public class PrometheusTextFormat {
         }
     }
 
+    static String getTypeNameSuffix(Collector.Type type) {
+        if (type.equals(Collector.Type.INFO)) {
+            return "_info";
+        }
+        return "";
+    }
+
+    static String getTypeStr(Collector.Type type) {
+        switch (type) {
+            case COUNTER:
+                return "counter";
+            case GAUGE:
+            case INFO:
+                return "gauge";
+            case SUMMARY:
+                return "summary";
+            case HISTOGRAM:
+                return "histogram";
+            case UNKNOWN:
+            default:
+                return "unknown";
+        }
+    }
+
     void writeType(Writer w, String name, String type) throws IOException {
         if (metricNameSet.contains(name)) {
             return;
diff --git a/stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/PrometheusTextFormatTest.java b/stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/PrometheusTextFormatTest.java
deleted file mode 100644
index bce5fce480..0000000000
--- a/stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/PrometheusTextFormatTest.java
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * 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.bookkeeper.stats.prometheus;
-
-import java.io.IOException;
-import java.io.StringWriter;
-import org.junit.Assert;
-import org.junit.Test;
-
-/**
- * Test for {@link PrometheusTextFormat}.
- */
-public class PrometheusTextFormatTest {
-
-    @Test
-    public void testPrometheusTypeDuplicate() throws IOException {
-        PrometheusTextFormat prometheusTextFormat = new PrometheusTextFormat();
-        StringWriter writer = new StringWriter();
-        prometheusTextFormat.writeType(writer, "counter", "gauge");
-        prometheusTextFormat.writeType(writer, "counter", "gauge");
-        String string = writer.toString();
-        Assert.assertEquals("# TYPE counter gauge\n", string);
-    }
-
-}
diff --git a/stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusFormatter.java b/stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusFormatter.java
index 7ef06eb4de..8855044f24 100644
--- a/stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusFormatter.java
+++ b/stats/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusFormatter.java
@@ -19,11 +19,19 @@ package org.apache.bookkeeper.stats.prometheus;
 import static com.google.common.base.Preconditions.checkArgument;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Splitter;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Multimap;
+import io.prometheus.client.CollectorRegistry;
+import io.prometheus.client.Gauge;
+import io.prometheus.client.hotspot.GarbageCollectorExports;
+import io.prometheus.client.hotspot.MemoryPoolsExports;
+import io.prometheus.client.hotspot.StandardExports;
+import io.prometheus.client.hotspot.ThreadExports;
+import java.io.IOException;
 import java.io.StringWriter;
 import java.util.List;
 import java.util.Map;
@@ -168,6 +176,54 @@ public class TestPrometheusFormatter {
         assertTrue(found);
     }
 
+    @Test
+    public void testWriteMetricsCollectedByPrometheusClient() {
+        CollectorRegistry registry = CollectorRegistry.defaultRegistry;
+        registry.register(new StandardExports());
+        registry.register(new MemoryPoolsExports());
+        registry.register(new GarbageCollectorExports());
+        registry.register(new ThreadExports());
+        registry.register(Gauge.build("jvm_memory_direct_bytes_used", "-").create().setChild(new Gauge.Child() {
+            @Override
+            public double get() {
+                return 1.0;
+            }
+        }));
+        registry.register(Gauge.build("jvm_memory_direct_bytes_max", "-").create().setChild(new Gauge.Child() {
+            @Override
+            public double get() {
+                return 100.0;
+            }
+        }));
+        PrometheusMetricsProvider provider = new PrometheusMetricsProvider(registry);
+        StringWriter writer = new StringWriter();
+        try {
+            provider.rotateLatencyCollection();
+            provider.writeAllMetrics(writer);
+            String output = writer.toString();
+            parseMetrics(output);
+            assertTrue(output.contains("# TYPE jvm_memory_direct_bytes_max gauge"));
+            assertTrue(output.contains("# TYPE jvm_memory_direct_bytes_used gauge"));
+            assertTrue(output.contains("# TYPE jvm_gc_collection_seconds summary"));
+            assertTrue(output.contains("# TYPE jvm_memory_pool_bytes_committed gauge"));
+            assertTrue(output.contains("# TYPE process_cpu_seconds counter"));
+        } catch (Exception e) {
+            fail();
+        }
+
+    }
+
+    @Test
+    public void testPrometheusTypeDuplicate() throws IOException {
+        PrometheusTextFormat prometheusTextFormat = new PrometheusTextFormat();
+        StringWriter writer = new StringWriter();
+        prometheusTextFormat.writeType(writer, "counter", "gauge");
+        prometheusTextFormat.writeType(writer, "counter", "gauge");
+        String string = writer.toString();
+        assertEquals("# TYPE counter gauge\n", string);
+    }
+
+
     /**
      * Hacky parsing of Prometheus text format. Sould be good enough for unit tests
      */