Enhancement Request 37553978 - [37149363->14.1.2.0.2] Metrics generated by Coherence should conform to OpenObservability spec (14.1.2.0 cl 114055 --> ce/14.1.2.0)

fryp · fryp · commit 2eabf20ce02e · 2025-02-04T12:15:44.000-05:00
[git-p4: depot-paths = "//dev/coherence-ce/release/coherence-ce-v14.1.2.0/": change = 114079]
diff --git a/prj/coherence-core/src/main/java/com/tangosol/internal/metrics/MetricsHttpHandler.java b/prj/coherence-core/src/main/java/com/tangosol/internal/metrics/MetricsHttpHandler.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2024, Oracle and/or its affiliates.
+ * Copyright (c) 2000, 2025, Oracle and/or its affiliates.
  *
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * https://oss.oracle.com/licenses/upl.
@@ -733,7 +733,7 @@ private void writeTags(Writer writer, Map<String, String> mapTags) throws IOExce
                           .append('"');
                     if (iterator.hasNext())
                         {
-                        writer.append(", ");
+                        writer.append(",");
                         }
                     }
 
diff --git a/prj/test/functional/metrics/pom.xml b/prj/test/functional/metrics/pom.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--
-  Copyright (c) 2000, 2024, Oracle and/or its affiliates.
+  Copyright (c) 2000, 2025, Oracle and/or its affiliates.
 
   Licensed under the Universal Permissive License v 1.0 as shown at
   https://oss.oracle.com/licenses/upl.
@@ -27,6 +27,54 @@
         <skipTests>false</skipTests>
       </properties>
     </profile>
+    <profile>
+      <id>metrics-format-test</id>
+      <activation>
+        <activeByDefault>false</activeByDefault>
+      </activation>
+      <properties>
+        <skipTests>false</skipTests>
+      </properties>
+      <build>
+        <plugins>
+          <plugin>
+            <groupId>org.apache.maven.plugins</groupId>
+            <artifactId>maven-failsafe-plugin</artifactId>
+            <version>${maven.failsafe.plugin.version}</version>
+            <executions>
+              <execution>
+                <goals>
+                  <goal>integration-test</goal>
+                  <goal>verify</goal>
+                </goals>
+                <configuration>
+                  <includes>
+                    <include>**/MetricsFormatTests.java</include>
+                  </includes>
+                </configuration>
+              </execution>
+            </executions>
+          </plugin>
+          <plugin>
+            <groupId>org.codehaus.mojo</groupId>
+            <artifactId>exec-maven-plugin</artifactId>
+            <executions>
+              <execution>
+                <id>Metrics Format Tests</id>
+                <phase>verify</phase>
+                <goals>
+                  <goal>exec</goal>
+                </goals>
+              </execution>
+            </executions>
+            <configuration>
+              <skip>${skipTests}</skip>
+              <executable>${project.basedir}/src/main/python/run_test.sh</executable>
+            </configuration>
+          </plugin>
+        </plugins>
+      </build>
+    </profile>
   </profiles>
 
   <properties>
diff --git a/prj/test/functional/metrics/src/main/java/metrics/MetricsFormatTests.java b/prj/test/functional/metrics/src/main/java/metrics/MetricsFormatTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2024, Oracle and/or its affiliates.
+ * Copyright (c) 2000, 2025, Oracle and/or its affiliates.
  *
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * https://oss.oracle.com/licenses/upl.
@@ -20,9 +20,13 @@
 import org.junit.runners.Parameterized;
 
 import java.io.BufferedReader;
+import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.io.PrintWriter;
+
 import java.net.HttpURLConnection;
 import java.net.URI;
+
 import java.util.ArrayList;
 import java.util.Collection;
 
@@ -132,6 +136,56 @@ public void shouldHaveCorrectNameFormat() throws Exception
             }
         }
 
+    /**
+     * Just extracts metrics into the file so their format can be verified by an external tool.
+     *
+     * @throws Exception
+     */
+    @Test
+    public void shouldHaveCorrectFormat() throws Exception
+        {
+        LocalPlatform    platform = LocalPlatform.get();
+        Capture<Integer> port     = new Capture<>(platform.getAvailablePorts());
+
+        // start Coherence with metrics configured with the test format
+        try (CoherenceClusterMember member = platform.launch(CoherenceClusterMember.class,
+                    SystemProperty.of(f_sProperty, f_fPropertyValue),
+                    SystemProperty.of(MetricsHttpHelper.PROP_METRICS_ENABLED, true),
+                    SystemProperty.of("coherence.metrics.http.port", port),
+                    IPv4Preferred.yes(),
+                    LocalHost.only()))
+            {
+            Eventually.assertDeferred(() -> member.isServiceRunning(MetricsHttpHelper.getServiceName()), is(true));
+
+            String sURL = "http://127.0.0.1:" + port.get() + "/metrics";
+
+            HttpURLConnection con = (HttpURLConnection) URI.create(sURL).toURL().openConnection();
+            con.setRequestProperty("Accept", "text/plain");
+            con.setRequestMethod("GET");
+
+            int responseCode = con.getResponseCode();
+            assertThat(responseCode, is(200));
+
+            try (PrintWriter writer = new PrintWriter("target/" + f_sFormat + ".metrics.txt");
+                 InputStream in = con.getInputStream())
+                {
+                BufferedReader isr = new BufferedReader(new InputStreamReader(in));
+                String line;
+                int counter = 0;
+                while ((line = isr.readLine()) != null )
+                    {
+                    writer.print(line + "\n");
+                    // limit example size otherwise verification takes too much time to complete
+                    if (counter++ == 110)
+                        {
+                        break;
+                        }
+                    }
+                writer.print("# EOF");
+                }
+            }
+        }
+
     // ----- data members ---------------------------------------------------
 
     /**
diff --git a/prj/test/functional/metrics/src/main/python/run_test.sh b/prj/test/functional/metrics/src/main/python/run_test.sh
@@ -0,0 +1,28 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2000, 2025, Oracle and/or its affiliates.
+#
+# Licensed under the Universal Permissive License v 1.0 as shown at
+# https://oss.oracle.com/licenses/upl.
+#
+
+echo "Metrics Format Test"
+echo "$(pwd)"
+
+cd src/main/python || exit 1
+rm -rf .pyenv || true
+git clone --branch v2.3.22 https://github.com/pyenv/pyenv.git .pyenv
+export PYENV_ROOT=$(pwd)/.pyenv
+export PATH=${PYENV_ROOT}/bin:${PATH}
+eval "$(pyenv init --path)"
+echo $PATH
+
+${PYENV_ROOT}/bin/pyenv install 3.11
+${PYENV_ROOT}/bin/pyenv versions
+${PYENV_ROOT}/bin/pyenv global 3.11
+which python
+
+pip install abnf
+
+echo "Current path: $(pwd)"
+python ./verify_metrics_format.py
diff --git a/prj/test/functional/metrics/src/main/python/verify_metrics_format.py b/prj/test/functional/metrics/src/main/python/verify_metrics_format.py
@@ -0,0 +1,90 @@
+#!/usr/bin/env python3
+#
+# Copyright (c) 2000, 2025, Oracle and/or its affiliates.
+#
+# Licensed under the Universal Permissive License v 1.0 as shown at
+# https://oss.oracle.com/licenses/upl.
+#
+
+import pathlib
+import os
+import datetime
+from abnf.parser import (
+    ABNFGrammarNodeVisitor,
+    ABNFGrammarRule,
+    Alternation,
+    CharValNodeVisitor,
+    Concatenation,
+    GrammarError,
+    Literal,
+    LiteralNode,
+    Match,
+    Node,
+    NumValVisitor,
+    Option,
+    ParseCache,
+    ParseError,
+    Prose,
+    Repeat,
+    Repetition,
+    Rule,
+    next_longest,
+    sorted_by_longest_match,
+)
+from urllib.request import urlopen
+
+class FromFileRule(Rule):
+    pass
+
+def fetch_abnf_source():
+    print("Fetching ABNF file...")
+    with urlopen('https://raw.githubusercontent.com/prometheus/OpenMetrics/refs/heads/main/specification/OpenMetrics.md') as response:
+        md = response.read().decode('utf-8')
+        abnf = []
+        copy = False
+        for line in md.split('\n'):
+            if "~~~~ abnf" in line:
+                copy = True
+                continue
+            elif "~~~~" in line and copy:
+                break
+            elif copy:
+                abnf.append(line)
+        if len(abnf) < 30:
+            raise Exception("Failed to extract ABNF grammar from the source document")
+        return '\n'.join(abnf)
+
+def test_file(parser, filename):
+    print(datetime.datetime.now())
+    print(f"Processing file: {filename}")
+    metrics = pathlib.Path(filename).read_text()
+    parser.parse_all(metrics)
+    print(datetime.datetime.now())
+    print(f"Parsed successfully: {filename}")
+
+abnf = fetch_abnf_source()
+print("Writing ABNF file...")
+with open("../../../target/metrics.abnf", "w") as f:
+    f.write(abnf)
+
+print("Loading ABNF file...")
+path = pathlib.Path("../../../target/metrics.abnf")
+
+print("Creating rule from the file...")
+FromFileRule.from_file(path)
+
+print("Creating parser...")
+parser = FromFileRule.get("exposition")
+
+files = os.listdir("../../../target/")
+print("Searching for metrics files")
+matching_files = [f for f in files if f.endswith(".metrics.txt")]
+if len(matching_files) == 0:
+    raise Exception("Metrics files to check are missing")
+
+for filename in matching_files:
+    if filename == "Dot Delimited.metrics.txt":
+        # dot delimited metrics don't conform to the specs
+        continue
+    print(f"Test file {filename}")
+    test_file(parser, "../../../target/" + filename)
diff --git a/prj/test/unit/coherence-core-tests/src/test/java/com/tangosol/internal/metrics/PrometheusFormatterTest.java b/prj/test/unit/coherence-core-tests/src/test/java/com/tangosol/internal/metrics/PrometheusFormatterTest.java
@@ -1,8 +1,8 @@
 /*
- * Copyright (c) 2000, 2022, Oracle and/or its affiliates.
+ * Copyright (c) 2000, 2025, Oracle and/or its affiliates.
  *
  * Licensed under the Universal Permissive License v 1.0 as shown at
- * http://oss.oracle.com/licenses/upl.
+ * https://oss.oracle.com/licenses/upl.
  */
 package com.tangosol.internal.metrics;
 
@@ -42,7 +42,7 @@ public void testMetricWithTags() throws IOException
         new PrometheusFormatter(false, MetricsHttpHandler.Format.Legacy, Collections.singletonList(metric))
                 .writeMetrics(writer);
 
-        String expected = "vendor:coherence_cluster_size{cluster=\"testCluster\", node_id=\"1\", site=\"testSite\"} 3\n";
+        String expected = "vendor:coherence_cluster_size{cluster=\"testCluster\",node_id=\"1\",site=\"testSite\"} 3\n";
 
         assertThat(writer.toString(), equalTo(expected));
         }
@@ -56,7 +56,7 @@ public void testMetricWithMicroprofileName() throws IOException
         new PrometheusFormatter(false, MetricsHttpHandler.Format.Microprofile, Collections.singletonList(metric))
                 .writeMetrics(writer);
 
-        String expected = "vendor_Coherence_ClusterSize{cluster=\"testCluster\", node_id=\"1\", site=\"testSite\"} 3\n";
+        String expected = "vendor_Coherence_ClusterSize{cluster=\"testCluster\",node_id=\"1\",site=\"testSite\"} 3\n";
 
         assertThat(writer.toString(), equalTo(expected));
         }
@@ -70,7 +70,7 @@ public void testMetricWithDotFormattedName() throws IOException
         new PrometheusFormatter(false, MetricsHttpHandler.Format.DotDelimited, Collections.singletonList(metric))
                 .writeMetrics(writer);
 
-        String expected = "coherence.cluster.size{cluster=\"testCluster\", node_id=\"1\", site=\"testSite\"} 3\n";
+        String expected = "coherence.cluster.size{cluster=\"testCluster\",node_id=\"1\",site=\"testSite\"} 3\n";
 
         assertThat(writer.toString(), equalTo(expected));
         }
@@ -86,7 +86,7 @@ public void testExtendedMetricWithTags() throws IOException
 
         String expected = "# TYPE vendor:coherence_cluster_size gauge\n"
                           + "# HELP vendor:coherence_cluster_size Cluster size\n"
-                          + "vendor:coherence_cluster_size{cluster=\"testCluster\", node_id=\"1\", site=\"testSite\"} 3\n";
+                          + "vendor:coherence_cluster_size{cluster=\"testCluster\",node_id=\"1\",site=\"testSite\"} 3\n";
 
         assertThat(writer.toString(), equalTo(expected));
         }

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright (c) 2000, 2024, Oracle and/or its affiliates.`
	`2`	`+ * Copyright (c) 2000, 2025, Oracle and/or its affiliates.`
`3`	`3`	`*`
`4`	`4`	`* Licensed under the Universal Permissive License v 1.0 as shown at`
`5`	`5`	`* https://oss.oracle.com/licenses/upl.`
`@@ -733,7 +733,7 @@ private void writeTags(Writer writer, Map<String, String> mapTags) throws IOExce`
`733`	`733`	`.append('"');`
`734`	`734`	`if (iterator.hasNext())`
`735`	`735`	`{`
`736`		`- writer.append(", ");`
	`736`	`+ writer.append(",");`
`737`	`737`	`}`
`738`	`738`	`}`
`739`	`739`