From 7a475e17c2728308d037a663c4dd4afcd62206ca Mon Sep 17 00:00:00 2001 From: "13997737+wolframhaussig@users.noreply.github.com" <13997737+wolframhaussig@users.noreply.github.com> Date: Thu, 1 Aug 2024 12:44:51 +0200 Subject: [PATCH 1/6] enhance class name detection --- .../agent/sdk/bytebuddy/ClassNameParser.java | 56 +++++++++++++++++ ...leMethodSignatureOffsetMappingFactory.java | 3 +- .../sdk/bytebuddy/ClassNameParserTest.java | 60 +++++++++++++++++++ .../sdk/bytebuddy/clazzes/AnonymousClass.java | 30 ++++++++++ .../sdk/bytebuddy/clazzes/Dollar$Class.java | 23 +++++++ .../sdk/bytebuddy/clazzes/ParentClass.java | 25 ++++++++ .../sdk/bytebuddy/clazzes/ScalaObject$.java | 27 +++++++++ .../sdk/bytebuddy/clazzes/SimpleClass.java | 23 +++++++ 8 files changed, 245 insertions(+), 2 deletions(-) create mode 100644 apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/ClassNameParser.java create mode 100644 apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/ClassNameParserTest.java create mode 100644 apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/AnonymousClass.java create mode 100644 apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/Dollar$Class.java create mode 100644 apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ParentClass.java create mode 100644 apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ScalaObject$.java create mode 100644 apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/SimpleClass.java 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..50ad8715f9 --- /dev/null +++ b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/bytebuddy/ClassNameParser.java @@ -0,0 +1,56 @@ +/* + * 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() {} + /** + * 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); + } else { + int dollarIndex = className.lastIndexOf('$'); + String innerClassName = className.substring(dollarIndex + 1); + if(innerClassName.matches("\\d+")) { + // this is an anonymous inner class + // we don't want to include the number in the class name + className = className.substring(0, dollarIndex); + } else { + className = innerClassName; + } + } + } + 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..402131a956 --- /dev/null +++ b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/ClassNameParserTest.java @@ -0,0 +1,60 @@ +/* + * 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.SimpleClass; +import co.elastic.apm.agent.sdk.bytebuddy.clazzes.AnonymousClass; +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 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"); + } + @Test + void testScalaObjectClassName() { + String className = ClassNameParser.parse(ScalaObject$.class.getName()); + assertThat(className).isEqualTo("ScalaObject"); + } +} 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/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..a9493284ee --- /dev/null +++ b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ParentClass.java @@ -0,0 +1,25 @@ +/* + * 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 { + } +} 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 { + +} From efc249ffbfc3242428f074b3b53dca3bff671f32 Mon Sep 17 00:00:00 2001 From: "13997737+wolframhaussig@users.noreply.github.com" <13997737+wolframhaussig@users.noreply.github.com> Date: Thu, 8 Aug 2024 12:06:40 +0200 Subject: [PATCH 2/6] added fix and test for nested scala objects --- .../agent/sdk/bytebuddy/ClassNameParser.java | 26 +++++++++++------ .../sdk/bytebuddy/ClassNameParserTest.java | 6 ++++ .../sdk/bytebuddy/clazzes/ParentObject.java | 29 +++++++++++++++++++ 3 files changed, 52 insertions(+), 9 deletions(-) create mode 100644 apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ParentObject.java 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 index 50ad8715f9..c3a17e2ac8 100644 --- 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 @@ -26,6 +26,18 @@ public class ClassNameParser { * Utility class, do not instantiate */ private ClassNameParser() {} + private static String getNestedClassName(String className) { + int dollarIndex = className.lastIndexOf('$'); + String innerClassName = className.substring(dollarIndex + 1); + if(innerClassName.matches("\\d+")) { + // this is an anonymous inner class + // we don't want to include the number in the class name + className = className.substring(0, dollarIndex); + } else { + className = innerClassName; + } + return className; + } /** * Parses the simple class name from a class name * @param className @@ -39,16 +51,12 @@ public static String parse(String className) { // 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); - } else { - int dollarIndex = className.lastIndexOf('$'); - String innerClassName = className.substring(dollarIndex + 1); - if(innerClassName.matches("\\d+")) { - // this is an anonymous inner class - // we don't want to include the number in the class name - className = className.substring(0, dollarIndex); - } else { - className = innerClassName; + if(className.contains("$")) + { + className = getNestedClassName(className); } + } else { + className = getNestedClassName(className); } } return className; 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 index 402131a956..941814e893 100644 --- 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 @@ -23,6 +23,7 @@ import org.junit.jupiter.api.Test; import co.elastic.apm.agent.sdk.bytebuddy.clazzes.ParentClass.InnerClass; +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.Dollar$Class; @@ -57,4 +58,9 @@ 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/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$ { + + } +} From b5f2406bc1a08c34d9aab9e52ab309782cff0c3b Mon Sep 17 00:00:00 2001 From: "13997737+wolframhaussig@users.noreply.github.com" <13997737+wolframhaussig@users.noreply.github.com> Date: Thu, 8 Aug 2024 12:14:14 +0200 Subject: [PATCH 3/6] added changelog entry --- CHANGELOG.asciidoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 99ad0570c8..865f779bf1 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -31,6 +31,9 @@ Use subheadings with the "=====" level for adding notes for unreleased changes: === Unreleased +===== Bug fixes +* Improve automatic span class name detection for Scala and nested/anonymous classes - {pull}3746[#3746] + [[release-notes-1.x]] === Java Agent version 1.x From 211a04321ba6dfdd15eee549224563143bb8ed1d Mon Sep 17 00:00:00 2001 From: "13997737+wolframhaussig@users.noreply.github.com" <13997737+wolframhaussig@users.noreply.github.com> Date: Mon, 12 Aug 2024 07:11:25 +0200 Subject: [PATCH 4/6] remove regex --- .../apm/agent/sdk/bytebuddy/ClassNameParser.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) 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 index c3a17e2ac8..7744c83243 100644 --- 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 @@ -26,10 +26,23 @@ 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; + } private static String getNestedClassName(String className) { int dollarIndex = className.lastIndexOf('$'); String innerClassName = className.substring(dollarIndex + 1); - if(innerClassName.matches("\\d+")) { + if(isNumeric(innerClassName)) { // this is an anonymous inner class // we don't want to include the number in the class name className = className.substring(0, dollarIndex); From 62d96d7a01ab15c29e0bfb437138df5855fd5670 Mon Sep 17 00:00:00 2001 From: "13997737+wolframhaussig@users.noreply.github.com" <13997737+wolframhaussig@users.noreply.github.com> Date: Mon, 12 Aug 2024 07:44:29 +0200 Subject: [PATCH 5/6] fix anonymous class names - anonymous classes are now shown as "ParentClass$1" instead of "ParentClass" - added more testcases --- .../agent/sdk/bytebuddy/ClassNameParser.java | 29 +++++++++++++- .../sdk/bytebuddy/ClassNameParserTest.java | 19 ++++++++- .../clazzes/AnonymousNestedClass.java | 39 +++++++++++++++++++ .../sdk/bytebuddy/clazzes/ParentClass.java | 11 ++++++ 4 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/AnonymousNestedClass.java 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 index 7744c83243..d8c327d0ed 100644 --- 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 @@ -39,13 +39,38 @@ private static boolean isNumeric(String str) { } 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 - // we don't want to include the number in the class name - className = className.substring(0, dollarIndex); + className = getAnonymousClassName(className); } else { className = innerClassName; } 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 index 941814e893..bc76d3dd18 100644 --- 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 @@ -23,9 +23,11 @@ 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$; @@ -42,6 +44,11 @@ void testNestedClassName() { 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 @@ -51,7 +58,17 @@ void testDollarClassName() { @Test void testAnonymousClassName() { String className = ClassNameParser.parse(AnonymousClass.getAnonymousClass().getName()); - assertThat(className).isEqualTo("AnonymousClass"); + 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() { 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/ParentClass.java b/apm-agent-plugin-sdk/src/test/java/co/elastic/apm/agent/sdk/bytebuddy/clazzes/ParentClass.java index a9493284ee..99799af8bf 100644 --- 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 @@ -21,5 +21,16 @@ 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 { + + } } } From 4b4ab46e17c24121258e1662a5114248f333232b Mon Sep 17 00:00:00 2001 From: "13997737+wolframhaussig@users.noreply.github.com" <13997737+wolframhaussig@users.noreply.github.com> Date: Wed, 14 Aug 2024 09:31:41 +0200 Subject: [PATCH 6/6] fix unit tests --- .../apm/agent/scheduled/TimerTaskInstrumentationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 {