Skip to content

Commit

Permalink
Fix latest dep tests (open-telemetry#11925)
Browse files Browse the repository at this point in the history
  • Loading branch information
@laurit
laurit committed Aug 5, 2024
1 parent 3d2ca40 commit 2dce859
Show file tree
Hide file tree
Showing 7 changed files with 191 additions and 66 deletions.
26 changes: 22 additions & 4 deletions instrumentation/async-http-client/async-http-client-2.0/javaagent/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,27 @@ dependencies {
testInstrumentation(project(":instrumentation:netty:netty-4.1:javaagent"))
}

otelJava {
// AHC uses Unsafe and so does not run on later java version
maxJavaVersionForTests.set(JavaVersion.VERSION_1_8)
val latestDepTest = findProperty("testLatestDeps") as Boolean
val testJavaVersion =
gradle.startParameter.projectProperties["testJavaVersion"]?.let(JavaVersion::toVersion)
?: JavaVersion.current()

if (!latestDepTest) {
otelJava {
// AHC uses Unsafe and so does not run on later java version
maxJavaVersionForTests.set(JavaVersion.VERSION_1_8)
}
}

tasks.withType<Test>().configureEach {
systemProperty("testLatestDeps", latestDepTest)
// async-http-client 3.0 requires java 11
// We are not using minJavaVersionSupported for latestDepTest because that way the instrumentation
// gets compiled with java 11 when running latestDepTest. This causes play-mvc-2.4 latest dep tests
// to fail because they require java 8 and instrumentation compiled with java 11 won't apply.
if (latestDepTest && testJavaVersion.isJava8) {
enabled = false
}
}

tasks {
Expand All @@ -35,7 +53,7 @@ tasks {

// async-http-client 2.0.0 does not work with Netty versions newer than this due to referencing an
// internal file.
if (!(findProperty("testLatestDeps") as Boolean)) {
if (!latestDepTest) {
configurations.configureEach {
if (!name.contains("muzzle")) {
resolutionStrategy {
Expand Down
28 changes: 23 additions & 5 deletions .../io/opentelemetry/javaagent/instrumentation/asynchttpclient/v2_0/AsyncHttpClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientInstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientResult;
import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
import java.lang.reflect.Method;
import java.net.URI;
import java.time.Duration;
import java.util.Map;
import java.util.concurrent.ExecutionException;
import org.asynchttpclient.AsyncCompletionHandler;
import org.asynchttpclient.AsyncHttpClient;
import org.asynchttpclient.DefaultAsyncHttpClientConfig;
import org.asynchttpclient.Dsl;
import org.asynchttpclient.Request;
import org.asynchttpclient.RequestBuilder;
Expand All @@ -31,11 +34,26 @@ class AsyncHttpClientTest extends AbstractHttpClientTest<Request> {
private static final int CONNECTION_TIMEOUT_MS = (int) CONNECTION_TIMEOUT.toMillis();

// request timeout is needed in addition to connect timeout on async-http-client versions 2.1.0+
private static final AsyncHttpClient client =
Dsl.asyncHttpClient(
Dsl.config()
.setConnectTimeout(CONNECTION_TIMEOUT_MS)
.setRequestTimeout(CONNECTION_TIMEOUT_MS));
private static final AsyncHttpClient client = Dsl.asyncHttpClient(configureTimeout(Dsl.config()));

private static DefaultAsyncHttpClientConfig.Builder configureTimeout(
DefaultAsyncHttpClientConfig.Builder builder) {
setTimeout(builder, "setConnectTimeout", CONNECTION_TIMEOUT_MS);
setTimeout(builder, "setRequestTimeout", CONNECTION_TIMEOUT_MS);
return builder;
}

private static void setTimeout(
DefaultAsyncHttpClientConfig.Builder builder, String methodName, int timeout) {
boolean testLatestDeps = Boolean.getBoolean("testLatestDeps");
try {
Method method =
builder.getClass().getMethod(methodName, testLatestDeps ? Duration.class : int.class);
method.invoke(builder, testLatestDeps ? Duration.ofMillis(timeout) : timeout);
} catch (Exception exception) {
throw new IllegalStateException("Failed to set timeout " + methodName, exception);
}
}

@Override
public Request buildRequest(String method, URI uri, Map<String, String> headers) {
Expand Down
2 changes: 1 addition & 1 deletion ...ovy/io/opentelemetry/javaagent/instrumentation/lettuce/v5_1/LettuceAsyncClientTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
package io.opentelemetry.javaagent.instrumentation.lettuce.v5_1

import io.lettuce.core.RedisClient
import io.opentelemetry.instrumentation.lettuce.v5_1.AbstractLettuceAsyncClientTest

import io.opentelemetry.instrumentation.test.AgentTestTrait

class LettuceAsyncClientTest extends AbstractLettuceAsyncClientTest implements AgentTestTrait {
Expand Down
2 changes: 1 addition & 1 deletion .../io/opentelemetry/javaagent/instrumentation/lettuce/v5_1/LettuceSyncClientAuthTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
package io.opentelemetry.javaagent.instrumentation.lettuce.v5_1

import io.lettuce.core.RedisClient
import io.opentelemetry.instrumentation.lettuce.v5_1.AbstractLettuceSyncClientAuthTest

import io.opentelemetry.instrumentation.test.AgentTestTrait

class LettuceSyncClientAuthTest extends AbstractLettuceSyncClientAuthTest implements AgentTestTrait {
Expand Down
39 changes: 37 additions & 2 deletions ...vy/io/opentelemetry/instrumentation/lettuce/v5_1/AbstractLettuceSyncClientAuthTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@
package io.opentelemetry.instrumentation.lettuce.v5_1

import io.lettuce.core.RedisClient
import io.lettuce.core.RedisCommandExecutionException
import io.opentelemetry.api.trace.StatusCode
import io.opentelemetry.instrumentation.test.InstrumentationSpecification
import io.opentelemetry.semconv.SemanticAttributes
import org.testcontainers.containers.GenericContainer
import spock.lang.Shared

import static io.opentelemetry.api.trace.SpanKind.CLIENT
import static io.opentelemetry.api.trace.StatusCode.ERROR

abstract class AbstractLettuceSyncClientAuthTest extends InstrumentationSpecification {
public static final int DB_INDEX = 0
Expand Down Expand Up @@ -60,8 +63,40 @@ abstract class AbstractLettuceSyncClientAuthTest extends InstrumentationSpecific

expect:
res == "OK"
assertTraces(1) {
trace(0, 1) {
assertTraces(Boolean.getBoolean("testLatestDeps") ? 3 : 1) {
if (Boolean.getBoolean("testLatestDeps")) {
trace(0, 1) {
span(0) {
name "CLIENT"
kind CLIENT
status ERROR
attributes {
"$SemanticAttributes.NET_SOCK_PEER_ADDR" "127.0.0.1"
"$SemanticAttributes.NET_SOCK_PEER_NAME" expectedHostAttributeValue
"$SemanticAttributes.NET_SOCK_PEER_PORT" port
"$SemanticAttributes.DB_SYSTEM" "redis"
"$SemanticAttributes.DB_STATEMENT" "CLIENT SETINFO lib-name Lettuce"
}
errorEvent(RedisCommandExecutionException.class, "NOAUTH Authentication required.")
}
}
trace(1, 1) {
span(0) {
name "CLIENT"
kind CLIENT
status ERROR
attributes {
"$SemanticAttributes.NET_SOCK_PEER_ADDR" "127.0.0.1"
"$SemanticAttributes.NET_SOCK_PEER_NAME" expectedHostAttributeValue
"$SemanticAttributes.NET_SOCK_PEER_PORT" port
"$SemanticAttributes.DB_SYSTEM" "redis"
"$SemanticAttributes.DB_STATEMENT" { it.startsWith("CLIENT SETINFO lib-ver") }
}
errorEvent(RedisCommandExecutionException.class, "NOAUTH Authentication required.")
}
}
}
trace(Boolean.getBoolean("testLatestDeps") ? 2 : 0, 1) {
span(0) {
name "AUTH"
kind CLIENT
Expand Down
17 changes: 5 additions & 12 deletions ...rc/main/java/io/opentelemetry/instrumentation/lettuce/v5_1/AbstractLettuceClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import io.lettuce.core.RedisClient;
import io.lettuce.core.api.StatefulRedisConnection;
import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.extension.RegisterExtension;
Expand All @@ -22,13 +21,6 @@
abstract class AbstractLettuceClientTest {
protected static final Logger logger = LoggerFactory.getLogger(AbstractLettuceClientTest.class);

@RegisterExtension
protected static final InstrumentationExtension testing = AgentInstrumentationExtension.create();

public InstrumentationExtension getInstrumentationExtension() {
return testing;
}

@RegisterExtension static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();

protected static final int DB_INDEX = 0;
Expand All @@ -40,15 +32,16 @@ public InstrumentationExtension getInstrumentationExtension() {
.waitingFor(Wait.forLogMessage(".*Ready to accept connections.*", 1));

protected static RedisClient redisClient;

protected static StatefulRedisConnection<String, String> connection;

protected abstract RedisClient createClient(String uri);

protected static String host;
protected static String ip;
protected static int port;
protected static String embeddedDbUri;

protected abstract RedisClient createClient(String uri);

protected abstract InstrumentationExtension getInstrumentationExtension();

protected ContainerConnection newContainerConnection() {
GenericContainer<?> server =
new GenericContainer<>("redis:6.2.3-alpine")
Expand Down
143 changes: 102 additions & 41 deletions ...ain/java/io/opentelemetry/instrumentation/lettuce/v5_1/AbstractLettuceSyncClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.instrumentation.test.utils.PortUtils;
import io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions;
import io.opentelemetry.semconv.SemanticAttributes;
import java.util.Base64;
import java.util.Map;
Expand Down Expand Up @@ -81,8 +82,13 @@ void testConnect() {
StatefulRedisConnection<String, String> testConnection = redisClient.connect();
cleanup.deferCleanup(testConnection);

// Lettuce tracing does not trace connect
assertThat(getInstrumentationExtension().spans()).isEmpty();
if (Boolean.getBoolean("testLatestDeps")) {
// ignore CLIENT SETINFO traces
getInstrumentationExtension().waitForTraces(2);
} else {
// Lettuce tracing does not trace connect
assertThat(getInstrumentationExtension().spans()).isEmpty();
}
}

@Test
Expand Down Expand Up @@ -133,6 +139,12 @@ void testSetCommandLocalhost() {
StatefulRedisConnection<String, String> testConnection = testConnectionClient.connect();
cleanup.deferCleanup(testConnection);

if (Boolean.getBoolean("testLatestDeps")) {
// ignore CLIENT SETINFO traces
getInstrumentationExtension().waitForTraces(2);
getInstrumentationExtension().clearData();
}

String res = testConnection.sync().set("TESTSETKEY", "TESTSETVAL");
assertThat(res).isEqualTo("OK");

Expand Down Expand Up @@ -237,6 +249,12 @@ void testListCommand() {
ContainerConnection containerConnection = newContainerConnection();
RedisCommands<String, String> commands = containerConnection.connection.sync();

if (Boolean.getBoolean("testLatestDeps")) {
// ignore CLIENT SETINFO traces
getInstrumentationExtension().waitForTraces(2);
getInstrumentationExtension().clearData();
}

long res = commands.lpush("TESTLIST", "TESTLIST ELEMENT");
assertThat(res).isEqualTo(1);

Expand Down Expand Up @@ -382,45 +400,82 @@ void testDebugSegfaultCommandWithNoArgumentProducesNoSpan() {

commands.debugSegfault();

getInstrumentationExtension()
.waitAndAssertTraces(
trace -> {
if (Boolean.getBoolean("testLatestDeps")) {
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("DEBUG")
.hasKind(SpanKind.CLIENT)
.hasAttributesSatisfyingExactly(
equalTo(SemanticAttributes.NET_SOCK_PEER_ADDR, "127.0.0.1"),
equalTo(
SemanticAttributes.NET_SOCK_PEER_NAME,
expectedHostAttributeValue),
equalTo(
SemanticAttributes.NET_SOCK_PEER_PORT,
containerConnection.port),
equalTo(SemanticAttributes.DB_SYSTEM, "redis"),
equalTo(SemanticAttributes.DB_STATEMENT, "DEBUG SEGFAULT")));
} else {
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("DEBUG")
.hasKind(SpanKind.CLIENT)
.hasAttributesSatisfyingExactly(
equalTo(SemanticAttributes.NET_SOCK_PEER_ADDR, "127.0.0.1"),
equalTo(
SemanticAttributes.NET_SOCK_PEER_NAME,
expectedHostAttributeValue),
equalTo(
SemanticAttributes.NET_SOCK_PEER_PORT,
containerConnection.port),
equalTo(SemanticAttributes.DB_SYSTEM, "redis"),
equalTo(SemanticAttributes.DB_STATEMENT, "DEBUG SEGFAULT"))
// these are no longer recorded since Lettuce 6.1.6
.hasEventsSatisfyingExactly(
event -> event.hasName("redis.encode.start"),
event -> event.hasName("redis.encode.end")));
}
});
if (Boolean.getBoolean("testLatestDeps")) {
getInstrumentationExtension()
.waitAndAssertTraces(
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("CLIENT")
.hasKind(SpanKind.CLIENT)
.hasAttributesSatisfyingExactly(
equalTo(SemanticAttributes.NET_SOCK_PEER_ADDR, "127.0.0.1"),
equalTo(
SemanticAttributes.NET_SOCK_PEER_NAME,
expectedHostAttributeValue),
equalTo(
SemanticAttributes.NET_SOCK_PEER_PORT,
containerConnection.port),
equalTo(SemanticAttributes.DB_SYSTEM, "redis"),
equalTo(
SemanticAttributes.DB_STATEMENT,
"CLIENT SETINFO lib-name Lettuce"))),
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("CLIENT")
.hasKind(SpanKind.CLIENT)
.hasAttributesSatisfyingExactly(
equalTo(SemanticAttributes.NET_SOCK_PEER_ADDR, "127.0.0.1"),
equalTo(
SemanticAttributes.NET_SOCK_PEER_NAME,
expectedHostAttributeValue),
equalTo(
SemanticAttributes.NET_SOCK_PEER_PORT,
containerConnection.port),
equalTo(SemanticAttributes.DB_SYSTEM, "redis"),
OpenTelemetryAssertions.satisfies(
SemanticAttributes.DB_STATEMENT,
stringAssert ->
stringAssert.startsWith("CLIENT SETINFO lib-ver")))),
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("DEBUG")
.hasKind(SpanKind.CLIENT)
.hasAttributesSatisfyingExactly(
equalTo(SemanticAttributes.NET_SOCK_PEER_ADDR, "127.0.0.1"),
equalTo(
SemanticAttributes.NET_SOCK_PEER_NAME,
expectedHostAttributeValue),
equalTo(
SemanticAttributes.NET_SOCK_PEER_PORT,
containerConnection.port),
equalTo(SemanticAttributes.DB_SYSTEM, "redis"),
equalTo(SemanticAttributes.DB_STATEMENT, "DEBUG SEGFAULT"))));
} else {
getInstrumentationExtension()
.waitAndAssertTraces(
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("DEBUG")
.hasKind(SpanKind.CLIENT)
.hasAttributesSatisfyingExactly(
equalTo(SemanticAttributes.NET_SOCK_PEER_ADDR, "127.0.0.1"),
equalTo(
SemanticAttributes.NET_SOCK_PEER_NAME,
expectedHostAttributeValue),
equalTo(
SemanticAttributes.NET_SOCK_PEER_PORT,
containerConnection.port),
equalTo(SemanticAttributes.DB_SYSTEM, "redis"),
equalTo(SemanticAttributes.DB_STATEMENT, "DEBUG SEGFAULT"))
// these are no longer recorded since Lettuce 6.1.6
.hasEventsSatisfyingExactly(
event -> event.hasName("redis.encode.start"),
event -> event.hasName("redis.encode.end"))));
}
}

@Test
Expand All @@ -429,6 +484,12 @@ void testShutdownCommandProducesNoSpan() {
ContainerConnection containerConnection = newContainerConnection();
RedisCommands<String, String> commands = containerConnection.connection.sync();

if (Boolean.getBoolean("testLatestDeps")) {
// ignore CLIENT SETINFO traces
getInstrumentationExtension().waitForTraces(2);
getInstrumentationExtension().clearData();
}

commands.shutdown(false);

getInstrumentationExtension()
Expand Down

0 comments on commit 2dce859

Please sign in to comment.