Update Armeria instrumentation to match latest decorator order and re… (#2021)

* Update Armeria instrumentation to match latest decorator order and remove dedupe logic for now.

* git add

* Revert unused

README.md

@@ -228,7 +228,7 @@ These are the supported libraries and frameworks:
 | [Akka HTTP](https://doc.akka.io/docs/akka-http/current/index.html)                                                                    | 10.0+                          |
 | [Apache HttpAsyncClient](https://hc.apache.org/index.html)                                                                            | 4.1+                           |
 | [Apache HttpClient](https://hc.apache.org/index.html)                                                                                 | 2.0+                           |
-| [Armeria](https://armeria.dev)                                                                                                        | 0.99.8+                        |
+| [Armeria](https://armeria.dev)                                                                                                        | 1.3+                           |
 | [AsyncHttpClient](https://github.com/AsyncHttpClient/async-http-client)                                                               | 1.9+ (not including 2.x yet)   |
 | [AWS Lambda](https://docs.aws.amazon.com/lambda/latest/dg/java-handler.html)                                                          | 1.0+                           |
 | [AWS SDK](https://aws.amazon.com/sdk-for-java/)                                                                                       | 1.11.x and 2.2.0+              |

instrumentation/armeria-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_0/ArmeriaInstrumentationModule.java

@@ -1,28 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.armeria.v1_0;
-
-import static java.util.Arrays.asList;
-
-import com.google.auto.service.AutoService;
-import io.opentelemetry.javaagent.tooling.InstrumentationModule;
-import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
-import java.util.List;
-
-@AutoService(InstrumentationModule.class)
-public class ArmeriaInstrumentationModule extends InstrumentationModule {
-  public ArmeriaInstrumentationModule() {
-    super("armeria", "armeria-1.0");
-  }
-
-  @Override
-  public List<TypeInstrumentation> typeInstrumentations() {
-    return asList(
-        new ArmeriaWebClientBuilderInstrumentation(),
-        new ArmeriaServerInstrumentation(),
-        new ArmeriaServerBuilderInstrumentation());
-  }
-}

instrumentation/armeria-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_0/ArmeriaServerBuilderInstrumentation.java

@@ -1,71 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.armeria.v1_0;
-
-import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
-import static net.bytebuddy.matcher.ElementMatchers.isMethod;
-import static net.bytebuddy.matcher.ElementMatchers.isPublic;
-import static net.bytebuddy.matcher.ElementMatchers.named;
-import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
-
-import com.linecorp.armeria.server.ServerBuilder;
-import io.opentelemetry.instrumentation.armeria.v1_0.server.OpenTelemetryService;
-import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.function.Function;
-import net.bytebuddy.asm.Advice;
-import net.bytebuddy.description.method.MethodDescription;
-import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.matcher.ElementMatcher;
-
-public class ArmeriaServerBuilderInstrumentation implements TypeInstrumentation {
-
-  @Override
-  public ElementMatcher<? super TypeDescription> typeMatcher() {
-    return named("com.linecorp.armeria.server.ServerBuilder");
-  }
-
-  @Override
-  public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
-    Map<ElementMatcher<MethodDescription>, String> transformers = new HashMap<>();
-    transformers.put(
-        isConstructor(),
-        ArmeriaServerBuilderInstrumentation.class.getName() + "$ConstructorAdvice");
-    transformers.put(
-        isMethod().and(isPublic()).and(named("decorator").and(takesArgument(0, Function.class))),
-        ArmeriaServerBuilderInstrumentation.class.getName() + "$SuppressDecoratorAdvice");
-    return transformers;
-  }
-
-  public static class ConstructorAdvice {
-    @Advice.OnMethodExit
-    public static void construct(@Advice.This ServerBuilder builder) {
-      builder.decorator(OpenTelemetryService.newDecorator());
-    }
-  }
-
-  // Intercept calls from app to register decorator and suppress them to avoid registering
-  // multiple decorators, one from user app and one from our auto instrumentation. Otherwise, we
-  // will end up with double telemetry.
-  // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/903
-  public static class SuppressDecoratorAdvice {
-    @Advice.OnMethodEnter(skipOn = Advice.OnNonDefaultValue.class)
-    public static boolean suppressDecorator(@Advice.Argument(0) Function<?, ?> decorator) {
-      return decorator != ArmeriaDecorators.SERVER_DECORATOR;
-    }
-
-    @Advice.OnMethodExit
-    public static void handleSuppression(
-        @Advice.This ServerBuilder builder,
-        @Advice.Enter boolean suppressed,
-        @Advice.Return(readOnly = false) ServerBuilder returned) {
-      if (suppressed) {
-        returned = builder;
-      }
-    }
-  }
-}

instrumentation/armeria-1.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/armeria/v1_0/ArmeriaNoDuplicateInstrumentationTest.groovy

@@ -1,33 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.armeria.v1_0
-
-import com.linecorp.armeria.client.WebClientBuilder
-import com.linecorp.armeria.server.ServerBuilder
-import io.opentelemetry.instrumentation.armeria.v1_0.AbstractArmeriaTest
-import io.opentelemetry.instrumentation.armeria.v1_0.client.OpenTelemetryClient
-import io.opentelemetry.instrumentation.armeria.v1_0.server.OpenTelemetryService
-import io.opentelemetry.instrumentation.test.AgentTestTrait
-
-class ArmeriaNoDuplicateInstrumentationTest extends AbstractArmeriaTest implements AgentTestTrait {
-  @Override
-  ServerBuilder configureServer(ServerBuilder sb) {
-    return sb.decorator(OpenTelemetryService.newDecorator())
-  }
-
-  @Override
-  WebClientBuilder configureClient(WebClientBuilder clientBuilder) {
-    return clientBuilder.decorator(OpenTelemetryClient.newDecorator())
-  }
-
-  def childSetupSpec() {
-    server.before()
-  }
-
-  def childCleanupSpec() {
-    server.after()
-  }
-}

instrumentation/armeria-1.3/javaagent/armeria-1.3-javaagent.gradle

@@ -4,14 +4,15 @@ muzzle {
   pass {
     group = "com.linecorp.armeria"
     module = "armeria"
-    versions = "[0.99.8,)"
+    versions = "[1.3.0,)"
+    assertInverse = true
   }
 }
 
 dependencies {
-  implementation project(':instrumentation:armeria-1.0:library')
+  implementation project(':instrumentation:armeria-1.3:library')
 
-  library group: 'com.linecorp.armeria', name: 'armeria', version: '0.99.8'
+  library group: 'com.linecorp.armeria', name: 'armeria', version: '1.3.0'
 
-  testImplementation project(':instrumentation:armeria-1.0:testing')
+  testImplementation project(':instrumentation:armeria-1.3:testing')
 }

instrumentation/armeria-1.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaDecorators.java

@@ -3,12 +3,12 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.armeria.v1_0;
+package io.opentelemetry.javaagent.instrumentation.armeria.v1_3;
 
 import com.linecorp.armeria.client.HttpClient;
 import com.linecorp.armeria.server.HttpService;
-import io.opentelemetry.instrumentation.armeria.v1_0.client.OpenTelemetryClient;
+import io.opentelemetry.instrumentation.armeria.v1_3.client.OpenTelemetryClient;
-import io.opentelemetry.instrumentation.armeria.v1_0.server.OpenTelemetryService;
+import io.opentelemetry.instrumentation.armeria.v1_3.server.OpenTelemetryService;
 import java.util.function.Function;
 
 // Holds singleton references to decorators to match against during suppression.

instrumentation/armeria-1.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaInstrumentationModule.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.armeria.v1_3;
+
+import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.ClassLoaderMatcher.hasClassesNamed;
+import static java.util.Arrays.asList;
+
+import com.google.auto.service.AutoService;
+import io.opentelemetry.javaagent.tooling.InstrumentationModule;
+import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
+import java.util.List;
+import net.bytebuddy.matcher.ElementMatcher;
+
+@AutoService(InstrumentationModule.class)
+public class ArmeriaInstrumentationModule extends InstrumentationModule {
+  public ArmeriaInstrumentationModule() {
+    super("armeria", "armeria-1.3");
+  }
+
+  @Override
+  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
+    // Unrelated class which was added in Armeria 1.3.0, the minimum version we support.
+    return hasClassesNamed("com.linecorp.armeria.server.metric.PrometheusExpositionServiceBuilder");
+  }
+
+  @Override
+  public List<TypeInstrumentation> typeInstrumentations() {
+    return asList(
+        new ArmeriaWebClientBuilderInstrumentation(), new ArmeriaServerBuilderInstrumentation());
+  }
+}

instrumentation/armeria-1.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaServerBuilderInstrumentation.java

@@ -3,11 +3,10 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.armeria.v1_0;
+package io.opentelemetry.javaagent.instrumentation.armeria.v1_3;
 
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.isPublic;
-import static net.bytebuddy.matcher.ElementMatchers.isStatic;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 
 import com.linecorp.armeria.server.ServerBuilder;
@@ -19,23 +18,24 @@ import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
 
-public class ArmeriaServerInstrumentation implements TypeInstrumentation {
+public class ArmeriaServerBuilderInstrumentation implements TypeInstrumentation {
+
   @Override
   public ElementMatcher<? super TypeDescription> typeMatcher() {
-    return named("com.linecorp.armeria.server.Server");
+    return named("com.linecorp.armeria.server.ServerBuilder");
   }
 
   @Override
   public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
     return Collections.singletonMap(
-        isMethod().and(isPublic()).and(isStatic()).and(named("builder")),
+        isMethod().and(isPublic()).and(named("build")),
-        ArmeriaServerInstrumentation.class.getName() + "$AddDecoratorAdvice");
+        ArmeriaServerBuilderInstrumentation.class.getName() + "$BuildAdvice");
   }
 
-  public static class AddDecoratorAdvice {
+  public static class BuildAdvice {
-    @Advice.OnMethodExit
+    @Advice.OnMethodEnter
-    public static void addDecorator(@Advice.Return ServerBuilder sb) {
+    public static void onEnter(@Advice.This ServerBuilder builder) {
-      sb.decorator(ArmeriaDecorators.SERVER_DECORATOR);
+      builder.decorator(ArmeriaDecorators.SERVER_DECORATOR);
     }
   }
 }

instrumentation/armeria-1.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaWebClientBuilderInstrumentation.java

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.armeria.v1_0;
+package io.opentelemetry.javaagent.instrumentation.armeria.v1_3;
 
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.isPublic;

instrumentation/armeria-1.3/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/armeria/v1_3/ArmeriaTest.groovy

@@ -3,11 +3,11 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.armeria.v1_0
+package io.opentelemetry.javaagent.instrumentation.armeria.v1_3
 
 import com.linecorp.armeria.client.WebClientBuilder
 import com.linecorp.armeria.server.ServerBuilder
-import io.opentelemetry.instrumentation.armeria.v1_0.AbstractArmeriaTest
+import io.opentelemetry.instrumentation.armeria.v1_3.AbstractArmeriaTest
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 
 class ArmeriaTest extends AbstractArmeriaTest implements AgentTestTrait {

instrumentation/armeria-1.3/library/armeria-1.3-library.gradle

@@ -2,7 +2,7 @@ apply from: "$rootDir/gradle/instrumentation-library.gradle"
 apply plugin: "net.ltgt.errorprone"
 
 dependencies {
-  library group: 'com.linecorp.armeria', name: 'armeria', version: '0.99.8'
+  library group: 'com.linecorp.armeria', name: 'armeria', version: '1.3.0'
 
-  testImplementation project(':instrumentation:armeria-1.0:testing')
+  testImplementation project(':instrumentation:armeria-1.3:testing')
 }

instrumentation/armeria-1.3/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/client/ArmeriaClientTracer.java

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.instrumentation.armeria.v1_0.client;
+package io.opentelemetry.instrumentation.armeria.v1_3.client;
 
 import com.linecorp.armeria.client.ClientRequestContext;
 import com.linecorp.armeria.common.HttpRequest;

instrumentation/armeria-1.3/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/client/OpenTelemetryClient.java

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.instrumentation.armeria.v1_0.client;
+package io.opentelemetry.instrumentation.armeria.v1_3.client;
 
 import com.linecorp.armeria.client.ClientRequestContext;
 import com.linecorp.armeria.client.HttpClient;

instrumentation/armeria-1.3/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/server/ArmeriaServerTracer.java

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.instrumentation.armeria.v1_0.server;
+package io.opentelemetry.instrumentation.armeria.v1_3.server;
 
 import com.linecorp.armeria.common.HttpRequest;
 import com.linecorp.armeria.common.logging.RequestLog;

instrumentation/armeria-1.3/library/src/main/java/io/opentelemetry/instrumentation/armeria/v1_3/server/OpenTelemetryService.java

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.instrumentation.armeria.v1_0.server;
+package io.opentelemetry.instrumentation.armeria.v1_3.server;
 
 import com.linecorp.armeria.common.HttpRequest;
 import com.linecorp.armeria.common.HttpResponse;

instrumentation/armeria-1.3/library/src/test/groovy/io/opentelemetry/instrumentation/armeria/v1_3/ArmeriaTest.groovy

@@ -3,12 +3,12 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.instrumentation.armeria.v1_0
+package io.opentelemetry.instrumentation.armeria.v1_3
 
 import com.linecorp.armeria.client.WebClientBuilder
 import com.linecorp.armeria.server.ServerBuilder
-import io.opentelemetry.instrumentation.armeria.v1_0.client.OpenTelemetryClient
+import io.opentelemetry.instrumentation.armeria.v1_3.client.OpenTelemetryClient
-import io.opentelemetry.instrumentation.armeria.v1_0.server.OpenTelemetryService
+import io.opentelemetry.instrumentation.armeria.v1_3.server.OpenTelemetryService
 import io.opentelemetry.instrumentation.test.InstrumentationTestTrait
 
 class ArmeriaTest extends AbstractArmeriaTest implements InstrumentationTestTrait {

instrumentation/armeria-1.3/testing/armeria-1.3-testing.gradle

@@ -3,8 +3,8 @@ apply from: "$rootDir/gradle/java.gradle"
 dependencies {
   api project(':testing-common')
 
-  api group: 'com.linecorp.armeria', name: 'armeria', version: '0.99.8'
+  api group: 'com.linecorp.armeria', name: 'armeria', version: '1.3.0'
-  api group: 'com.linecorp.armeria', name: 'armeria-junit4', version: '0.99.8'
+  api group: 'com.linecorp.armeria', name: 'armeria-junit4', version: '1.3.0'
 
   implementation deps.guava
 

instrumentation/armeria-1.3/testing/src/main/groovy/io/opentelemetry/instrumentation/armeria/v1_3/AbstractArmeriaTest.groovy

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.instrumentation.armeria.v1_0
+package io.opentelemetry.instrumentation.armeria.v1_3
 
 import static io.opentelemetry.api.trace.Span.Kind.CLIENT
 import static io.opentelemetry.api.trace.Span.Kind.SERVER
@@ -14,10 +14,15 @@ import com.linecorp.armeria.common.HttpMethod
 import com.linecorp.armeria.common.HttpRequest
 import com.linecorp.armeria.common.HttpResponse
 import com.linecorp.armeria.common.HttpStatus
+import com.linecorp.armeria.server.DecoratingHttpServiceFunction
+import com.linecorp.armeria.server.HttpService
 import com.linecorp.armeria.server.ServerBuilder
+import com.linecorp.armeria.server.ServiceRequestContext
 import com.linecorp.armeria.testing.junit4.server.ServerRule
+import io.opentelemetry.api.trace.Span
 import io.opentelemetry.api.trace.attributes.SemanticAttributes
 import io.opentelemetry.instrumentation.test.InstrumentationSpecification
+import java.util.function.Function
 import spock.lang.Shared
 import spock.lang.Unroll
 
@@ -34,11 +39,41 @@ abstract class AbstractArmeriaTest extends InstrumentationSpecification {
   protected ServerRule server = new ServerRule() {
     @Override
     protected void configure(ServerBuilder sb) throws Exception {
-      sb = configureServer(sb)
       sb.service("/exact", { ctx, req -> HttpResponse.of(HttpStatus.OK) })
       sb.service("/items/{itemsId}", { ctx, req -> HttpResponse.of(HttpStatus.OK) })
       sb.service("/httperror", { ctx, req -> HttpResponse.of(HttpStatus.NOT_IMPLEMENTED) })
       sb.service("/exception", { ctx, req -> throw new IllegalStateException("illegal") })
+
+      // Make sure user decorators see spans.
+      sb.decorator(new DecoratingHttpServiceFunction() {
+        @Override
+        HttpResponse serve(HttpService delegate, ServiceRequestContext ctx, HttpRequest req) throws Exception {
+          if (!Span.current().spanContext.isValid()) {
+            // Return an invalid code to fail any assertion
+            return HttpResponse.of(600)
+          }
+          ctx.addAdditionalResponseHeader("decoratinghttpservicefunction", "ok")
+          return delegate.serve(ctx, req)
+        }
+      })
+      sb.decorator(new Function<HttpService, HttpService>() {
+        @Override
+        HttpService apply(HttpService delegate) {
+          return new HttpService() {
+            @Override
+            HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exception {
+              if (!Span.current().spanContext.isValid()) {
+                // Return an invalid code to fail any assertion
+                return HttpResponse.of(601)
+              }
+              ctx.addAdditionalResponseHeader("decoratingfunction", "ok")
+              return delegate.serve(ctx, req)
+            }
+          }
+        }
+      })
+
+      sb = configureServer(sb)
     }
   }
 
@@ -47,6 +82,8 @@ abstract class AbstractArmeriaTest extends InstrumentationSpecification {
   def "HTTP #method #path"() {
     when:
     def response = client.execute(HttpRequest.of(method, path)).aggregate().join()
+    response.headers().get("decoratinghttpservicefunction") == "ok"
+    response.headers().get("decoratingfunction") == "ok"
 
     then:
     response.status().code() == code

settings.gradle

@@ -60,9 +60,9 @@ include ':instrumentation:apache-camel-2.20:javaagent'
 include ':instrumentation:apache-httpasyncclient-4.1:javaagent'
 include ':instrumentation:apache-httpclient:apache-httpclient-2.0:javaagent'
 include ':instrumentation:apache-httpclient:apache-httpclient-4.0:javaagent'
-include ':instrumentation:armeria-1.0:javaagent'
+include ':instrumentation:armeria-1.3:javaagent'
-include ':instrumentation:armeria-1.0:library'
+include ':instrumentation:armeria-1.3:library'
-include ':instrumentation:armeria-1.0:testing'
+include ':instrumentation:armeria-1.3:testing'
 include ':instrumentation:async-http-client-1.9:javaagent'
 include ':instrumentation:aws-lambda-1.0:javaagent'
 include ':instrumentation:aws-lambda-1.0:library'