diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 864acac52b..e8e14d88be 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -34,6 +34,7 @@ Use subheadings with the "=====" level for adding notes for unreleased changes: [float] ===== Bug fixes * Fix log4j2 log correlation with shaded application jar - {pull}3764[#3764] +* Improve automatic span class name detection for Scala and nested/anonymous classes - {pull}3746[#3746] [float] ===== Features diff --git a/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/ClassNameParser.java b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/ClassNameParser.java new file mode 100644 index 0000000000..d8c327d0ed --- /dev/null +++ b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/ClassNameParser.java @@ -0,0 +1,102 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package co.elastic.apm.agent.sdk.bytebuddy; + +/** + * Utility class to extract the correct simple class name + */ +public class ClassNameParser { + /** + * Utility class, do not instantiate + */ + private ClassNameParser() {} + /** + * returns true if the string only contains digits + * @param str the string to check + * @return + */ + private static boolean isNumeric(String str) { + for(int i = 0; i< str.length(); i++) { + if(!Character.isDigit(str.charAt(i))) { + return false; + } + } + return true; + } + /** + * this method parses the anonymous class name from the full class name + * @param className + * @return + */ + private static String getAnonymousClassName(String className) { + int lastDollarIndex = className.lastIndexOf('$'); + int currentDollarIndex; + String nestedClassName; + // we do not want to show just a number for anonymous classes, so we walk back until we find a named (normal or nested) class + do { + currentDollarIndex = className.lastIndexOf('$', lastDollarIndex -1); + nestedClassName = className.substring(currentDollarIndex + 1, lastDollarIndex); + lastDollarIndex = currentDollarIndex; + } while(currentDollarIndex != -1 && isNumeric(nestedClassName)); + return className.substring(currentDollarIndex + 1); + } + /** + * Parses the class name from nested and anonymous classes (containing a $ sign) + *
+ * Note: We cannot know if the $ sign is part of the class name or denotes a nested/anonymous class. As $ signs should not be used in class names anyway, + * we handle them always as a class separator + *
+ * @param className + * @return + */ + private static String getNestedClassName(String className) { + int dollarIndex = className.lastIndexOf('$'); + String innerClassName = className.substring(dollarIndex + 1); + if(isNumeric(innerClassName)) { + // this is an anonymous inner class + className = getAnonymousClassName(className); + } else { + className = innerClassName; + } + return className; + } + /** + * Parses the simple class name from a class name + * @param className + * @return + */ + public static String parse(String className) { + //remove package name + className = className.substring(className.lastIndexOf('.') + 1); + if(className.contains("$")) { + if(className.endsWith("$")) { + // this can happen if the source code was in Scala and the object keyword was used + // https://www.toptal.com/scala/scala-bytecode-and-the-jvm + className = className.substring(0, className.length() - 1); + if(className.contains("$")) + { + className = getNestedClassName(className); + } + } else { + className = getNestedClassName(className); + } + } + return className; + } +} diff --git a/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/SimpleMethodSignatureOffsetMappingFactory.java b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/SimpleMethodSignatureOffsetMappingFactory.java index d0bc841cd6..3df8507ea1 100644 --- a/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/SimpleMethodSignatureOffsetMappingFactory.java +++ b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/SimpleMethodSignatureOffsetMappingFactory.java @@ -51,8 +51,7 @@ public Advice.OffsetMapping make(ParameterDescription.InDefinedShape target, public Target resolve(TypeDescription instrumentedType, MethodDescription instrumentedMethod, Assigner assigner, Advice.ArgumentHandler argumentHandler, Sort sort) { final String className = instrumentedMethod.getDeclaringType().getTypeName(); - String simpleClassName = className.substring(className.lastIndexOf('$') + 1); - simpleClassName = simpleClassName.substring(simpleClassName.lastIndexOf('.') + 1); + final String simpleClassName = ClassNameParser.parse(className); final String signature = String.format("%s#%s", simpleClassName, instrumentedMethod.getName()); return Target.ForStackManipulation.of(signature); } diff --git a/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/ClassNameParserTest.java b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/ClassNameParserTest.java new file mode 100644 index 0000000000..bc76d3dd18 --- /dev/null +++ b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/ClassNameParserTest.java @@ -0,0 +1,83 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package co.elastic.apm.agent.sdk.bytebuddy; + +import static org.assertj.core.api.Assertions.*; + +import org.junit.jupiter.api.Test; + +import co.elastic.apm.agent.sdk.bytebuddy.clazzes.ParentClass.InnerClass; +import co.elastic.apm.agent.sdk.bytebuddy.clazzes.ParentClass.InnerClass.NestedInnerClass; +import co.elastic.apm.agent.sdk.bytebuddy.clazzes.ParentObject.ChildScalaObject$; +import co.elastic.apm.agent.sdk.bytebuddy.clazzes.SimpleClass; +import co.elastic.apm.agent.sdk.bytebuddy.clazzes.AnonymousClass; +import co.elastic.apm.agent.sdk.bytebuddy.clazzes.AnonymousNestedClass; +import co.elastic.apm.agent.sdk.bytebuddy.clazzes.Dollar$Class; +import co.elastic.apm.agent.sdk.bytebuddy.clazzes.ScalaObject$; + +class ClassNameParserTest { + + @Test + void testSimpleClassName() { + String className = ClassNameParser.parse(SimpleClass.class.getName()); + assertThat(className).isEqualTo("SimpleClass"); + } + @Test + void testNestedClassName() { + String className = ClassNameParser.parse(InnerClass.class.getName()); + assertThat(className).isEqualTo("InnerClass"); + } + @Test + void testDoubleNestedClassName() { + String className = ClassNameParser.parse(NestedInnerClass.class.getName()); + assertThat(className).isEqualTo("NestedInnerClass"); + } + @Test + void testDollarClassName() { + String className = ClassNameParser.parse(Dollar$Class.class.getName()); + //We have no way to know if the class name is Dollar$Class or Class is a nested class of Dollar + //As dollar signs should not be used in names, it should be fine to detect Class instead of Dollar$Class + assertThat(className).describedAs("Dollar signs should not be used in names").isEqualTo("Class"); + } + @Test + void testAnonymousClassName() { + String className = ClassNameParser.parse(AnonymousClass.getAnonymousClass().getName()); + assertThat(className).isEqualTo("AnonymousClass$1"); + } + @Test + void testAnonymousNestedClassName() { + String className = ClassNameParser.parse(InnerClass.getAnonymousClass().getName()); + assertThat(className).isEqualTo("InnerClass$1"); + } + @Test + void testAnonymousInAnonymousClassName() { + String className = ClassNameParser.parse(AnonymousNestedClass.getAnonymousClass().getName()); + assertThat(className).isEqualTo("AnonymousNestedClass$1$1"); + } + @Test + void testScalaObjectClassName() { + String className = ClassNameParser.parse(ScalaObject$.class.getName()); + assertThat(className).isEqualTo("ScalaObject"); + } + @Test + void testNestedScalaObjectClassName() { + String className = ClassNameParser.parse(ChildScalaObject$.class.getName()); + assertThat(className).isEqualTo("ChildScalaObject"); + } +} diff --git a/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/AnonymousClass.java b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/AnonymousClass.java new file mode 100644 index 0000000000..ec1234c2c7 --- /dev/null +++ b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/AnonymousClass.java @@ -0,0 +1,30 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package co.elastic.apm.agent.sdk.bytebuddy.clazzes; + +public class AnonymousClass { + public static Class> getAnonymousClass() { + return new Object() { + @Override + public String toString() { + return "foo"; + } + }.getClass(); + } +} diff --git a/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/AnonymousNestedClass.java b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/AnonymousNestedClass.java new file mode 100644 index 0000000000..14c01ce0cb --- /dev/null +++ b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/AnonymousNestedClass.java @@ -0,0 +1,39 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package co.elastic.apm.agent.sdk.bytebuddy.clazzes; + +public class AnonymousNestedClass { + public static Class> getAnonymousClass() { + Factory f = new Factory() { + @Override + public Object getObject() { + return new Object() { + @Override + public String toString() { + return "foo"; + } + }; + } + }; + return f.getObject().getClass(); + } + public static interface Factory { + Object getObject(); + } +} diff --git a/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/Dollar$Class.java b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/Dollar$Class.java new file mode 100644 index 0000000000..08aa4c7e3d --- /dev/null +++ b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/Dollar$Class.java @@ -0,0 +1,23 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package co.elastic.apm.agent.sdk.bytebuddy.clazzes; + +public class Dollar$Class { + +} diff --git a/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ParentClass.java b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ParentClass.java new file mode 100644 index 0000000000..99799af8bf --- /dev/null +++ b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ParentClass.java @@ -0,0 +1,36 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package co.elastic.apm.agent.sdk.bytebuddy.clazzes; + +public class ParentClass { + + public static class InnerClass { + public static Class> getAnonymousClass() { + return new Object() { + @Override + public String toString() { + return "foo"; + } + }.getClass(); + } + public static class NestedInnerClass { + + } + } +} diff --git a/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ParentObject.java b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ParentObject.java new file mode 100644 index 0000000000..3a73eeb973 --- /dev/null +++ b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ParentObject.java @@ -0,0 +1,29 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package co.elastic.apm.agent.sdk.bytebuddy.clazzes; + +public class ParentObject { + /** + * Scala objects generate a class with a dollar at the end + * https://www.toptal.com/scala/scala-bytecode-and-the-jvm + */ + public class ChildScalaObject$ { + + } +} diff --git a/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ScalaObject$.java b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ScalaObject$.java new file mode 100644 index 0000000000..c20f336742 --- /dev/null +++ b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ScalaObject$.java @@ -0,0 +1,27 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package co.elastic.apm.agent.sdk.bytebuddy.clazzes; + +/** + * Scala objects generate a class with a dollar at the end + * https://www.toptal.com/scala/scala-bytecode-and-the-jvm + */ +public class ScalaObject$ { + +} diff --git a/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/SimpleClass.java b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/SimpleClass.java new file mode 100644 index 0000000000..e0153bffa8 --- /dev/null +++ b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/SimpleClass.java @@ -0,0 +1,23 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package co.elastic.apm.agent.sdk.bytebuddy.clazzes; + +public class SimpleClass { + +} diff --git a/apm-agent-plugins/apm-scheduled-annotation-plugin/src/test/java/co/elastic/apm/agent/scheduled/TimerTaskInstrumentationTest.java b/apm-agent-plugins/apm-scheduled-annotation-plugin/src/test/java/co/elastic/apm/agent/scheduled/TimerTaskInstrumentationTest.java index 49c8a4e1a0..d5a8a1163c 100644 --- a/apm-agent-plugins/apm-scheduled-annotation-plugin/src/test/java/co/elastic/apm/agent/scheduled/TimerTaskInstrumentationTest.java +++ b/apm-agent-plugins/apm-scheduled-annotation-plugin/src/test/java/co/elastic/apm/agent/scheduled/TimerTaskInstrumentationTest.java @@ -80,7 +80,7 @@ public void run() { }, 1); reporter.awaitTransactionCount(1); - assertThat(reporter.getTransactions().get(0).getNameAsString()).isEqualTo("1#run"); + assertThat(reporter.getTransactions().get(0).getNameAsString()).isEqualTo("TimerTaskInstrumentationTest$1#run"); } public static class TestTimerTask extends TimerTask {