فهرست منبع

Fix NoClassDefFoundError happening when snakeyaml is used on a custom JRE (#7598)

Fixes #7580
Mateusz Rzeszutek 2 سال پیش
والد
کامیت
268165c668

+ 1 - 1
dependencyManagement/build.gradle.kts

@@ -106,7 +106,7 @@ val DEPENDENCIES = listOf(
   // Note that this is only referenced as "org.springframework.boot" in build files, not the artifact name.
   "org.springframework.boot:spring-boot-dependencies:2.7.5",
   "javax.validation:validation-api:2.0.1.Final",
-  "org.yaml:snakeyaml:1.33"
+  "org.snakeyaml:snakeyaml-engine:2.6"
 )
 
 javaPlatform {

+ 1 - 1
instrumentation/jmx-metrics/library/build.gradle.kts

@@ -3,7 +3,7 @@ plugins {
 }
 
 dependencies {
-  implementation("org.yaml:snakeyaml")
+  implementation("org.snakeyaml:snakeyaml-engine")
 
   testImplementation(project(":testing-common"))
 }

+ 6 - 7
instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/JmxConfig.java

@@ -19,15 +19,14 @@ public class JmxConfig {
   //   rules:
   //     - JMX_DEFINITION1
   //     - JMX_DEFINITION2
-  // The parser is guaranteed to call setRules with a non-null argument, or throw an exception
-  private List<JmxRule> rules;
+  private final List<JmxRule> rules;
 
-  public List<JmxRule> getRules() {
-    return rules;
+  public JmxConfig(List<JmxRule> rules) {
+    this.rules = rules;
   }
 
-  public void setRules(List<JmxRule> rules) {
-    this.rules = rules;
+  public List<JmxRule> getRules() {
+    return rules;
   }
 
   /**
@@ -35,7 +34,7 @@ public class JmxConfig {
    * MetricConfiguration.
    *
    * @param configuration MetricConfiguration to add MetricDefs to
-   * @throws an exception if the rule conversion cannot be performed
+   * @throws Exception an exception if the rule conversion cannot be performed
    */
   void addMetricDefsTo(MetricConfiguration configuration) throws Exception {
     for (JmxRule rule : rules) {

+ 15 - 10
instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/JmxRule.java

@@ -16,6 +16,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import javax.annotation.Nullable;
 import javax.management.MalformedObjectNameException;
 import javax.management.ObjectName;
 
@@ -38,16 +39,16 @@ public class JmxRule extends MetricStructure {
   //     ATTRIBUTE3:
   //       METRIC_FIELDS3
   // The parser never calls setters for these fields with null arguments
-  private String bean;
+  @Nullable private String bean;
   private List<String> beans;
-  private String prefix;
+  @Nullable private String prefix;
   private Map<String, Metric> mapping;
 
   public String getBean() {
     return bean;
   }
 
-  public void setBean(String bean) throws Exception {
+  public void setBean(String bean) {
     this.bean = validateBean(bean);
   }
 
@@ -55,14 +56,18 @@ public class JmxRule extends MetricStructure {
     return beans;
   }
 
-  private static String validateBean(String name) throws MalformedObjectNameException {
-    String trimmed = name.trim();
-    // Check the syntax of the provided name by attempting to create an ObjectName from it.
-    new ObjectName(trimmed);
-    return trimmed;
+  private static String validateBean(String name) {
+    try {
+      String trimmed = name.trim();
+      // Check the syntax of the provided name by attempting to create an ObjectName from it.
+      new ObjectName(trimmed);
+      return trimmed;
+    } catch (MalformedObjectNameException e) {
+      throw new IllegalArgumentException("'" + name + "' is not a valid JMX object name", e);
+    }
   }
 
-  public void setBeans(List<String> beans) throws Exception {
+  public void setBeans(List<String> beans) {
     List<String> list = new ArrayList<>();
     for (String name : beans) {
       list.add(validateBean(name));
@@ -113,7 +118,7 @@ public class JmxRule extends MetricStructure {
    * consistency or semantic issues, an exception will be thrown.
    *
    * @return a valid MetricDefinition object
-   * @throws an exception if any issues within the rule are detected
+   * @throws Exception an exception if any issues within the rule are detected
    */
   public MetricDef buildMetricDef() throws Exception {
     BeanGroup group;

+ 7 - 4
instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/Metric.java

@@ -6,6 +6,7 @@
 package io.opentelemetry.instrumentation.jmx.yaml;
 
 import io.opentelemetry.instrumentation.jmx.engine.MetricInfo;
+import javax.annotation.Nullable;
 
 /**
  * A class representing metric definition as a part of YAML metric rule. Objects of this class are
@@ -16,9 +17,8 @@ public class Metric extends MetricStructure {
   // Used by the YAML parser
   //   metric: METRIC_NAME
   //   desc: DESCRIPTION
-  // The parser never calls setters for these fields with null arguments
-  private String metric;
-  private String desc;
+  @Nullable private String metric;
+  @Nullable private String desc;
 
   public String getMetric() {
     return metric;
@@ -43,7 +43,10 @@ public class Metric extends MetricStructure {
   }
 
   MetricInfo buildMetricInfo(
-      String prefix, String attributeName, String defaultUnit, MetricInfo.Type defaultType) {
+      @Nullable String prefix,
+      String attributeName,
+      String defaultUnit,
+      MetricInfo.Type defaultType) {
     String metricName;
     if (metric == null) {
       metricName = prefix == null ? attributeName : (prefix + attributeName);

+ 0 - 6
instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/MetricStructure.java

@@ -29,22 +29,16 @@ abstract class MetricStructure {
   //      KEY2: SPECIFICATION2
   //    unit: UNIT
 
-  private String type; // unused, for YAML parser only
   private Map<String, String> metricAttribute; // unused, for YAML parser only
   private String unit;
 
   private MetricInfo.Type metricType;
   private List<MetricAttribute> metricAttributes;
 
-  public String getType() {
-    return type;
-  }
-
   public void setType(String t) {
     // Do not complain about case variations
     t = t.trim().toUpperCase();
     this.metricType = MetricInfo.Type.valueOf(t);
-    this.type = t;
   }
 
   public String getUnit() {

+ 107 - 11
instrumentation/jmx-metrics/library/src/main/java/io/opentelemetry/instrumentation/jmx/yaml/RuleParser.java

@@ -5,14 +5,20 @@
 
 package io.opentelemetry.instrumentation.jmx.yaml;
 
+import static java.util.Collections.emptyList;
 import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.WARNING;
 
 import io.opentelemetry.instrumentation.jmx.engine.MetricConfiguration;
 import java.io.InputStream;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 import java.util.logging.Logger;
-import org.yaml.snakeyaml.Yaml;
-import org.yaml.snakeyaml.constructor.Constructor;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
+import org.snakeyaml.engine.v2.api.Load;
+import org.snakeyaml.engine.v2.api.LoadSettings;
 
 /** Parse a YAML file containing a number of rules. */
 public class RuleParser {
@@ -40,9 +46,103 @@ public class RuleParser {
 
   private RuleParser() {}
 
-  public JmxConfig loadConfig(InputStream is) throws Exception {
-    Yaml yaml = new Yaml(new Constructor(JmxConfig.class));
-    return yaml.load(is);
+  @SuppressWarnings("unchecked")
+  public JmxConfig loadConfig(InputStream is) {
+    LoadSettings settings = LoadSettings.builder().build();
+    Load yaml = new Load(settings);
+
+    Map<String, Object> data = (Map<String, Object>) yaml.loadFromInputStream(is);
+    if (data == null) {
+      return new JmxConfig(emptyList());
+    }
+
+    List<Object> rules = (List<Object>) data.remove("rules");
+    if (rules == null) {
+      return new JmxConfig(emptyList());
+    }
+
+    failOnExtraKeys(data);
+    return new JmxConfig(
+        rules.stream()
+            .map(obj -> (Map<String, Object>) obj)
+            .map(RuleParser::parseJmxRule)
+            .collect(Collectors.toList()));
+  }
+
+  @SuppressWarnings("unchecked")
+  private static JmxRule parseJmxRule(Map<String, Object> ruleYaml) {
+    JmxRule jmxRule = new JmxRule();
+
+    String bean = (String) ruleYaml.remove("bean");
+    if (bean != null) {
+      jmxRule.setBean(bean);
+    }
+    List<String> beans = (List<String>) ruleYaml.remove("beans");
+    if (beans != null) {
+      jmxRule.setBeans(beans);
+    }
+    String prefix = (String) ruleYaml.remove("prefix");
+    if (prefix != null) {
+      jmxRule.setPrefix(prefix);
+    }
+    jmxRule.setMapping(parseMappings((Map<String, Object>) ruleYaml.remove("mapping")));
+    parseMetricStructure(ruleYaml, jmxRule);
+
+    failOnExtraKeys(ruleYaml);
+    return jmxRule;
+  }
+
+  @SuppressWarnings("unchecked")
+  private static Map<String, Metric> parseMappings(@Nullable Map<String, Object> mappingYaml) {
+    Map<String, Metric> mappings = new LinkedHashMap<>();
+    if (mappingYaml != null) {
+      mappingYaml.forEach(
+          (name, metricYaml) ->
+              mappings.put(
+                  name, metricYaml == null ? null : parseMetric((Map<String, Object>) metricYaml)));
+    }
+    return mappings;
+  }
+
+  private static Metric parseMetric(Map<String, Object> metricYaml) {
+    Metric metric = new Metric();
+
+    String metricName = (String) metricYaml.remove("metric");
+    if (metricName != null) {
+      metric.setMetric(metricName);
+    }
+    String desc = (String) metricYaml.remove("desc");
+    if (desc != null) {
+      metric.setDesc(desc);
+    }
+    parseMetricStructure(metricYaml, metric);
+
+    failOnExtraKeys(metricYaml);
+    return metric;
+  }
+
+  @SuppressWarnings("unchecked")
+  private static void parseMetricStructure(
+      Map<String, Object> metricStructureYaml, MetricStructure out) {
+    String type = (String) metricStructureYaml.remove("type");
+    if (type != null) {
+      out.setType(type);
+    }
+    Map<String, String> metricAttribute =
+        (Map<String, String>) metricStructureYaml.remove("metricAttribute");
+    if (metricAttribute != null) {
+      out.setMetricAttribute(metricAttribute);
+    }
+    String unit = (String) metricStructureYaml.remove("unit");
+    if (unit != null) {
+      out.setUnit(unit);
+    }
+  }
+
+  private static void failOnExtraKeys(Map<String, Object> yaml) {
+    if (!yaml.isEmpty()) {
+      throw new IllegalArgumentException("Unrecognized keys found: " + yaml.keySet());
+    }
   }
 
   /**
@@ -55,13 +155,9 @@ public class RuleParser {
    */
   public void addMetricDefsTo(MetricConfiguration conf, InputStream is, String id) {
     try {
-
       JmxConfig config = loadConfig(is);
-      if (config != null) {
-        logger.log(
-            INFO, "{0}: found {1} metric rules", new Object[] {id, config.getRules().size()});
-        config.addMetricDefsTo(conf);
-      }
+      logger.log(INFO, "{0}: found {1} metric rules", new Object[] {id, config.getRules().size()});
+      config.addMetricDefsTo(conf);
     } catch (Exception exception) {
       logger.log(
           WARNING,

+ 26 - 35
instrumentation/jmx-metrics/library/src/test/java/io/opentelemetry/instrumentation/jmx/engine/RuleParserTest.java

@@ -67,7 +67,7 @@ class RuleParserTest {
           + "        metric: METRIC_NAME3\n";
 
   @Test
-  void testConf2() throws Exception {
+  void testConf2() {
     InputStream is = new ByteArrayInputStream(CONF2.getBytes(StandardCharsets.UTF_8));
     JmxConfig config = parser.loadConfig(is);
     assertThat(config).isNotNull();
@@ -102,7 +102,7 @@ class RuleParserTest {
           + "      ATTRIBUTE35:\n";
 
   @Test
-  void testConf3() throws Exception {
+  void testConf3() {
     InputStream is = new ByteArrayInputStream(CONF3.getBytes(StandardCharsets.UTF_8));
     JmxConfig config = parser.loadConfig(is);
     assertThat(config).isNotNull();
@@ -160,24 +160,28 @@ class RuleParserTest {
     assertThat(metricDef).isNotNull();
     assertThat(metricDef.getMetricExtractors()).hasSize(3);
 
-    MetricExtractor m1 = metricDef.getMetricExtractors()[0];
-    assertThat(m1.getMetricValueExtractor().getAttributeName()).isEqualTo("A.b");
-    assertThat(m1.getAttributes()).hasSize(3);
-
-    MetricInfo mb1 = m1.getInfo();
-    assertThat(mb1.getMetricName()).isEqualTo("PREFIX.METRIC_NAME1");
-    assertThat(mb1.getDescription()).isEqualTo("DESCRIPTION1");
-    assertThat(mb1.getUnit()).isEqualTo("UNIT1");
-    assertThat(mb1.getType()).isEqualTo(MetricInfo.Type.COUNTER);
-
-    MetricExtractor m3 = metricDef.getMetricExtractors()[2];
-    assertThat(m3.getMetricValueExtractor().getAttributeName()).isEqualTo("ATTRIBUTE3");
-
-    MetricInfo mb3 = m3.getInfo();
-    assertThat(mb3.getMetricName()).isEqualTo("PREFIX.ATTRIBUTE3");
-    // syntax extension - defining a default unit and type
-    assertThat(mb3.getType()).isEqualTo(MetricInfo.Type.UPDOWNCOUNTER);
-    assertThat(mb3.getUnit()).isEqualTo("DEFAULT_UNIT");
+    assertThat(metricDef.getMetricExtractors())
+        .anySatisfy(
+            m -> {
+              assertThat(m.getMetricValueExtractor().getAttributeName()).isEqualTo("A.b");
+              assertThat(m.getAttributes()).hasSize(3);
+
+              MetricInfo mb1 = m.getInfo();
+              assertThat(mb1.getMetricName()).isEqualTo("PREFIX.METRIC_NAME1");
+              assertThat(mb1.getDescription()).isEqualTo("DESCRIPTION1");
+              assertThat(mb1.getUnit()).isEqualTo("UNIT1");
+              assertThat(mb1.getType()).isEqualTo(MetricInfo.Type.COUNTER);
+            })
+        .anySatisfy(
+            m -> {
+              assertThat(m.getMetricValueExtractor().getAttributeName()).isEqualTo("ATTRIBUTE3");
+
+              MetricInfo mb3 = m.getInfo();
+              assertThat(mb3.getMetricName()).isEqualTo("PREFIX.ATTRIBUTE3");
+              // syntax extension - defining a default unit and type
+              assertThat(mb3.getType()).isEqualTo(MetricInfo.Type.UPDOWNCOUNTER);
+              assertThat(mb3.getUnit()).isEqualTo("DEFAULT_UNIT");
+            });
   }
 
   private static final String CONF5 = // minimal valid definition
@@ -278,10 +282,10 @@ class RuleParserTest {
   private static final String EMPTY_CONF = "---\n";
 
   @Test
-  void testEmptyConf() throws Exception {
+  void testEmptyConf() {
     InputStream is = new ByteArrayInputStream(EMPTY_CONF.getBytes(StandardCharsets.UTF_8));
     JmxConfig config = parser.loadConfig(is);
-    assertThat(config).isNull();
+    assertThat(config.getRules()).isEmpty();
   }
 
   /*
@@ -417,19 +421,6 @@ class RuleParserTest {
     runNegativeTest(yaml);
   }
 
-  @Test
-  void testEmptyPrefix() {
-    String yaml =
-        "---\n"
-            + "rules:\n"
-            + "  - bean: domain:name=you\n"
-            + "    prefix:\n"
-            + "    mapping:\n"
-            + "      A:\n"
-            + "        metric: METRIC_NAME\n";
-    runNegativeTest(yaml);
-  }
-
   @Test
   void testTypoInMetric() {
     String yaml =

+ 1 - 1
instrumentation/spring/spring-boot-resources/library/build.gradle.kts

@@ -9,7 +9,7 @@ dependencies {
   compileOnly("com.google.auto.service:auto-service-annotations")
   testCompileOnly("com.google.auto.service:auto-service-annotations")
 
-  implementation("org.yaml:snakeyaml")
+  implementation("org.snakeyaml:snakeyaml-engine")
 
   testImplementation("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi")
 }

+ 5 - 3
instrumentation/spring/spring-boot-resources/library/src/main/java/io/opentelemetry/instrumentation/spring/resources/SpringBootServiceNameDetector.java

@@ -30,7 +30,8 @@ import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
-import org.yaml.snakeyaml.Yaml;
+import org.snakeyaml.engine.v2.api.Load;
+import org.snakeyaml.engine.v2.api.LoadSettings;
 
 /**
  * A ResourceProvider that will attempt to guess the application name for a Spring Boot service.
@@ -174,9 +175,10 @@ public class SpringBootServiceNameDetector implements ConditionalResourceProvide
   @Nullable
   @SuppressWarnings("unchecked")
   private static String parseNameFromYaml(InputStream in) {
-    Yaml yaml = new Yaml();
     try {
-      Map<String, Object> data = yaml.load(in);
+      LoadSettings settings = LoadSettings.builder().build();
+      Load yaml = new Load(settings);
+      Map<String, Object> data = (Map<String, Object>) yaml.loadFromInputStream(in);
       Map<String, Map<String, Object>> spring =
           (Map<String, Map<String, Object>>) data.get("spring");
       if (spring != null) {