From 512c31803c96ca5ba1b97028863d4f33b093e23c Mon Sep 17 00:00:00 2001 From: John Bley Date: Thu, 6 Feb 2025 16:24:46 -0500 Subject: [PATCH 01/62] Initial veeeeery messing prototype of the nocode instrumentation concept --- .../bootstrap/nocode/NocodeRules.java | 60 ++++++++ instrumentation/nocode/build.gradle.kts | 11 ++ .../instrumentation/nocode/JSPS.java | 83 +++++++++++ .../nocode/NocodeInstrumentation.java | 100 +++++++++++++ .../instrumentation/nocode/NocodeModule.java | 47 ++++++ .../nocode/NocodeSingletons.java | 28 ++++ .../instrumentation/nocode/YamlParser.java | 90 ++++++++++++ .../instrumentation/nocode/JSPSTest.java | 136 ++++++++++++++++++ settings.gradle.kts | 1 + 9 files changed, 556 insertions(+) create mode 100644 bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java create mode 100644 instrumentation/nocode/build.gradle.kts create mode 100644 instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java create mode 100644 instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java create mode 100644 instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java create mode 100644 instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java create mode 100644 instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java create mode 100644 instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java diff --git a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java new file mode 100644 index 000000000..0f0b50483 --- /dev/null +++ b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java @@ -0,0 +1,60 @@ +package com.splunk.opentelemetry.javaagent.nocode; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; + +public class NocodeRules { + + public static class Rule { + public final String className; + public final String methodName; + public final Map attributes; // key name to jsps + public Rule(String className, String methodName, Map attributes) { + this.className = className; + this.methodName = methodName; + this.attributes = Collections.unmodifiableMap(new HashMap<>(attributes)); + } + public Map getAttributes() { + return attributes; + } + public String toString() { + return className+"."+methodName+"=attrs:"+attributes; + } + } + + // FIXME this is awkward atomicity due to unique nature of initialization and access patterns. + // revisit if this placement actually works + private static final AtomicReference> GlobalRules = new AtomicReference<>(Collections.EMPTY_LIST); + private static final AtomicReference>> c2m2r = new AtomicReference<>(Collections.emptyMap()); + + public static void setGlobalRules(List rules) { + GlobalRules.set(Collections.unmodifiableList(new ArrayList<>(rules))); + Map> lookups = new HashMap<>(); + for(Rule r : rules) { + Map method2rule = lookups.computeIfAbsent(r.className, k -> new HashMap<>()); + method2rule.put(r.methodName, r); + } + // FIXME awkward structure, particularly since subfields aren't easily marked unmodifiable + c2m2r.set(Collections.unmodifiableMap(lookups)); + } + + public static List getGlobalRules() { + return GlobalRules.get(); + } + + public static Rule findRuleByClassAndMethod(String className, String methodName) { + Map> lookups = c2m2r.get(); + Map m2r = lookups.get(className); + if (m2r == null) { + System.out.println("JBLEY NO M2R"); + return null; + } + return m2r.get(methodName); + } + + +} diff --git a/instrumentation/nocode/build.gradle.kts b/instrumentation/nocode/build.gradle.kts new file mode 100644 index 000000000..cbf6d6fb8 --- /dev/null +++ b/instrumentation/nocode/build.gradle.kts @@ -0,0 +1,11 @@ +plugins { + id("splunk.instrumentation-conventions") +} + + +dependencies { + compileOnly("io.opentelemetry.javaagent:opentelemetry-javaagent-tooling") + compileOnly("org.snakeyaml:snakeyaml-engine:2.8") + //implementation("org.yaml:snakeyaml:2.3") + // implementation("org.snakeyaml:snakeyaml-engine:2.8") +} diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java new file mode 100644 index 000000000..d228a6237 --- /dev/null +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java @@ -0,0 +1,83 @@ +package com.splunk.opentelemetry.instrumentation.nocode; + +import java.lang.reflect.Method; + +public class JSPS { + + public static String evaluate(String jsps, Object thiz, Object[] params) { + try { + return unsafeEvaluate(jsps, thiz, params); + } catch (Throwable t) { + // FIXME better logging + System.out.println("can't eval jsps: "+t); + return null; + } + } + + + // FIXME improve performance - reduce allocations, etc. + private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) throws Exception { + jsps = jsps.trim(); + int nextDot = jsps.indexOf('.'); + String var = jsps.substring(0, nextDot).trim(); + Object curObject = null; + if (var.equals("this")) { + curObject = thiz; + } else if (var.startsWith("param")) { + int varIndex = Integer.parseInt(var.substring("param".length())); + curObject = params[varIndex]; + } + int curIndex = nextDot; + while(curIndex < jsps.length() && (jsps.charAt(curIndex) == '.' || Character.isWhitespace(jsps.charAt(curIndex)))) { + curIndex++; + } + while(curIndex < jsps.length()) { + int openParen = jsps.indexOf('(', curIndex); + String method = jsps.substring(curIndex, openParen).trim(); + int closeParen = jsps.indexOf(')', openParen); + String paramString = jsps.substring(openParen + 1, closeParen).trim(); + if (paramString.isEmpty()) { + // FIXME how does javaagent open the class? getting exceptions gere + // e.g., with hashmap.entrySet().size() on the entryset accessor + Object nextObject = curObject.getClass().getMethod(method).invoke(curObject); + curObject = nextObject; + } else { + if (paramString.startsWith("\"") && paramString.endsWith("\"")) { + String passed = paramString.substring(1, paramString.length()-1); + Method m = findMethod(curObject, method, + String.class, Object.class); + curObject = m.invoke(curObject, passed); + } else if (paramString.equals("true") || paramString.equals("false")) { + Method m = findMethod(curObject, method, + boolean.class, Boolean.class, Object.class); + curObject = m.invoke(curObject, Boolean.parseBoolean(paramString)); + } else if (paramString.matches("[0-9]+")) { + Method m = findMethod(curObject, method, + int.class, Integer.class, long.class, Long.class, Object.class); + int passed = Integer.parseInt(paramString); + curObject = m.invoke(curObject, passed); + } else { + throw new UnsupportedOperationException("Can't parse \""+paramString+"\" as literal parameter"); + } + } + curIndex = closeParen + 1; + while(curIndex < jsps.length() && (jsps.charAt(curIndex) == '.' || Character.isWhitespace(jsps.charAt(curIndex)))) { + curIndex++; + } + } + return curObject == null ? null : curObject.toString(); + } + + // FIXME must be a better way to look through a series of type options + private static Method findMethod(Object curObject, String methodName, Class... paramTypesToTryInOrder) throws NoSuchMethodException{ + Class c = curObject.getClass(); + for(Class paramType : paramTypesToTryInOrder) { + try { + return c.getMethod(methodName, paramType); + } catch (NoSuchMethodException e) { + // try next one + } + } + throw new NoSuchMethodException(methodName + " with single parameter matching given type"); + } +} diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java new file mode 100644 index 000000000..6d88b4da2 --- /dev/null +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -0,0 +1,100 @@ +package com.splunk.opentelemetry.instrumentation.nocode; + +import com.splunk.opentelemetry.javaagent.nocode.NocodeRules; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; +import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +import java.lang.reflect.Method; +import java.util.Collections; +import java.util.Map; + +import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; +import static com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons.instrumentor; + +import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.named; + +public class NocodeInstrumentation implements TypeInstrumentation { + private final NocodeRules.Rule rule; + + public NocodeInstrumentation(NocodeRules.Rule rule) { + this.rule = rule; + } + + @Override + public ElementMatcher typeMatcher() { + return hasSuperType(named(rule.className)); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod(named(rule.methodName), + NocodeInstrumentation.class.getName()+"$NocodeAdvice"); + } + + @SuppressWarnings("unused") + public static class NocodeAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.Origin("#t") Class declaringClass, + @Advice.Origin("#m") String methodName, + @Advice.Local("otelMethod") ClassAndMethod classAndMethod, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope, + @Advice.This Object thiz, + @Advice.AllArguments Object[] methodParams + ) { + System.out.println("JBLEY INJECTED START"); + Context parentContext = currentContext(); + classAndMethod = ClassAndMethod.create(declaringClass, methodName); + // FIXME probably need to rework logic here to just use otel span + // creation api rather than this abstraction around it + if (!instrumentor().shouldStart(parentContext, classAndMethod)) { + return; + } + context = instrumentor().start(parentContext, classAndMethod); + scope = context.makeCurrent(); + Map attributes = Collections.EMPTY_MAP; + NocodeRules.Rule rule = NocodeRules.findRuleByClassAndMethod(declaringClass.getName(), methodName); // FIXME declaring class? + if (rule != null) { + System.out.println("JBLEY ADVICE RULE LOOKUP FOUND "+rule); + attributes = rule.getAttributes(); + } + + for(String key : attributes.keySet()) { + String jsps = attributes.get(key); + String value = JSPS.evaluate(jsps, thiz, methodParams); + System.out.println("JBLEY ATTRIBUTE "+key+" = "+value); + if (value != null) { + // FIXME java8 bridge nonsense + Span.current().setAttribute(key, value); + } + } + } + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void stopSpan( + @Advice.Origin Method method, + @Advice.Local("otelMethod") ClassAndMethod classAndMethod, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) + { + // FIXME @Advice.Thrown and Return + System.out.println("JBLEY INJECTED END"); + if (scope == null) { + return; + } + scope.close(); + // FIXME what is this nonsense copied from AsyncOperationSupport or something like that? + instrumentor().end(context, classAndMethod, null, null); + + } + } +} diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java new file mode 100644 index 000000000..df7196ef0 --- /dev/null +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java @@ -0,0 +1,47 @@ + +package com.splunk.opentelemetry.instrumentation.nocode; + +import com.google.auto.service.AutoService; +import com.splunk.opentelemetry.javaagent.nocode.NocodeRules; +import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +@AutoService(InstrumentationModule.class) +public class NocodeModule extends InstrumentationModule { + + // FIXME style + + public NocodeModule() { + super("nocode"); + YamlParser yp = new YamlParser(); + NocodeRules.setGlobalRules(yp.getInstrumentationRules()); +// GlobalYaml.set(new YamlParser()); + } + + @Override + public List typeInstrumentations() { + ArrayList answer = new ArrayList<>(); + // FIXME is doing this parsing in two places necessary for classloader stuff or can I just use the singleton? + for(NocodeRules.Rule rule : NocodeRules.getGlobalRules()) { + answer.add(new NocodeInstrumentation(rule)); + System.out.println("Added instrumentation for rule "+ rule);; + } + return answer; + } + + @Override + public List getAdditionalHelperClassNames() { + return Arrays.asList( + "com.splunk.opentelemetry.instrumentation.nocode.JSPS", + "com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons" + ); + } + + @Override + public int order() { + return Integer.MAX_VALUE; + } +} diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java new file mode 100644 index 000000000..216f56c4f --- /dev/null +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java @@ -0,0 +1,28 @@ +package com.splunk.opentelemetry.instrumentation.nocode; + +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor; +import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesGetter; +import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeSpanNameExtractor; +import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor; + +public class NocodeSingletons { + // FIXME so much copy and paste from Methods instrumentation + private static final Instrumenter INSTRUMENTER; + + static { + CodeAttributesGetter cag = ClassAndMethod.codeAttributesGetter(); + + INSTRUMENTER = Instrumenter.builder( + GlobalOpenTelemetry.get(), + "com.splunk.opentelemetry.instrumentation.nocode", + CodeSpanNameExtractor.create(cag)).addAttributesExtractor(CodeAttributesExtractor.create(cag)) + .buildInstrumenter(SpanKindExtractor.alwaysInternal()); + } + + public static Instrumenter instrumentor() { + return INSTRUMENTER; + } +} diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java new file mode 100644 index 000000000..dae93f118 --- /dev/null +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -0,0 +1,90 @@ +package com.splunk.opentelemetry.instrumentation.nocode; + +import com.splunk.opentelemetry.javaagent.nocode.NocodeRules; +import java.io.BufferedReader; +import java.io.File; +import java.io.FileReader; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +// FIXME rename and refactor if new non-static structure works +public class YamlParser { + + +private final List InstrumentationRules; + + public YamlParser() { + InstrumentationRules = Collections.unmodifiableList(new ArrayList<>(load())); + } + + public List getInstrumentationRules() { + return InstrumentationRules; + } + + // FIXME hardcoded file name, obviously needs to be an env variable + // FIXME surely there is a utility for this somewhere in the agent's scope + private static String readYamlFile() throws Exception { + File f = new File("/Users/jbley/dev/java/nocode/nocode.yml"); + BufferedReader br = new BufferedReader(new FileReader(f)); + String line; + StringBuffer buf = new StringBuffer(); + while((line = br.readLine()) != null) { + buf.append(line); + buf.append(System.lineSeparator()); + } + br.close(); + return buf.toString(); + } + + + private static List load() { + try { + return loadUnsafe(); + } catch (Exception e) { + // FIXME error handling + System.out.println("JBLEY can't load yaml: "+e); + return Collections.emptyList(); + } + } + + private static List loadUnsafe() throws Exception { + String yamlString = readYamlFile(); + System.out.println("----- yaml ------"); + System.out.println(yamlString); + System.out.println("----- end ------"); + + // FIXME why can't I figure out how to reference the snakeyaml that's already inside the agent jar? + // This nonsense is here to do a reflective load of the yaml parser that's already there + // and parse the config file + Class loadSettingsC = Class.forName("org.snakeyaml.engine.v2.api.LoadSettings"); + Class loadC = Class.forName("org.snakeyaml.engine.v2.api.Load"); + Object lsb = loadSettingsC.getMethod("builder").invoke(null); + Object loadSettings = lsb.getClass().getMethod("build").invoke(lsb); + Object load = loadC.getConstructor(loadSettingsC).newInstance(loadSettings); + Iterable parsedYaml = (Iterable) load.getClass().getMethod("loadAllFromString", String.class).invoke(load, yamlString); + ArrayList answer = new ArrayList<>(); + for(Object yamlBit : parsedYaml) { + List l = (List) yamlBit; + for(Object sub : l) { + LinkedHashMap lhm = (LinkedHashMap) sub; + String className = lhm.get("class").toString(); + String methodName = lhm.get("method").toString(); + List attrs = (List) lhm.get("attributes"); + Map ruleAttributes = new HashMap<>(); + for(Object attr : attrs) { + LinkedHashMap attrMap = (LinkedHashMap) attr; + ruleAttributes.put(attrMap.get("key").toString(), attrMap.get("value").toString()); + } + answer.add(new NocodeRules.Rule(className, methodName, ruleAttributes)); + } + } + return answer; + } + + + +} diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java new file mode 100644 index 000000000..7fb0e43e5 --- /dev/null +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -0,0 +1,136 @@ +package com.splunk.opentelemetry.instrumentation.nocode; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +public class JSPSTest { + private static final Map thiz = new HashMap<>(); + private static final Set param0 = new HashSet<>(); + + static { + thiz.put("key", "value"); + param0.add("present"); + } + + @Test + public void testBasicBehavior() { + String[][] tests = new String[][] { + // FIXME support bare "this" + {"this.toString()", "{key=value}"}, + {"this.toString().length()", "11"}, + {"this.get(\"key\")", "value"}, + {"this.get(\"key\").substring(1)", "alue"}, + {"param0.isEmpty()", "false"}, + }; + for(String[] test : tests) { + assertEquals(test[1], JSPS.evaluate(test[0], thiz, new Object[]{param0}), test[0]); + } + } + + // FIXME support escaping quotes in string? + // FIXME test all supported types in type chains + + @Test + public void testInvalidJspsReturnNull() { + String[] invalids = new String[] { + "nosuchvar", + "nosuchvar.toString()", + // "this .", // FIXME currently passes + // "this . ", // FIXME currently passes! + "this.noSuchMethod()", + "param1.toString()", // out of bounds + "this.getOrDefault(\"key\", \"multiparamnotsupported\")", + "this.get(\"noclosequote)", + "this.get(\"nocloseparan\"", + "this.noparens", + "this.noparens.anotherMethod()", + "this.wrongOrder)(", + "this.get(NotALiteralParameter);", + "this.get(12.2)", + "this.get(this)", + "this.get(\"NoSuchKey\")", // evals completely but returns null + }; + for(String invalid : invalids) { + String answer = JSPS.evaluate(invalid, thiz, new Object[]{param0}); + assertNull(answer, "Expected null for \"" + invalid + "\" but was \"" + answer + "\""); + } + } + + @Test + public void testIntegerLiteralLongerThanOneDigit() { + Map o = new HashMap<>(); + o.put("key", "really long value"); + String jsps = "this.get(\"key\").substring(10)"; + assertEquals("g value", JSPS.evaluate(jsps, o, new Object[0])); + } + + public static class TakeString { + public String take(String s) { + return s; + } + } + public static class TakeObject { + public String take(Object o) { + return o.toString(); + } + } + + public static class TakeBooleanPrimitive { + public String take(boolean param) { + return Boolean.toString(param); + } + } + public static class TakeBoolean { + public String take(Boolean param) { + return param.toString(); + } + } + + @Test + public void testBooleanLiteralParamTypes() { + TakeBooleanPrimitive b = new TakeBooleanPrimitive(); + TakeBoolean B = new TakeBoolean(); + TakeObject O = new TakeObject(); + assertEquals("true", JSPS.evaluate("this.take(true)", b, new Object[0])); + assertEquals("false", JSPS.evaluate("this.take(false)", B, new Object[0])); + assertEquals("true", JSPS.evaluate("this.take(true)", O, new Object[0])); + } + + @Test + public void testStringLiteralParamTypes() { + TakeString S = new TakeString(); + TakeObject O = new TakeObject(); + assertEquals("a", JSPS.evaluate("this.take(\"a\")", S, new Object[0])); + assertEquals("a", JSPS.evaluate("this.take(\"a\")", O, new Object[0])); + } + + @Test + public void testWhitespace() { + String[] tests = new String[]{ + "this.get(\"key\").substring(1)", + " this.get(\"key\").substring(1)", + "this .get(\"key\").substring(1)", + "this. get(\"key\").substring(1)", + "this.get (\"key\").substring(1)", + "this.get( \"key\").substring(1)", + "this.get(\"key\" ).substring(1)", + "this.get(\"key\") .substring(1)", + "this.get(\"key\"). substring(1)", + "this.get(\"key\").substring (1)", + "this.get(\"key\").substring( 1)", + "this.get(\"key\").substring(1 )", + }; + for(String test : tests) { + assertEquals("alue", JSPS.evaluate(test, thiz, new Object[]{param0}), test); + + } + } + +} diff --git a/settings.gradle.kts b/settings.gradle.kts index 70b24d172..52a5428a7 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -31,6 +31,7 @@ include( "instrumentation:jvm-metrics", "instrumentation:khttp", "instrumentation:liberty", + "instrumentation:nocode", "instrumentation:servlet-3-testing", "instrumentation:tomcat", "instrumentation:tomee", From 44bf2ae31d04d08256e675831cae3007a5865a3e Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 7 Feb 2025 06:57:35 -0500 Subject: [PATCH 02/62] Add and fix integral literal tests, misc other FIXME cleanups --- .../instrumentation/nocode/JSPS.java | 13 +++++-- .../nocode/NocodeInstrumentation.java | 5 ++- .../instrumentation/nocode/NocodeModule.java | 1 - .../instrumentation/nocode/JSPSTest.java | 37 ++++++++++++++++++- 4 files changed, 47 insertions(+), 9 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java index d228a6237..0fa63fc6e 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java @@ -52,10 +52,15 @@ private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) boolean.class, Boolean.class, Object.class); curObject = m.invoke(curObject, Boolean.parseBoolean(paramString)); } else if (paramString.matches("[0-9]+")) { - Method m = findMethod(curObject, method, - int.class, Integer.class, long.class, Long.class, Object.class); - int passed = Integer.parseInt(paramString); - curObject = m.invoke(curObject, passed); + try { + Method m = findMethod(curObject, method, int.class, Integer.class, Object.class); + int passed = Integer.parseInt(paramString); + curObject = m.invoke(curObject, passed); + } catch (NoSuchMethodException tryLongInstead) { + Method m = findMethod(curObject, method, long.class, Long.class, Object.class); + long passed = Long.parseLong(paramString); + curObject = m.invoke(curObject, passed); + } } else { throw new UnsupportedOperationException("Can't parse \""+paramString+"\" as literal parameter"); } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index 6d88b4da2..c0f0e4608 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -30,7 +30,8 @@ public NocodeInstrumentation(NocodeRules.Rule rule) { @Override public ElementMatcher typeMatcher() { - return hasSuperType(named(rule.className)); + // exact match for now, if updated think about class name to use in naming the span/attributes + return named(rule.className); } @Override @@ -62,7 +63,7 @@ public static void onEnter( context = instrumentor().start(parentContext, classAndMethod); scope = context.makeCurrent(); Map attributes = Collections.EMPTY_MAP; - NocodeRules.Rule rule = NocodeRules.findRuleByClassAndMethod(declaringClass.getName(), methodName); // FIXME declaring class? + NocodeRules.Rule rule = NocodeRules.findRuleByClassAndMethod(declaringClass.getName(), methodName); if (rule != null) { System.out.println("JBLEY ADVICE RULE LOOKUP FOUND "+rule); attributes = rule.getAttributes(); diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java index df7196ef0..032fa5c65 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java @@ -18,7 +18,6 @@ public NocodeModule() { super("nocode"); YamlParser yp = new YamlParser(); NocodeRules.setGlobalRules(yp.getInstrumentationRules()); -// GlobalYaml.set(new YamlParser()); } @Override diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index 7fb0e43e5..7ededc814 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -35,8 +35,6 @@ public void testBasicBehavior() { } // FIXME support escaping quotes in string? - // FIXME test all supported types in type chains - @Test public void testInvalidJspsReturnNull() { String[] invalids = new String[] { @@ -92,6 +90,27 @@ public String take(Boolean param) { return param.toString(); } } + public static class TakeIntegerPrimitive { + public String take(int param) { + return Integer.toString(param); + } + } + public static class TakeInteger { + public String take(Integer param) { + return param.toString(); + } + } + public static class TakeLongPrimitize { + public String take(long param) { + return Long.toString(param); + } + } + public static class TakeLong { + public String take(Long param) { + return param.toString(); + } + } + @Test public void testBooleanLiteralParamTypes() { @@ -111,6 +130,20 @@ public void testStringLiteralParamTypes() { assertEquals("a", JSPS.evaluate("this.take(\"a\")", O, new Object[0])); } + @Test + public void testIntegerLiteralParamTypes() { + TakeIntegerPrimitive i = new TakeIntegerPrimitive(); + TakeInteger I = new TakeInteger(); + TakeLongPrimitize l = new TakeLongPrimitize(); + TakeLong L = new TakeLong(); + TakeObject O = new TakeObject(); + assertEquals("13", JSPS.evaluate("this.take(13)", i, new Object[0])); + assertEquals("13", JSPS.evaluate("this.take(13)", I, new Object[0])); + assertEquals("13", JSPS.evaluate("this.take(13)", l, new Object[0])); + assertEquals("13", JSPS.evaluate("this.take(13)", L, new Object[0])); + assertEquals("13", JSPS.evaluate("this.take(13)", O, new Object[0])); + } + @Test public void testWhitespace() { String[] tests = new String[]{ From 3af909ea3730ae8e6791727a55ccbcdbc057a80e Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 7 Feb 2025 07:39:14 -0500 Subject: [PATCH 03/62] Cleanup initialization logic and immutability around lookup structure --- .../bootstrap/nocode/NocodeRules.java | 25 ++++++++----------- .../instrumentation/nocode/JSPSTest.java | 2 ++ 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java index 0f0b50483..cf9c441cf 100644 --- a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java +++ b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java @@ -26,20 +26,20 @@ public String toString() { } } - // FIXME this is awkward atomicity due to unique nature of initialization and access patterns. - // revisit if this placement actually works + // Unfortunately the particular sequence of who needs access to this and who can create (based on + // having the yaml parser library loaded) is awkward. Would prefer this to be a simple + // static final immutable computed at startup... private static final AtomicReference> GlobalRules = new AtomicReference<>(Collections.EMPTY_LIST); - private static final AtomicReference>> c2m2r = new AtomicReference<>(Collections.emptyMap()); + // Using className.methodName as the key + private static final AtomicReference> Name2Rule = new AtomicReference<>(Collections.emptyMap()); public static void setGlobalRules(List rules) { GlobalRules.set(Collections.unmodifiableList(new ArrayList<>(rules))); - Map> lookups = new HashMap<>(); + Map n2r = new HashMap<>(); for(Rule r : rules) { - Map method2rule = lookups.computeIfAbsent(r.className, k -> new HashMap<>()); - method2rule.put(r.methodName, r); + n2r.put(r.className + "." + r.methodName, r); } - // FIXME awkward structure, particularly since subfields aren't easily marked unmodifiable - c2m2r.set(Collections.unmodifiableMap(lookups)); + Name2Rule.set(Collections.unmodifiableMap(n2r)); } public static List getGlobalRules() { @@ -47,13 +47,8 @@ public static List getGlobalRules() { } public static Rule findRuleByClassAndMethod(String className, String methodName) { - Map> lookups = c2m2r.get(); - Map m2r = lookups.get(className); - if (m2r == null) { - System.out.println("JBLEY NO M2R"); - return null; - } - return m2r.get(methodName); + Map n2r = Name2Rule.get(); + return n2r == null ? null : n2r.get(className + "." + methodName); } diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index 7ededc814..e41249332 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -10,6 +10,7 @@ import java.util.Map; import java.util.Set; +// FIXME need tests for the instrumentation too! public class JSPSTest { private static final Map thiz = new HashMap<>(); private static final Set param0 = new HashSet<>(); @@ -28,6 +29,7 @@ public void testBasicBehavior() { {"this.get(\"key\")", "value"}, {"this.get(\"key\").substring(1)", "alue"}, {"param0.isEmpty()", "false"}, + {"param0.contains(\"present\")", "true"}, }; for(String[] test : tests) { assertEquals(test[1], JSPS.evaluate(test[0], thiz, new Object[]{param0}), test[0]); From ff7dcbd0eafb21ac1af849a869732d88a3e2f257 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 7 Feb 2025 07:44:58 -0500 Subject: [PATCH 04/62] Tests, FIXMEs, some debug printouts removed --- .../instrumentation/nocode/NocodeInstrumentation.java | 1 - .../instrumentation/nocode/NocodeModule.java | 3 --- .../opentelemetry/instrumentation/nocode/YamlParser.java | 3 --- .../opentelemetry/instrumentation/nocode/JSPSTest.java | 9 +++++++++ 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index c0f0e4608..4aeef8179 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -65,7 +65,6 @@ public static void onEnter( Map attributes = Collections.EMPTY_MAP; NocodeRules.Rule rule = NocodeRules.findRuleByClassAndMethod(declaringClass.getName(), methodName); if (rule != null) { - System.out.println("JBLEY ADVICE RULE LOOKUP FOUND "+rule); attributes = rule.getAttributes(); } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java index 032fa5c65..d7f7ad6ba 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java @@ -12,8 +12,6 @@ @AutoService(InstrumentationModule.class) public class NocodeModule extends InstrumentationModule { - // FIXME style - public NocodeModule() { super("nocode"); YamlParser yp = new YamlParser(); @@ -26,7 +24,6 @@ public List typeInstrumentations() { // FIXME is doing this parsing in two places necessary for classloader stuff or can I just use the singleton? for(NocodeRules.Rule rule : NocodeRules.getGlobalRules()) { answer.add(new NocodeInstrumentation(rule)); - System.out.println("Added instrumentation for rule "+ rule);; } return answer; } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index dae93f118..c8f38e3fc 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -53,9 +53,6 @@ private static List load() { private static List loadUnsafe() throws Exception { String yamlString = readYamlFile(); - System.out.println("----- yaml ------"); - System.out.println(yamlString); - System.out.println("----- end ------"); // FIXME why can't I figure out how to reference the snakeyaml that's already inside the agent jar? // This nonsense is here to do a reflective load of the yaml parser that's already there diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index e41249332..1875b0b64 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -5,6 +5,7 @@ import org.junit.jupiter.api.Test; +import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -56,6 +57,7 @@ public void testInvalidJspsReturnNull() { "this.get(12.2)", "this.get(this)", "this.get(\"NoSuchKey\")", // evals completely but returns null + "param1.toString()", // no such param }; for(String invalid : invalids) { String answer = JSPS.evaluate(invalid, thiz, new Object[]{param0}); @@ -168,4 +170,11 @@ public void testWhitespace() { } } + public void testManyParams() { + Object[] params = new Object[13]; + Arrays.fill(params, new Object()); + assertEquals("java.lang.Object", JSPS.evaluate("param12.getClass().getName()", new Object(), params)); + } + + } From 00fe0ce78a7a54a1841787cae81445df8b59c19c Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 7 Feb 2025 07:54:15 -0500 Subject: [PATCH 05/62] Make yml file actually configurable --- .../instrumentation/nocode/YamlParser.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index c8f38e3fc..574373265 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -1,6 +1,7 @@ package com.splunk.opentelemetry.instrumentation.nocode; import com.splunk.opentelemetry.javaagent.nocode.NocodeRules; +import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; import java.io.BufferedReader; import java.io.File; import java.io.FileReader; @@ -14,8 +15,9 @@ // FIXME rename and refactor if new non-static structure works public class YamlParser { + public static final String NOCODE_YMLFILE_ENV_KEY = "SPLUNK_OTEL_INSTRUMENTATION_NOCODE_YML_FILE"; -private final List InstrumentationRules; + private final List InstrumentationRules; public YamlParser() { InstrumentationRules = Collections.unmodifiableList(new ArrayList<>(load())); @@ -25,10 +27,9 @@ public List getInstrumentationRules() { return InstrumentationRules; } - // FIXME hardcoded file name, obviously needs to be an env variable - // FIXME surely there is a utility for this somewhere in the agent's scope - private static String readYamlFile() throws Exception { - File f = new File("/Users/jbley/dev/java/nocode/nocode.yml"); + // FIXME surely there is a utility for this somewhere in the agent's scope (or just use Reader in snakeyaml) + private static String readYamlFile(String yamlFileName) throws Exception { + File f = new File(yamlFileName); BufferedReader br = new BufferedReader(new FileReader(f)); String line; StringBuffer buf = new StringBuffer(); @@ -52,7 +53,12 @@ private static List load() { } private static List loadUnsafe() throws Exception { - String yamlString = readYamlFile(); + String yamlFileName = System.getenv(NOCODE_YMLFILE_ENV_KEY); + if (yamlFileName == null || yamlFileName.trim().isEmpty()) { + return Collections.emptyList(); + } + String yamlString = readYamlFile(yamlFileName.trim()); + // FIXME why can't I figure out how to reference the snakeyaml that's already inside the agent jar? // This nonsense is here to do a reflective load of the yaml parser that's already there From cbedaa6f5b0f6f25061ac89128fc37e297cd8d4d Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 7 Feb 2025 07:55:46 -0500 Subject: [PATCH 06/62] FIXME cleanup --- .../opentelemetry/instrumentation/nocode/NocodeModule.java | 1 - .../splunk/opentelemetry/instrumentation/nocode/YamlParser.java | 1 - 2 files changed, 2 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java index d7f7ad6ba..94a052e57 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java @@ -21,7 +21,6 @@ public NocodeModule() { @Override public List typeInstrumentations() { ArrayList answer = new ArrayList<>(); - // FIXME is doing this parsing in two places necessary for classloader stuff or can I just use the singleton? for(NocodeRules.Rule rule : NocodeRules.getGlobalRules()) { answer.add(new NocodeInstrumentation(rule)); } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index 574373265..66e60537f 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -12,7 +12,6 @@ import java.util.List; import java.util.Map; -// FIXME rename and refactor if new non-static structure works public class YamlParser { public static final String NOCODE_YMLFILE_ENV_KEY = "SPLUNK_OTEL_INSTRUMENTATION_NOCODE_YML_FILE"; From a4007e9a26cff91f8ea858b7455afed001a90932 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 7 Feb 2025 17:26:01 -0500 Subject: [PATCH 07/62] Factor out interfaces desired by the instrumentation apparatus --- .../bootstrap/nocode/NocodeRules.java | 9 ++-- .../instrumentation/nocode/JSPS.java | 1 + .../nocode/NocodeAttributesExtractor.java | 50 +++++++++++++++++++ .../nocode/NocodeInstrumentation.java | 33 +++++------- .../nocode/NocodeMethodInvocation.java | 34 +++++++++++++ .../instrumentation/nocode/NocodeModule.java | 5 +- .../nocode/NocodeSingletons.java | 14 ++---- .../nocode/NocodeSpanNameExtractor.java | 28 +++++++++++ .../instrumentation/nocode/YamlParser.java | 3 +- 9 files changed, 139 insertions(+), 38 deletions(-) create mode 100644 instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java create mode 100644 instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java create mode 100644 instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java diff --git a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java index cf9c441cf..f65af039a 100644 --- a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java +++ b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java @@ -12,17 +12,16 @@ public class NocodeRules { public static class Rule { public final String className; public final String methodName; + public final String spanName; // may be null - use default of "class.method" public final Map attributes; // key name to jsps - public Rule(String className, String methodName, Map attributes) { + public Rule(String className, String methodName, String spanName, Map attributes) { this.className = className; this.methodName = methodName; + this.spanName = spanName; this.attributes = Collections.unmodifiableMap(new HashMap<>(attributes)); } - public Map getAttributes() { - return attributes; - } public String toString() { - return className+"."+methodName+"=attrs:"+attributes; + return className+"."+methodName+":spanName="+spanName+",attrs="+attributes; } } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java index 0fa63fc6e..1825a2bf0 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java @@ -2,6 +2,7 @@ import java.lang.reflect.Method; +// JSPS stands for Java-like String-Producing Statement. FIXME describe in more detail and pick a better nane public class JSPS { public static String evaluate(String jsps, Object thiz, Object[] params) { diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java new file mode 100644 index 000000000..dd35d45f5 --- /dev/null +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java @@ -0,0 +1,50 @@ +package com.splunk.opentelemetry.instrumentation.nocode; + +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor; +import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; +import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import javax.annotation.Nullable; +import java.util.Collections; +import java.util.Map; + +public class NocodeAttributesExtractor implements AttributesExtractor { + private final AttributesExtractor codeExtractor; + + public NocodeAttributesExtractor() { + codeExtractor = CodeAttributesExtractor.create(ClassAndMethod.codeAttributesGetter()); + } + @Override + public void onStart(AttributesBuilder attributesBuilder, Context context, NocodeMethodInvocation mi) { + System.out.println("JBLEY NCAE"); + codeExtractor.onStart(attributesBuilder, context, mi.getClassAndMethod()); + + Map attributes = Collections.EMPTY_MAP; + if (mi.getRule() != null) { + attributes = mi.getRule().attributes; + } + for(String key : attributes.keySet()) { + String jsps = attributes.get(key); + String value = JSPS.evaluate(jsps, mi.getThiz(), mi.getParameters()); + System.out.println("JBLEY ATTRIBUTE "+key+" = "+value); + if (value != null) { + // FIXME java8 bridge nonsense + Span.current().setAttribute(key, value); + } + } + + + } + + // FIXME this Void might be the RESULT type which might be the return value; look into it + @Override + public void onEnd(AttributesBuilder attributesBuilder, Context context, + NocodeMethodInvocation nocodeMethodInvocation, @Nullable Void unused, + @Nullable Throwable throwable) { + codeExtractor.onEnd(attributesBuilder, context, nocodeMethodInvocation.getClassAndMethod(), unused, throwable); + + + } +} diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index 4aeef8179..d184ccc41 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -46,43 +46,34 @@ public static class NocodeAdvice { public static void onEnter( @Advice.Origin("#t") Class declaringClass, @Advice.Origin("#m") String methodName, - @Advice.Local("otelMethod") ClassAndMethod classAndMethod, + @Advice.Local("otelInvocation") NocodeMethodInvocation otelInvocation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope, @Advice.This Object thiz, @Advice.AllArguments Object[] methodParams ) { System.out.println("JBLEY INJECTED START"); + NocodeRules.Rule rule = NocodeRules.findRuleByClassAndMethod(declaringClass.getName(), methodName); + otelInvocation = new NocodeMethodInvocation( + rule, + ClassAndMethod.create(declaringClass, methodName), + thiz, + methodParams); Context parentContext = currentContext(); - classAndMethod = ClassAndMethod.create(declaringClass, methodName); + // FIXME probably need to rework logic here to just use otel span // creation api rather than this abstraction around it - if (!instrumentor().shouldStart(parentContext, classAndMethod)) { + if (!instrumentor().shouldStart(parentContext, otelInvocation)) { return; } - context = instrumentor().start(parentContext, classAndMethod); + context = instrumentor().start(parentContext, otelInvocation); scope = context.makeCurrent(); - Map attributes = Collections.EMPTY_MAP; - NocodeRules.Rule rule = NocodeRules.findRuleByClassAndMethod(declaringClass.getName(), methodName); - if (rule != null) { - attributes = rule.getAttributes(); - } - - for(String key : attributes.keySet()) { - String jsps = attributes.get(key); - String value = JSPS.evaluate(jsps, thiz, methodParams); - System.out.println("JBLEY ATTRIBUTE "+key+" = "+value); - if (value != null) { - // FIXME java8 bridge nonsense - Span.current().setAttribute(key, value); - } - } } @Advice.OnMethodExit(suppress = Throwable.class) public static void stopSpan( @Advice.Origin Method method, - @Advice.Local("otelMethod") ClassAndMethod classAndMethod, + @Advice.Local("otelInvocation") NocodeMethodInvocation otelInvocation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { @@ -93,7 +84,7 @@ public static void stopSpan( } scope.close(); // FIXME what is this nonsense copied from AsyncOperationSupport or something like that? - instrumentor().end(context, classAndMethod, null, null); + instrumentor().end(context, otelInvocation, null, null); } } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java new file mode 100644 index 000000000..f5bfb5813 --- /dev/null +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java @@ -0,0 +1,34 @@ +package com.splunk.opentelemetry.instrumentation.nocode; + +import com.splunk.opentelemetry.javaagent.nocode.NocodeRules; +import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; + +public class NocodeMethodInvocation { + private final NocodeRules.Rule rule; + private final ClassAndMethod classAndMethod; + private final Object thiz; + private final Object[] parameters; + + public NocodeMethodInvocation(NocodeRules.Rule rule, ClassAndMethod cm, Object thiz, Object[] parameters) { + this.rule = rule; + this.classAndMethod = cm; + this.thiz = thiz; + this.parameters = parameters; + } + + public NocodeRules.Rule getRule() { + return rule; + } + + public Object getThiz() { + return thiz; + } + + public Object[] getParameters() { + return parameters; + } + + public ClassAndMethod getClassAndMethod() { + return classAndMethod; + } +} diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java index 94a052e57..568d30624 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java @@ -31,7 +31,10 @@ public List typeInstrumentations() { public List getAdditionalHelperClassNames() { return Arrays.asList( "com.splunk.opentelemetry.instrumentation.nocode.JSPS", - "com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons" + "com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons", + "com.splunk.opentelemetry.instrumentation.nocode.NocodeAttributesExtractor", + "com.splunk.opentelemetry.instrumentation.nocode.NocodeMethodInvocation", + "com.splunk.opentelemetry.instrumentation.nocode.NocodeSpanNameExtractor" ); } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java index 216f56c4f..f4d165dc2 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java @@ -1,28 +1,22 @@ package com.splunk.opentelemetry.instrumentation.nocode; import io.opentelemetry.api.GlobalOpenTelemetry; -import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor; -import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesGetter; -import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeSpanNameExtractor; -import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor; public class NocodeSingletons { // FIXME so much copy and paste from Methods instrumentation - private static final Instrumenter INSTRUMENTER; + private static final Instrumenter INSTRUMENTER; static { - CodeAttributesGetter cag = ClassAndMethod.codeAttributesGetter(); - - INSTRUMENTER = Instrumenter.builder( + INSTRUMENTER = Instrumenter.builder( GlobalOpenTelemetry.get(), "com.splunk.opentelemetry.instrumentation.nocode", - CodeSpanNameExtractor.create(cag)).addAttributesExtractor(CodeAttributesExtractor.create(cag)) + new NocodeSpanNameExtractor()).addAttributesExtractor(new NocodeAttributesExtractor()) .buildInstrumenter(SpanKindExtractor.alwaysInternal()); } - public static Instrumenter instrumentor() { + public static Instrumenter instrumentor() { return INSTRUMENTER; } } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java new file mode 100644 index 000000000..70a3f33fa --- /dev/null +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java @@ -0,0 +1,28 @@ +package com.splunk.opentelemetry.instrumentation.nocode; + +import com.splunk.opentelemetry.javaagent.nocode.NocodeRules; +import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeSpanNameExtractor; +import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; +import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor; + +public class NocodeSpanNameExtractor implements SpanNameExtractor { + private final SpanNameExtractor defaultNamer; + + public NocodeSpanNameExtractor() { + this.defaultNamer = CodeSpanNameExtractor.create(ClassAndMethod.codeAttributesGetter()); + } + + @Override + public String extract(NocodeMethodInvocation mi) { + NocodeRules.Rule rule = mi.getRule(); + if (rule != null && rule.spanName != null) { + // FIXME might want to allow string literal as a "statement" so that + // the name could be hardcoded to something? + String name = JSPS.evaluate(rule.spanName, mi.getThiz(), mi.getParameters()); + if (name != null) { + return name; + } + } + return defaultNamer.extract(mi.getClassAndMethod()); + } +} diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index 66e60537f..395fd26f0 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -75,13 +75,14 @@ private static List loadUnsafe() throws Exception { LinkedHashMap lhm = (LinkedHashMap) sub; String className = lhm.get("class").toString(); String methodName = lhm.get("method").toString(); + String spanName = lhm.get("spanName") == null ? null : lhm.get("spanName").toString(); List attrs = (List) lhm.get("attributes"); Map ruleAttributes = new HashMap<>(); for(Object attr : attrs) { LinkedHashMap attrMap = (LinkedHashMap) attr; ruleAttributes.put(attrMap.get("key").toString(), attrMap.get("value").toString()); } - answer.add(new NocodeRules.Rule(className, methodName, ruleAttributes)); + answer.add(new NocodeRules.Rule(className, methodName, spanName, ruleAttributes)); } } return answer; From 862f833b4b8a70ad31debdbf3463a5ad74c97c24 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 7 Feb 2025 17:27:59 -0500 Subject: [PATCH 08/62] Doc a desired test --- .../splunk/opentelemetry/instrumentation/nocode/JSPSTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index 1875b0b64..1905c038d 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -46,6 +46,8 @@ public void testInvalidJspsReturnNull() { // "this .", // FIXME currently passes // "this . ", // FIXME currently passes! "this.noSuchMethod()", + "toString()", + //"this.toString()toString()", // FIXME would like this to not work "param1.toString()", // out of bounds "this.getOrDefault(\"key\", \"multiparamnotsupported\")", "this.get(\"noclosequote)", From 90ed8bb05bcc41fa3d7de43c2441731c97ae74c1 Mon Sep 17 00:00:00 2001 From: John Bley Date: Sun, 9 Feb 2025 06:40:41 -0500 Subject: [PATCH 09/62] Make thrown exception capture work, clean up some FIXMEs, doc a few more --- .../nocode/NocodeAttributesExtractor.java | 7 ++----- .../nocode/NocodeInstrumentation.java | 17 +++++++++-------- .../nocode/NocodeSingletons.java | 1 - .../instrumentation/nocode/YamlParser.java | 2 ++ 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java index dd35d45f5..d105c5012 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java @@ -1,7 +1,6 @@ package com.splunk.opentelemetry.instrumentation.nocode; import io.opentelemetry.api.common.AttributesBuilder; -import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor; import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; @@ -10,6 +9,7 @@ import java.util.Collections; import java.util.Map; +// FIXME clean up printouts throughout public class NocodeAttributesExtractor implements AttributesExtractor { private final AttributesExtractor codeExtractor; @@ -18,7 +18,6 @@ public NocodeAttributesExtractor() { } @Override public void onStart(AttributesBuilder attributesBuilder, Context context, NocodeMethodInvocation mi) { - System.out.println("JBLEY NCAE"); codeExtractor.onStart(attributesBuilder, context, mi.getClassAndMethod()); Map attributes = Collections.EMPTY_MAP; @@ -30,15 +29,13 @@ public void onStart(AttributesBuilder attributesBuilder, Context context, Nocode String value = JSPS.evaluate(jsps, mi.getThiz(), mi.getParameters()); System.out.println("JBLEY ATTRIBUTE "+key+" = "+value); if (value != null) { - // FIXME java8 bridge nonsense - Span.current().setAttribute(key, value); + attributesBuilder.put(key, value); } } } - // FIXME this Void might be the RESULT type which might be the return value; look into it @Override public void onEnd(AttributesBuilder attributesBuilder, Context context, NocodeMethodInvocation nocodeMethodInvocation, @Nullable Void unused, diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index d184ccc41..cbee34193 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -61,8 +61,6 @@ public static void onEnter( methodParams); Context parentContext = currentContext(); - // FIXME probably need to rework logic here to just use otel span - // creation api rather than this abstraction around it if (!instrumentor().shouldStart(parentContext, otelInvocation)) { return; } @@ -70,22 +68,25 @@ public static void onEnter( scope = context.makeCurrent(); } - @Advice.OnMethodExit(suppress = Throwable.class) + public NocodeAdvice() { + super(); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( @Advice.Origin Method method, @Advice.Local("otelInvocation") NocodeMethodInvocation otelInvocation, @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) + @Advice.Local("otelScope") Scope scope, + @Advice.Thrown Throwable error) { - // FIXME @Advice.Thrown and Return System.out.println("JBLEY INJECTED END"); if (scope == null) { return; } scope.close(); - // FIXME what is this nonsense copied from AsyncOperationSupport or something like that? - instrumentor().end(context, otelInvocation, null, null); - + // I wonder why the orignal methods instrumentation had support for modifying return value? + instrumentor().end(context, otelInvocation, null, error); } } } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java index f4d165dc2..ac898f0f6 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java @@ -5,7 +5,6 @@ import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor; public class NocodeSingletons { - // FIXME so much copy and paste from Methods instrumentation private static final Instrumenter INSTRUMENTER; static { diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index 395fd26f0..ccda36223 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -14,6 +14,8 @@ public class YamlParser { + // FIXME support span kind + // FIXME support method override selection - e.g., with classfile method signature or something public static final String NOCODE_YMLFILE_ENV_KEY = "SPLUNK_OTEL_INSTRUMENTATION_NOCODE_YML_FILE"; private final List InstrumentationRules; From e08ae118b69df1949c0f37be0e2439bc4aa7a6f3 Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 10 Feb 2025 09:37:15 -0500 Subject: [PATCH 10/62] Start cleaning up parsing logic --- .../opentelemetry/instrumentation/nocode/JSPS.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java index 1825a2bf0..96cad9c68 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java @@ -29,10 +29,10 @@ private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) curObject = params[varIndex]; } int curIndex = nextDot; - while(curIndex < jsps.length() && (jsps.charAt(curIndex) == '.' || Character.isWhitespace(jsps.charAt(curIndex)))) { - curIndex++; - } while(curIndex < jsps.length()) { + while(jsps.charAt(curIndex) == '.' || Character.isWhitespace(jsps.charAt(curIndex))) { + curIndex++; + } int openParen = jsps.indexOf('(', curIndex); String method = jsps.substring(curIndex, openParen).trim(); int closeParen = jsps.indexOf(')', openParen); @@ -40,8 +40,7 @@ private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) if (paramString.isEmpty()) { // FIXME how does javaagent open the class? getting exceptions gere // e.g., with hashmap.entrySet().size() on the entryset accessor - Object nextObject = curObject.getClass().getMethod(method).invoke(curObject); - curObject = nextObject; + curObject = curObject.getClass().getMethod(method).invoke(curObject); } else { if (paramString.startsWith("\"") && paramString.endsWith("\"")) { String passed = paramString.substring(1, paramString.length()-1); @@ -67,9 +66,6 @@ private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) } } curIndex = closeParen + 1; - while(curIndex < jsps.length() && (jsps.charAt(curIndex) == '.' || Character.isWhitespace(jsps.charAt(curIndex)))) { - curIndex++; - } } return curObject == null ? null : curObject.toString(); } From 29f5744ffda06703c1d213a5b9e2da7fbd212687 Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 10 Feb 2025 09:50:13 -0500 Subject: [PATCH 11/62] Clean up parsing of . separator a bit --- .../opentelemetry/instrumentation/nocode/JSPS.java | 2 ++ .../instrumentation/nocode/JSPSTest.java | 11 ++++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java index 96cad9c68..695f27f3c 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java @@ -17,6 +17,7 @@ public static String evaluate(String jsps, Object thiz, Object[] params) { // FIXME improve performance - reduce allocations, etc. + // FIXME Might be nice to support escaping quotes in string literals... private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) throws Exception { jsps = jsps.trim(); int nextDot = jsps.indexOf('.'); @@ -30,6 +31,7 @@ private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) } int curIndex = nextDot; while(curIndex < jsps.length()) { + curIndex = jsps.indexOf('.', curIndex); while(jsps.charAt(curIndex) == '.' || Character.isWhitespace(jsps.charAt(curIndex))) { curIndex++; } diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index 1905c038d..91f588a39 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -24,7 +24,7 @@ public class JSPSTest { @Test public void testBasicBehavior() { String[][] tests = new String[][] { - // FIXME support bare "this" + // Might be nice to support a bare "param0" or "this" but as a workaround can always use "this.toString()" {"this.toString()", "{key=value}"}, {"this.toString().length()", "11"}, {"this.get(\"key\")", "value"}, @@ -37,18 +37,19 @@ public void testBasicBehavior() { } } - // FIXME support escaping quotes in string? @Test public void testInvalidJspsReturnNull() { String[] invalids = new String[] { "nosuchvar", "nosuchvar.toString()", - // "this .", // FIXME currently passes - // "this . ", // FIXME currently passes! + "this .", + "this . ", "this.noSuchMethod()", "toString()", - //"this.toString()toString()", // FIXME would like this to not work + "this.toString()extrastuffatend", + "this.toString()toString()", "param1.toString()", // out of bounds + "param999.toString()", "this.getOrDefault(\"key\", \"multiparamnotsupported\")", "this.get(\"noclosequote)", "this.get(\"nocloseparan\"", From 39d4159bc5ffbd1f13bf34745c6e08a2ffa1a146 Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 10 Feb 2025 09:58:47 -0500 Subject: [PATCH 12/62] Clean up imports and package names --- .../javaagent/bootstrap/nocode/NocodeRules.java | 2 +- .../instrumentation/nocode/NocodeInstrumentation.java | 6 +----- .../instrumentation/nocode/NocodeMethodInvocation.java | 2 +- .../opentelemetry/instrumentation/nocode/NocodeModule.java | 2 +- .../instrumentation/nocode/NocodeSpanNameExtractor.java | 2 +- .../opentelemetry/instrumentation/nocode/YamlParser.java | 3 +-- 6 files changed, 6 insertions(+), 11 deletions(-) diff --git a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java index f65af039a..6ef17b00b 100644 --- a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java +++ b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java @@ -1,4 +1,4 @@ -package com.splunk.opentelemetry.javaagent.nocode; +package com.splunk.opentelemetry.javaagent.bootstrap.nocode; import java.util.ArrayList; import java.util.Collections; diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index cbee34193..4902998b1 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -1,7 +1,6 @@ package com.splunk.opentelemetry.instrumentation.nocode; -import com.splunk.opentelemetry.javaagent.nocode.NocodeRules; -import io.opentelemetry.api.trace.Span; +import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; @@ -12,13 +11,10 @@ import net.bytebuddy.matcher.ElementMatcher; import java.lang.reflect.Method; -import java.util.Collections; -import java.util.Map; import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; import static com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons.instrumentor; -import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.named; public class NocodeInstrumentation implements TypeInstrumentation { diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java index f5bfb5813..1ff05751c 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java @@ -1,6 +1,6 @@ package com.splunk.opentelemetry.instrumentation.nocode; -import com.splunk.opentelemetry.javaagent.nocode.NocodeRules; +import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; public class NocodeMethodInvocation { diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java index 568d30624..9cb32ed8c 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java @@ -2,7 +2,7 @@ package com.splunk.opentelemetry.instrumentation.nocode; import com.google.auto.service.AutoService; -import com.splunk.opentelemetry.javaagent.nocode.NocodeRules; +import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import java.util.ArrayList; diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java index 70a3f33fa..dc33ef423 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java @@ -1,6 +1,6 @@ package com.splunk.opentelemetry.instrumentation.nocode; -import com.splunk.opentelemetry.javaagent.nocode.NocodeRules; +import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeSpanNameExtractor; import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor; diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index ccda36223..453222ba2 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -1,7 +1,6 @@ package com.splunk.opentelemetry.instrumentation.nocode; -import com.splunk.opentelemetry.javaagent.nocode.NocodeRules; -import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; import java.io.BufferedReader; import java.io.File; import java.io.FileReader; From 3bbe1f1e37596b359a3044a36d7467ec79c41916 Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 10 Feb 2025 10:03:17 -0500 Subject: [PATCH 13/62] Use file i/o directly rather than slurping in a string for yaml parsing --- .../instrumentation/nocode/YamlParser.java | 21 ++++--------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index 453222ba2..db4b92dc1 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -4,6 +4,7 @@ import java.io.BufferedReader; import java.io.File; import java.io.FileReader; +import java.io.Reader; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -27,21 +28,6 @@ public List getInstrumentationRules() { return InstrumentationRules; } - // FIXME surely there is a utility for this somewhere in the agent's scope (or just use Reader in snakeyaml) - private static String readYamlFile(String yamlFileName) throws Exception { - File f = new File(yamlFileName); - BufferedReader br = new BufferedReader(new FileReader(f)); - String line; - StringBuffer buf = new StringBuffer(); - while((line = br.readLine()) != null) { - buf.append(line); - buf.append(System.lineSeparator()); - } - br.close(); - return buf.toString(); - } - - private static List load() { try { return loadUnsafe(); @@ -57,7 +43,7 @@ private static List loadUnsafe() throws Exception { if (yamlFileName == null || yamlFileName.trim().isEmpty()) { return Collections.emptyList(); } - String yamlString = readYamlFile(yamlFileName.trim()); + Reader yamlReader = new FileReader(yamlFileName.trim()); // FIXME why can't I figure out how to reference the snakeyaml that's already inside the agent jar? @@ -68,7 +54,7 @@ private static List loadUnsafe() throws Exception { Object lsb = loadSettingsC.getMethod("builder").invoke(null); Object loadSettings = lsb.getClass().getMethod("build").invoke(lsb); Object load = loadC.getConstructor(loadSettingsC).newInstance(loadSettings); - Iterable parsedYaml = (Iterable) load.getClass().getMethod("loadAllFromString", String.class).invoke(load, yamlString); + Iterable parsedYaml = (Iterable) load.getClass().getMethod("loadAllFromReader", Reader.class).invoke(load, yamlReader); ArrayList answer = new ArrayList<>(); for(Object yamlBit : parsedYaml) { List l = (List) yamlBit; @@ -86,6 +72,7 @@ private static List loadUnsafe() throws Exception { answer.add(new NocodeRules.Rule(className, methodName, spanName, ruleAttributes)); } } + yamlReader.close(); return answer; } From a641880c6cdfd3949da96795254ec49d9402fb95 Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 10 Feb 2025 10:26:21 -0500 Subject: [PATCH 14/62] Add spanKind support --- .../javaagent/bootstrap/nocode/NocodeRules.java | 4 +++- .../instrumentation/nocode/NocodeModule.java | 1 + .../instrumentation/nocode/NocodeSingletons.java | 2 +- .../nocode/NocodeSpanKindExtractor.java | 14 ++++++++++++++ .../instrumentation/nocode/YamlParser.java | 5 ++--- 5 files changed, 21 insertions(+), 5 deletions(-) create mode 100644 instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java diff --git a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java index 6ef17b00b..a43177df5 100644 --- a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java +++ b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java @@ -13,11 +13,13 @@ public static class Rule { public final String className; public final String methodName; public final String spanName; // may be null - use default of "class.method" + public final String spanKind; // matches the SpanKind enum, null means default to INTERNAL public final Map attributes; // key name to jsps - public Rule(String className, String methodName, String spanName, Map attributes) { + public Rule(String className, String methodName, String spanName, String spanKind, Map attributes) { this.className = className; this.methodName = methodName; this.spanName = spanName; + this.spanKind = spanKind; this.attributes = Collections.unmodifiableMap(new HashMap<>(attributes)); } public String toString() { diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java index 9cb32ed8c..aadbf703b 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java @@ -34,6 +34,7 @@ public List getAdditionalHelperClassNames() { "com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons", "com.splunk.opentelemetry.instrumentation.nocode.NocodeAttributesExtractor", "com.splunk.opentelemetry.instrumentation.nocode.NocodeMethodInvocation", + "com.splunk.opentelemetry.instrumentation.nocode.NocodeSpanKindExtractor", "com.splunk.opentelemetry.instrumentation.nocode.NocodeSpanNameExtractor" ); } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java index ac898f0f6..195e77e4d 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java @@ -12,7 +12,7 @@ public class NocodeSingletons { GlobalOpenTelemetry.get(), "com.splunk.opentelemetry.instrumentation.nocode", new NocodeSpanNameExtractor()).addAttributesExtractor(new NocodeAttributesExtractor()) - .buildInstrumenter(SpanKindExtractor.alwaysInternal()); + .buildInstrumenter(new NocodeSpanKindExtractor()); } public static Instrumenter instrumentor() { diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java new file mode 100644 index 000000000..5ef8867fa --- /dev/null +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java @@ -0,0 +1,14 @@ +package com.splunk.opentelemetry.instrumentation.nocode; + +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor; + +public class NocodeSpanKindExtractor implements SpanKindExtractor { + @Override + public SpanKind extract(NocodeMethodInvocation mi) { + if (mi.getRule() == null || mi.getRule().spanKind == null) { + return SpanKind.INTERNAL; + } + return SpanKind.valueOf(mi.getRule().spanKind.toUpperCase()); + } +} diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index db4b92dc1..f7d4b8312 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -13,8 +13,6 @@ import java.util.Map; public class YamlParser { - - // FIXME support span kind // FIXME support method override selection - e.g., with classfile method signature or something public static final String NOCODE_YMLFILE_ENV_KEY = "SPLUNK_OTEL_INSTRUMENTATION_NOCODE_YML_FILE"; @@ -63,13 +61,14 @@ private static List loadUnsafe() throws Exception { String className = lhm.get("class").toString(); String methodName = lhm.get("method").toString(); String spanName = lhm.get("spanName") == null ? null : lhm.get("spanName").toString(); + String spanKind = lhm.get("spanKind") == null ? null : lhm.get("spanKind").toString(); List attrs = (List) lhm.get("attributes"); Map ruleAttributes = new HashMap<>(); for(Object attr : attrs) { LinkedHashMap attrMap = (LinkedHashMap) attr; ruleAttributes.put(attrMap.get("key").toString(), attrMap.get("value").toString()); } - answer.add(new NocodeRules.Rule(className, methodName, spanName, ruleAttributes)); + answer.add(new NocodeRules.Rule(className, methodName, spanName, spanKind, ruleAttributes)); } } yamlReader.close(); From c3d44c294189c14caed3b62ccca04432f7addf16 Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 10 Feb 2025 10:33:27 -0500 Subject: [PATCH 15/62] Clean up debug printouts --- .../instrumentation/nocode/NocodeAttributesExtractor.java | 2 -- .../instrumentation/nocode/NocodeInstrumentation.java | 4 +--- .../opentelemetry/instrumentation/nocode/YamlParser.java | 2 -- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java index d105c5012..6bd4cd5c6 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java @@ -9,7 +9,6 @@ import java.util.Collections; import java.util.Map; -// FIXME clean up printouts throughout public class NocodeAttributesExtractor implements AttributesExtractor { private final AttributesExtractor codeExtractor; @@ -27,7 +26,6 @@ public void onStart(AttributesBuilder attributesBuilder, Context context, Nocode for(String key : attributes.keySet()) { String jsps = attributes.get(key); String value = JSPS.evaluate(jsps, mi.getThiz(), mi.getParameters()); - System.out.println("JBLEY ATTRIBUTE "+key+" = "+value); if (value != null) { attributesBuilder.put(key, value); } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index 4902998b1..da8b4d683 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -48,7 +48,6 @@ public static void onEnter( @Advice.This Object thiz, @Advice.AllArguments Object[] methodParams ) { - System.out.println("JBLEY INJECTED START"); NocodeRules.Rule rule = NocodeRules.findRuleByClassAndMethod(declaringClass.getName(), methodName); otelInvocation = new NocodeMethodInvocation( rule, @@ -76,12 +75,11 @@ public static void stopSpan( @Advice.Local("otelScope") Scope scope, @Advice.Thrown Throwable error) { - System.out.println("JBLEY INJECTED END"); if (scope == null) { return; } scope.close(); - // I wonder why the orignal methods instrumentation had support for modifying return value? + // I wonder why the original methods instrumentation had support for modifying the return value? instrumentor().end(context, otelInvocation, null, error); } } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index f7d4b8312..952043f5f 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -1,8 +1,6 @@ package com.splunk.opentelemetry.instrumentation.nocode; import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; -import java.io.BufferedReader; -import java.io.File; import java.io.FileReader; import java.io.Reader; import java.util.ArrayList; From e055398b6b092a9533fc4daf4c60215c6bf4550c Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 10 Feb 2025 13:46:31 -0500 Subject: [PATCH 16/62] Clean up logging/error handling --- .../splunk/opentelemetry/instrumentation/nocode/JSPS.java | 6 +++--- .../instrumentation/nocode/NocodeSpanNameExtractor.java | 2 -- .../opentelemetry/instrumentation/nocode/YamlParser.java | 5 +++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java index 695f27f3c..a9d286600 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java @@ -1,16 +1,17 @@ package com.splunk.opentelemetry.instrumentation.nocode; import java.lang.reflect.Method; +import java.util.logging.Logger; // JSPS stands for Java-like String-Producing Statement. FIXME describe in more detail and pick a better nane public class JSPS { + private final static Logger logger = Logger.getLogger(JSPS.class.getName()); public static String evaluate(String jsps, Object thiz, Object[] params) { try { return unsafeEvaluate(jsps, thiz, params); } catch (Throwable t) { - // FIXME better logging - System.out.println("can't eval jsps: "+t); + logger.warning("Can't evaluate {"+jsps+"}: "+t); return null; } } @@ -72,7 +73,6 @@ private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) return curObject == null ? null : curObject.toString(); } - // FIXME must be a better way to look through a series of type options private static Method findMethod(Object curObject, String methodName, Class... paramTypesToTryInOrder) throws NoSuchMethodException{ Class c = curObject.getClass(); for(Class paramType : paramTypesToTryInOrder) { diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java index dc33ef423..c92080638 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java @@ -16,8 +16,6 @@ public NocodeSpanNameExtractor() { public String extract(NocodeMethodInvocation mi) { NocodeRules.Rule rule = mi.getRule(); if (rule != null && rule.spanName != null) { - // FIXME might want to allow string literal as a "statement" so that - // the name could be hardcoded to something? String name = JSPS.evaluate(rule.spanName, mi.getThiz(), mi.getParameters()); if (name != null) { return name; diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index 952043f5f..7989f03fe 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -9,8 +9,10 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.logging.Logger; public class YamlParser { + private static final Logger logger = Logger.getLogger(YamlParser.class.getName()); // FIXME support method override selection - e.g., with classfile method signature or something public static final String NOCODE_YMLFILE_ENV_KEY = "SPLUNK_OTEL_INSTRUMENTATION_NOCODE_YML_FILE"; @@ -28,8 +30,7 @@ private static List load() { try { return loadUnsafe(); } catch (Exception e) { - // FIXME error handling - System.out.println("JBLEY can't load yaml: "+e); + logger.severe("Can't load configured yaml: "+e); return Collections.emptyList(); } } From 48c5170c0eb351f667476f705c525ede8110fc05 Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 10 Feb 2025 14:03:05 -0500 Subject: [PATCH 17/62] Doc JSPS a bit --- .../opentelemetry/instrumentation/nocode/JSPS.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java index a9d286600..47a26010f 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java @@ -3,7 +3,18 @@ import java.lang.reflect.Method; import java.util.logging.Logger; -// JSPS stands for Java-like String-Producing Statement. FIXME describe in more detail and pick a better nane +// JSPS stands for Java-like String-Producing Statement. A JSPS is +// essentially a single call in Java (as though it ends with a semicolon), with +// some limitations. Its purpose is to allow pieces of nocode instrumentation +// (attributes, span name) to be derived from the instrumentated context. +// As some illustrative examples: + // this.getHeaders().get("X-Custom-Header").substring(5) + // param0.getDetails().getCustomerAccount().getAccountType() +// The limitations are: + // no access to variables other than 'this' and 'paramN' (N indexed at 0) + // no control flow (if), no local variables, basically nothing other than a single chain of method calls + // Methods calls are limited to either 0 or 1 parameters currently + // Parameters must be literals and only integral (int/long), string, and boolean literals are currently supported public class JSPS { private final static Logger logger = Logger.getLogger(JSPS.class.getName()); @@ -17,7 +28,6 @@ public static String evaluate(String jsps, Object thiz, Object[] params) { } - // FIXME improve performance - reduce allocations, etc. // FIXME Might be nice to support escaping quotes in string literals... private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) throws Exception { jsps = jsps.trim(); From 6eb81e95d9778de9dfbfa6a6994bd16d59b8cc2a Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 10 Feb 2025 16:51:25 -0500 Subject: [PATCH 18/62] Use snakeyaml directly instead of through reflection --- instrumentation/nocode/build.gradle.kts | 2 +- .../instrumentation/nocode/YamlParser.java | 18 ++++++------------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/instrumentation/nocode/build.gradle.kts b/instrumentation/nocode/build.gradle.kts index cbf6d6fb8..107df5671 100644 --- a/instrumentation/nocode/build.gradle.kts +++ b/instrumentation/nocode/build.gradle.kts @@ -7,5 +7,5 @@ dependencies { compileOnly("io.opentelemetry.javaagent:opentelemetry-javaagent-tooling") compileOnly("org.snakeyaml:snakeyaml-engine:2.8") //implementation("org.yaml:snakeyaml:2.3") - // implementation("org.snakeyaml:snakeyaml-engine:2.8") + //implementation("org.snakeyaml:snakeyaml-engine:2.8") } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index 7989f03fe..1f32efbe3 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -1,6 +1,8 @@ package com.splunk.opentelemetry.instrumentation.nocode; import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; +import org.snakeyaml.engine.v2.api.Load; +import org.snakeyaml.engine.v2.api.LoadSettings; import java.io.FileReader; import java.io.Reader; import java.util.ArrayList; @@ -42,21 +44,13 @@ private static List loadUnsafe() throws Exception { } Reader yamlReader = new FileReader(yamlFileName.trim()); - - // FIXME why can't I figure out how to reference the snakeyaml that's already inside the agent jar? - // This nonsense is here to do a reflective load of the yaml parser that's already there - // and parse the config file - Class loadSettingsC = Class.forName("org.snakeyaml.engine.v2.api.LoadSettings"); - Class loadC = Class.forName("org.snakeyaml.engine.v2.api.Load"); - Object lsb = loadSettingsC.getMethod("builder").invoke(null); - Object loadSettings = lsb.getClass().getMethod("build").invoke(lsb); - Object load = loadC.getConstructor(loadSettingsC).newInstance(loadSettings); - Iterable parsedYaml = (Iterable) load.getClass().getMethod("loadAllFromReader", Reader.class).invoke(load, yamlReader); + Load load = new Load(LoadSettings.builder().build()); + Iterable parsedYaml = load.loadAllFromReader(yamlReader); ArrayList answer = new ArrayList<>(); for(Object yamlBit : parsedYaml) { List l = (List) yamlBit; - for(Object sub : l) { - LinkedHashMap lhm = (LinkedHashMap) sub; + for(Object yamlRule : l) { + LinkedHashMap lhm = (LinkedHashMap) yamlRule; String className = lhm.get("class").toString(); String methodName = lhm.get("method").toString(); String spanName = lhm.get("spanName") == null ? null : lhm.get("spanName").toString(); From b8c57e32dfa1934b7180173250b3ca8f9a39d60e Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 10 Feb 2025 16:54:15 -0500 Subject: [PATCH 19/62] Fallback to internal spanKind --- .../instrumentation/nocode/NocodeSpanKindExtractor.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java index 5ef8867fa..7b7338a0f 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java @@ -9,6 +9,10 @@ public SpanKind extract(NocodeMethodInvocation mi) { if (mi.getRule() == null || mi.getRule().spanKind == null) { return SpanKind.INTERNAL; } - return SpanKind.valueOf(mi.getRule().spanKind.toUpperCase()); + try { + return SpanKind.valueOf(mi.getRule().spanKind.toUpperCase()); + } catch (IllegalArgumentException noMatchingValue) { + return SpanKind.INTERNAL; + } } } From 16fc9b16b94e8cf715676cffa506ea3b3b33b21a Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 10 Feb 2025 16:57:12 -0500 Subject: [PATCH 20/62] Build script cleanup --- instrumentation/nocode/build.gradle.kts | 2 -- 1 file changed, 2 deletions(-) diff --git a/instrumentation/nocode/build.gradle.kts b/instrumentation/nocode/build.gradle.kts index 107df5671..20ca90a74 100644 --- a/instrumentation/nocode/build.gradle.kts +++ b/instrumentation/nocode/build.gradle.kts @@ -6,6 +6,4 @@ plugins { dependencies { compileOnly("io.opentelemetry.javaagent:opentelemetry-javaagent-tooling") compileOnly("org.snakeyaml:snakeyaml-engine:2.8") - //implementation("org.yaml:snakeyaml:2.3") - //implementation("org.snakeyaml:snakeyaml-engine:2.8") } From 757e16e36d99f5ac6c919eabbd0d19e4eed66755 Mon Sep 17 00:00:00 2001 From: John Bley Date: Tue, 11 Feb 2025 10:58:09 -0500 Subject: [PATCH 21/62] Use superType instead of exact match to filter class names, just like upstream methods instrumentation --- .../instrumentation/nocode/NocodeInstrumentation.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index da8b4d683..97eedbf7d 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -15,6 +15,7 @@ import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; import static com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons.instrumentor; +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.named; public class NocodeInstrumentation implements TypeInstrumentation { @@ -26,8 +27,7 @@ public NocodeInstrumentation(NocodeRules.Rule rule) { @Override public ElementMatcher typeMatcher() { - // exact match for now, if updated think about class name to use in naming the span/attributes - return named(rule.className); + return hasSuperType(named(rule.className)); } @Override From 00ca67c36fbf17663edb7cb1c7d6067df8e163c8 Mon Sep 17 00:00:00 2001 From: John Bley Date: Tue, 11 Feb 2025 11:56:02 -0500 Subject: [PATCH 22/62] Add tests for extractors --- .../instrumentation/nocode/ExtractorTest.java | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/ExtractorTest.java diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/ExtractorTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/ExtractorTest.java new file mode 100644 index 000000000..32a656f44 --- /dev/null +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/ExtractorTest.java @@ -0,0 +1,76 @@ +package com.splunk.opentelemetry.instrumentation.nocode; + +import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; +import org.junit.jupiter.api.Test; +import java.util.*; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +public class ExtractorTest { + private static final Map thiz = new HashMap<>(); + private static final Set param0 = new HashSet<>(); + private static final ClassAndMethod cm = ClassAndMethod.create(Object.class, "method");; + + static { + thiz.put("key", "value"); + param0.add("present"); + } + + + @Test + public void testSpanKind() { + NocodeSpanKindExtractor ex = new NocodeSpanKindExtractor(); + + NocodeRules.Rule server = new NocodeRules.Rule("java.lang.Object", "method", "n", "SERVER", Collections.EMPTY_MAP); + assertEquals(SpanKind.SERVER, ex.extract(new NocodeMethodInvocation(server, cm, null, null))); + + NocodeRules.Rule invalid = new NocodeRules.Rule("java.lang.Object", "method", "n", "INVALID", Collections.EMPTY_MAP); + assertEquals(SpanKind.INTERNAL, ex.extract(new NocodeMethodInvocation(invalid, cm, null, null))); + + NocodeRules.Rule unspecified = new NocodeRules.Rule("java.lang.Object", "method", "n", null, Collections.EMPTY_MAP); + assertEquals(SpanKind.INTERNAL, ex.extract(new NocodeMethodInvocation(unspecified, cm, null, null))); + } + + @Test + public void testSpanName() { + NocodeSpanNameExtractor ex = new NocodeSpanNameExtractor(); + + NocodeRules.Rule unspecified = new NocodeRules.Rule("java.lang.Object", "method", null, null, Collections.EMPTY_MAP); + assertEquals("Object.method", ex.extract(new NocodeMethodInvocation(unspecified, cm, null, null))); + + NocodeRules.Rule specified = new NocodeRules.Rule("java.lang.Object", "method", "this.get(\"key\")", null, Collections.EMPTY_MAP); + assertEquals("value", ex.extract(new NocodeMethodInvocation(specified, cm, thiz, null))); + } + + @Test + public void testAttributes() { + NocodeAttributesExtractor ex = new NocodeAttributesExtractor(); + + Map attsSpecs = new HashMap<>(); + attsSpecs.put("key1", "this.get(\"key\").toString().substring(1)"); + attsSpecs.put("key2", "param0.contains(\"present\")"); + attsSpecs.put("invalid", "invalid syntax.)(=no value, key not present"); + NocodeRules.Rule rule = new NocodeRules.Rule("java.lang.Object","method", null, null, attsSpecs); + AttributesBuilder ab = Attributes.builder(); + ex.onStart(ab, Context.current(), new NocodeMethodInvocation(rule, cm, thiz, new Object[]{param0})); + Map, Object> atts = ab.build().asMap(); + System.out.println(atts); + assertEquals(4, atts.size()); + assertEquals("alue", atts.get(AttributeKey.stringKey("key1"))); + assertEquals("true", atts.get(AttributeKey.stringKey("key2"))); + assertEquals("method", atts.get(AttributeKey.stringKey("code.function"))); + assertEquals("java.lang.Object", atts.get(AttributeKey.stringKey("code.namespace"))); + assertFalse(atts.containsKey(AttributeKey.stringKey("invalid"))); + } + + + + +} From bbbc4004c185a5443c94b14dba95a9d4677e2d28 Mon Sep 17 00:00:00 2001 From: John Bley Date: Tue, 11 Feb 2025 13:31:22 -0500 Subject: [PATCH 23/62] End-to-end integation test, some bootstrap classpath issues with the extractor still --- instrumentation/nocode/build.gradle.kts | 5 +- .../nocode/src/test/config/nocode.yml | 14 +++++ .../nocode/IntegrationTest.java | 51 +++++++++++++++++++ .../instrumentation/nocode/SampleClass.java | 19 +++++++ 4 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 instrumentation/nocode/src/test/config/nocode.yml create mode 100644 instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/IntegrationTest.java create mode 100644 instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/SampleClass.java diff --git a/instrumentation/nocode/build.gradle.kts b/instrumentation/nocode/build.gradle.kts index 20ca90a74..4ed8b9944 100644 --- a/instrumentation/nocode/build.gradle.kts +++ b/instrumentation/nocode/build.gradle.kts @@ -2,8 +2,11 @@ plugins { id("splunk.instrumentation-conventions") } - dependencies { compileOnly("io.opentelemetry.javaagent:opentelemetry-javaagent-tooling") compileOnly("org.snakeyaml:snakeyaml-engine:2.8") } + +tasks.withType().configureEach { + environment("SPLUNK_OTEL_INSTRUMENTATION_NOCODE_YML_FILE", "./src/test/config/nocode.yml") +} diff --git a/instrumentation/nocode/src/test/config/nocode.yml b/instrumentation/nocode/src/test/config/nocode.yml new file mode 100644 index 000000000..e72508953 --- /dev/null +++ b/instrumentation/nocode/src/test/config/nocode.yml @@ -0,0 +1,14 @@ +- class: com.splunk.opentelemetry.instrumentation.nocode.SampleClass + method: doSomething + spanName: this.getName() + attributes: + - key: "details" + value: this.getDetails() + +- class: com.splunk.opentelemetry.instrumentation.nocode.SampleClass + method: throwException + spanKind: SERVER + attributes: + - key: "five" + value: "param0.toString()" + diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/IntegrationTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/IntegrationTest.java new file mode 100644 index 000000000..d19b9340d --- /dev/null +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/IntegrationTest.java @@ -0,0 +1,51 @@ +package com.splunk.opentelemetry.instrumentation.nocode; + +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; + +// This test has "test/config/nocode.yml" applied to it by the gradle environment setting +public class IntegrationTest { + @RegisterExtension + static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); + + @Test + public void testBasicMethod() { + new SampleClass().doSomething(); + // FIXME what is scope version verification and why doesn't it pass? + testing.waitAndAssertTracesWithoutScopeVersionVerification( + trace -> + trace.hasSpansSatisfyingExactly( + span -> + span.hasName("name") + .hasKind(SpanKind.INTERNAL) + .hasAttributesSatisfying( + equalTo(AttributeKey.stringKey("details"), "details")))); + } + + @Test + public void testThrowException() { + try { + new SampleClass().throwException(5); + } catch (Exception expected) { + // nop + } + + testing.waitAndAssertTracesWithoutScopeVersionVerification( + trace -> + trace.hasSpansSatisfyingExactly( + span -> + span.hasName("SampleClass.throwException") + .hasKind(SpanKind.SERVER) + .hasEventsSatisfyingExactly( + event -> + event.hasName("exception")) + .hasAttributesSatisfying( + equalTo(AttributeKey.stringKey("five"), "5")))); + } + +} diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/SampleClass.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/SampleClass.java new file mode 100644 index 000000000..63bbfcff3 --- /dev/null +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/SampleClass.java @@ -0,0 +1,19 @@ +package com.splunk.opentelemetry.instrumentation.nocode; + +public class SampleClass { + public String getName() { + return "name"; + } + + public String getDetails() { + return "details"; + } + + public void throwException(int parameter) { + throw new UnsupportedOperationException("oh no"); + } + + public void doSomething() { + } + +} From fc25038cbd3929a0379f2e6fcb1359369206f9a5 Mon Sep 17 00:00:00 2001 From: John Bley Date: Wed, 12 Feb 2025 06:31:52 -0500 Subject: [PATCH 24/62] Replace extractor tests with integration ones --- .../nocode/src/test/config/nocode.yml | 12 ++- .../instrumentation/nocode/ExtractorTest.java | 76 ------------------- .../nocode/IntegrationTest.java | 14 ++++ .../instrumentation/nocode/SampleClass.java | 3 + 4 files changed, 27 insertions(+), 78 deletions(-) delete mode 100644 instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/ExtractorTest.java diff --git a/instrumentation/nocode/src/test/config/nocode.yml b/instrumentation/nocode/src/test/config/nocode.yml index e72508953..c76d91fd1 100644 --- a/instrumentation/nocode/src/test/config/nocode.yml +++ b/instrumentation/nocode/src/test/config/nocode.yml @@ -1,6 +1,6 @@ - class: com.splunk.opentelemetry.instrumentation.nocode.SampleClass method: doSomething - spanName: this.getName() + spanName: "this.getName()" attributes: - key: "details" value: this.getDetails() @@ -10,5 +10,13 @@ spanKind: SERVER attributes: - key: "five" - value: "param0.toString()" + value: param0.toString().substring(0) + +- class: com.splunk.opentelemetry.instrumentation.nocode.SampleClass + method: doInvalidRule + spanName: "this.thereIsNoSuchMethod()" + spanKind: INVALID + attributes: + - key: "notpresent" + value: "invalid.noSuchStatement()" diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/ExtractorTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/ExtractorTest.java deleted file mode 100644 index 32a656f44..000000000 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/ExtractorTest.java +++ /dev/null @@ -1,76 +0,0 @@ -package com.splunk.opentelemetry.instrumentation.nocode; - -import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.common.AttributesBuilder; -import io.opentelemetry.api.trace.SpanKind; -import io.opentelemetry.context.Context; -import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; -import org.junit.jupiter.api.Test; -import java.util.*; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; - -public class ExtractorTest { - private static final Map thiz = new HashMap<>(); - private static final Set param0 = new HashSet<>(); - private static final ClassAndMethod cm = ClassAndMethod.create(Object.class, "method");; - - static { - thiz.put("key", "value"); - param0.add("present"); - } - - - @Test - public void testSpanKind() { - NocodeSpanKindExtractor ex = new NocodeSpanKindExtractor(); - - NocodeRules.Rule server = new NocodeRules.Rule("java.lang.Object", "method", "n", "SERVER", Collections.EMPTY_MAP); - assertEquals(SpanKind.SERVER, ex.extract(new NocodeMethodInvocation(server, cm, null, null))); - - NocodeRules.Rule invalid = new NocodeRules.Rule("java.lang.Object", "method", "n", "INVALID", Collections.EMPTY_MAP); - assertEquals(SpanKind.INTERNAL, ex.extract(new NocodeMethodInvocation(invalid, cm, null, null))); - - NocodeRules.Rule unspecified = new NocodeRules.Rule("java.lang.Object", "method", "n", null, Collections.EMPTY_MAP); - assertEquals(SpanKind.INTERNAL, ex.extract(new NocodeMethodInvocation(unspecified, cm, null, null))); - } - - @Test - public void testSpanName() { - NocodeSpanNameExtractor ex = new NocodeSpanNameExtractor(); - - NocodeRules.Rule unspecified = new NocodeRules.Rule("java.lang.Object", "method", null, null, Collections.EMPTY_MAP); - assertEquals("Object.method", ex.extract(new NocodeMethodInvocation(unspecified, cm, null, null))); - - NocodeRules.Rule specified = new NocodeRules.Rule("java.lang.Object", "method", "this.get(\"key\")", null, Collections.EMPTY_MAP); - assertEquals("value", ex.extract(new NocodeMethodInvocation(specified, cm, thiz, null))); - } - - @Test - public void testAttributes() { - NocodeAttributesExtractor ex = new NocodeAttributesExtractor(); - - Map attsSpecs = new HashMap<>(); - attsSpecs.put("key1", "this.get(\"key\").toString().substring(1)"); - attsSpecs.put("key2", "param0.contains(\"present\")"); - attsSpecs.put("invalid", "invalid syntax.)(=no value, key not present"); - NocodeRules.Rule rule = new NocodeRules.Rule("java.lang.Object","method", null, null, attsSpecs); - AttributesBuilder ab = Attributes.builder(); - ex.onStart(ab, Context.current(), new NocodeMethodInvocation(rule, cm, thiz, new Object[]{param0})); - Map, Object> atts = ab.build().asMap(); - System.out.println(atts); - assertEquals(4, atts.size()); - assertEquals("alue", atts.get(AttributeKey.stringKey("key1"))); - assertEquals("true", atts.get(AttributeKey.stringKey("key2"))); - assertEquals("method", atts.get(AttributeKey.stringKey("code.function"))); - assertEquals("java.lang.Object", atts.get(AttributeKey.stringKey("code.namespace"))); - assertFalse(atts.containsKey(AttributeKey.stringKey("invalid"))); - } - - - - -} diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/IntegrationTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/IntegrationTest.java index d19b9340d..27adf4fec 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/IntegrationTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/IntegrationTest.java @@ -7,6 +7,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; +import static org.assertj.core.api.Assertions.doesNotHave; // This test has "test/config/nocode.yml" applied to it by the gradle environment setting public class IntegrationTest { @@ -27,6 +28,19 @@ public void testBasicMethod() { equalTo(AttributeKey.stringKey("details"), "details")))); } + @Test + public void testRuleWithManyInvalidFields() { + new SampleClass().doInvalidRule(); + testing.waitAndAssertTracesWithoutScopeVersionVerification( + trace -> + trace.hasSpansSatisfyingExactly( + span -> + span.hasName("SampleClass.doInvalidRule") + .hasKind(SpanKind.INTERNAL) + .hasTotalAttributeCount(2))); // two code. attribute but nothing from the invalid rule + } + + @Test public void testThrowException() { try { diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/SampleClass.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/SampleClass.java index 63bbfcff3..57b2cb8e3 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/SampleClass.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/SampleClass.java @@ -16,4 +16,7 @@ public void throwException(int parameter) { public void doSomething() { } + public void doInvalidRule() { + } + } From 0a8bd67dc865aea4f52e4d85f6dcaeafb6dfe2ee Mon Sep 17 00:00:00 2001 From: John Bley Date: Wed, 12 Feb 2025 08:08:12 -0500 Subject: [PATCH 25/62] Rename instrumentation test --- .../splunk/opentelemetry/instrumentation/nocode/JSPSTest.java | 1 - .../{IntegrationTest.java => NocodeInstrumentationTest.java} | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) rename instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/{IntegrationTest.java => NocodeInstrumentationTest.java} (96%) diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index 91f588a39..13ad1d701 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -11,7 +11,6 @@ import java.util.Map; import java.util.Set; -// FIXME need tests for the instrumentation too! public class JSPSTest { private static final Map thiz = new HashMap<>(); private static final Set param0 = new HashSet<>(); diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/IntegrationTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java similarity index 96% rename from instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/IntegrationTest.java rename to instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java index 27adf4fec..3aee8c08c 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/IntegrationTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java @@ -7,10 +7,9 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; -import static org.assertj.core.api.Assertions.doesNotHave; // This test has "test/config/nocode.yml" applied to it by the gradle environment setting -public class IntegrationTest { +public class NocodeInstrumentationTest { @RegisterExtension static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); From 5429d1873e9ad6c92c7a19747aa54357c1548f83 Mon Sep 17 00:00:00 2001 From: John Bley Date: Wed, 12 Feb 2025 08:10:28 -0500 Subject: [PATCH 26/62] Inline code to instrument inside the test --- .../nocode/src/test/config/nocode.yml | 6 ++--- .../nocode/NocodeInstrumentationTest.java | 20 +++++++++++++++++ .../instrumentation/nocode/SampleClass.java | 22 ------------------- 3 files changed, 23 insertions(+), 25 deletions(-) delete mode 100644 instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/SampleClass.java diff --git a/instrumentation/nocode/src/test/config/nocode.yml b/instrumentation/nocode/src/test/config/nocode.yml index c76d91fd1..b17af28f1 100644 --- a/instrumentation/nocode/src/test/config/nocode.yml +++ b/instrumentation/nocode/src/test/config/nocode.yml @@ -1,18 +1,18 @@ -- class: com.splunk.opentelemetry.instrumentation.nocode.SampleClass +- class: com.splunk.opentelemetry.instrumentation.nocode.NocodeInstrumentationTest$SampleClass method: doSomething spanName: "this.getName()" attributes: - key: "details" value: this.getDetails() -- class: com.splunk.opentelemetry.instrumentation.nocode.SampleClass +- class: com.splunk.opentelemetry.instrumentation.nocode.NocodeInstrumentationTest$SampleClass method: throwException spanKind: SERVER attributes: - key: "five" value: param0.toString().substring(0) -- class: com.splunk.opentelemetry.instrumentation.nocode.SampleClass +- class: com.splunk.opentelemetry.instrumentation.nocode.NocodeInstrumentationTest$SampleClass method: doInvalidRule spanName: "this.thereIsNoSuchMethod()" spanKind: INVALID diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java index 3aee8c08c..161f59ceb 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java @@ -61,4 +61,24 @@ public void testThrowException() { equalTo(AttributeKey.stringKey("five"), "5")))); } + public static class SampleClass { + public String getName() { + return "name"; + } + + public String getDetails() { + return "details"; + } + + public void throwException(int parameter) { + throw new UnsupportedOperationException("oh no"); + } + + public void doSomething() { + } + + public void doInvalidRule() { + } + + } } diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/SampleClass.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/SampleClass.java deleted file mode 100644 index 57b2cb8e3..000000000 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/SampleClass.java +++ /dev/null @@ -1,22 +0,0 @@ -package com.splunk.opentelemetry.instrumentation.nocode; - -public class SampleClass { - public String getName() { - return "name"; - } - - public String getDetails() { - return "details"; - } - - public void throwException(int parameter) { - throw new UnsupportedOperationException("oh no"); - } - - public void doSomething() { - } - - public void doInvalidRule() { - } - -} From e9088519b5e6ca8a178c9588b5e287641efd87d8 Mon Sep 17 00:00:00 2001 From: John Bley Date: Wed, 12 Feb 2025 08:11:54 -0500 Subject: [PATCH 27/62] Organize imports --- .../nocode/NocodeAttributesExtractor.java | 2 +- .../nocode/NocodeInstrumentation.java | 14 ++++++-------- .../instrumentation/nocode/NocodeSingletons.java | 1 - .../instrumentation/nocode/YamlParser.java | 4 ++-- .../instrumentation/nocode/JSPSTest.java | 3 +-- .../nocode/NocodeInstrumentationTest.java | 3 ++- 6 files changed, 12 insertions(+), 15 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java index 6bd4cd5c6..c607bb4b1 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java @@ -5,9 +5,9 @@ import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor; import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; -import javax.annotation.Nullable; import java.util.Collections; import java.util.Map; +import javax.annotation.Nullable; public class NocodeAttributesExtractor implements AttributesExtractor { private final AttributesExtractor codeExtractor; diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index 97eedbf7d..5a493e4e5 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -1,23 +1,21 @@ package com.splunk.opentelemetry.instrumentation.nocode; +import static com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons.instrumentor; +import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.named; + import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.lang.reflect.Method; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; -import java.lang.reflect.Method; - -import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; -import static com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons.instrumentor; - -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; -import static net.bytebuddy.matcher.ElementMatchers.named; - public class NocodeInstrumentation implements TypeInstrumentation { private final NocodeRules.Rule rule; diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java index 195e77e4d..4523bc881 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java @@ -2,7 +2,6 @@ import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor; public class NocodeSingletons { private static final Instrumenter INSTRUMENTER; diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index 1f32efbe3..b79732528 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -1,8 +1,6 @@ package com.splunk.opentelemetry.instrumentation.nocode; import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; -import org.snakeyaml.engine.v2.api.Load; -import org.snakeyaml.engine.v2.api.LoadSettings; import java.io.FileReader; import java.io.Reader; import java.util.ArrayList; @@ -12,6 +10,8 @@ import java.util.List; import java.util.Map; import java.util.logging.Logger; +import org.snakeyaml.engine.v2.api.Load; +import org.snakeyaml.engine.v2.api.LoadSettings; public class YamlParser { private static final Logger logger = Logger.getLogger(YamlParser.class.getName()); diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index 13ad1d701..3e1beb9d0 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -3,13 +3,12 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; -import org.junit.jupiter.api.Test; - import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import org.junit.jupiter.api.Test; public class JSPSTest { private static final Map thiz = new HashMap<>(); diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java index 161f59ceb..a9d122554 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java @@ -1,12 +1,13 @@ package com.splunk.opentelemetry.instrumentation.nocode; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; + import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; // This test has "test/config/nocode.yml" applied to it by the gradle environment setting public class NocodeInstrumentationTest { From bbcab5311370118ca7e29bac321fcc7714ea81ce Mon Sep 17 00:00:00 2001 From: John Bley Date: Thu, 13 Feb 2025 10:59:40 -0500 Subject: [PATCH 28/62] Fixed the open-access reflection issue for many cases --- .../instrumentation/nocode/JSPS.java | 59 +++++++++++++++---- .../nocode/src/test/config/nocode.yml | 2 + .../instrumentation/nocode/JSPSTest.java | 1 + .../nocode/NocodeInstrumentationTest.java | 9 +++ 4 files changed, 61 insertions(+), 10 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java index 47a26010f..0a59445e4 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java @@ -1,6 +1,7 @@ package com.splunk.opentelemetry.instrumentation.nocode; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.logging.Logger; // JSPS stands for Java-like String-Producing Statement. A JSPS is @@ -16,7 +17,8 @@ // Methods calls are limited to either 0 or 1 parameters currently // Parameters must be literals and only integral (int/long), string, and boolean literals are currently supported public class JSPS { - private final static Logger logger = Logger.getLogger(JSPS.class.getName()); + private static final Logger logger = Logger.getLogger(JSPS.class.getName()); + private static final Class[] NoParamTypes = new Class[0]; public static String evaluate(String jsps, Object thiz, Object[] params) { try { @@ -51,26 +53,25 @@ private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) int closeParen = jsps.indexOf(')', openParen); String paramString = jsps.substring(openParen + 1, closeParen).trim(); if (paramString.isEmpty()) { - // FIXME how does javaagent open the class? getting exceptions gere - // e.g., with hashmap.entrySet().size() on the entryset accessor - curObject = curObject.getClass().getMethod(method).invoke(curObject); + Method m = findMatchingMethod(curObject, method, NoParamTypes); + curObject = m.invoke(curObject); } else { if (paramString.startsWith("\"") && paramString.endsWith("\"")) { String passed = paramString.substring(1, paramString.length()-1); - Method m = findMethod(curObject, method, + Method m = findMethodWithPossibleTypes(curObject, method, String.class, Object.class); curObject = m.invoke(curObject, passed); } else if (paramString.equals("true") || paramString.equals("false")) { - Method m = findMethod(curObject, method, + Method m = findMethodWithPossibleTypes(curObject, method, boolean.class, Boolean.class, Object.class); curObject = m.invoke(curObject, Boolean.parseBoolean(paramString)); } else if (paramString.matches("[0-9]+")) { try { - Method m = findMethod(curObject, method, int.class, Integer.class, Object.class); + Method m = findMethodWithPossibleTypes(curObject, method, int.class, Integer.class, Object.class); int passed = Integer.parseInt(paramString); curObject = m.invoke(curObject, passed); } catch (NoSuchMethodException tryLongInstead) { - Method m = findMethod(curObject, method, long.class, Long.class, Object.class); + Method m = findMethodWithPossibleTypes(curObject, method, long.class, Long.class, Object.class); long passed = Long.parseLong(paramString); curObject = m.invoke(curObject, passed); } @@ -83,11 +84,49 @@ private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) return curObject == null ? null : curObject.toString(); } - private static Method findMethod(Object curObject, String methodName, Class... paramTypesToTryInOrder) throws NoSuchMethodException{ + // This sequence of methods is here because: + // - we want to try a variety of parameter types to match a literal + // e.g., someMethod(5) could match someMethod(int) or someMethod(Object) + // - module rules around reflection make some "legal" things harder and require + // looking for a public class/interface matching the method to call + // e.g., a naive reflective call through + // this.getSomeHashMap().entrySet().size() + // would fail simply because the HashMap$EntrySet implementation + // is not public, even though the interface it's being call through is. + private static Method findMatchingMethod(Object curObject, String methodName, Class[] actualParamTypes) throws NoSuchMethodException{ + Method m = findMatchingMethod(methodName, curObject.getClass(), actualParamTypes); + if (m == null) { + throw new NoSuchMethodException("Can't find matching method for "+methodName+" on "+curObject.getClass().getName()); + } + return m; + } + // Returns null for none found + private static Method findMatchingMethod(String methodName, Class c, Class[] actualParamTypes) { + if (c == null) { + return null; + } + if (Modifier.isPublic(c.getModifiers())) { + try { + return c.getMethod(methodName, actualParamTypes); + } catch (NoSuchMethodException nsme) { + // keep trying + } + } + // not public, try interfaces and supertype + for(Class iface : c.getInterfaces()) { + Method m = findMatchingMethod(methodName, iface, actualParamTypes); + if (m != null) { + return m; + } + } + return findMatchingMethod(methodName, c.getSuperclass(), actualParamTypes); + } + + private static Method findMethodWithPossibleTypes(Object curObject, String methodName, Class... paramTypesToTryInOrder) throws NoSuchMethodException{ Class c = curObject.getClass(); for(Class paramType : paramTypesToTryInOrder) { try { - return c.getMethod(methodName, paramType); + return findMatchingMethod(curObject, methodName, new Class[]{paramType}); } catch (NoSuchMethodException e) { // try next one } diff --git a/instrumentation/nocode/src/test/config/nocode.yml b/instrumentation/nocode/src/test/config/nocode.yml index b17af28f1..91b664bd8 100644 --- a/instrumentation/nocode/src/test/config/nocode.yml +++ b/instrumentation/nocode/src/test/config/nocode.yml @@ -4,6 +4,8 @@ attributes: - key: "details" value: this.getDetails() + - key: "map.size" + value: this.getMap().entrySet().size() - class: com.splunk.opentelemetry.instrumentation.nocode.NocodeInstrumentationTest$SampleClass method: throwException diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index 3e1beb9d0..6d3b4ebda 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -29,6 +29,7 @@ public void testBasicBehavior() { {"this.get(\"key\").substring(1)", "alue"}, {"param0.isEmpty()", "false"}, {"param0.contains(\"present\")", "true"}, + {"this.entrySet().size()", "1"}, }; for(String[] test : tests) { assertEquals(test[1], JSPS.evaluate(test[0], thiz, new Object[]{param0}), test[0]); diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java index a9d122554..78872e143 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java @@ -8,6 +8,8 @@ import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import java.util.HashMap; +import java.util.Map; // This test has "test/config/nocode.yml" applied to it by the gradle environment setting public class NocodeInstrumentationTest { @@ -25,6 +27,7 @@ public void testBasicMethod() { span.hasName("name") .hasKind(SpanKind.INTERNAL) .hasAttributesSatisfying( + equalTo(AttributeKey.stringKey("map.size"), "2"), equalTo(AttributeKey.stringKey("details"), "details")))); } @@ -70,6 +73,12 @@ public String getName() { public String getDetails() { return "details"; } + public Map getMap() { + HashMap answer = new HashMap<>(); + answer.put("key", "value"); + answer.put("key2", "value2"); + return answer; + } public void throwException(int parameter) { throw new UnsupportedOperationException("oh no"); From bd4c6fd0f08f7ca6f8930afad957f9db87557daa Mon Sep 17 00:00:00 2001 From: John Bley Date: Thu, 13 Feb 2025 12:43:47 -0500 Subject: [PATCH 29/62] spotlessApply --- .../bootstrap/nocode/NocodeRules.java | 39 ++++- .../instrumentation/nocode/JSPS.java | 75 +++++++--- .../nocode/NocodeAttributesExtractor.java | 39 +++-- .../nocode/NocodeInstrumentation.java | 42 ++++-- .../nocode/NocodeMethodInvocation.java | 19 ++- .../instrumentation/nocode/NocodeModule.java | 20 ++- .../nocode/NocodeSingletons.java | 28 +++- .../nocode/NocodeSpanKindExtractor.java | 16 ++ .../nocode/NocodeSpanNameExtractor.java | 16 ++ .../instrumentation/nocode/YamlParser.java | 27 +++- .../instrumentation/nocode/JSPSTest.java | 141 ++++++++++-------- .../nocode/NocodeInstrumentationTest.java | 43 ++++-- 12 files changed, 358 insertions(+), 147 deletions(-) diff --git a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java index a43177df5..a96dacdda 100644 --- a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java +++ b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java @@ -1,3 +1,19 @@ +/* + * Copyright Splunk Inc. + * + * Licensed 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 com.splunk.opentelemetry.javaagent.bootstrap.nocode; import java.util.ArrayList; @@ -15,29 +31,38 @@ public static class Rule { public final String spanName; // may be null - use default of "class.method" public final String spanKind; // matches the SpanKind enum, null means default to INTERNAL public final Map attributes; // key name to jsps - public Rule(String className, String methodName, String spanName, String spanKind, Map attributes) { + + public Rule( + String className, + String methodName, + String spanName, + String spanKind, + Map attributes) { this.className = className; this.methodName = methodName; this.spanName = spanName; this.spanKind = spanKind; this.attributes = Collections.unmodifiableMap(new HashMap<>(attributes)); } + public String toString() { - return className+"."+methodName+":spanName="+spanName+",attrs="+attributes; + return className + "." + methodName + ":spanName=" + spanName + ",attrs=" + attributes; } } // Unfortunately the particular sequence of who needs access to this and who can create (based on // having the yaml parser library loaded) is awkward. Would prefer this to be a simple // static final immutable computed at startup... - private static final AtomicReference> GlobalRules = new AtomicReference<>(Collections.EMPTY_LIST); + private static final AtomicReference> GlobalRules = + new AtomicReference<>(Collections.EMPTY_LIST); // Using className.methodName as the key - private static final AtomicReference> Name2Rule = new AtomicReference<>(Collections.emptyMap()); + private static final AtomicReference> Name2Rule = + new AtomicReference<>(Collections.emptyMap()); public static void setGlobalRules(List rules) { GlobalRules.set(Collections.unmodifiableList(new ArrayList<>(rules))); Map n2r = new HashMap<>(); - for(Rule r : rules) { + for (Rule r : rules) { n2r.put(r.className + "." + r.methodName, r); } Name2Rule.set(Collections.unmodifiableMap(n2r)); @@ -48,9 +73,7 @@ public static List getGlobalRules() { } public static Rule findRuleByClassAndMethod(String className, String methodName) { - Map n2r = Name2Rule.get(); + Map n2r = Name2Rule.get(); return n2r == null ? null : n2r.get(className + "." + methodName); } - - } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java index 0a59445e4..71f832439 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java @@ -1,3 +1,19 @@ +/* + * Copyright Splunk Inc. + * + * Licensed 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 com.splunk.opentelemetry.instrumentation.nocode; import java.lang.reflect.Method; @@ -9,13 +25,15 @@ // some limitations. Its purpose is to allow pieces of nocode instrumentation // (attributes, span name) to be derived from the instrumentated context. // As some illustrative examples: - // this.getHeaders().get("X-Custom-Header").substring(5) - // param0.getDetails().getCustomerAccount().getAccountType() +// this.getHeaders().get("X-Custom-Header").substring(5) +// param0.getDetails().getCustomerAccount().getAccountType() // The limitations are: - // no access to variables other than 'this' and 'paramN' (N indexed at 0) - // no control flow (if), no local variables, basically nothing other than a single chain of method calls - // Methods calls are limited to either 0 or 1 parameters currently - // Parameters must be literals and only integral (int/long), string, and boolean literals are currently supported +// no access to variables other than 'this' and 'paramN' (N indexed at 0) +// no control flow (if), no local variables, basically nothing other than a single chain of method +// calls +// Methods calls are limited to either 0 or 1 parameters currently +// Parameters must be literals and only integral (int/long), string, and boolean literals are +// currently supported public class JSPS { private static final Logger logger = Logger.getLogger(JSPS.class.getName()); private static final Class[] NoParamTypes = new Class[0]; @@ -24,12 +42,11 @@ public static String evaluate(String jsps, Object thiz, Object[] params) { try { return unsafeEvaluate(jsps, thiz, params); } catch (Throwable t) { - logger.warning("Can't evaluate {"+jsps+"}: "+t); + logger.warning("Can't evaluate {" + jsps + "}: " + t); return null; } } - // FIXME Might be nice to support escaping quotes in string literals... private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) throws Exception { jsps = jsps.trim(); @@ -43,9 +60,9 @@ private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) curObject = params[varIndex]; } int curIndex = nextDot; - while(curIndex < jsps.length()) { + while (curIndex < jsps.length()) { curIndex = jsps.indexOf('.', curIndex); - while(jsps.charAt(curIndex) == '.' || Character.isWhitespace(jsps.charAt(curIndex))) { + while (jsps.charAt(curIndex) == '.' || Character.isWhitespace(jsps.charAt(curIndex))) { curIndex++; } int openParen = jsps.indexOf('(', curIndex); @@ -57,26 +74,31 @@ private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) curObject = m.invoke(curObject); } else { if (paramString.startsWith("\"") && paramString.endsWith("\"")) { - String passed = paramString.substring(1, paramString.length()-1); - Method m = findMethodWithPossibleTypes(curObject, method, - String.class, Object.class); + String passed = paramString.substring(1, paramString.length() - 1); + Method m = findMethodWithPossibleTypes(curObject, method, String.class, Object.class); curObject = m.invoke(curObject, passed); } else if (paramString.equals("true") || paramString.equals("false")) { - Method m = findMethodWithPossibleTypes(curObject, method, - boolean.class, Boolean.class, Object.class); + Method m = + findMethodWithPossibleTypes( + curObject, method, boolean.class, Boolean.class, Object.class); curObject = m.invoke(curObject, Boolean.parseBoolean(paramString)); } else if (paramString.matches("[0-9]+")) { try { - Method m = findMethodWithPossibleTypes(curObject, method, int.class, Integer.class, Object.class); + Method m = + findMethodWithPossibleTypes( + curObject, method, int.class, Integer.class, Object.class); int passed = Integer.parseInt(paramString); curObject = m.invoke(curObject, passed); } catch (NoSuchMethodException tryLongInstead) { - Method m = findMethodWithPossibleTypes(curObject, method, long.class, Long.class, Object.class); + Method m = + findMethodWithPossibleTypes( + curObject, method, long.class, Long.class, Object.class); long passed = Long.parseLong(paramString); curObject = m.invoke(curObject, passed); } } else { - throw new UnsupportedOperationException("Can't parse \""+paramString+"\" as literal parameter"); + throw new UnsupportedOperationException( + "Can't parse \"" + paramString + "\" as literal parameter"); } } curIndex = closeParen + 1; @@ -93,13 +115,16 @@ private static String unsafeEvaluate(String jsps, Object thiz, Object[] params) // this.getSomeHashMap().entrySet().size() // would fail simply because the HashMap$EntrySet implementation // is not public, even though the interface it's being call through is. - private static Method findMatchingMethod(Object curObject, String methodName, Class[] actualParamTypes) throws NoSuchMethodException{ + private static Method findMatchingMethod( + Object curObject, String methodName, Class[] actualParamTypes) throws NoSuchMethodException { Method m = findMatchingMethod(methodName, curObject.getClass(), actualParamTypes); if (m == null) { - throw new NoSuchMethodException("Can't find matching method for "+methodName+" on "+curObject.getClass().getName()); + throw new NoSuchMethodException( + "Can't find matching method for " + methodName + " on " + curObject.getClass().getName()); } return m; } + // Returns null for none found private static Method findMatchingMethod(String methodName, Class c, Class[] actualParamTypes) { if (c == null) { @@ -113,7 +138,7 @@ private static Method findMatchingMethod(String methodName, Class c, Class[] act } } // not public, try interfaces and supertype - for(Class iface : c.getInterfaces()) { + for (Class iface : c.getInterfaces()) { Method m = findMatchingMethod(methodName, iface, actualParamTypes); if (m != null) { return m; @@ -122,11 +147,13 @@ private static Method findMatchingMethod(String methodName, Class c, Class[] act return findMatchingMethod(methodName, c.getSuperclass(), actualParamTypes); } - private static Method findMethodWithPossibleTypes(Object curObject, String methodName, Class... paramTypesToTryInOrder) throws NoSuchMethodException{ + private static Method findMethodWithPossibleTypes( + Object curObject, String methodName, Class... paramTypesToTryInOrder) + throws NoSuchMethodException { Class c = curObject.getClass(); - for(Class paramType : paramTypesToTryInOrder) { + for (Class paramType : paramTypesToTryInOrder) { try { - return findMatchingMethod(curObject, methodName, new Class[]{paramType}); + return findMatchingMethod(curObject, methodName, new Class[] {paramType}); } catch (NoSuchMethodException e) { // try next one } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java index c607bb4b1..e249a75cb 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java @@ -1,3 +1,19 @@ +/* + * Copyright Splunk Inc. + * + * Licensed 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 com.splunk.opentelemetry.instrumentation.nocode; import io.opentelemetry.api.common.AttributesBuilder; @@ -9,37 +25,40 @@ import java.util.Map; import javax.annotation.Nullable; -public class NocodeAttributesExtractor implements AttributesExtractor { +public class NocodeAttributesExtractor + implements AttributesExtractor { private final AttributesExtractor codeExtractor; public NocodeAttributesExtractor() { codeExtractor = CodeAttributesExtractor.create(ClassAndMethod.codeAttributesGetter()); } + @Override - public void onStart(AttributesBuilder attributesBuilder, Context context, NocodeMethodInvocation mi) { + public void onStart( + AttributesBuilder attributesBuilder, Context context, NocodeMethodInvocation mi) { codeExtractor.onStart(attributesBuilder, context, mi.getClassAndMethod()); Map attributes = Collections.EMPTY_MAP; if (mi.getRule() != null) { attributes = mi.getRule().attributes; } - for(String key : attributes.keySet()) { + for (String key : attributes.keySet()) { String jsps = attributes.get(key); String value = JSPS.evaluate(jsps, mi.getThiz(), mi.getParameters()); if (value != null) { attributesBuilder.put(key, value); } } - - } @Override - public void onEnd(AttributesBuilder attributesBuilder, Context context, - NocodeMethodInvocation nocodeMethodInvocation, @Nullable Void unused, + public void onEnd( + AttributesBuilder attributesBuilder, + Context context, + NocodeMethodInvocation nocodeMethodInvocation, + @Nullable Void unused, @Nullable Throwable throwable) { - codeExtractor.onEnd(attributesBuilder, context, nocodeMethodInvocation.getClassAndMethod(), unused, throwable); - - + codeExtractor.onEnd( + attributesBuilder, context, nocodeMethodInvocation.getClassAndMethod(), unused, throwable); } } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index 5a493e4e5..c394adf5e 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -1,3 +1,19 @@ +/* + * Copyright Splunk Inc. + * + * Licensed 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 com.splunk.opentelemetry.instrumentation.nocode; import static com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons.instrumentor; @@ -30,13 +46,13 @@ public ElementMatcher typeMatcher() { @Override public void transform(TypeTransformer transformer) { - transformer.applyAdviceToMethod(named(rule.methodName), - NocodeInstrumentation.class.getName()+"$NocodeAdvice"); + transformer.applyAdviceToMethod( + named(rule.methodName), NocodeInstrumentation.class.getName() + "$NocodeAdvice"); } @SuppressWarnings("unused") public static class NocodeAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) + @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter( @Advice.Origin("#t") Class declaringClass, @Advice.Origin("#m") String methodName, @@ -44,14 +60,12 @@ public static void onEnter( @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope, @Advice.This Object thiz, - @Advice.AllArguments Object[] methodParams - ) { - NocodeRules.Rule rule = NocodeRules.findRuleByClassAndMethod(declaringClass.getName(), methodName); - otelInvocation = new NocodeMethodInvocation( - rule, - ClassAndMethod.create(declaringClass, methodName), - thiz, - methodParams); + @Advice.AllArguments Object[] methodParams) { + NocodeRules.Rule rule = + NocodeRules.findRuleByClassAndMethod(declaringClass.getName(), methodName); + otelInvocation = + new NocodeMethodInvocation( + rule, ClassAndMethod.create(declaringClass, methodName), thiz, methodParams); Context parentContext = currentContext(); if (!instrumentor().shouldStart(parentContext, otelInvocation)) { @@ -71,13 +85,13 @@ public static void stopSpan( @Advice.Local("otelInvocation") NocodeMethodInvocation otelInvocation, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope, - @Advice.Thrown Throwable error) - { + @Advice.Thrown Throwable error) { if (scope == null) { return; } scope.close(); - // I wonder why the original methods instrumentation had support for modifying the return value? + // I wonder why the original methods instrumentation had support for modifying the return + // value? instrumentor().end(context, otelInvocation, null, error); } } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java index 1ff05751c..1e06d0096 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java @@ -1,3 +1,19 @@ +/* + * Copyright Splunk Inc. + * + * Licensed 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 com.splunk.opentelemetry.instrumentation.nocode; import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; @@ -9,7 +25,8 @@ public class NocodeMethodInvocation { private final Object thiz; private final Object[] parameters; - public NocodeMethodInvocation(NocodeRules.Rule rule, ClassAndMethod cm, Object thiz, Object[] parameters) { + public NocodeMethodInvocation( + NocodeRules.Rule rule, ClassAndMethod cm, Object thiz, Object[] parameters) { this.rule = rule; this.classAndMethod = cm; this.thiz = thiz; diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java index aadbf703b..2fdd87ad7 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java @@ -1,3 +1,18 @@ +/* + * Copyright Splunk Inc. + * + * Licensed 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 com.splunk.opentelemetry.instrumentation.nocode; @@ -21,7 +36,7 @@ public NocodeModule() { @Override public List typeInstrumentations() { ArrayList answer = new ArrayList<>(); - for(NocodeRules.Rule rule : NocodeRules.getGlobalRules()) { + for (NocodeRules.Rule rule : NocodeRules.getGlobalRules()) { answer.add(new NocodeInstrumentation(rule)); } return answer; @@ -35,8 +50,7 @@ public List getAdditionalHelperClassNames() { "com.splunk.opentelemetry.instrumentation.nocode.NocodeAttributesExtractor", "com.splunk.opentelemetry.instrumentation.nocode.NocodeMethodInvocation", "com.splunk.opentelemetry.instrumentation.nocode.NocodeSpanKindExtractor", - "com.splunk.opentelemetry.instrumentation.nocode.NocodeSpanNameExtractor" - ); + "com.splunk.opentelemetry.instrumentation.nocode.NocodeSpanNameExtractor"); } @Override diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java index 4523bc881..960da0a25 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java @@ -1,3 +1,19 @@ +/* + * Copyright Splunk Inc. + * + * Licensed 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 com.splunk.opentelemetry.instrumentation.nocode; import io.opentelemetry.api.GlobalOpenTelemetry; @@ -7,11 +23,13 @@ public class NocodeSingletons { private static final Instrumenter INSTRUMENTER; static { - INSTRUMENTER = Instrumenter.builder( - GlobalOpenTelemetry.get(), - "com.splunk.opentelemetry.instrumentation.nocode", - new NocodeSpanNameExtractor()).addAttributesExtractor(new NocodeAttributesExtractor()) - .buildInstrumenter(new NocodeSpanKindExtractor()); + INSTRUMENTER = + Instrumenter.builder( + GlobalOpenTelemetry.get(), + "com.splunk.opentelemetry.instrumentation.nocode", + new NocodeSpanNameExtractor()) + .addAttributesExtractor(new NocodeAttributesExtractor()) + .buildInstrumenter(new NocodeSpanKindExtractor()); } public static Instrumenter instrumentor() { diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java index 7b7338a0f..80b639c34 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java @@ -1,3 +1,19 @@ +/* + * Copyright Splunk Inc. + * + * Licensed 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 com.splunk.opentelemetry.instrumentation.nocode; import io.opentelemetry.api.trace.SpanKind; diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java index c92080638..86db707ef 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanNameExtractor.java @@ -1,3 +1,19 @@ +/* + * Copyright Splunk Inc. + * + * Licensed 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 com.splunk.opentelemetry.instrumentation.nocode; import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index b79732528..a44bf43ac 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -1,3 +1,19 @@ +/* + * Copyright Splunk Inc. + * + * Licensed 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 com.splunk.opentelemetry.instrumentation.nocode; import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; @@ -32,7 +48,7 @@ private static List load() { try { return loadUnsafe(); } catch (Exception e) { - logger.severe("Can't load configured yaml: "+e); + logger.severe("Can't load configured yaml: " + e); return Collections.emptyList(); } } @@ -47,9 +63,9 @@ private static List loadUnsafe() throws Exception { Load load = new Load(LoadSettings.builder().build()); Iterable parsedYaml = load.loadAllFromReader(yamlReader); ArrayList answer = new ArrayList<>(); - for(Object yamlBit : parsedYaml) { + for (Object yamlBit : parsedYaml) { List l = (List) yamlBit; - for(Object yamlRule : l) { + for (Object yamlRule : l) { LinkedHashMap lhm = (LinkedHashMap) yamlRule; String className = lhm.get("class").toString(); String methodName = lhm.get("method").toString(); @@ -57,7 +73,7 @@ private static List loadUnsafe() throws Exception { String spanKind = lhm.get("spanKind") == null ? null : lhm.get("spanKind").toString(); List attrs = (List) lhm.get("attributes"); Map ruleAttributes = new HashMap<>(); - for(Object attr : attrs) { + for (Object attr : attrs) { LinkedHashMap attrMap = (LinkedHashMap) attr; ruleAttributes.put(attrMap.get("key").toString(), attrMap.get("value").toString()); } @@ -67,7 +83,4 @@ private static List loadUnsafe() throws Exception { yamlReader.close(); return answer; } - - - } diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index 6d3b4ebda..7b27846fd 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -1,3 +1,19 @@ +/* + * Copyright Splunk Inc. + * + * Licensed 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 com.splunk.opentelemetry.instrumentation.nocode; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -10,7 +26,7 @@ import java.util.Set; import org.junit.jupiter.api.Test; -public class JSPSTest { +public class JSPSTest { private static final Map thiz = new HashMap<>(); private static final Set param0 = new HashSet<>(); @@ -21,48 +37,51 @@ public class JSPSTest { @Test public void testBasicBehavior() { - String[][] tests = new String[][] { - // Might be nice to support a bare "param0" or "this" but as a workaround can always use "this.toString()" - {"this.toString()", "{key=value}"}, - {"this.toString().length()", "11"}, - {"this.get(\"key\")", "value"}, - {"this.get(\"key\").substring(1)", "alue"}, - {"param0.isEmpty()", "false"}, - {"param0.contains(\"present\")", "true"}, - {"this.entrySet().size()", "1"}, - }; - for(String[] test : tests) { - assertEquals(test[1], JSPS.evaluate(test[0], thiz, new Object[]{param0}), test[0]); + String[][] tests = + new String[][] { + // Might be nice to support a bare "param0" or "this" but as a workaround can always use + // "this.toString()" + {"this.toString()", "{key=value}"}, + {"this.toString().length()", "11"}, + {"this.get(\"key\")", "value"}, + {"this.get(\"key\").substring(1)", "alue"}, + {"param0.isEmpty()", "false"}, + {"param0.contains(\"present\")", "true"}, + {"this.entrySet().size()", "1"}, + }; + for (String[] test : tests) { + assertEquals(test[1], JSPS.evaluate(test[0], thiz, new Object[] {param0}), test[0]); } } @Test public void testInvalidJspsReturnNull() { - String[] invalids = new String[] { - "nosuchvar", - "nosuchvar.toString()", - "this .", - "this . ", - "this.noSuchMethod()", - "toString()", - "this.toString()extrastuffatend", - "this.toString()toString()", - "param1.toString()", // out of bounds - "param999.toString()", - "this.getOrDefault(\"key\", \"multiparamnotsupported\")", - "this.get(\"noclosequote)", - "this.get(\"nocloseparan\"", - "this.noparens", - "this.noparens.anotherMethod()", - "this.wrongOrder)(", - "this.get(NotALiteralParameter);", - "this.get(12.2)", - "this.get(this)", - "this.get(\"NoSuchKey\")", // evals completely but returns null - "param1.toString()", // no such param - }; - for(String invalid : invalids) { - String answer = JSPS.evaluate(invalid, thiz, new Object[]{param0}); + String[] invalids = + new String[] { + "nosuchvar", + "nosuchvar.toString()", + "this .", + "this . ", + "this.noSuchMethod()", + "toString()", + "this.toString()extrastuffatend", + "this.toString()toString()", + "param1.toString()", // out of bounds + "param999.toString()", + "this.getOrDefault(\"key\", \"multiparamnotsupported\")", + "this.get(\"noclosequote)", + "this.get(\"nocloseparan\"", + "this.noparens", + "this.noparens.anotherMethod()", + "this.wrongOrder)(", + "this.get(NotALiteralParameter);", + "this.get(12.2)", + "this.get(this)", + "this.get(\"NoSuchKey\")", // evals completely but returns null + "param1.toString()", // no such param + }; + for (String invalid : invalids) { + String answer = JSPS.evaluate(invalid, thiz, new Object[] {param0}); assertNull(answer, "Expected null for \"" + invalid + "\" but was \"" + answer + "\""); } } @@ -80,6 +99,7 @@ public String take(String s) { return s; } } + public static class TakeObject { public String take(Object o) { return o.toString(); @@ -91,33 +111,37 @@ public String take(boolean param) { return Boolean.toString(param); } } + public static class TakeBoolean { public String take(Boolean param) { return param.toString(); } } + public static class TakeIntegerPrimitive { public String take(int param) { return Integer.toString(param); } } + public static class TakeInteger { public String take(Integer param) { return param.toString(); } } + public static class TakeLongPrimitize { public String take(long param) { return Long.toString(param); } } + public static class TakeLong { public String take(Long param) { return param.toString(); } } - @Test public void testBooleanLiteralParamTypes() { TakeBooleanPrimitive b = new TakeBooleanPrimitive(); @@ -152,31 +176,30 @@ public void testIntegerLiteralParamTypes() { @Test public void testWhitespace() { - String[] tests = new String[]{ - "this.get(\"key\").substring(1)", - " this.get(\"key\").substring(1)", - "this .get(\"key\").substring(1)", - "this. get(\"key\").substring(1)", - "this.get (\"key\").substring(1)", - "this.get( \"key\").substring(1)", - "this.get(\"key\" ).substring(1)", - "this.get(\"key\") .substring(1)", - "this.get(\"key\"). substring(1)", - "this.get(\"key\").substring (1)", - "this.get(\"key\").substring( 1)", - "this.get(\"key\").substring(1 )", - }; - for(String test : tests) { - assertEquals("alue", JSPS.evaluate(test, thiz, new Object[]{param0}), test); - + String[] tests = + new String[] { + "this.get(\"key\").substring(1)", + " this.get(\"key\").substring(1)", + "this .get(\"key\").substring(1)", + "this. get(\"key\").substring(1)", + "this.get (\"key\").substring(1)", + "this.get( \"key\").substring(1)", + "this.get(\"key\" ).substring(1)", + "this.get(\"key\") .substring(1)", + "this.get(\"key\"). substring(1)", + "this.get(\"key\").substring (1)", + "this.get(\"key\").substring( 1)", + "this.get(\"key\").substring(1 )", + }; + for (String test : tests) { + assertEquals("alue", JSPS.evaluate(test, thiz, new Object[] {param0}), test); } } public void testManyParams() { Object[] params = new Object[13]; Arrays.fill(params, new Object()); - assertEquals("java.lang.Object", JSPS.evaluate("param12.getClass().getName()", new Object(), params)); + assertEquals( + "java.lang.Object", JSPS.evaluate("param12.getClass().getName()", new Object(), params)); } - - } diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java index 78872e143..723715588 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java @@ -1,3 +1,19 @@ +/* + * Copyright Splunk Inc. + * + * Licensed 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 com.splunk.opentelemetry.instrumentation.nocode; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; @@ -6,10 +22,10 @@ import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; import java.util.HashMap; import java.util.Map; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; // This test has "test/config/nocode.yml" applied to it by the gradle environment setting public class NocodeInstrumentationTest { @@ -40,10 +56,10 @@ public void testRuleWithManyInvalidFields() { span -> span.hasName("SampleClass.doInvalidRule") .hasKind(SpanKind.INTERNAL) - .hasTotalAttributeCount(2))); // two code. attribute but nothing from the invalid rule + .hasTotalAttributeCount( + 2))); // two code. attribute but nothing from the invalid rule } - @Test public void testThrowException() { try { @@ -58,11 +74,8 @@ public void testThrowException() { span -> span.hasName("SampleClass.throwException") .hasKind(SpanKind.SERVER) - .hasEventsSatisfyingExactly( - event -> - event.hasName("exception")) - .hasAttributesSatisfying( - equalTo(AttributeKey.stringKey("five"), "5")))); + .hasEventsSatisfyingExactly(event -> event.hasName("exception")) + .hasAttributesSatisfying(equalTo(AttributeKey.stringKey("five"), "5")))); } public static class SampleClass { @@ -73,8 +86,9 @@ public String getName() { public String getDetails() { return "details"; } - public Map getMap() { - HashMap answer = new HashMap<>(); + + public Map getMap() { + HashMap answer = new HashMap<>(); answer.put("key", "value"); answer.put("key2", "value2"); return answer; @@ -84,11 +98,8 @@ public void throwException(int parameter) { throw new UnsupportedOperationException("oh no"); } - public void doSomething() { - } - - public void doInvalidRule() { - } + public void doSomething() {} + public void doInvalidRule() {} } } From 8c311609cddca924eaa7e158ee54c2b5268f1c6e Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:35:25 -0500 Subject: [PATCH 30/62] Update bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java final class Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java index a96dacdda..d22486b2b 100644 --- a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java +++ b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java @@ -25,7 +25,7 @@ public class NocodeRules { - public static class Rule { + public final static class Rule { public final String className; public final String methodName; public final String spanName; // may be null - use default of "class.method" From 5bf0a940bc4b0ac7fed8fd55ddc992212d154053 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:35:41 -0500 Subject: [PATCH 31/62] Update bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java final class Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java index d22486b2b..4720ceed0 100644 --- a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java +++ b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java @@ -23,7 +23,7 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicReference; -public class NocodeRules { +public final class NocodeRules { public final static class Rule { public final String className; From fbebfe36f09da42398e1b08260c443b4da742350 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:40:12 -0500 Subject: [PATCH 32/62] Simplify global rules data structure, naming, and logic per feedback --- .../bootstrap/nocode/NocodeRules.java | 23 ++++++------------- .../nocode/NocodeInstrumentation.java | 2 +- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java index 4720ceed0..546d399d3 100644 --- a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java +++ b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java @@ -21,6 +21,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicReference; public final class NocodeRules { @@ -50,30 +51,20 @@ public String toString() { } } - // Unfortunately the particular sequence of who needs access to this and who can create (based on - // having the yaml parser library loaded) is awkward. Would prefer this to be a simple - // static final immutable computed at startup... - private static final AtomicReference> GlobalRules = - new AtomicReference<>(Collections.EMPTY_LIST); // Using className.methodName as the key - private static final AtomicReference> Name2Rule = - new AtomicReference<>(Collections.emptyMap()); + private static final ConcurrentHashMap Name2Rule = new ConcurrentHashMap<>(); public static void setGlobalRules(List rules) { - GlobalRules.set(Collections.unmodifiableList(new ArrayList<>(rules))); - Map n2r = new HashMap<>(); for (Rule r : rules) { - n2r.put(r.className + "." + r.methodName, r); + Name2Rule.put(r.className + "." + r.methodName, r); } - Name2Rule.set(Collections.unmodifiableMap(n2r)); } - public static List getGlobalRules() { - return GlobalRules.get(); + public static Iterable getGlobalRules() { + return Name2Rule.values(); } - public static Rule findRuleByClassAndMethod(String className, String methodName) { - Map n2r = Name2Rule.get(); - return n2r == null ? null : n2r.get(className + "." + methodName); + public static Rule find(String className, String methodName) { + return Name2Rule.get(className + "." + methodName); } } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index c394adf5e..b9b8ca2c5 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -62,7 +62,7 @@ public static void onEnter( @Advice.This Object thiz, @Advice.AllArguments Object[] methodParams) { NocodeRules.Rule rule = - NocodeRules.findRuleByClassAndMethod(declaringClass.getName(), methodName); + NocodeRules.find(declaringClass.getName(), methodName); otelInvocation = new NocodeMethodInvocation( rule, ClassAndMethod.create(declaringClass, methodName), thiz, methodParams); From a4a4ea7491054c9a05df59e5e7a97cbdf95edb2d Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:44:17 -0500 Subject: [PATCH 33/62] Naming, docs feedback --- .../instrumentation/nocode/JSPS.java | 43 +++++++++---------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java index 71f832439..eb78da0eb 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java @@ -19,22 +19,21 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.logging.Logger; - -// JSPS stands for Java-like String-Producing Statement. A JSPS is -// essentially a single call in Java (as though it ends with a semicolon), with -// some limitations. Its purpose is to allow pieces of nocode instrumentation -// (attributes, span name) to be derived from the instrumentated context. -// As some illustrative examples: -// this.getHeaders().get("X-Custom-Header").substring(5) -// param0.getDetails().getCustomerAccount().getAccountType() -// The limitations are: -// no access to variables other than 'this' and 'paramN' (N indexed at 0) -// no control flow (if), no local variables, basically nothing other than a single chain of method -// calls -// Methods calls are limited to either 0 or 1 parameters currently -// Parameters must be literals and only integral (int/long), string, and boolean literals are -// currently supported -public class JSPS { +/** + * JSPS stands for Java-like String-Producing Statement. A JSPS is + * essentially a single call in Java (as though it ends with a semicolon), with + * some limitations. Its purpose is to allow pieces of nocode instrumentation + * (attributes, span name) to be derived from the instrumentated context. + * As some illustrative examples: + * this.getHeaders().get("X-Custom-Header").substring(5) + * param0.getDetails().getCustomerAccount().getAccountType() + * The limitations are: + * - no access to variables other than 'this' and 'paramN' (N indexed at 0) + * - no control flow (if), no local variables, basically nothing other than a single chain of method calls + * - Method calls are limited to either 0 or 1 parameters currently + * - Parameters must be literals and only integral (int/long), string, and boolean literals are currently supported + */ +public final class JSPS { private static final Logger logger = Logger.getLogger(JSPS.class.getName()); private static final Class[] NoParamTypes = new Class[0]; @@ -126,25 +125,25 @@ private static Method findMatchingMethod( } // Returns null for none found - private static Method findMatchingMethod(String methodName, Class c, Class[] actualParamTypes) { - if (c == null) { + private static Method findMatchingMethod(String methodName, Class clazz, Class[] actualParamTypes) { + if (clazz == null) { return null; } - if (Modifier.isPublic(c.getModifiers())) { + if (Modifier.isPublic(clazz.getModifiers())) { try { - return c.getMethod(methodName, actualParamTypes); + return clazz.getMethod(methodName, actualParamTypes); } catch (NoSuchMethodException nsme) { // keep trying } } // not public, try interfaces and supertype - for (Class iface : c.getInterfaces()) { + for (Class iface : clazz.getInterfaces()) { Method m = findMatchingMethod(methodName, iface, actualParamTypes); if (m != null) { return m; } } - return findMatchingMethod(methodName, c.getSuperclass(), actualParamTypes); + return findMatchingMethod(methodName, clazz.getSuperclass(), actualParamTypes); } private static Method findMethodWithPossibleTypes( From 58d5307df36d06084fd82c1ec3271ab823b26b7f Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:45:02 -0500 Subject: [PATCH 34/62] Update instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java final class Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../instrumentation/nocode/NocodeAttributesExtractor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java index e249a75cb..8953b7725 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java @@ -25,7 +25,7 @@ import java.util.Map; import javax.annotation.Nullable; -public class NocodeAttributesExtractor +public final class NocodeAttributesExtractor implements AttributesExtractor { private final AttributesExtractor codeExtractor; From 52e691b0acc783a825f5133dc9a7a5d848af2c20 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:45:14 -0500 Subject: [PATCH 35/62] Update instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java final class Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../instrumentation/nocode/NocodeMethodInvocation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java index 1e06d0096..4f318ef9b 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java @@ -19,7 +19,7 @@ import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; -public class NocodeMethodInvocation { +public final class NocodeMethodInvocation { private final NocodeRules.Rule rule; private final ClassAndMethod classAndMethod; private final Object thiz; From 244269002615b6928d3be4103b43f1fa1ed7d796 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:46:59 -0500 Subject: [PATCH 36/62] Update instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java final class Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../instrumentation/nocode/NocodeInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index b9b8ca2c5..f031b998d 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -32,7 +32,7 @@ import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; -public class NocodeInstrumentation implements TypeInstrumentation { +public final class NocodeInstrumentation implements TypeInstrumentation { private final NocodeRules.Rule rule; public NocodeInstrumentation(NocodeRules.Rule rule) { From d354adbcaadb4ad7c7f2d7037c11395d81c2443e Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:48:46 -0500 Subject: [PATCH 37/62] Update instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../instrumentation/nocode/NocodeInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index f031b998d..8a5921efa 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -47,7 +47,7 @@ public ElementMatcher typeMatcher() { @Override public void transform(TypeTransformer transformer) { transformer.applyAdviceToMethod( - named(rule.methodName), NocodeInstrumentation.class.getName() + "$NocodeAdvice"); + named(rule.methodName), this.getClass().getName() + "$NocodeAdvice"); } @SuppressWarnings("unused") From 8e296115bfd5ed778f1c8b3679da9c533f1d613b Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:49:15 -0500 Subject: [PATCH 38/62] Update instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../instrumentation/nocode/NocodeInstrumentation.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index 8a5921efa..53c33ad69 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -75,10 +75,6 @@ public static void onEnter( scope = context.makeCurrent(); } - public NocodeAdvice() { - super(); - } - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( @Advice.Origin Method method, From 6e5cce212636db6590e4f25be2a093fdbc01c9ea Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:50:45 -0500 Subject: [PATCH 39/62] Update instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../opentelemetry/instrumentation/nocode/NocodeModule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java index 2fdd87ad7..51ccd7cf5 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java @@ -35,7 +35,7 @@ public NocodeModule() { @Override public List typeInstrumentations() { - ArrayList answer = new ArrayList<>(); + List answer = new ArrayList<>(); for (NocodeRules.Rule rule : NocodeRules.getGlobalRules()) { answer.add(new NocodeInstrumentation(rule)); } From 105f29a3b8cccc200751ba8b9345413477d97304 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:55:37 -0500 Subject: [PATCH 40/62] Reduce use of public in test code --- .../instrumentation/nocode/JSPSTest.java | 17 +++++++++-------- .../nocode/NocodeInstrumentationTest.java | 8 ++++---- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index 7b27846fd..4abfd22bd 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -26,7 +26,7 @@ import java.util.Set; import org.junit.jupiter.api.Test; -public class JSPSTest { +class JSPSTest { private static final Map thiz = new HashMap<>(); private static final Set param0 = new HashSet<>(); @@ -36,7 +36,7 @@ public class JSPSTest { } @Test - public void testBasicBehavior() { + void testBasicBehavior() { String[][] tests = new String[][] { // Might be nice to support a bare "param0" or "this" but as a workaround can always use @@ -55,7 +55,7 @@ public void testBasicBehavior() { } @Test - public void testInvalidJspsReturnNull() { + void testInvalidJspsReturnNull() { String[] invalids = new String[] { "nosuchvar", @@ -87,7 +87,7 @@ public void testInvalidJspsReturnNull() { } @Test - public void testIntegerLiteralLongerThanOneDigit() { + void testIntegerLiteralLongerThanOneDigit() { Map o = new HashMap<>(); o.put("key", "really long value"); String jsps = "this.get(\"key\").substring(10)"; @@ -143,7 +143,7 @@ public String take(Long param) { } @Test - public void testBooleanLiteralParamTypes() { + void testBooleanLiteralParamTypes() { TakeBooleanPrimitive b = new TakeBooleanPrimitive(); TakeBoolean B = new TakeBoolean(); TakeObject O = new TakeObject(); @@ -153,7 +153,7 @@ public void testBooleanLiteralParamTypes() { } @Test - public void testStringLiteralParamTypes() { + void testStringLiteralParamTypes() { TakeString S = new TakeString(); TakeObject O = new TakeObject(); assertEquals("a", JSPS.evaluate("this.take(\"a\")", S, new Object[0])); @@ -175,7 +175,7 @@ public void testIntegerLiteralParamTypes() { } @Test - public void testWhitespace() { + void testWhitespace() { String[] tests = new String[] { "this.get(\"key\").substring(1)", @@ -196,7 +196,8 @@ public void testWhitespace() { } } - public void testManyParams() { + @Test + void testManyParams() { Object[] params = new Object[13]; Arrays.fill(params, new Object()); assertEquals( diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java index 723715588..945e9b8ab 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java @@ -28,12 +28,12 @@ import org.junit.jupiter.api.extension.RegisterExtension; // This test has "test/config/nocode.yml" applied to it by the gradle environment setting -public class NocodeInstrumentationTest { +class NocodeInstrumentationTest { @RegisterExtension static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); @Test - public void testBasicMethod() { + void testBasicMethod() { new SampleClass().doSomething(); // FIXME what is scope version verification and why doesn't it pass? testing.waitAndAssertTracesWithoutScopeVersionVerification( @@ -48,7 +48,7 @@ public void testBasicMethod() { } @Test - public void testRuleWithManyInvalidFields() { + void testRuleWithManyInvalidFields() { new SampleClass().doInvalidRule(); testing.waitAndAssertTracesWithoutScopeVersionVerification( trace -> @@ -61,7 +61,7 @@ public void testRuleWithManyInvalidFields() { } @Test - public void testThrowException() { + void testThrowException() { try { new SampleClass().throwException(5); } catch (Exception expected) { From 07ff3295fff3f95b17ca779d44c630bca9d7b24c Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:57:20 -0500 Subject: [PATCH 41/62] Update instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../opentelemetry/instrumentation/nocode/NocodeModule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java index 51ccd7cf5..f58723882 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java @@ -25,7 +25,7 @@ import java.util.List; @AutoService(InstrumentationModule.class) -public class NocodeModule extends InstrumentationModule { +public final class NocodeModule extends InstrumentationModule { public NocodeModule() { super("nocode"); From 36e0258e15116e593c245b9123ed5323a4a4435a Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:57:31 -0500 Subject: [PATCH 42/62] Update instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../splunk/opentelemetry/instrumentation/nocode/YamlParser.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index a44bf43ac..7046753bc 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -29,7 +29,7 @@ import org.snakeyaml.engine.v2.api.Load; import org.snakeyaml.engine.v2.api.LoadSettings; -public class YamlParser { +public final class YamlParser { private static final Logger logger = Logger.getLogger(YamlParser.class.getName()); // FIXME support method override selection - e.g., with classfile method signature or something public static final String NOCODE_YMLFILE_ENV_KEY = "SPLUNK_OTEL_INSTRUMENTATION_NOCODE_YML_FILE"; From 539c71b7dc1cbbcb2869e9c611dc518a00dd1893 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:58:02 -0500 Subject: [PATCH 43/62] Update instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../splunk/opentelemetry/instrumentation/nocode/YamlParser.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index 7046753bc..b5d5db584 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -48,7 +48,7 @@ private static List load() { try { return loadUnsafe(); } catch (Exception e) { - logger.severe("Can't load configured yaml: " + e); + logger.log(Level.SEVERE, "Can't load configured nocode yaml.", e) return Collections.emptyList(); } } From 629e5065b098e6bb344322b6a68798defe30ffe0 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 05:58:28 -0500 Subject: [PATCH 44/62] Update instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com> --- .../splunk/opentelemetry/instrumentation/nocode/YamlParser.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index b5d5db584..72bc2f6c7 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -62,7 +62,7 @@ private static List loadUnsafe() throws Exception { Load load = new Load(LoadSettings.builder().build()); Iterable parsedYaml = load.loadAllFromReader(yamlReader); - ArrayList answer = new ArrayList<>(); + List answer = new ArrayList<>(); for (Object yamlBit : parsedYaml) { List l = (List) yamlBit; for (Object yamlRule : l) { From 0dded04f0fd382fdc5797306320bdbfe8a0cedf8 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 06:01:12 -0500 Subject: [PATCH 45/62] Make PR feedback compile --- .../opentelemetry/instrumentation/nocode/YamlParser.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index 72bc2f6c7..d6065e3a0 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -25,6 +25,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.logging.Level; import java.util.logging.Logger; import org.snakeyaml.engine.v2.api.Load; import org.snakeyaml.engine.v2.api.LoadSettings; @@ -48,7 +49,7 @@ private static List load() { try { return loadUnsafe(); } catch (Exception e) { - logger.log(Level.SEVERE, "Can't load configured nocode yaml.", e) + logger.log(Level.SEVERE, "Can't load configured nocode yaml.", e); return Collections.emptyList(); } } From 4faae8bbd036905b5ac875ffe9c99f5742f21c7b Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 06:03:05 -0500 Subject: [PATCH 46/62] Use more kinds of whitespace in test --- .../opentelemetry/instrumentation/nocode/JSPSTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index 4abfd22bd..25e15334c 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -185,9 +185,9 @@ void testWhitespace() { "this.get (\"key\").substring(1)", "this.get( \"key\").substring(1)", "this.get(\"key\" ).substring(1)", - "this.get(\"key\") .substring(1)", - "this.get(\"key\"). substring(1)", - "this.get(\"key\").substring (1)", + "this.get(\"key\")\t.substring(1)", + "this.get(\"key\").\nsubstring(1)", + "this.get(\"key\").substring\r(1)", "this.get(\"key\").substring( 1)", "this.get(\"key\").substring(1 )", }; From cb8f827a7f863c66da42b0006c7bea4bf029750c Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 06:08:06 -0500 Subject: [PATCH 47/62] Clean up types, names, local variables for yaml parsing. --- .../instrumentation/nocode/YamlParser.java | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index d6065e3a0..9c8663cff 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -65,18 +65,16 @@ private static List loadUnsafe() throws Exception { Iterable parsedYaml = load.loadAllFromReader(yamlReader); List answer = new ArrayList<>(); for (Object yamlBit : parsedYaml) { - List l = (List) yamlBit; - for (Object yamlRule : l) { - LinkedHashMap lhm = (LinkedHashMap) yamlRule; - String className = lhm.get("class").toString(); - String methodName = lhm.get("method").toString(); - String spanName = lhm.get("spanName") == null ? null : lhm.get("spanName").toString(); - String spanKind = lhm.get("spanKind") == null ? null : lhm.get("spanKind").toString(); - List attrs = (List) lhm.get("attributes"); + List> rulesMap = (List>) yamlBit; + for (Map yamlRule : rulesMap) { + String className = yamlRule.get("class").toString(); + String methodName = yamlRule.get("method").toString(); + String spanName = yamlRule.get("spanName") == null ? null : yamlRule.get("spanName").toString(); + String spanKind = yamlRule.get("spanKind") == null ? null : yamlRule.get("spanKind").toString(); + List> attrs = (List>) yamlRule.get("attributes"); Map ruleAttributes = new HashMap<>(); - for (Object attr : attrs) { - LinkedHashMap attrMap = (LinkedHashMap) attr; - ruleAttributes.put(attrMap.get("key").toString(), attrMap.get("value").toString()); + for (Map attr : attrs) { + ruleAttributes.put(attr.get("key").toString(), attr.get("value").toString()); } answer.add(new NocodeRules.Rule(className, methodName, spanName, spanKind, ruleAttributes)); } From 7641c245caa7fcd3d1e81b010fc1277d56383a99 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 06:09:58 -0500 Subject: [PATCH 48/62] Rename field --- .../opentelemetry/instrumentation/nocode/YamlParser.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index 9c8663cff..fb657bc36 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -22,7 +22,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.logging.Level; @@ -35,14 +34,14 @@ public final class YamlParser { // FIXME support method override selection - e.g., with classfile method signature or something public static final String NOCODE_YMLFILE_ENV_KEY = "SPLUNK_OTEL_INSTRUMENTATION_NOCODE_YML_FILE"; - private final List InstrumentationRules; + private final List instrumentationRules; public YamlParser() { - InstrumentationRules = Collections.unmodifiableList(new ArrayList<>(load())); + instrumentationRules = Collections.unmodifiableList(new ArrayList<>(load())); } public List getInstrumentationRules() { - return InstrumentationRules; + return instrumentationRules; } private static List load() { From 282fa2fb1d3409b9819813ceccede477db1c497f Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 06:14:00 -0500 Subject: [PATCH 49/62] Comment a bit of danger for any future code editors --- .../instrumentation/nocode/NocodeMethodInvocation.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java index 4f318ef9b..b18a7a4c7 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java @@ -41,6 +41,10 @@ public Object getThiz() { return thiz; } + /** + * Please be careful with this, it's directly tied to @Advice.AllArguments. + * @return + */ public Object[] getParameters() { return parameters; } From a2b789d79e751908560817969d6dd6dfaa326707 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 06:17:31 -0500 Subject: [PATCH 50/62] Clean up null checking logic --- .../instrumentation/nocode/NocodeAttributesExtractor.java | 5 +---- .../instrumentation/nocode/NocodeMethodInvocation.java | 7 +++++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java index 8953b7725..f6651887a 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java @@ -38,10 +38,7 @@ public void onStart( AttributesBuilder attributesBuilder, Context context, NocodeMethodInvocation mi) { codeExtractor.onStart(attributesBuilder, context, mi.getClassAndMethod()); - Map attributes = Collections.EMPTY_MAP; - if (mi.getRule() != null) { - attributes = mi.getRule().attributes; - } + Map attributes = mi.getRuleAttributes(); for (String key : attributes.keySet()) { String jsps = attributes.get(key); String value = JSPS.evaluate(jsps, mi.getThiz(), mi.getParameters()); diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java index b18a7a4c7..a04528e5c 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java @@ -18,6 +18,8 @@ import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; +import java.util.Collections; +import java.util.Map; public final class NocodeMethodInvocation { private final NocodeRules.Rule rule; @@ -52,4 +54,9 @@ public Object[] getParameters() { public ClassAndMethod getClassAndMethod() { return classAndMethod; } + + public Map getRuleAttributes() { + return rule == null ? Collections.EMPTY_MAP : rule.attributes; + } + } From 8468dd383c4d2ebc80c45e7cde6c442909e6eb39 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 06:28:26 -0500 Subject: [PATCH 51/62] Parameterize some of the jsps tests --- .../instrumentation/nocode/JSPSTest.java | 97 +++++++++---------- 1 file changed, 46 insertions(+), 51 deletions(-) diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index 25e15334c..75aec6a91 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -25,6 +25,8 @@ import java.util.Map; import java.util.Set; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; class JSPSTest { private static final Map thiz = new HashMap<>(); @@ -35,7 +37,6 @@ class JSPSTest { param0.add("present"); } - @Test void testBasicBehavior() { String[][] tests = new String[][] { @@ -54,36 +55,33 @@ void testBasicBehavior() { } } - @Test - void testInvalidJspsReturnNull() { - String[] invalids = - new String[] { - "nosuchvar", - "nosuchvar.toString()", - "this .", - "this . ", - "this.noSuchMethod()", - "toString()", - "this.toString()extrastuffatend", - "this.toString()toString()", - "param1.toString()", // out of bounds - "param999.toString()", - "this.getOrDefault(\"key\", \"multiparamnotsupported\")", - "this.get(\"noclosequote)", - "this.get(\"nocloseparan\"", - "this.noparens", - "this.noparens.anotherMethod()", - "this.wrongOrder)(", - "this.get(NotALiteralParameter);", - "this.get(12.2)", - "this.get(this)", - "this.get(\"NoSuchKey\")", // evals completely but returns null - "param1.toString()", // no such param - }; - for (String invalid : invalids) { - String answer = JSPS.evaluate(invalid, thiz, new Object[] {param0}); - assertNull(answer, "Expected null for \"" + invalid + "\" but was \"" + answer + "\""); - } + @ParameterizedTest + @ValueSource(strings = { + "nosuchvar", + "nosuchvar.toString()", + "this .", + "this . ", + "this.noSuchMethod()", + "toString()", + "this.toString()extrastuffatend", + "this.toString()toString()", + "param1.toString()", // out of bounds + "param999.toString()", + "this.getOrDefault(\"key\", \"multiparamnotsupported\")", + "this.get(\"noclosequote)", + "this.get(\"nocloseparan\"", + "this.noparens", + "this.noparens.anotherMethod()", + "this.wrongOrder)(", + "this.get(NotALiteralParameter);", + "this.get(12.2)", + "this.get(this)", + "this.get(\"NoSuchKey\")", // evals completely but returns null + "param1.toString()", // no such param + }) + void testInvalidJspsReturnNull(String invalid) { + String answer = JSPS.evaluate(invalid, thiz, new Object[] {param0}); + assertNull(answer, "Expected null for \"" + invalid + "\" but was \"" + answer + "\""); } @Test @@ -174,26 +172,23 @@ public void testIntegerLiteralParamTypes() { assertEquals("13", JSPS.evaluate("this.take(13)", O, new Object[0])); } - @Test - void testWhitespace() { - String[] tests = - new String[] { - "this.get(\"key\").substring(1)", - " this.get(\"key\").substring(1)", - "this .get(\"key\").substring(1)", - "this. get(\"key\").substring(1)", - "this.get (\"key\").substring(1)", - "this.get( \"key\").substring(1)", - "this.get(\"key\" ).substring(1)", - "this.get(\"key\")\t.substring(1)", - "this.get(\"key\").\nsubstring(1)", - "this.get(\"key\").substring\r(1)", - "this.get(\"key\").substring( 1)", - "this.get(\"key\").substring(1 )", - }; - for (String test : tests) { - assertEquals("alue", JSPS.evaluate(test, thiz, new Object[] {param0}), test); - } + @ParameterizedTest + @ValueSource(strings = { + "this.get(\"key\").substring(1)", + " this.get(\"key\").substring(1)", + "this .get(\"key\").substring(1)", + "this. get(\"key\").substring(1)", + "this.get (\"key\").substring(1)", + "this.get( \"key\").substring(1)", + "this.get(\"key\" ).substring(1)", + "this.get(\"key\")\t.substring(1)", + "this.get(\"key\").\nsubstring(1)", + "this.get(\"key\").substring\r(1)", + "this.get(\"key\").substring( 1)", + "this.get(\"key\").substring(1 )", + }) + void testWhitespace(String test) { + assertEquals("alue", JSPS.evaluate(test, thiz, new Object[] {param0}), test); } @Test From 2cb3adaf548e5d4558d9ebb98b4c2b2d6a962d7f Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 06:30:18 -0500 Subject: [PATCH 52/62] Most of spotlessApply --- .../bootstrap/nocode/NocodeRules.java | 4 +- .../nocode/NocodeAttributesExtractor.java | 3 +- .../nocode/NocodeInstrumentation.java | 3 +- .../nocode/NocodeMethodInvocation.java | 4 +- .../instrumentation/nocode/YamlParser.java | 14 ++-- .../instrumentation/nocode/JSPSTest.java | 76 ++++++++++--------- 6 files changed, 52 insertions(+), 52 deletions(-) diff --git a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java index 546d399d3..0193db6e4 100644 --- a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java +++ b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java @@ -16,17 +16,15 @@ package com.splunk.opentelemetry.javaagent.bootstrap.nocode; -import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicReference; public final class NocodeRules { - public final static class Rule { + public static final class Rule { public final String className; public final String methodName; public final String spanName; // may be null - use default of "class.method" diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java index f6651887a..6a901e39e 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java @@ -21,7 +21,6 @@ import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor; import io.opentelemetry.instrumentation.api.incubator.semconv.util.ClassAndMethod; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; -import java.util.Collections; import java.util.Map; import javax.annotation.Nullable; @@ -38,7 +37,7 @@ public void onStart( AttributesBuilder attributesBuilder, Context context, NocodeMethodInvocation mi) { codeExtractor.onStart(attributesBuilder, context, mi.getClassAndMethod()); - Map attributes = mi.getRuleAttributes(); + Map attributes = mi.getRuleAttributes(); for (String key : attributes.keySet()) { String jsps = attributes.get(key); String value = JSPS.evaluate(jsps, mi.getThiz(), mi.getParameters()); diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index 53c33ad69..69269460f 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -61,8 +61,7 @@ public static void onEnter( @Advice.Local("otelScope") Scope scope, @Advice.This Object thiz, @Advice.AllArguments Object[] methodParams) { - NocodeRules.Rule rule = - NocodeRules.find(declaringClass.getName(), methodName); + NocodeRules.Rule rule = NocodeRules.find(declaringClass.getName(), methodName); otelInvocation = new NocodeMethodInvocation( rule, ClassAndMethod.create(declaringClass, methodName), thiz, methodParams); diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java index a04528e5c..2956f07da 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java @@ -45,6 +45,7 @@ public Object getThiz() { /** * Please be careful with this, it's directly tied to @Advice.AllArguments. + * * @return */ public Object[] getParameters() { @@ -55,8 +56,7 @@ public ClassAndMethod getClassAndMethod() { return classAndMethod; } - public Map getRuleAttributes() { + public Map getRuleAttributes() { return rule == null ? Collections.EMPTY_MAP : rule.attributes; } - } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index fb657bc36..cb466452c 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -64,15 +64,17 @@ private static List loadUnsafe() throws Exception { Iterable parsedYaml = load.loadAllFromReader(yamlReader); List answer = new ArrayList<>(); for (Object yamlBit : parsedYaml) { - List> rulesMap = (List>) yamlBit; - for (Map yamlRule : rulesMap) { + List> rulesMap = (List>) yamlBit; + for (Map yamlRule : rulesMap) { String className = yamlRule.get("class").toString(); String methodName = yamlRule.get("method").toString(); - String spanName = yamlRule.get("spanName") == null ? null : yamlRule.get("spanName").toString(); - String spanKind = yamlRule.get("spanKind") == null ? null : yamlRule.get("spanKind").toString(); - List> attrs = (List>) yamlRule.get("attributes"); + String spanName = + yamlRule.get("spanName") == null ? null : yamlRule.get("spanName").toString(); + String spanKind = + yamlRule.get("spanKind") == null ? null : yamlRule.get("spanKind").toString(); + List> attrs = (List>) yamlRule.get("attributes"); Map ruleAttributes = new HashMap<>(); - for (Map attr : attrs) { + for (Map attr : attrs) { ruleAttributes.put(attr.get("key").toString(), attr.get("value").toString()); } answer.add(new NocodeRules.Rule(className, methodName, spanName, spanKind, ruleAttributes)); diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index 75aec6a91..76149c186 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -56,29 +56,30 @@ void testBasicBehavior() { } @ParameterizedTest - @ValueSource(strings = { - "nosuchvar", - "nosuchvar.toString()", - "this .", - "this . ", - "this.noSuchMethod()", - "toString()", - "this.toString()extrastuffatend", - "this.toString()toString()", - "param1.toString()", // out of bounds - "param999.toString()", - "this.getOrDefault(\"key\", \"multiparamnotsupported\")", - "this.get(\"noclosequote)", - "this.get(\"nocloseparan\"", - "this.noparens", - "this.noparens.anotherMethod()", - "this.wrongOrder)(", - "this.get(NotALiteralParameter);", - "this.get(12.2)", - "this.get(this)", - "this.get(\"NoSuchKey\")", // evals completely but returns null - "param1.toString()", // no such param - }) + @ValueSource( + strings = { + "nosuchvar", + "nosuchvar.toString()", + "this .", + "this . ", + "this.noSuchMethod()", + "toString()", + "this.toString()extrastuffatend", + "this.toString()toString()", + "param1.toString()", // out of bounds + "param999.toString()", + "this.getOrDefault(\"key\", \"multiparamnotsupported\")", + "this.get(\"noclosequote)", + "this.get(\"nocloseparan\"", + "this.noparens", + "this.noparens.anotherMethod()", + "this.wrongOrder)(", + "this.get(NotALiteralParameter);", + "this.get(12.2)", + "this.get(this)", + "this.get(\"NoSuchKey\")", // evals completely but returns null + "param1.toString()", // no such param + }) void testInvalidJspsReturnNull(String invalid) { String answer = JSPS.evaluate(invalid, thiz, new Object[] {param0}); assertNull(answer, "Expected null for \"" + invalid + "\" but was \"" + answer + "\""); @@ -173,20 +174,21 @@ public void testIntegerLiteralParamTypes() { } @ParameterizedTest - @ValueSource(strings = { - "this.get(\"key\").substring(1)", - " this.get(\"key\").substring(1)", - "this .get(\"key\").substring(1)", - "this. get(\"key\").substring(1)", - "this.get (\"key\").substring(1)", - "this.get( \"key\").substring(1)", - "this.get(\"key\" ).substring(1)", - "this.get(\"key\")\t.substring(1)", - "this.get(\"key\").\nsubstring(1)", - "this.get(\"key\").substring\r(1)", - "this.get(\"key\").substring( 1)", - "this.get(\"key\").substring(1 )", - }) + @ValueSource( + strings = { + "this.get(\"key\").substring(1)", + " this.get(\"key\").substring(1)", + "this .get(\"key\").substring(1)", + "this. get(\"key\").substring(1)", + "this.get (\"key\").substring(1)", + "this.get( \"key\").substring(1)", + "this.get(\"key\" ).substring(1)", + "this.get(\"key\")\t.substring(1)", + "this.get(\"key\").\nsubstring(1)", + "this.get(\"key\").substring\r(1)", + "this.get(\"key\").substring( 1)", + "this.get(\"key\").substring(1 )", + }) void testWhitespace(String test) { assertEquals("alue", JSPS.evaluate(test, thiz, new Object[] {param0}), test); } From 2dc267515621712394329bceff2abfab3ce06575 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 06:34:52 -0500 Subject: [PATCH 53/62] Clean up javadoc style/use html rather than markdown for JSPS comment --- .../instrumentation/nocode/JSPS.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java index eb78da0eb..9a992f646 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java @@ -24,14 +24,20 @@ * essentially a single call in Java (as though it ends with a semicolon), with * some limitations. Its purpose is to allow pieces of nocode instrumentation * (attributes, span name) to be derived from the instrumentated context. + *

* As some illustrative examples: - * this.getHeaders().get("X-Custom-Header").substring(5) - * param0.getDetails().getCustomerAccount().getAccountType() + * 

+ *   this.getHeaders().get("X-Custom-Header").substring(5)
+ *   param0.getDetails().getCustomerAccount().getAccountType()
+ *
+ *

* The limitations are: - * - no access to variables other than 'this' and 'paramN' (N indexed at 0) - * - no control flow (if), no local variables, basically nothing other than a single chain of method calls - * - Method calls are limited to either 0 or 1 parameters currently - * - Parameters must be literals and only integral (int/long), string, and boolean literals are currently supported + *

    + *
  • no access to variables other than 'this' and 'paramN' (N indexed at 0)
    • + *
  • no control flow (if), no local variables, basically nothing other than a single chain of method calls
    • + *
  • Method calls are limited to either 0 or 1 parameters currently
    • + *
  • Parameters must be literals and only integral (int/long), string, and boolean literals are currently supported
    • + *
*/ public final class JSPS { private static final Logger logger = Logger.getLogger(JSPS.class.getName()); From 7f8569b121adb2dee72f0fc9acc7c475e1070abb Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 06:35:13 -0500 Subject: [PATCH 54/62] spotlessApply --- .../instrumentation/nocode/JSPS.java | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java index 9a992f646..c8d132705 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JSPS.java @@ -19,24 +19,28 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.logging.Logger; + /** - * JSPS stands for Java-like String-Producing Statement. A JSPS is - * essentially a single call in Java (as though it ends with a semicolon), with - * some limitations. Its purpose is to allow pieces of nocode instrumentation - * (attributes, span name) to be derived from the instrumentated context. - *

- * As some illustrative examples: + * JSPS stands for Java-like String-Producing Statement. A JSPS is essentially a single call in Java + * (as though it ends with a semicolon), with some limitations. Its purpose is to allow pieces of + * nocode instrumentation (attributes, span name) to be derived from the instrumentated context. + * + *

As some illustrative examples: + * * 

  *   this.getHeaders().get("X-Custom-Header").substring(5)
  *   param0.getDetails().getCustomerAccount().getAccountType()
  *
- *

- * The limitations are: + * + *

The limitations are: + * *

    - *
  • no access to variables other than 'this' and 'paramN' (N indexed at 0)
    • - *
  • no control flow (if), no local variables, basically nothing other than a single chain of method calls
    • - *
  • Method calls are limited to either 0 or 1 parameters currently
    • - *
  • Parameters must be literals and only integral (int/long), string, and boolean literals are currently supported
    • + *
  • no access to variables other than 'this' and 'paramN' (N indexed at 0) + *
  • no control flow (if), no local variables, basically nothing other than a single chain of + * method calls + *
  • Method calls are limited to either 0 or 1 parameters currently + *
  • Parameters must be literals and only integral (int/long), string, and boolean literals are + * currently supported *
*/ public final class JSPS { @@ -131,7 +135,8 @@ private static Method findMatchingMethod( } // Returns null for none found - private static Method findMatchingMethod(String methodName, Class clazz, Class[] actualParamTypes) { + private static Method findMatchingMethod( + String methodName, Class clazz, Class[] actualParamTypes) { if (clazz == null) { return null; } From 014158ee92333ca42eb36f27b39a52656c968013 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 06:42:12 -0500 Subject: [PATCH 55/62] Parameterize basic jsps behavior tests --- .../instrumentation/nocode/JSPSTest.java | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java index 76149c186..78f076f06 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/JSPSTest.java @@ -18,14 +18,18 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.params.provider.Arguments.arguments; import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; class JSPSTest { @@ -37,22 +41,23 @@ class JSPSTest { param0.add("present"); } - void testBasicBehavior() { - String[][] tests = - new String[][] { - // Might be nice to support a bare "param0" or "this" but as a workaround can always use - // "this.toString()" - {"this.toString()", "{key=value}"}, - {"this.toString().length()", "11"}, - {"this.get(\"key\")", "value"}, - {"this.get(\"key\").substring(1)", "alue"}, - {"param0.isEmpty()", "false"}, - {"param0.contains(\"present\")", "true"}, - {"this.entrySet().size()", "1"}, - }; - for (String[] test : tests) { - assertEquals(test[1], JSPS.evaluate(test[0], thiz, new Object[] {param0}), test[0]); - } + static Stream jspsToExpected() { + return Stream.of( + // Might be nice to support a bare "param0" or "this" but as a workaround can always use + // "this.toString()" + arguments("this.toString()", "{key=value}"), + arguments("this.toString().length()", "11"), + arguments("this.get(\"key\")", "value"), + arguments("this.get(\"key\").substring(1)", "alue"), + arguments("param0.isEmpty()", "false"), + arguments("param0.contains(\"present\")", "true"), + arguments("this.entrySet().size()", "1")); + } + + @ParameterizedTest + @MethodSource("jspsToExpected") + void testBasicBehavior(String jsps, String expected) { + assertEquals(expected, JSPS.evaluate(jsps, thiz, new Object[] {param0}), jsps); } @ParameterizedTest From f22f7bb91c9fe47cf78ace6cbf57f8af3a13f641 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 09:58:25 -0500 Subject: [PATCH 56/62] Feedback nits --- .../instrumentation/nocode/NocodeInstrumentation.java | 5 +++-- .../instrumentation/nocode/NocodeMethodInvocation.java | 2 +- .../instrumentation/nocode/NocodeSpanKindExtractor.java | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index 69269460f..4b6dec061 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -85,8 +85,9 @@ public static void stopSpan( return; } scope.close(); - // I wonder why the original methods instrumentation had support for modifying the return - // value? + // This is heavily based on the "methods" instrumentation from upstream, but + // for now we're not supporting modifying return types/async result codes, etc. + // This could be expanded in the future. instrumentor().end(context, otelInvocation, null, error); } } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java index 2956f07da..eea5e6e06 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java @@ -57,6 +57,6 @@ public ClassAndMethod getClassAndMethod() { } public Map getRuleAttributes() { - return rule == null ? Collections.EMPTY_MAP : rule.attributes; + return rule == null ? Collections.emptyMap(): rule.attributes; } } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java index 80b639c34..ee25915ba 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanKindExtractor.java @@ -18,6 +18,7 @@ import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor; +import java.util.Locale; public class NocodeSpanKindExtractor implements SpanKindExtractor { @Override @@ -26,7 +27,7 @@ public SpanKind extract(NocodeMethodInvocation mi) { return SpanKind.INTERNAL; } try { - return SpanKind.valueOf(mi.getRule().spanKind.toUpperCase()); + return SpanKind.valueOf(mi.getRule().spanKind.toUpperCase(Locale.ROOT)); } catch (IllegalArgumentException noMatchingValue) { return SpanKind.INTERNAL; } From ea9082aceee9c04d6b50d889f90ce60b78855420 Mon Sep 17 00:00:00 2001 From: John Bley Date: Fri, 14 Feb 2025 10:14:29 -0500 Subject: [PATCH 57/62] spotlessApply --- .../instrumentation/nocode/NocodeMethodInvocation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java index eea5e6e06..32be9aad1 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeMethodInvocation.java @@ -57,6 +57,6 @@ public ClassAndMethod getClassAndMethod() { } public Map getRuleAttributes() { - return rule == null ? Collections.emptyMap(): rule.attributes; + return rule == null ? Collections.emptyMap() : rule.attributes; } } From 58c71e20491e95b5f5f9e3a8a32d38c1d092a98f Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Mon, 17 Feb 2025 15:28:12 +0200 Subject: [PATCH 58/62] Let muzzle plugin find helper classes --- instrumentation/nocode/build.gradle.kts | 4 ++ .../nocode/NocodeInitializer.java | 36 ++++++++++ .../nocode/NocodeInstrumentation.java | 14 ++-- .../instrumentation/nocode/NocodeModule.java | 18 ++--- .../nocode/NocodeSingletons.java | 6 +- .../instrumentation/nocode/YamlParser.java | 66 ++++++++++--------- .../nocode/NocodeInstrumentationTest.java | 7 +- 7 files changed, 95 insertions(+), 56 deletions(-) create mode 100644 instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInitializer.java diff --git a/instrumentation/nocode/build.gradle.kts b/instrumentation/nocode/build.gradle.kts index 4ed8b9944..61b83c985 100644 --- a/instrumentation/nocode/build.gradle.kts +++ b/instrumentation/nocode/build.gradle.kts @@ -1,10 +1,14 @@ plugins { id("splunk.instrumentation-conventions") + id("splunk.muzzle-conventions") } dependencies { + compileOnly(project(":custom")) compileOnly("io.opentelemetry.javaagent:opentelemetry-javaagent-tooling") compileOnly("org.snakeyaml:snakeyaml-engine:2.8") + + add("codegen", project(":bootstrap")) } tasks.withType().configureEach { diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInitializer.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInitializer.java new file mode 100644 index 000000000..9eda6599e --- /dev/null +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInitializer.java @@ -0,0 +1,36 @@ +/* + * Copyright Splunk Inc. + * + * Licensed 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 com.splunk.opentelemetry.instrumentation.nocode; + +import static io.opentelemetry.sdk.autoconfigure.AutoConfigureUtil.getConfig; + +import com.google.auto.service.AutoService; +import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; +import io.opentelemetry.javaagent.tooling.BeforeAgentListener; +import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; +import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; + +@AutoService(BeforeAgentListener.class) +public class NocodeInitializer implements BeforeAgentListener { + + @Override + public void beforeAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk) { + ConfigProperties config = getConfig(autoConfiguredOpenTelemetrySdk); + YamlParser yp = new YamlParser(config); + NocodeRules.setGlobalRules(yp.getInstrumentationRules()); + } +} diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index 4b6dec061..50881c2d7 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -16,10 +16,11 @@ package com.splunk.opentelemetry.instrumentation.nocode; -import static com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons.instrumentor; +import static com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons.instrumenter; import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.none; import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; import io.opentelemetry.context.Context; @@ -41,13 +42,14 @@ public NocodeInstrumentation(NocodeRules.Rule rule) { @Override public ElementMatcher typeMatcher() { - return hasSuperType(named(rule.className)); + return rule != null ? hasSuperType(named(rule.className)) : none(); } @Override public void transform(TypeTransformer transformer) { transformer.applyAdviceToMethod( - named(rule.methodName), this.getClass().getName() + "$NocodeAdvice"); + rule != null ? named(rule.methodName) : none(), + this.getClass().getName() + "$NocodeAdvice"); } @SuppressWarnings("unused") @@ -67,10 +69,10 @@ public static void onEnter( rule, ClassAndMethod.create(declaringClass, methodName), thiz, methodParams); Context parentContext = currentContext(); - if (!instrumentor().shouldStart(parentContext, otelInvocation)) { + if (!instrumenter().shouldStart(parentContext, otelInvocation)) { return; } - context = instrumentor().start(parentContext, otelInvocation); + context = instrumenter().start(parentContext, otelInvocation); scope = context.makeCurrent(); } @@ -88,7 +90,7 @@ public static void stopSpan( // This is heavily based on the "methods" instrumentation from upstream, but // for now we're not supporting modifying return types/async result codes, etc. // This could be expanded in the future. - instrumentor().end(context, otelInvocation, null, error); + instrumenter().end(context, otelInvocation, null, error); } } } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java index f58723882..b987d45ea 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java @@ -21,7 +21,6 @@ import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; @AutoService(InstrumentationModule.class) @@ -29,8 +28,6 @@ public final class NocodeModule extends InstrumentationModule { public NocodeModule() { super("nocode"); - YamlParser yp = new YamlParser(); - NocodeRules.setGlobalRules(yp.getInstrumentationRules()); } @Override @@ -39,18 +36,17 @@ public List typeInstrumentations() { for (NocodeRules.Rule rule : NocodeRules.getGlobalRules()) { answer.add(new NocodeInstrumentation(rule)); } + // ensure that there is at least one instrumentation so that muzzle reference collection could + // work + if (answer.isEmpty()) { + answer.add(new NocodeInstrumentation(null)); + } return answer; } @Override - public List getAdditionalHelperClassNames() { - return Arrays.asList( - "com.splunk.opentelemetry.instrumentation.nocode.JSPS", - "com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons", - "com.splunk.opentelemetry.instrumentation.nocode.NocodeAttributesExtractor", - "com.splunk.opentelemetry.instrumentation.nocode.NocodeMethodInvocation", - "com.splunk.opentelemetry.instrumentation.nocode.NocodeSpanKindExtractor", - "com.splunk.opentelemetry.instrumentation.nocode.NocodeSpanNameExtractor"); + public boolean isHelperClass(String className) { + return className.startsWith("com.splunk.opentelemetry.instrumentation"); } @Override diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java index 960da0a25..c6ef04a1c 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java @@ -25,14 +25,12 @@ public class NocodeSingletons { static { INSTRUMENTER = Instrumenter.builder( - GlobalOpenTelemetry.get(), - "com.splunk.opentelemetry.instrumentation.nocode", - new NocodeSpanNameExtractor()) + GlobalOpenTelemetry.get(), "com.splunk.nocode", new NocodeSpanNameExtractor()) .addAttributesExtractor(new NocodeAttributesExtractor()) .buildInstrumenter(new NocodeSpanKindExtractor()); } - public static Instrumenter instrumentor() { + public static Instrumenter instrumenter() { return INSTRUMENTER; } } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java index cb466452c..9f2ccd93d 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java @@ -17,8 +17,10 @@ package com.splunk.opentelemetry.instrumentation.nocode; import com.splunk.opentelemetry.javaagent.bootstrap.nocode.NocodeRules; -import java.io.FileReader; -import java.io.Reader; +import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -32,55 +34,57 @@ public final class YamlParser { private static final Logger logger = Logger.getLogger(YamlParser.class.getName()); // FIXME support method override selection - e.g., with classfile method signature or something - public static final String NOCODE_YMLFILE_ENV_KEY = "SPLUNK_OTEL_INSTRUMENTATION_NOCODE_YML_FILE"; + private static final String NOCODE_YMLFILE = "splunk.otel.instrumentation.nocode.yml.file"; private final List instrumentationRules; - public YamlParser() { - instrumentationRules = Collections.unmodifiableList(new ArrayList<>(load())); + public YamlParser(ConfigProperties config) { + instrumentationRules = Collections.unmodifiableList(new ArrayList<>(load(config))); } public List getInstrumentationRules() { return instrumentationRules; } - private static List load() { + private static List load(ConfigProperties config) { + String yamlFileName = config.getString(NOCODE_YMLFILE); + if (yamlFileName == null || yamlFileName.trim().isEmpty()) { + return Collections.emptyList(); + } + try { - return loadUnsafe(); + return loadUnsafe(yamlFileName); } catch (Exception e) { logger.log(Level.SEVERE, "Can't load configured nocode yaml.", e); return Collections.emptyList(); } } - private static List loadUnsafe() throws Exception { - String yamlFileName = System.getenv(NOCODE_YMLFILE_ENV_KEY); - if (yamlFileName == null || yamlFileName.trim().isEmpty()) { - return Collections.emptyList(); - } - Reader yamlReader = new FileReader(yamlFileName.trim()); - - Load load = new Load(LoadSettings.builder().build()); - Iterable parsedYaml = load.loadAllFromReader(yamlReader); + private static List loadUnsafe(String yamlFileName) throws Exception { List answer = new ArrayList<>(); - for (Object yamlBit : parsedYaml) { - List> rulesMap = (List>) yamlBit; - for (Map yamlRule : rulesMap) { - String className = yamlRule.get("class").toString(); - String methodName = yamlRule.get("method").toString(); - String spanName = - yamlRule.get("spanName") == null ? null : yamlRule.get("spanName").toString(); - String spanKind = - yamlRule.get("spanKind") == null ? null : yamlRule.get("spanKind").toString(); - List> attrs = (List>) yamlRule.get("attributes"); - Map ruleAttributes = new HashMap<>(); - for (Map attr : attrs) { - ruleAttributes.put(attr.get("key").toString(), attr.get("value").toString()); + try (InputStream inputStream = Files.newInputStream(Paths.get(yamlFileName.trim()))) { + Load load = new Load(LoadSettings.builder().build()); + Iterable parsedYaml = load.loadAllFromInputStream(inputStream); + for (Object yamlBit : parsedYaml) { + List> rulesMap = (List>) yamlBit; + for (Map yamlRule : rulesMap) { + String className = yamlRule.get("class").toString(); + String methodName = yamlRule.get("method").toString(); + String spanName = + yamlRule.get("spanName") == null ? null : yamlRule.get("spanName").toString(); + String spanKind = + yamlRule.get("spanKind") == null ? null : yamlRule.get("spanKind").toString(); + List> attrs = (List>) yamlRule.get("attributes"); + Map ruleAttributes = new HashMap<>(); + for (Map attr : attrs) { + ruleAttributes.put(attr.get("key").toString(), attr.get("value").toString()); + } + answer.add( + new NocodeRules.Rule(className, methodName, spanName, spanKind, ruleAttributes)); } - answer.add(new NocodeRules.Rule(className, methodName, spanName, spanKind, ruleAttributes)); } } - yamlReader.close(); + return answer; } } diff --git a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java index 945e9b8ab..a938aca40 100644 --- a/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java +++ b/instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java @@ -35,8 +35,7 @@ class NocodeInstrumentationTest { @Test void testBasicMethod() { new SampleClass().doSomething(); - // FIXME what is scope version verification and why doesn't it pass? - testing.waitAndAssertTracesWithoutScopeVersionVerification( + testing.waitAndAssertTraces( trace -> trace.hasSpansSatisfyingExactly( span -> @@ -50,7 +49,7 @@ void testBasicMethod() { @Test void testRuleWithManyInvalidFields() { new SampleClass().doInvalidRule(); - testing.waitAndAssertTracesWithoutScopeVersionVerification( + testing.waitAndAssertTraces( trace -> trace.hasSpansSatisfyingExactly( span -> @@ -68,7 +67,7 @@ void testThrowException() { // nop } - testing.waitAndAssertTracesWithoutScopeVersionVerification( + testing.waitAndAssertTraces( trace -> trace.hasSpansSatisfyingExactly( span -> From 43e5449bcc172649e42c16c5df19bd4605188ec9 Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 17 Feb 2025 09:35:39 -0500 Subject: [PATCH 59/62] Logic and naming fixups from PR feedback --- .../javaagent/bootstrap/nocode/NocodeRules.java | 10 +++++----- .../instrumentation/nocode/NocodeInstrumentation.java | 6 +++++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java index 0193db6e4..7e27a2c0f 100644 --- a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java +++ b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java @@ -20,7 +20,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; public final class NocodeRules { @@ -50,19 +49,20 @@ public String toString() { } // Using className.methodName as the key - private static final ConcurrentHashMap Name2Rule = new ConcurrentHashMap<>(); + private static final HashMap name2Rule = new HashMap<>(); + // Called by the NocodeInitializer public static void setGlobalRules(List rules) { for (Rule r : rules) { - Name2Rule.put(r.className + "." + r.methodName, r); + name2Rule.put(r.className + "." + r.methodName, r); } } public static Iterable getGlobalRules() { - return Name2Rule.values(); + return name2Rule.values(); } public static Rule find(String className, String methodName) { - return Name2Rule.get(className + "." + methodName); + return name2Rule.get(className + "." + methodName); } } diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index 50881c2d7..c887d92a1 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -42,7 +42,11 @@ public NocodeInstrumentation(NocodeRules.Rule rule) { @Override public ElementMatcher typeMatcher() { - return rule != null ? hasSuperType(named(rule.className)) : none(); + // names have to match exactly for now to enable rule lookup + // at advice time. In the future, we could support + // more complex rules here if we dynamically generated advice classes for + // each rule. + return rule != null ? named(rule.className) : none(); } @Override From 957fb3b7ebb9553a9eabae88778a5c5bf41f4d66 Mon Sep 17 00:00:00 2001 From: John Bley Date: Mon, 17 Feb 2025 09:40:44 -0500 Subject: [PATCH 60/62] spotlessApply --- .../instrumentation/nocode/NocodeInstrumentation.java | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java index c887d92a1..990524259 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java @@ -18,7 +18,6 @@ import static com.splunk.opentelemetry.instrumentation.nocode.NocodeSingletons.instrumenter; import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; -import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.none; From 575b0b87650987e3efbbbe03f0d7556c166495d8 Mon Sep 17 00:00:00 2001 From: John Bley Date: Tue, 18 Feb 2025 08:14:04 -0500 Subject: [PATCH 61/62] Add comment about instrumentation order --- .../opentelemetry/instrumentation/nocode/NocodeModule.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java index b987d45ea..2f44e5a9f 100644 --- a/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java +++ b/instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeModule.java @@ -49,6 +49,10 @@ public boolean isHelperClass(String className) { return className.startsWith("com.splunk.opentelemetry.instrumentation"); } + // If nocode instrumentation is added to something with existing auto-instrumentation, + // it would generally be better to run the nocode bits after the "regular" bits. + // E.g., if we want to add nocode to a servlet call, then we want to make sure that + // the otel-standard servlet instrumentation runs first to handle context propagation, etc. @Override public int order() { return Integer.MAX_VALUE; From 4857123c042fa97c8b4ef303aa73543b1bc324d7 Mon Sep 17 00:00:00 2001 From: John Bley Date: Tue, 18 Feb 2025 12:37:07 -0500 Subject: [PATCH 62/62] Non-initialized singleton, better toString --- .../javaagent/bootstrap/nocode/NocodeRules.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java index 7e27a2c0f..2a74ce1cc 100644 --- a/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java +++ b/bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java @@ -44,10 +44,19 @@ public Rule( } public String toString() { - return className + "." + methodName + ":spanName=" + spanName + ",attrs=" + attributes; + return "Nocode rule: " + + className + + "." + + methodName + + ":spanName=" + + spanName + + ",attrs=" + + attributes; } } + private NocodeRules() {} + // Using className.methodName as the key private static final HashMap name2Rule = new HashMap<>();