From d46d73d4b39a124b42c019f28c728af3cee2c9a5 Mon Sep 17 00:00:00 2001
From: Trask Stalnaker <trask.stalnaker@gmail.com>
Date: Tue, 27 Jul 2021 14:01:10 -0700
Subject: [PATCH] Improve benchmark-overhead-jmh consistency (#3680)

* Improve benchmark-overhead-jmh consistency

* HttpURLConnection
---
 benchmark-overhead-jmh/build.gradle.kts       | 12 ++++-----
 .../benchmark/servlet/ServletBenchmark.java   | 27 ++++++++++++++-----
 .../src/jmh/resources/application.properties  |  3 +++
 3 files changed, 28 insertions(+), 14 deletions(-)
 create mode 100644 benchmark-overhead-jmh/src/jmh/resources/application.properties

diff --git a/benchmark-overhead-jmh/build.gradle.kts b/benchmark-overhead-jmh/build.gradle.kts
index d995d50341..00f647fb27 100644
--- a/benchmark-overhead-jmh/build.gradle.kts
+++ b/benchmark-overhead-jmh/build.gradle.kts
@@ -9,13 +9,8 @@ plugins {
 
 dependencies {
   jmhImplementation("org.springframework.boot:spring-boot-starter-web:2.5.2")
-  jmhImplementation(project(":testing-common")) {
-    exclude("ch.qos.logback")
-  }
 
-  // this only exists to make Intellij happy since it doesn't (currently at least) understand our
-  // inclusion of this artifact inside of :testing-common
-  jmhCompileOnly(project(path = ":testing:armeria-shaded-for-testing", configuration = "shadow"))
+  testImplementation("org.assertj:assertj-core")
 }
 
 tasks {
@@ -42,7 +37,10 @@ tasks {
     val args = listOf(
       "-javaagent:${shadowTask.archiveFile.get()}",
       "-Dotel.traces.exporter=none",
-      "-Dotel.metrics.exporter=none"
+      "-Dotel.metrics.exporter=none",
+      // avoid instrumenting HttpURLConnection for now since it is used to make the requests
+      // and this benchmark is focused on servlet overhead for now
+      "-Dotel.instrumentation.http-url-connection.enabled=false"
     )
     // see https://github.com/melix/jmh-gradle-plugin/issues/200
     jvmArgsPrepend.add(args.joinToString(" "))
diff --git a/benchmark-overhead-jmh/src/jmh/java/io/opentelemetry/javaagent/benchmark/servlet/ServletBenchmark.java b/benchmark-overhead-jmh/src/jmh/java/io/opentelemetry/javaagent/benchmark/servlet/ServletBenchmark.java
index 362046a544..ddf090a3c6 100644
--- a/benchmark-overhead-jmh/src/jmh/java/io/opentelemetry/javaagent/benchmark/servlet/ServletBenchmark.java
+++ b/benchmark-overhead-jmh/src/jmh/java/io/opentelemetry/javaagent/benchmark/servlet/ServletBenchmark.java
@@ -6,7 +6,10 @@
 package io.opentelemetry.javaagent.benchmark.servlet;
 
 import io.opentelemetry.javaagent.benchmark.servlet.app.HelloWorldApplication;
-import io.opentelemetry.testing.internal.armeria.client.WebClient;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
 import java.util.concurrent.TimeUnit;
 import org.openjdk.jmh.annotations.Benchmark;
 import org.openjdk.jmh.annotations.BenchmarkMode;
@@ -28,12 +31,13 @@ public class ServletBenchmark {
     HelloWorldApplication.main();
   }
 
-  // using shaded armeria http client from testing-common artifact since it won't be instrumented
-  private WebClient client;
+  private URL client;
+  private byte[] buffer;
 
   @Setup
-  public void setup() {
-    client = WebClient.builder().build();
+  public void setup() throws IOException {
+    client = new URL("http://localhost:8080");
+    buffer = new byte[8192];
   }
 
   @TearDown
@@ -42,7 +46,16 @@ public class ServletBenchmark {
   }
 
   @Benchmark
-  public Object execute() {
-    return client.get("http://localhost:8080").aggregate().join();
+  public void execute() throws IOException {
+    HttpURLConnection connection = (HttpURLConnection) client.openConnection();
+    InputStream inputStream = connection.getInputStream();
+    drain(inputStream);
+    inputStream.close();
+    connection.disconnect();
+  }
+
+  @SuppressWarnings("StatementWithEmptyBody")
+  private void drain(InputStream inputStream) throws IOException {
+    while (inputStream.read(buffer) != -1) {}
   }
 }
diff --git a/benchmark-overhead-jmh/src/jmh/resources/application.properties b/benchmark-overhead-jmh/src/jmh/resources/application.properties
new file mode 100644
index 0000000000..4ab5a6d91a
--- /dev/null
+++ b/benchmark-overhead-jmh/src/jmh/resources/application.properties
@@ -0,0 +1,3 @@
+# infinite keep alive helps a little with benchmark variance
+server.tomcat.keep-alive-timeout=-1
+server.tomcat.max-keep-alive-requests=-1