enable more metrics

Author: jaydeluca

docs/instrumentation-list.yaml

@@ -14,22 +14,25 @@ fi
 
 readonly INSTRUMENTATIONS=(
   # <module path (colon-separated)> : <javaagent|library> : [ gradle-task-suffix ]
-#  "alibaba-druid-1.0:javaagent:test"
-#  "alibaba-druid-1.0:javaagent:testStableSemconv"
-#  "apache-dbcp-2.0:javaagent:test"
-#  "apache-dbcp-2.0:javaagent:testStableSemconv"
-#  "apache-httpclient:apache-httpclient-5.0:javaagent:test"
-#  "c3p0-0.9:javaagent:test"
-#  "c3p0-0.9:javaagent:testStableSemconv"
-#  "hikaricp-3.0:javaagent:test"
-#  "hikaricp-3.0:javaagent:testStableSemconv"
-#  "tomcat:tomcat-jdbc:javaagent:test"
-#  "tomcat:tomcat-jdbc:javaagent:testStableSemconv"
-#  "oracle-ucp-11.2:javaagent:test"
-#  "oracle-ucp-11.2:javaagent:testStableSemconv"
+  "alibaba-druid-1.0:javaagent:test"
+  "alibaba-druid-1.0:javaagent:testStableSemconv"
+  "apache-dbcp-2.0:javaagent:test"
+  "apache-dbcp-2.0:javaagent:testStableSemconv"
+  "apache-httpclient:apache-httpclient-5.0:javaagent:test"
+  "c3p0-0.9:javaagent:test"
+  "c3p0-0.9:javaagent:testStableSemconv"
+  "clickhouse-client-0.5:javaagent:test"
+  "clickhouse-client-0.5:javaagent:testStableSemconv"
+  "hikaricp-3.0:javaagent:test"
+  "hikaricp-3.0:javaagent:testStableSemconv"
+  "tomcat:tomcat-jdbc:javaagent:test"
+  "tomcat:tomcat-jdbc:javaagent:testStableSemconv"
+  "oracle-ucp-11.2:javaagent:test"
+  "oracle-ucp-11.2:javaagent:testStableSemconv"
   "oshi:javaagent:test"
   "oshi:javaagent:testExperimental"
-#  "vibur-dbcp-11.0:javaagent:test"
+  "vibur-dbcp-11.0:javaagent:test"
+  "vibur-dbcp-11.0:javaagent:testStableSemconv"
 )
 
 readonly TELEMETRY_DIR_NAME=".telemetry"

instrumentation-docs/collect.sh

@@ -22,6 +22,35 @@ tasks {
 }
 ```
 
+Sometimes instrumentation will behave differently based on configuration options, and we can
+differentiate between these configurations by using the `metaDataConfig` system property. When the
+telemetry is written to a file, the value of this property will be included, or it will default to
+a `default` attribution.
+
+For example, to collect and write metadata for the `otel.semconv-stability.opt-in=database` option
+set for an instrumentation:
+
+```kotlin
+val collectMetadata = findProperty("collectMetadata")?.toString() ?: "false"
+
+tasks {
+  val testStableSemconv by registering(Test::class) {
+    jvmArgs("-Dotel.semconv-stability.opt-in=database")
+
+    systemProperty("collectMetadata", collectMetadata)
+    systemProperty("metaDataConfig", "otel.semconv-stability.opt-in=database")
+  }
+
+  test {
+    systemProperty("collectMetadata", collectMetadata)
+  }
+
+  check {
+    dependsOn(testStableSemconv)
+  }
+}
+```
+
 Then, prior to running the analyzer, run the following command to generate `.telemetry` files:
 
 `./gradlew test -PcollectMetadata=true`
@@ -36,8 +65,6 @@ or use the helper script that will run only the currently supported tests (recom
 ./instrumentation-docs/collect.sh
 ```
 
-This script will also clean up all `.telemetry` files after the analysis is done.
-
 ## Instrumentation Hierarchy
 
 An "InstrumentationModule" represents a module that that targets specific code in a
@@ -101,7 +128,8 @@ public class SpringWebInstrumentationModule extends InstrumentationModule
   * List of settings that are available for the instrumentation module
   * Each setting has a name, description, type, and default value
 * metrics
-  * List of metrics that the instrumentation module collects, including the metric name, description, type, and attributes
+  * List of metrics that the instrumentation module collects, including the metric name, description, type, and attributes.
+  * Separate lists for the metrics emitted by default vs via configuration options.
 
 ## Methodology
 
@@ -150,3 +178,6 @@ generate the metrics section of the instrumentation-list.yaml file.
 
 The data is written into a `.telemetry` directory in the root of each instrumentation module. This
 data will be excluded from git and just generated on demand.
+
+Each file has a `when` value along with the list of metrics that indicates whether the telemetry is emitted by default or via a
+configuration option.

instrumentation-docs/readme.md

@@ -100,7 +100,7 @@ List<InstrumentationModule> analyze() throws IOException {
           if (entry.getValue() == null || entry.getValue().getMetrics() == null) {
             continue;
           }
-          module.getMetrics().putIfAbsent(entry.getKey(), entry.getValue().getMetrics());
+          module.getMetrics().put(entry.getKey(), entry.getValue().getMetrics());
         }
       }
     }

instrumentation-docs/src/main/java/io/opentelemetry/instrumentation/docs/InstrumentationAnalyzer.java

@@ -17,6 +17,7 @@
 import java.io.BufferedWriter;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -141,8 +142,12 @@ private static Map<String, Object> baseProperties(InstrumentationModule module)
         List<EmittedMetrics.Metric> metrics =
             module.getMetrics().getOrDefault(group, Collections.emptyList());
         List<Map<String, Object>> metricsList = new ArrayList<>();
+
+        // sort metrics by name for some determinism in the order
+        metrics.sort(Comparator.comparing(EmittedMetrics.Metric::getName));
+
         for (EmittedMetrics.Metric metric : metrics) {
-          metricsList.add(getObjectMap(metric));
+          metricsList.add(getMetricsMap(metric));
         }
         telemetryEntry.put("metrics", metricsList);
         telemetryList.add(telemetryEntry);
@@ -216,27 +221,7 @@ private static Map<String, Object> configurationToMap(ConfigurationOption config
     return conf;
   }
 
-//  private static Map<String, List<Map<String, Object>>> getMetricsList(
-//      InstrumentationModule module) {
-//    Map<String, List<Map<String, Object>>> metricsMap = new LinkedHashMap<>();
-//    if (module.getMetrics() == null) {
-//      return metricsMap;
-//    }
-//
-//    for (Map.Entry<String, List<EmittedMetrics.Metric>> metricEntry :
-//        module.getMetrics().entrySet()) {
-//      List<Map<String, Object>> metricsList = new ArrayList<>();
-//      for (EmittedMetrics.Metric metric : metricEntry.getValue()) {
-//        Map<String, Object> innerMetricMap = getObjectMap(metric);
-//        metricsList.add(innerMetricMap);
-//      }
-//      metricsMap.put(metricEntry.getKey(), metricsList);
-//    }
-//
-//    return metricsMap;
-//  }
-
-  private static Map<String, Object> getObjectMap(EmittedMetrics.Metric metric) {
+  private static Map<String, Object> getMetricsMap(EmittedMetrics.Metric metric) {
     Map<String, Object> innerMetricMap = new LinkedHashMap<>();
     innerMetricMap.put("name", metric.getName());
     innerMetricMap.put("description", metric.getDescription());

instrumentation-docs/src/main/java/io/opentelemetry/instrumentation/docs/utils/YamlHelper.java

@@ -22,13 +22,18 @@ dependencies {
   testLibrary("org.apache.httpcomponents.client5:httpclient5:5.2.3")
 }
 
+val collectMetadata = findProperty("collectMetadata")?.toString() ?: "false"
+
 tasks {
   test {
     usesService(gradle.sharedServices.registrations["testcontainersBuildService"].service)
+    systemProperty("collectMetadata", collectMetadata)
   }
 
   val testStableSemconv by registering(Test::class) {
     jvmArgs("-Dotel.semconv-stability.opt-in=database")
+    systemProperty("collectMetadata", collectMetadata)
+    systemProperty("metaDataConfig", "otel.semconv-stability.opt-in=database")
   }
 
   check {

instrumentation/clickhouse-client-0.5/javaagent/build.gradle.kts

@@ -5,16 +5,20 @@
 
 package io.opentelemetry.javaagent.instrumentation.clickhouse;
 
+import static io.opentelemetry.instrumentation.testing.junit.db.DbClientMetricsTestUtil.assertDurationMetric;
 import static io.opentelemetry.instrumentation.testing.junit.db.SemconvStabilityUtil.maybeStable;
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
 import static io.opentelemetry.semconv.ErrorAttributes.ERROR_TYPE;
 import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS;
 import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT;
 import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_NAME;
+import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_NAMESPACE;
 import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION;
+import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION_NAME;
 import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_RESPONSE_STATUS_CODE;
 import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_STATEMENT;
 import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SYSTEM;
+import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SYSTEM_NAME;
 import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.catchThrowable;
@@ -115,6 +119,15 @@ void testConnectionStringWithoutDatabaseSpecifiedStillGeneratesSpans()
                         .hasNoParent()
                         .hasAttributesSatisfyingExactly(
                             attributeAssertions("select * from " + tableName, "SELECT"))));
+
+    assertDurationMetric(
+        testing,
+        "io.opentelemetry.clickhouse-client-0.5",
+        DB_SYSTEM_NAME,
+        DB_OPERATION_NAME,
+        DB_NAMESPACE,
+        SERVER_ADDRESS,
+        SERVER_PORT);
   }
 
   @Test

instrumentation/clickhouse-client-0.5/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/clickhouse/ClickHouseClientTest.java

@@ -22,20 +22,18 @@ dependencies {
   testImplementation(project(":instrumentation:oshi:testing"))
 }
 
-val collectMetadata = findProperty("collectMetadata")?.toString() ?: "false"
-
-
 tasks {
-  test {
-    systemProperty("collectMetadata", collectMetadata)
+  withType<Test>().configureEach {
+    systemProperty("collectMetadata", findProperty("collectMetadata")?.toString() ?: "false")
   }
 
   val testExperimental by registering(Test::class) {
     jvmArgs("-Dotel.instrumentation.oshi.experimental-metrics.enabled=true")
-    jvmArgs("-Dotel.semconv-stability.opt-in=database")
-
-    systemProperty("collectMetadata", collectMetadata)
-    systemProperty("metaDataConfig", "otel.semconv-stability.opt-in=database")
+    systemProperty("testExperimental", "true")
+    systemProperty("metaDataConfig", "otel.instrumentation.oshi.experimental-metrics.enabled=true")
   }
 
+  check {
+    dependsOn(testExperimental)
+  }
 }

instrumentation/oshi/javaagent/build.gradle.kts

@@ -1,6 +1,6 @@
 description: When the OSHI library is detected on the classpath, this instrumentation will use the system class loader to load classes from the oshi-core jar that are then used to generate system metrics.
 configurations:
   - name: otel.instrumentation.oshi.experimental-metrics.enabled
-    description: Enable the OSHI runtime metrics.
+    description: Enable the OSHI process runtime metrics.
     type: boolean
     default: false

instrumentation/oshi/metadata.yaml

@@ -9,6 +9,7 @@
 
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledIfSystemProperty;
 
 public abstract class AbstractProcessMetricsTest {
 
@@ -17,6 +18,7 @@ public abstract class AbstractProcessMetricsTest {
   protected abstract InstrumentationExtension testing();
 
   @Test
+  @EnabledIfSystemProperty(named = "testExperimental", matches = "true")
   void test() {
     // when
     registerMetrics();