javadoc + timeout

Author: zeitlinger

smoke-tests/src/main/java/io/opentelemetry/smoketest/SmokeTestInstrumentationExtension.java

@@ -44,9 +44,6 @@
 public class SmokeTestInstrumentationExtension extends InstrumentationExtension
     implements TelemetryRetrieverProvider {
 
-  // todo timeout configuration
-  // todo move builder docs to builder methods
-
   private final TestContainerManager containerManager = createContainerManager();
 
   private TelemetryRetriever telemetryRetriever;
@@ -70,6 +67,7 @@ public interface GetTargetImage {
   private final List<ResourceMapping> extraResources;
   private final TargetWaitStrategy waitStrategy;
   private final List<Integer> extraPorts;
+  private final Duration telemetryTimeout;
 
   private SmokeTestInstrumentationExtension(
       GetTargetImage getTargetImage,
@@ -79,7 +77,8 @@ private SmokeTestInstrumentationExtension(
       Map<String, String> extraEnv,
       List<ResourceMapping> extraResources,
       TargetWaitStrategy waitStrategy,
-      List<Integer> extraPorts) {
+      List<Integer> extraPorts,
+      Duration telemetryTimeout) {
     super(new SmokeTestRunner());
     this.getTargetImage = getTargetImage;
     this.command = command;
@@ -89,6 +88,7 @@ private SmokeTestInstrumentationExtension(
     this.extraResources = extraResources;
     this.waitStrategy = waitStrategy;
     this.extraPorts = extraPorts;
+    this.telemetryTimeout = telemetryTimeout;
   }
 
   public WebClient client() {
@@ -98,7 +98,8 @@ public WebClient client() {
   @Override
   public void beforeAll(ExtensionContext context) throws Exception {
     containerManager.startEnvironmentOnce();
-    telemetryRetriever = new TelemetryRetriever(containerManager.getBackendMappedPort());
+    telemetryRetriever =
+        new TelemetryRetriever(containerManager.getBackendMappedPort(), telemetryTimeout);
     super.beforeAll(context);
   }
 
@@ -124,67 +125,26 @@ public SmokeTestOutput start(int jdk) {
   }
 
   public SmokeTestOutput start(String jdk, String serverVersion, boolean windows) {
-    String targetImage = getTargetImage(jdk, serverVersion, windows);
     autoCleanup.deferCleanup(() -> containerManager.stopTarget());
 
     return new SmokeTestOutput(
         containerManager.startTarget(
-            targetImage,
+            getTargetImage.getTargetImage(jdk, serverVersion, windows),
             agentPath,
-            getJvmArgsEnvVarName(),
-            getExtraEnv(),
-            getSetServiceName(),
-            getExtraResources(),
-            getExtraPorts(),
-            getWaitStrategy(),
-            getCommand()));
+            jvmArgsEnvVarName,
+            extraEnv,
+            setServiceName,
+            extraResources,
+            extraPorts,
+            waitStrategy,
+            command));
   }
 
   @Override
   public TelemetryRetriever getTelemetryRetriever() {
     return telemetryRetriever;
   }
 
-  public String getTargetImage(String jdk, String serverVersion, boolean windows) {
-    return getTargetImage.getTargetImage(jdk, serverVersion, windows);
-  }
-
-  public String[] getCommand() {
-    return command;
-  }
-
-  /** Subclasses can override this method to pass jvm arguments in another environment variable */
-  public String getJvmArgsEnvVarName() {
-    return jvmArgsEnvVarName;
-  }
-
-  /** Subclasses can override this method to customise target application's environment */
-  public Map<String, String> getExtraEnv() {
-    return extraEnv;
-  }
-
-  /** Subclasses can override this method to disable setting default service name */
-  public boolean getSetServiceName() {
-    return setServiceName;
-  }
-
-  /** Subclasses can override this method to provide additional files to copy to target container */
-  public List<ResourceMapping> getExtraResources() {
-    return extraResources;
-  }
-
-  /**
-   * Subclasses can override this method to provide additional ports that should be exposed from the
-   * target container
-   */
-  public List<Integer> getExtraPorts() {
-    return extraPorts;
-  }
-
-  public TargetWaitStrategy getWaitStrategy() {
-    return waitStrategy;
-  }
-
   public static Builder builder(Function<String, String> getTargetImage) {
     return builder((jdk, serverVersion, windows) -> getTargetImage.apply(jdk));
   }
@@ -219,53 +179,68 @@ public static class Builder {
     private List<ResourceMapping> extraResources = List.of();
     private TargetWaitStrategy waitStrategy;
     private List<Integer> extraPorts = List.of();
+    private Duration telemetryTimeout = Duration.ofSeconds(30);
 
     private Builder(GetTargetImage getTargetImage) {
       this.getTargetImage = getTargetImage;
     }
 
+    /** Sets the command to run in the target container. */
     @CanIgnoreReturnValue
     public Builder command(String... command) {
       this.command = command;
       return this;
     }
 
+    /** Sets the environment variable name used to pass JVM arguments to the target application. */
     @CanIgnoreReturnValue
     public Builder jvmArgsEnvVarName(String jvmArgsEnvVarName) {
       this.jvmArgsEnvVarName = jvmArgsEnvVarName;
       return this;
     }
 
+    /** Enables or disables setting the default service name for the target application. */
     @CanIgnoreReturnValue
     public Builder setServiceName(boolean setServiceName) {
       this.setServiceName = setServiceName;
       return this;
     }
 
+    /** Adds an environment variable to the target application's environment. */
     @CanIgnoreReturnValue
     public Builder env(String key, String value) {
       this.extraEnv.put(key, value);
       return this;
     }
 
+    /** Specifies additional files to copy to the target container. */
     @CanIgnoreReturnValue
     public Builder extraResources(ResourceMapping... resources) {
       this.extraResources = List.of(resources);
       return this;
     }
 
+    /** Sets the wait strategy for the target container startup. */
     @CanIgnoreReturnValue
     public Builder waitStrategy(@Nullable TargetWaitStrategy waitStrategy) {
       this.waitStrategy = waitStrategy;
       return this;
     }
 
+    /** Specifies additional ports to expose from the target container. */
     @CanIgnoreReturnValue
     public Builder extraPorts(Integer... ports) {
       this.extraPorts = List.of(ports);
       return this;
     }
 
+    /** Sets the timeout duration for retrieving telemetry data. */
+    @CanIgnoreReturnValue
+    public Builder telemetryTimeout(Duration telemetryTimeout) {
+      this.telemetryTimeout = telemetryTimeout;
+      return this;
+    }
+
     public SmokeTestInstrumentationExtension build() {
       return new SmokeTestInstrumentationExtension(
           getTargetImage,
@@ -275,7 +250,8 @@ public SmokeTestInstrumentationExtension build() {
           extraEnv,
           extraResources,
           waitStrategy,
-          extraPorts);
+          extraPorts,
+          telemetryTimeout);
     }
   }
 }

smoke-tests/src/main/java/io/opentelemetry/smoketest/TelemetryRetriever.java

@@ -18,6 +18,7 @@
 import io.opentelemetry.testing.internal.protobuf.GeneratedMessage;
 import io.opentelemetry.testing.internal.protobuf.InvalidProtocolBufferException;
 import io.opentelemetry.testing.internal.protobuf.util.JsonFormat;
+import java.time.Duration;
 import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -28,9 +29,11 @@
 public class TelemetryRetriever {
   private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
   private final WebClient client;
+  private final Duration telemetryTimeout;
 
-  public TelemetryRetriever(int backendPort) {
+  public TelemetryRetriever(int backendPort, Duration telemetryTimeout) {
     client = WebClient.of("http://localhost:" + backendPort);
+    this.telemetryTimeout = telemetryTimeout;
   }
 
   public void clearTelemetry() {
@@ -91,7 +94,7 @@ Collection<T> waitForTelemetry(String path, Supplier<B> builderConstructor) {
   @SuppressWarnings("SystemOut")
   private String waitForContent(String path) throws InterruptedException {
     long previousSize = 0;
-    long deadline = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(30);
+    long deadline = System.currentTimeMillis() + telemetryTimeout.toMillis();
     String content = "[]";
     while (System.currentTimeMillis() < deadline) {
       content = client.get(path).aggregate().join().contentUtf8();

smoke-tests/src/test/java/io/opentelemetry/smoketest/JavaSmokeTest.java

@@ -42,7 +42,8 @@ public WebClient client() {
   @BeforeAll
   static void setUp() {
     containerManager.startEnvironmentOnce();
-    telemetryRetriever = new TelemetryRetriever(containerManager.getBackendMappedPort());
+    telemetryRetriever =
+        new TelemetryRetriever(containerManager.getBackendMappedPort(), telemetryTimeout);
   }
 
   protected SmokeTestOutput startTarget(int jdk) {

smoke-tests/src/test/java/io/opentelemetry/smoketest/SpringBootSmokeTest.java

@@ -5,24 +5,23 @@
 
 package io.opentelemetry.smoketest;
 
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies;
+
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.sdk.trace.data.SpanData;
 import io.opentelemetry.semconv.ServiceAttributes;
 import io.opentelemetry.semconv.incubating.OsIncubatingAttributes;
 import io.opentelemetry.semconv.incubating.TelemetryIncubatingAttributes;
 import io.opentelemetry.semconv.incubating.ThreadIncubatingAttributes;
-import org.junit.jupiter.api.condition.DisabledIf;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.ValueSource;
-
 import java.util.List;
 import java.util.Set;
 import java.util.jar.Attributes;
 import java.util.jar.JarFile;
 import java.util.stream.Collectors;
-
-import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
-import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies;
+import org.junit.jupiter.api.condition.DisabledIf;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 @DisabledIf("io.opentelemetry.smoketest.TestContainerManager#useWindowsContainers")
 class SpringBootSmokeTest extends JavaSmokeTest {