Post-review code changes, renaming, more comments.

open-telemetry · Nov 15, 2022 · 87f166a · 87f166a
1 parent 5afd3ce
commit 87f166a
Show file tree

Hide file tree

Showing 14 changed files with 125 additions and 54 deletions.
diff --git a/...c/main/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstaller.java b/...c/main/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstaller.java
@@ -59,7 +59,7 @@ private static void addRulesForPlatform(String platform, MetricConfiguration con
       if (inputStream != null) {
         JmxMetricInsight.getLogger().log(FINE, "Opened input stream {0}", yamlResource);
         RuleParser parserInstance = RuleParser.get();
-        parserInstance.addMetricDefs(conf, inputStream);
+        parserInstance.addMetricDefsTo(conf, inputStream);
       } else {
         JmxMetricInsight.getLogger().log(INFO, "No support found for {0}", platform);
       }
@@ -85,7 +85,7 @@ private static void buildFromUserRules(
       JmxMetricInsight.getLogger().log(FINE, "JMX config file name: {0}", jmxDir);
       RuleParser parserInstance = RuleParser.get();
       try (InputStream inputStream = Files.newInputStream(new File(jmxDir.trim()).toPath())) {
-        parserInstance.addMetricDefs(conf, inputStream);
+        parserInstance.addMetricDefsTo(conf, inputStream);
       } catch (Exception e) {
         JmxMetricInsight.getLogger().warning(e.getMessage());
       }

diff --git a/...rics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/AttributeInfo.java b/...rics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/AttributeInfo.java
@@ -5,16 +5,18 @@
 
 package io.opentelemetry.instrumentation.jmx.engine;
 
+import javax.annotation.Nullable;
+
 /**
  * A class holding relevant information about an MBean attribute which will be used for collecting
  * metric values. The info comes directly from the relevant MBeans.
  */
 class AttributeInfo {
 
   private boolean usesDoubles;
-  private String description;
+  @Nullable private String description;
 
-  AttributeInfo(Number sampleValue, String description) {
+  AttributeInfo(Number sampleValue, @Nullable String description) {
     if (sampleValue instanceof Byte
         || sampleValue instanceof Short
         || sampleValue instanceof Integer
@@ -31,6 +33,7 @@ boolean usesDoubleValues() {
     return usesDoubles;
   }
 
+  @Nullable
   String getDescription() {
     return description;
   }

diff --git a/...ary/src/main/java/io/opentelemetry/instrumentation/jmx/engine/BeanAttributeExtractor.java b/...ary/src/main/java/io/opentelemetry/instrumentation/jmx/engine/BeanAttributeExtractor.java
@@ -10,6 +10,7 @@
 
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.Nullable;
 import javax.management.InstanceNotFoundException;
 import javax.management.MBeanAttributeInfo;
 import javax.management.MBeanInfo;
@@ -98,6 +99,7 @@ String getAttributeName() {
    * @param objectName the ObjectName identifying the MBean
    * @return AttributeInfo if the attribute is properly recognized, or null
    */
+  @Nullable
   AttributeInfo getAttributeInfo(MBeanServer server, ObjectName objectName) {
     if (logger.isLoggable(FINE)) {
       logger.log(FINE, "Resolving {0} for {1}", new Object[] {getAttributeName(), objectName});
@@ -168,6 +170,7 @@ AttributeInfo getAttributeInfo(MBeanServer server, ObjectName objectName) {
    *     malfunctions will be silent to avoid flooding the log.
    * @return the attribute value, if found, or null if an error occurred
    */
+  @Nullable
   private Object extractAttributeValue(MBeanServer server, ObjectName objectName, Logger logger) {
     try {
       Object value = server.getAttribute(objectName, baseName);
@@ -206,10 +209,12 @@ private Object extractAttributeValue(MBeanServer server, ObjectName objectName,
     return null;
   }
 
+  @Nullable
   private Object extractAttributeValue(MBeanServer server, ObjectName objectName) {
     return extractAttributeValue(server, objectName, null);
   }
 
+  @Nullable
   Number extractNumericalAttribute(MBeanServer server, ObjectName objectName) {
     Object value = extractAttributeValue(server, objectName);
     if (value instanceof Number) {
@@ -219,10 +224,12 @@ Number extractNumericalAttribute(MBeanServer server, ObjectName objectName) {
   }
 
   @Override
+  @Nullable
   public String extractValue(MBeanServer server, ObjectName objectName) {
     return extractStringAttribute(server, objectName);
   }
 
+  @Nullable
   private String extractStringAttribute(MBeanServer server, ObjectName objectName) {
     Object value = extractAttributeValue(server, objectName);
     if (value instanceof String) {

diff --git a/...x-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/BeanPack.java b/...x-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/BeanPack.java
@@ -5,6 +5,7 @@
 
 package io.opentelemetry.instrumentation.jmx.engine;
 
+import javax.annotation.Nullable;
 import javax.management.ObjectName;
 import javax.management.QueryExp;
 
@@ -14,7 +15,7 @@
  */
 public class BeanPack {
   // How to specify the MBean(s)
-  private final QueryExp queryExp;
+  @Nullable private final QueryExp queryExp;
   private final ObjectName[] namePatterns;
 
   /**
@@ -24,11 +25,12 @@ public class BeanPack {
    * @param namePatterns an array of ObjectNames used to look for MBeans; usually they will be
    *     patterns. If multiple patterns are provided, they work as logical OR.
    */
-  public BeanPack(QueryExp queryExp, ObjectName... namePatterns) {
+  public BeanPack(@Nullable QueryExp queryExp, ObjectName... namePatterns) {
     this.queryExp = queryExp;
     this.namePatterns = namePatterns;
   }
 
+  @Nullable
   QueryExp getQueryExp() {
     return queryExp;
   }

diff --git a/...s/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/JmxMetricInsight.java b/...s/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/JmxMetricInsight.java
@@ -5,7 +5,7 @@
 
 package io.opentelemetry.instrumentation.jmx.engine;
 
-import static java.util.logging.Level.CONFIG;
+import static java.util.logging.Level.INFO;
 
 import io.opentelemetry.api.OpenTelemetry;
 import java.util.logging.Logger;
@@ -36,7 +36,7 @@ private JmxMetricInsight(OpenTelemetry openTelemetry, long discoveryDelay) {
   public void start(MetricConfiguration conf) {
     if (conf.isEmpty()) {
       logger.log(
-          CONFIG,
+          INFO,
           "Empty JMX configuration, no metrics will be collected for InstrumentationScope "
               + INSTRUMENTATION_SCOPE);
     } else {

diff --git a/...cs/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricAttribute.java b/...cs/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricAttribute.java
@@ -29,23 +29,4 @@ public String getAttributeName() {
   String acquireAttributeValue(MBeanServer server, ObjectName objectName) {
     return extractor.extractValue(server, objectName);
   }
-
-  public static MetricAttributeExtractor fromConstant(String constantValue) {
-    return (a, b) -> {
-      return constantValue;
-    };
-  }
-
-  public static MetricAttributeExtractor fromObjectNameParameter(String parameterKey) {
-    if (parameterKey.isEmpty()) {
-      throw new IllegalArgumentException("Empty parameter name");
-    }
-    return (dummy, objectName) -> {
-      return objectName.getKeyProperty(parameterKey);
-    };
-  }
-
-  public static MetricAttributeExtractor fromBeanAttribute(String attributeName) {
-    return BeanAttributeExtractor.fromName(attributeName);
-  }
 }
diff --git a/...y/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricAttributeExtractor.java b/...y/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricAttributeExtractor.java
@@ -5,6 +5,7 @@
 
 package io.opentelemetry.instrumentation.jmx.engine;
 
+import javax.annotation.Nullable;
 import javax.management.MBeanServer;
 import javax.management.ObjectName;
 
@@ -23,5 +24,25 @@ public interface MetricAttributeExtractor {
    *     from an MBean attribute, or from the ObjectName parameter
    * @return the value of the attribute, can be null if extraction failed
    */
-  String extractValue(MBeanServer server, ObjectName objectName);
+  @Nullable
+  String extractValue(@Nullable MBeanServer server, @Nullable ObjectName objectName);
+
+  static MetricAttributeExtractor fromConstant(String constantValue) {
+    return (a, b) -> {
+      return constantValue;
+    };
+  }
+
+  static MetricAttributeExtractor fromObjectNameParameter(String parameterKey) {
+    if (parameterKey.isEmpty()) {
+      throw new IllegalArgumentException("Empty parameter name");
+    }
+    return (dummy, objectName) -> {
+      return objectName.getKeyProperty(parameterKey);
+    };
+  }
+
+  static MetricAttributeExtractor fromBeanAttribute(String attributeName) {
+    return BeanAttributeExtractor.fromName(attributeName);
+  }
 }
diff --git a/...trics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricBanner.java b/...trics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricBanner.java
@@ -5,6 +5,8 @@
 
 package io.opentelemetry.instrumentation.jmx.engine;
 
+import javax.annotation.Nullable;
+
 /**
  * A class providing the user visible characteristics (name, type, description and units) of a
  * metric to be reported with OpenTelemetry.
@@ -22,8 +24,8 @@ public enum Type {
 
   // How to report the metric using OpenTelemetry API
   private final String metricName; // used as Instrument name
-  private final String description;
-  private final String unit;
+  @Nullable private final String description;
+  @Nullable private final String unit;
   private final Type type;
 
   /**
@@ -34,7 +36,8 @@ public enum Type {
    * @param unit a human readable unit of measurement
    * @param type the instrument typ to be used for the metric
    */
-  public MetricBanner(String metricName, String description, String unit, Type type) {
+  public MetricBanner(
+      String metricName, @Nullable String description, String unit, @Nullable Type type) {
     this.metricName = metricName;
     this.description = description;
     this.unit = unit;
@@ -45,10 +48,12 @@ String getMetricName() {
     return metricName;
   }
 
+  @Nullable
   String getDescription() {
     return description;
   }
 
+  @Nullable
   String getUnit() {
     return unit;
   }

diff --git a/...-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricDef.java b/...-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricDef.java
@@ -9,17 +9,65 @@
  * A class providing a complete definition on how to create an Open Telemetry metric out of the JMX
  * system: how to extract values from MBeans and how to model, name and decorate them with
  * attributes using OpenTelemetry Metric API. Objects of this class are immutable.
- *
- * <p>Example: The JVM provides an MBean with ObjectName "java.lang:type=Threading", and one of the
- * MBean attributes is "ThreadCount". This MBean can be used to provide a metric showing the current
- * number of threads run by the JVM as follows:
- *
- * <p>new MetricDef(new BeanPack(null, new ObjectName("java.lang:type=Threading")),
- *
- * <p>new MetricExtractor(new BeanAttributeExtractor("ThreadCount"),
- *
- * <p>new MetricBanner("process.runtime.jvm.threads", "Current number of threads", "1" )));
  */
+
+// Example: The rule described by the following YAML definition
+//
+//  - bean: java.lang:name=*,type=MemoryPool
+//    metricAttribute:
+//      pool: param(name)
+//      type: beanattr(Type)
+//    mapping:
+//      Usage.used:
+//        metric: my.own.jvm.memory.pool.used
+//        type: updowncounter
+//        desc: Pool memory currently used
+//        unit: By
+//      Usage.max:
+//        metric: my.own.jvm.memory.pool.max
+//        type: updowncounter
+//        desc: Maximum obtainable memory pool size
+//        unit: By
+//
+// can be created using the following snippet:
+//
+//  MetricAttribute poolAttribute =
+//      new MetricAttribute("pool", MetricAttributeExtractor.fromObjectNameParameter("name"));
+//  MetricAttribute typeAttribute =
+//      new MetricAttribute("type", MetricAttributeExtractor.fromBeanAttribute("Type"));
+//
+//  MetricBanner poolUsedBanner =
+//      new MetricBanner(
+//          "my.own.jvm.memory.pool.used",
+//          "Pool memory currently used",
+//          "By",
+//          MetricBanner.Type.UPDOWNCOUNTER);
+//  MetricBanner poolLimitBanner =
+//      new MetricBanner(
+//          "my.own.jvm.memory.pool.limit",
+//          "Maximum obtainable memory pool size",
+//          "By",
+//          MetricBanner.Type.UPDOWNCOUNTER);
+//
+//  MetricExtractor usageUsedExtractor =
+//      new MetricExtractor(
+//          new BeanAttributeExtractor("Usage", "used"),
+//          poolUsedBanner,
+//          poolAttribute,
+//          typeAttribute);
+//  MetricExtractor usageMaxExtractor =
+//      new MetricExtractor(
+//          new BeanAttributeExtractor("Usage", "max"),
+//          poolLimitBanner,
+//          poolAttribute,
+//          typeAttribute);
+//
+//  MetricDef def =
+//      new MetricDef(
+//          new BeanPack(null, new ObjectName("java.lang:name=*,type=MemoryPool")),
+//          usageUsedExtractor,
+//          usageMaxExtractor);
+
 public class MetricDef {
 
   // Describes the MBeans to use

diff --git a/...cs/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricExtractor.java b/...cs/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricExtractor.java
@@ -5,6 +5,8 @@
 
 package io.opentelemetry.instrumentation.jmx.engine;
 
+import javax.annotation.Nullable;
+
 /**
  * A class holding the info needed to support a single metric: how to define it in OpenTelemetry and
  * how to provide the metric values.
@@ -22,7 +24,7 @@ public class MetricExtractor {
   // Defines the Measurement attributes to be used when reporting the metric value.
   private final MetricAttribute[] attributes;
 
-  private volatile DetectionStatus status;
+  @Nullable private volatile DetectionStatus status;
 
   public MetricExtractor(
       BeanAttributeExtractor attributeExtractor,
@@ -49,6 +51,7 @@ void setStatus(DetectionStatus status) {
     this.status = status;
   }
 
+  @Nullable
   DetectionStatus getStatus() {
     return status;
   }

diff --git a/...cs/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricRegistrar.java b/...cs/library/src/main/java/io/opentelemetry/instrumentation/jmx/engine/MetricRegistrar.java
@@ -5,7 +5,7 @@
 
 package io.opentelemetry.instrumentation.jmx.engine;
 
-import static java.util.logging.Level.CONFIG;
+import static java.util.logging.Level.INFO;
 
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.common.Attributes;
@@ -83,7 +83,7 @@ void enrollExtractor(
             } else {
               builder.buildWithCallback(longTypeCallback(extractor));
             }
-            logger.log(CONFIG, "Created Counter for {0}", metricName);
+            logger.log(INFO, "Created Counter for {0}", metricName);
           }
           break;
 
@@ -104,7 +104,7 @@ void enrollExtractor(
             } else {
               builder.buildWithCallback(longTypeCallback(extractor));
             }
-            logger.log(CONFIG, "Created UpDownCounter for {0}", metricName);
+            logger.log(INFO, "Created UpDownCounter for {0}", metricName);
           }
           break;
 
@@ -125,7 +125,7 @@ void enrollExtractor(
             } else {
               builder.ofLongs().buildWithCallback(longTypeCallback(extractor));
             }
-            logger.log(CONFIG, "Created Gauge for {0}", metricName);
+            logger.log(INFO, "Created Gauge for {0}", metricName);
           }
       }
     }

diff --git a/...mx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/JmxConfig.java b/...mx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/JmxConfig.java
@@ -37,7 +37,7 @@ public void setRules(List<JmxRule> rules) {
    * @param configuration MetricConfiguration to add MetricDefs to
    * @throws an exception if the rule conversion cannot be performed
    */
-  public void addMetricDefs(MetricConfiguration configuration) throws Exception {
+  void addMetricDefsTo(MetricConfiguration configuration) throws Exception {
     for (JmxRule rule : rules) {
       MetricDef metricDef = rule.buildMetricDef();
       configuration.addMetricDef(metricDef);