Add ability to set span status through nocode expressions (#2231)

* Initial work on setting span status with returnValue/error variables

* Add some barebones documentation

* spotlessApply (runs on .md too, huh)

* Tweak example in docs to be a tiny bit more realistic

* Log an edge case and document that this is not stable yet

* spotlessApply

Author: johnbley

bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeEvaluation.java

@@ -22,6 +22,9 @@ public class NocodeEvaluation {
 
   public interface Evaluator {
     Object evaluate(String expression, Object thiz, Object[] params);
+
+    Object evaluateAtEnd(
+        String expression, Object thiz, Object[] params, Object returnValue, Throwable error);
   }
 
   private static final AtomicReference<Evaluator> globalEvaluator = new AtomicReference<>();
@@ -35,5 +38,11 @@ public static Object evaluate(String expression, Object thiz, Object[] params) {
     return e == null ? null : e.evaluate(expression, thiz, params);
   }
 
+  public static Object evaluateAtEnd(
+      String expression, Object thiz, Object[] params, Object returnValue, Throwable error) {
+    Evaluator e = globalEvaluator.get();
+    return e == null ? null : e.evaluateAtEnd(expression, thiz, params, returnValue, error);
+  }
+
   private NocodeEvaluation() {}
 }

bootstrap/src/main/java/com/splunk/opentelemetry/javaagent/bootstrap/nocode/NocodeRules.java

@@ -28,18 +28,22 @@ public static final class Rule {
     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 String spanStatus; // may be null, should return string from StatusCodes
+
     public final Map<String, String> attributes; // key name to jexl expression
 
     public Rule(
         String className,
         String methodName,
         String spanName,
         String spanKind,
+        String spanStatus,
         Map<String, String> attributes) {
       this.className = className;
       this.methodName = methodName;
       this.spanName = spanName;
       this.spanKind = spanKind;
+      this.spanStatus = spanStatus;
       this.attributes = Collections.unmodifiableMap(new HashMap<>(attributes));
     }
 
@@ -50,6 +54,10 @@ public String toString() {
           + methodName
           + ":spanName="
           + spanName
+          + ":spanKind="
+          + spanKind
+          + ":spanStatus="
+          + spanStatus
           + ",attrs="
           + attributes;
     }

instrumentation/nocode/README.md

@@ -0,0 +1,32 @@
+# "nocode" instrumentation
+
+Stability: under active development; breaking changes can occur
+
+Please don't use this if you have the ability to edit the code being instrumented.
+
+Set `SPLUNK_OTEL_INSTRUMENTATION_NOCODE_YML_FILE=/path/to/some.yml`
+
+Where the yml looks like
+```
+- class: foo.Foo
+  method: foo
+  spanName: this.getName()
+  attributes:
+    - key: "business.context"
+      value: this.getDetails().get("context")
+
+- class: foo.Foo
+  method: doStuff
+  spanKind: CLIENT
+  spanStatus: 'returnValue.code() > 3 ? "OK" : "ERROR"`
+  attributes:
+    - key: "special.header"
+      value: 'param0.headers().get("special-header").substring(5)"'
+```
+
+Expressions are written in [JEXL](https://commons.apache.org/proper/commons-jexl/reference/syntax.html) and may use
+the following variables:
+  - `this` - which may be null for a static method
+  - `param0` through `paramN` where 0 indexes the first parameter to the method
+  - `returnValue` which is only defined for `spanStatus` and may be null (if an exception is thrown or the method returns void)
+  - `error` which is only defined for `spanStatus` and is the `Throwable` thrown by the method invocation (or null if a normal return)

instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/JexlEvaluator.java

@@ -52,13 +52,14 @@ public JexlEvaluator() {
             .create();
   }
 
-  @Override
-  public Object evaluate(String expression, Object thiz, Object[] params) {
-    JexlContext context = new MapContext();
+  private static void setBeginningVariables(JexlContext context, Object thiz, Object[] params) {
     context.set("this", thiz);
     for (int i = 0; i < params.length; i++) {
       context.set("param" + i, params[i]);
     }
+  }
+
+  private Object evaluateExpression(String expression, JexlContext context) {
     try {
       // could cache the Expression in the Rule if desired
       return jexl.createExpression(expression).evaluate(context);
@@ -67,4 +68,25 @@ public Object evaluate(String expression, Object thiz, Object[] params) {
       return null;
     }
   }
+
+  private void setEndingVariables(JexlContext context, Object returnValue, Throwable error) {
+    context.set("returnValue", returnValue);
+    context.set("error", error);
+  }
+
+  @Override
+  public Object evaluate(String expression, Object thiz, Object[] params) {
+    JexlContext context = new MapContext();
+    setBeginningVariables(context, thiz, params);
+    return evaluateExpression(expression, context);
+  }
+
+  @Override
+  public Object evaluateAtEnd(
+      String expression, Object thiz, Object[] params, Object returnValue, Throwable error) {
+    JexlContext context = new MapContext();
+    setBeginningVariables(context, thiz, params);
+    setEndingVariables(context, returnValue, error);
+    return evaluateExpression(expression, context);
+  }
 }

instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeAttributesExtractor.java

@@ -26,8 +26,8 @@
 import javax.annotation.Nullable;
 
 public final class NocodeAttributesExtractor
-    implements AttributesExtractor<NocodeMethodInvocation, Void> {
-  private final AttributesExtractor<ClassAndMethod, Void> codeExtractor;
+    implements AttributesExtractor<NocodeMethodInvocation, Object> {
+  private final AttributesExtractor<ClassAndMethod, Object> codeExtractor;
 
   public NocodeAttributesExtractor() {
     codeExtractor = CodeAttributesExtractor.create(ClassAndMethod.codeAttributesGetter());
@@ -62,7 +62,7 @@ public void onEnd(
       AttributesBuilder attributesBuilder,
       Context context,
       NocodeMethodInvocation nocodeMethodInvocation,
-      @Nullable Void unused,
+      @Nullable Object unused,
       @Nullable Throwable throwable) {
     codeExtractor.onEnd(
         attributesBuilder, context, nocodeMethodInvocation.getClassAndMethod(), unused, throwable);

instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentation.java

@@ -30,6 +30,7 @@
 import java.lang.reflect.Method;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.implementation.bytecode.assign.Assigner;
 import net.bytebuddy.matcher.ElementMatcher;
 
 public final class NocodeInstrumentation implements TypeInstrumentation {
@@ -85,6 +86,7 @@ public static void stopSpan(
         @Advice.Local("otelInvocation") NocodeMethodInvocation otelInvocation,
         @Advice.Local("otelContext") Context context,
         @Advice.Local("otelScope") Scope scope,
+        @Advice.Return(typing = Assigner.Typing.DYNAMIC) Object returnValue,
         @Advice.Thrown Throwable error) {
       if (scope == null) {
         return;
@@ -93,7 +95,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.
-      instrumenter().end(context, otelInvocation, null, error);
+      instrumenter().end(context, otelInvocation, returnValue, error);
     }
   }
 }

instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSingletons.java

@@ -20,17 +20,18 @@
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 
 public class NocodeSingletons {
-  private static final Instrumenter<NocodeMethodInvocation, Void> INSTRUMENTER;
+  private static final Instrumenter<NocodeMethodInvocation, Object> INSTRUMENTER;
 
   static {
     INSTRUMENTER =
-        Instrumenter.<NocodeMethodInvocation, Void>builder(
+        Instrumenter.<NocodeMethodInvocation, Object>builder(
                 GlobalOpenTelemetry.get(), "com.splunk.nocode", new NocodeSpanNameExtractor())
             .addAttributesExtractor(new NocodeAttributesExtractor())
+            .setSpanStatusExtractor(new NocodeSpanStatusExtractor())
             .buildInstrumenter(new NocodeSpanKindExtractor());
   }
 
-  public static Instrumenter<NocodeMethodInvocation, Void> instrumenter() {
+  public static Instrumenter<NocodeMethodInvocation, Object> instrumenter() {
     return INSTRUMENTER;
   }
 }

instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeSpanStatusExtractor.java

@@ -0,0 +1,60 @@
+/*
+ * 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.NocodeEvaluation;
+import io.opentelemetry.api.trace.StatusCode;
+import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusBuilder;
+import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusExtractor;
+import java.util.Locale;
+import java.util.logging.Logger;
+import javax.annotation.Nullable;
+
+public class NocodeSpanStatusExtractor
+    implements SpanStatusExtractor<NocodeMethodInvocation, Object> {
+  private static final Logger logger = Logger.getLogger(NocodeSpanStatusExtractor.class.getName());
+
+  @Override
+  public void extract(
+      SpanStatusBuilder spanStatusBuilder,
+      NocodeMethodInvocation mi,
+      @Nullable Object returnValue,
+      @Nullable Throwable error) {
+
+    if (mi.getRule() == null || mi.getRule().spanStatus == null) {
+
+      // FIXME would love to use a DefaultSpanStatusExtractor as a fallback but it is not public
+      // so here is a copy of its (admittedly simple) guts
+      if (error != null) {
+        spanStatusBuilder.setStatus(StatusCode.ERROR);
+      }
+      return;
+    }
+    Object status =
+        NocodeEvaluation.evaluateAtEnd(
+            mi.getRule().spanStatus, mi.getThiz(), mi.getParameters(), returnValue, error);
+    if (status != null) {
+      try {
+        StatusCode code = StatusCode.valueOf(status.toString().toUpperCase(Locale.ROOT));
+        spanStatusBuilder.setStatus(code);
+      } catch (IllegalArgumentException noMatchingValue) {
+        // nop, should remain UNSET
+        logger.fine("Invalid span status ignored: " + status);
+      }
+    }
+  }
+}

instrumentation/nocode/src/main/java/com/splunk/opentelemetry/instrumentation/nocode/YamlParser.java

@@ -75,13 +75,19 @@ private static List<NocodeRules.Rule> loadUnsafe(String yamlFileName) throws Exc
               yamlRule.get("spanName") == null ? null : yamlRule.get("spanName").toString();
           String spanKind =
               yamlRule.get("spanKind") == null ? null : yamlRule.get("spanKind").toString();
-          List<Map<String, Object>> attrs = (List<Map<String, Object>>) yamlRule.get("attributes");
+          String spanStatus =
+              yamlRule.get("spanStatus") == null ? null : yamlRule.get("spanStatus").toString();
+
           Map<String, String> ruleAttributes = new HashMap<>();
-          for (Map<String, Object> attr : attrs) {
-            ruleAttributes.put(attr.get("key").toString(), attr.get("value").toString());
+          List<Map<String, Object>> attrs = (List<Map<String, Object>>) yamlRule.get("attributes");
+          if (attrs != null) {
+            for (Map<String, Object> attr : attrs) {
+              ruleAttributes.put(attr.get("key").toString(), attr.get("value").toString());
+            }
           }
           answer.add(
-              new NocodeRules.Rule(className, methodName, spanName, spanKind, ruleAttributes));
+              new NocodeRules.Rule(
+                  className, methodName, spanName, spanKind, spanStatus, ruleAttributes));
         }
       }
     }

instrumentation/nocode/src/test/config/nocode.yml

@@ -26,10 +26,16 @@
     - key: "five"
       value: param0.toString().substring(0)
 
+- class: com.splunk.opentelemetry.instrumentation.nocode.NocodeInstrumentationTest$SampleClass
+  method: echo
+  spanStatus: 'returnValue.booleanValue() ? "ERROR" : "OK"'
+
+
 - class: com.splunk.opentelemetry.instrumentation.nocode.NocodeInstrumentationTest$SampleClass
   method: doInvalidRule
   spanName: "this.thereIsNoSuchMethod()"
   spanKind: INVALID
+  spanStatus: invalid jexl that does not parse
   attributes:
     - key: "notpresent"
       value: "invalid.noSuchStatement()"

instrumentation/nocode/src/test/java/com/splunk/opentelemetry/instrumentation/nocode/NocodeInstrumentationTest.java

@@ -88,6 +88,27 @@ void testThrowException() {
                         .hasAttributesSatisfying(equalTo(AttributeKey.stringKey("five"), "5"))));
   }
 
+  @Test
+  void testEchoTrueIsError() {
+    new SampleClass().echo(true);
+    testing.waitAndAssertTraces(
+        trace ->
+            trace.hasSpansSatisfyingExactly(
+                span ->
+                    span.hasName("SampleClass.echo")
+                        .hasStatusSatisfying(StatusDataAssert::isError)));
+  }
+
+  @Test
+  void testEchoFalseIsOK() {
+    new SampleClass().echo(false);
+    testing.waitAndAssertTraces(
+        trace ->
+            trace.hasSpansSatisfyingExactly(
+                span ->
+                    span.hasName("SampleClass.echo").hasStatusSatisfying(StatusDataAssert::isOk)));
+  }
+
   public static class SampleClass {
     public String getName() {
       return "name";
@@ -131,5 +152,9 @@ public void throwException(int parameter) {
     public void doSomething() {}
 
     public void doInvalidRule() {}
+
+    public boolean echo(boolean b) {
+      return b;
+    }
   }
 }