Code review followup changes

open-telemetry · Sep 26, 2024 · c15f40d · c15f40d
1 parent 7d0be38
commit c15f40d
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 153 deletions.
diff --git a/...ationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/JvmIntegrationTest.java b/...ationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/JvmIntegrationTest.java
@@ -7,6 +7,8 @@
 
 import io.opentelemetry.contrib.jmxscraper.JmxScraperContainer;
 import io.opentelemetry.contrib.jmxscraper.TestAppContainer;
+import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest;
+import java.util.List;
 import org.testcontainers.containers.GenericContainer;
 
 public class JvmIntegrationTest extends TargetSystemIntegrationTest {
@@ -21,4 +23,9 @@ protected GenericContainer<?> createTargetContainer(int jmxPort) {
   protected JmxScraperContainer customizeScraperContainer(JmxScraperContainer scraper) {
     return scraper.withTargetSystem("jvm");
   }
+
+  @Override
+  protected void verifyMetrics(List<ExportMetricsServiceRequest> metrics) {
+    // TODO: Verify gathered metrics
+  }
 }
diff --git a/.../java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java b/.../java/io/opentelemetry/contrib/jmxscraper/target_systems/TargetSystemIntegrationTest.java
@@ -120,10 +120,11 @@ void endToEndTest() {
     scraper = customizeScraperContainer(scraper);
     scraper.start();
 
-    // TODO: replace with real assertions
-    assertThat(otlpServer.getMetrics()).isEmpty();
+    verifyMetrics(otlpServer.getMetrics());
   }
 
+  protected abstract void verifyMetrics(List<ExportMetricsServiceRequest> metrics);
+
   protected JmxScraperContainer customizeScraperContainer(JmxScraperContainer scraper) {
     return scraper;
   }

diff --git a/...onTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java b/...onTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/TomcatIntegrationTest.java
@@ -6,7 +6,9 @@
 package io.opentelemetry.contrib.jmxscraper.target_systems;
 
 import io.opentelemetry.contrib.jmxscraper.JmxScraperContainer;
+import io.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest;
 import java.time.Duration;
+import java.util.List;
 import org.testcontainers.containers.GenericContainer;
 import org.testcontainers.containers.wait.strategy.Wait;
 import org.testcontainers.images.builder.ImageFromDockerfile;
@@ -44,4 +46,9 @@ protected GenericContainer<?> createTargetContainer(int jmxPort) {
   protected JmxScraperContainer customizeScraperContainer(JmxScraperContainer scraper) {
     return scraper.withTargetSystem("tomcat");
   }
+
+  @Override
+  protected void verifyMetrics(List<ExportMetricsServiceRequest> metrics) {
+    // TODO: Verify gathered metrics
+  }
 }
diff --git a/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/JmxScraperConfig.java b/jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/JmxScraperConfig.java
@@ -5,7 +5,7 @@
 
 package io.opentelemetry.contrib.jmxscraper.config;
 
-import static io.opentelemetry.contrib.jmxscraper.util.StringUtils.isBlank;
+import static io.opentelemetry.contrib.jmxscraper.internal.StringUtils.isBlank;
 
 import java.util.Arrays;
 import java.util.Collections;
@@ -35,19 +35,20 @@ public class JmxScraperConfig {
   static final String JMX_REALM = "otel.jmx.realm";
 
   static final List<String> AVAILABLE_TARGET_SYSTEMS =
-      Arrays.asList(
-          "activemq",
-          "cassandra",
-          "hbase",
-          "hadoop",
-          "jetty",
-          "jvm",
-          "kafka",
-          "kafka-consumer",
-          "kafka-producer",
-          "solr",
-          "tomcat",
-          "wildfly");
+      Collections.unmodifiableList(
+          Arrays.asList(
+              "activemq",
+              "cassandra",
+              "hbase",
+              "hadoop",
+              "jetty",
+              "jvm",
+              "kafka",
+              "kafka-consumer",
+              "kafka-producer",
+              "solr",
+              "tomcat",
+              "wildfly"));
 
   private String serviceUrl = "";
   private String customJmxScrapingConfigPath = "";
@@ -164,7 +165,7 @@ public static JmxScraperConfig fromProperties(
   /**
    * Sets system properties from effective configuration, must be called once and early before any
    * OTel SDK or SSL/TLS stack initialization. This allows to override JVM system properties using
-   * user-providded configuration and also to set standard OTel SDK configuration.
+   * user-provided configuration and also to set standard OTel SDK configuration.
    */
   public void propagateSystemProperties() {
     for (Map.Entry<Object, Object> entry : properties.entrySet()) {

diff --git a/.../contrib/jmxscraper/util/StringUtils.java → ...trib/jmxscraper/internal/StringUtils.java b/.../contrib/jmxscraper/util/StringUtils.java → ...trib/jmxscraper/internal/StringUtils.java
@@ -3,10 +3,14 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.contrib.jmxscraper.util;
+package io.opentelemetry.contrib.jmxscraper.internal;
 
 import javax.annotation.Nullable;
 
+/**
+ * This class is internal and is hence not for public use. Its APIs are unstable and can change at
+ * any time. </br> This is a utility class implementing miscellaneous String operations.
+ */
 public final class StringUtils {
   private StringUtils() {}