From 28bbbe79f58b64dd836466be631001aa3e18ecf0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:49:24 +0000 Subject: [PATCH 01/22] Initial plan From 510216030b5333c96a34f5d0bbb5d04f3c615397 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:04:55 +0000 Subject: [PATCH 02/22] Classify Redisson server errors Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../javaagent-unit-tests/build.gradle.kts | 1 + .../v3_0/RedissonDbAttributesGetterTest.java | 94 +++++++++++++++++++ .../v3_0/RedissonDbAttributesGetter.java | 52 ++++++++++ .../redisson/AbstractRedissonClientTest.java | 47 ++++++++++ 4 files changed, 194 insertions(+) create mode 100644 instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java diff --git a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/build.gradle.kts b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/build.gradle.kts index 758645a31022..a6f47d94c3c5 100644 --- a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/build.gradle.kts +++ b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/build.gradle.kts @@ -4,6 +4,7 @@ plugins { dependencies { testImplementation(project(":instrumentation-api-incubator")) + testImplementation(project(":instrumentation:redisson:redisson-common-3.0:javaagent")) testImplementation(project(":instrumentation:redisson:redisson-3.0:javaagent")) testImplementation("io.opentelemetry.javaagent:opentelemetry-testing-common") testImplementation("org.redisson:redisson:${baseVersion("3.0.0").orLatest("3.16.+")}") diff --git a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java new file mode 100644 index 000000000000..6400827fe088 --- /dev/null +++ b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java @@ -0,0 +1,94 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0; + +import static io.opentelemetry.semconv.ErrorAttributes.ERROR_TYPE; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.params.provider.Arguments.argumentSet; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientAttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.redisson.client.RedisConnectionException; +import org.redisson.client.RedisException; +import org.redisson.client.RedisLoadingException; +import org.redisson.client.RedisOutOfMemoryException; + +class RedissonDbAttributesGetterTest { + + private static final RedissonDbAttributesGetter GETTER = new RedissonDbAttributesGetter(); + private static final RedissonRequest REQUEST = + RedissonRequest.create(null, new Object(), null, null); + private static final AttributesExtractor EXTRACTOR = + DbClientAttributesExtractor.create(GETTER); + + @ParameterizedTest + @MethodSource("redisErrorMessages") + void extractsKnownRedisErrorTypes(String message, String expectedErrorType) { + assertThat(GETTER.getErrorType(REQUEST, null, new RedisException(message))) + .isEqualTo(expectedErrorType); + } + + private static Stream redisErrorMessages() { + return Stream.of( + argumentSet( + "generic error", + "ERR unknown command with variable details. channel: redis command: GET", + "ERR"), + argumentSet( + "wrong type", + "WRONGTYPE Operation against a key holding the wrong kind of value. channel: redis command: GET", + "WRONGTYPE"), + argumentSet("malformed prefix", "ERR", null), + argumentSet("unknown prefix", "CUSTOM variable details. channel: redis command: GET", null), + argumentSet("message without decoder marker", "ERR connection refused", null), + argumentSet("non-server error", "Unable to decode response. channel: redis", null)); + } + + @Test + void missingMessageHasNoErrorType() { + assertThat(GETTER.getErrorType(REQUEST, null, new RedisException())).isNull(); + assertThat(GETTER.getErrorType(REQUEST, null, null)).isNull(); + } + + @Test + void dedicatedExceptionKeepsItsExceptionClassFallback() { + RedisLoadingException error = new RedisLoadingException("LOADING Redis is loading"); + + assertThat(GETTER.getErrorType(REQUEST, null, error)).isNull(); + assertThat(extractedErrorType(error)).isEqualTo(RedisLoadingException.class.getName()); + } + + @Test + void dedicatedExceptionWithRemovedPrefixKeepsItsExceptionClassFallback() { + RedisOutOfMemoryException error = + new RedisOutOfMemoryException("maxmemory threshold exceeded. channel: redis command: SET"); + + assertThat(GETTER.getErrorType(REQUEST, null, error)).isNull(); + assertThat(extractedErrorType(error)).isEqualTo(RedisOutOfMemoryException.class.getName()); + } + + @Test + void connectionExceptionIsNotClassifiedAsServerError() { + RedisConnectionException error = new RedisConnectionException("ERR connection refused"); + + assertThat(GETTER.getErrorType(REQUEST, null, error)).isNull(); + assertThat(extractedErrorType(error)).isEqualTo(RedisConnectionException.class.getName()); + } + + private static String extractedErrorType(Throwable error) { + AttributesBuilder attributes = Attributes.builder(); + EXTRACTOR.onEnd(attributes, Context.root(), REQUEST, null, error); + return attributes.build().get(ERROR_TYPE); + } +} diff --git a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetter.java b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetter.java index 1bbe40e94c4d..4649656fe696 100644 --- a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetter.java +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetter.java @@ -10,6 +10,7 @@ import io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DbSystemNameIncubatingValues; import java.net.InetSocketAddress; import javax.annotation.Nullable; +import org.redisson.client.RedisException; final class RedissonDbAttributesGetter implements DbClientAttributesGetter { @@ -43,6 +44,57 @@ public Long getDbOperationBatchSize(RedissonRequest request) { return request.getOperationBatchSize(); } + @Override + @Nullable + public String getErrorType( + RedissonRequest request, @Nullable Void response, @Nullable Throwable error) { + if (error == null || error.getClass() != RedisException.class) { + return null; + } + + String message = error.getMessage(); + if (message == null) { + return null; + } + + int serverErrorMarker = message.indexOf(". channel:"); + if (serverErrorMarker < 0) { + return null; + } + + int separator = message.indexOf(' '); + if (separator <= 0 || separator >= serverErrorMarker) { + return null; + } + + String errorType = message.substring(0, separator); + switch (errorType) { + case "ERR": + case "WRONGTYPE": + case "EXECABORT": + case "CROSSSLOT": + case "MASTERDOWN": + case "NOREPLICAS": + case "READONLY": + case "MISCONF": + case "NOPERM": + case "NOSCRIPT": + case "NOAUTH": + case "WRONGPASS": + case "MOVED": + case "ASK": + case "TRYAGAIN": + case "LOADING": + case "OOM": + case "CLUSTERDOWN": + case "BUSY": + case "WAIT": + return errorType; + default: + return null; + } + } + @Nullable @Override public String getServerAddress(RedissonRequest request) { diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index b61d0c1ae180..f974cff7243d 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -19,6 +19,7 @@ import static io.opentelemetry.semconv.DbAttributes.DB_OPERATION_NAME; import static io.opentelemetry.semconv.DbAttributes.DB_QUERY_TEXT; import static io.opentelemetry.semconv.DbAttributes.DB_SYSTEM_NAME; +import static io.opentelemetry.semconv.ErrorAttributes.ERROR_TYPE; import static io.opentelemetry.semconv.NetworkAttributes.NETWORK_PEER_ADDRESS; import static io.opentelemetry.semconv.NetworkAttributes.NETWORK_PEER_PORT; import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS; @@ -26,6 +27,7 @@ import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DbSystemNameIncubatingValues.REDIS; import static java.util.Arrays.asList; import static java.util.Collections.nCopies; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.params.provider.Arguments.argumentSet; import io.opentelemetry.api.common.AttributeKey; @@ -33,6 +35,7 @@ import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.sdk.testing.assertj.TraceAssert; +import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.net.InetAddress; import java.net.UnknownHostException; @@ -67,6 +70,7 @@ import org.redisson.api.RScoredSortedSet; import org.redisson.api.RSet; import org.redisson.api.RedissonClient; +import org.redisson.client.RedisException; import org.redisson.config.Config; import org.redisson.config.SingleServerConfig; import org.slf4j.Logger; @@ -238,6 +242,49 @@ void testDurationMetric() { testing, instrumentationName.get(), expectedKeys.toArray(new AttributeKey[0])); } + @Test + void wrongTypeErrorIsRecordedInSpanAndMetrics() throws IOException, InterruptedException { + assertThat( + redisServer.execInContainer("redis-cli", "lpush", "wrong-type", "value").getExitCode()) + .isZero(); + assertThatThrownBy(() -> redisson.getBucket("wrong-type").get()) + .isInstanceOf(RedisException.class); + + AtomicReference instrumentationName = new AtomicReference<>(); + testing.waitAndAssertTraces( + trace -> { + instrumentationName.set(trace.getSpan(0).getInstrumentationScopeInfo().getName()); + trace.hasSpansSatisfyingExactly( + span -> + span.hasName("GET " + address) + .hasKind(CLIENT) + .hasAttributesSatisfyingExactly( + equalTo(NETWORK_PEER_ADDRESS, ip), + equalTo(NETWORK_PEER_PORT, port), + equalTo(SERVER_ADDRESS, host), + equalTo(SERVER_PORT, port), + equalTo(DB_SYSTEM_NAME, REDIS), + equalTo(DB_NAMESPACE, dbNamespace("0")), + equalTo(DB_QUERY_TEXT, "GET wrong-type"), + equalTo(DB_OPERATION_NAME, "GET"), + equalTo(ERROR_TYPE, "WRONGTYPE"))); + }); + + testing.waitAndAssertMetrics( + instrumentationName.get(), + metrics -> + metrics + .hasName("db.client.operation.duration") + .hasHistogramSatisfying( + histogram -> + histogram.hasPointsSatisfying( + point -> + point.hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()) + .containsEntry(ERROR_TYPE, "WRONGTYPE"))))); + } + @Test void configuredDatabaseIndex() throws InvocationTargetException, IllegalAccessException { RedissonClient databaseOne = Redisson.create(createConfig(1, null)); From 36a4c0b4691679edffb2e398d0f3c0d59ce724d4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 20 Oct 2026 23:08:53 +0000 Subject: [PATCH 03/22] Capture Redisson server error replies Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .github/config/latest-dep-versions.json | 1 + .../v3_0/RedissonDbAttributesGetterTest.java | 91 +++++++++------ .../v3_0/RedisCommandDataInstrumentation.java | 13 +++ .../RedisCommandDecoderInstrumentation.java | 50 ++++++++ .../v3_0/RedissonInstrumentationModule.java | 1 + .../redisson-3.17/javaagent/build.gradle.kts | 1 + .../RedisCommandDataInstrumentation.java | 14 +++ .../RedisCommandDecoderInstrumentation.java | 50 ++++++++ .../v3_17/RedissonInstrumentationModule.java | 1 + .../common/v3_0/EndOperationListener.java | 6 +- .../v3_0/RedissonDbAttributesGetter.java | 47 +------- .../common/v3_0/RedissonErrorCapture.java | 107 ++++++++++++++++++ .../AbstractRedissonAsyncClientTest.java | 51 +++++++++ .../redisson/AbstractRedissonClientTest.java | 59 ++++++++++ 14 files changed, 411 insertions(+), 81 deletions(-) create mode 100644 instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDecoderInstrumentation.java create mode 100644 instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDecoderInstrumentation.java create mode 100644 instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java diff --git a/.github/config/latest-dep-versions.json b/.github/config/latest-dep-versions.json index b5edef4501a6..f7f0ddc46497 100644 --- a/.github/config/latest-dep-versions.json +++ b/.github/config/latest-dep-versions.json @@ -478,6 +478,7 @@ "org.playframework:play_3#+": "3.0.12", "org.quartz-scheduler:quartz#+": "2.5.2", "org.redisson:redisson#+": "4.8.0", + "org.redisson:redisson#3.+": "3.52.0", "org.redisson:redisson#3.11.+": "3.11.6", "org.redisson:redisson#3.16.+": "3.16.8", "org.redisson:redisson#3.17.+": "3.17.7", diff --git a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java index 6400827fe088..8caeefc5ae13 100644 --- a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java +++ b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java @@ -6,23 +6,20 @@ package io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0; import static io.opentelemetry.semconv.ErrorAttributes.ERROR_TYPE; +import static java.nio.charset.StandardCharsets.US_ASCII; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.params.provider.Arguments.argumentSet; +import io.netty.buffer.ByteBuf; +import io.netty.buffer.Unpooled; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientAttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; -import java.util.stream.Stream; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; import org.redisson.client.RedisConnectionException; import org.redisson.client.RedisException; import org.redisson.client.RedisLoadingException; -import org.redisson.client.RedisOutOfMemoryException; class RedissonDbAttributesGetterTest { @@ -32,27 +29,62 @@ class RedissonDbAttributesGetterTest { private static final AttributesExtractor EXTRACTOR = DbClientAttributesExtractor.create(GETTER); - @ParameterizedTest - @MethodSource("redisErrorMessages") - void extractsKnownRedisErrorTypes(String message, String expectedErrorType) { - assertThat(GETTER.getErrorType(REQUEST, null, new RedisException(message))) - .isEqualTo(expectedErrorType); + @Test + void capturesUnknownServerPrefixFromRespReply() { + ByteBuf response = Unpooled.copiedBuffer("-CUSTOM diagnostic details\r\n", US_ASCII); + String previous = + RedissonErrorCapture.currentServerErrorType() + .set(RedissonErrorCapture.parseServerErrorType(response)); + RedisException error = new RedisException("CUSTOM diagnostic details. channel: redis"); + try { + RedissonErrorCapture.captureServerError(error); + } finally { + RedissonErrorCapture.currentServerErrorType().restore(previous); + response.release(); + } + + assertThat(GETTER.getErrorType(REQUEST, null, error)).isEqualTo("CUSTOM"); + assertThat(extractedErrorType(error)).isEqualTo("CUSTOM"); + + RedissonErrorCapture.clearServerError(error); + assertThat(extractedErrorType(error)).isEqualTo(RedisException.class.getName()); + assertThat(RedissonErrorCapture.currentServerErrorType().get()).isNull(); + assertThat(extractedErrorType(new RedisException("CUSTOM other operation"))) + .isEqualTo(RedisException.class.getName()); } - private static Stream redisErrorMessages() { - return Stream.of( - argumentSet( - "generic error", - "ERR unknown command with variable details. channel: redis command: GET", - "ERR"), - argumentSet( - "wrong type", - "WRONGTYPE Operation against a key holding the wrong kind of value. channel: redis command: GET", - "WRONGTYPE"), - argumentSet("malformed prefix", "ERR", null), - argumentSet("unknown prefix", "CUSTOM variable details. channel: redis command: GET", null), - argumentSet("message without decoder marker", "ERR connection refused", null), - argumentSet("non-server error", "Unable to decode response. channel: redis", null)); + @Test + void malformedAndNonErrorRepliesDoNotProduceServerErrorType() { + ByteBuf success = Unpooled.copiedBuffer("+OK\r\n", US_ASCII); + try { + assertThat(RedissonErrorCapture.parseServerErrorType(success)).isNull(); + } finally { + success.release(); + } + + ByteBuf emptyPrefix = Unpooled.copiedBuffer("- \r\n", US_ASCII); + try { + assertThat(RedissonErrorCapture.parseServerErrorType(emptyPrefix)).isNull(); + } finally { + emptyPrefix.release(); + } + + ByteBuf incomplete = Unpooled.copiedBuffer("-CUSTOM", US_ASCII); + try { + assertThat(RedissonErrorCapture.parseServerErrorType(incomplete)).isNull(); + } finally { + incomplete.release(); + } + } + + @Test + void clientExceptionMessageIsNotClassifiedAsServerReply() { + RedisException error = new RedisException("ERR connection refused"); + + RedissonErrorCapture.captureServerError(error); + + assertThat(GETTER.getErrorType(REQUEST, null, error)).isNull(); + assertThat(extractedErrorType(error)).isEqualTo(RedisException.class.getName()); } @Test @@ -69,15 +101,6 @@ void dedicatedExceptionKeepsItsExceptionClassFallback() { assertThat(extractedErrorType(error)).isEqualTo(RedisLoadingException.class.getName()); } - @Test - void dedicatedExceptionWithRemovedPrefixKeepsItsExceptionClassFallback() { - RedisOutOfMemoryException error = - new RedisOutOfMemoryException("maxmemory threshold exceeded. channel: redis command: SET"); - - assertThat(GETTER.getErrorType(REQUEST, null, error)).isNull(); - assertThat(extractedErrorType(error)).isEqualTo(RedisOutOfMemoryException.class.getName()); - } - @Test void connectionExceptionIsNotClassifiedAsServerError() { RedisConnectionException error = new RedisConnectionException("ERR connection refused"); diff --git a/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDataInstrumentation.java b/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDataInstrumentation.java index 2d974b1da8e8..baece4e3daa7 100644 --- a/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDataInstrumentation.java +++ b/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDataInstrumentation.java @@ -13,6 +13,7 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.CompletableFutureWrapper; +import io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.RedissonErrorCapture; import java.util.concurrent.CompletableFuture; import net.bytebuddy.asm.Advice; import net.bytebuddy.asm.Advice.AssignReturned; @@ -39,6 +40,9 @@ public void transform(TypeTransformer transformer) { transformer.applyAdviceToMethod( isConstructor().and(takesArgument(0, CompletableFuture.class)), getClass().getName() + "$WrapCompletableFutureAdvice"); + transformer.applyAdviceToMethod( + named("tryFailure").and(takesArgument(0, Throwable.class)), + getClass().getName() + "$CaptureErrorAdvice"); } @SuppressWarnings("unused") @@ -61,4 +65,13 @@ public static CompletableFuture onEnter( return CompletableFutureWrapper.wrap(completableFuture); } } + + @SuppressWarnings("unused") + public static class CaptureErrorAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + public static void onEnter(@Advice.Argument(0) Throwable error) { + RedissonErrorCapture.captureServerError(error); + } + } } diff --git a/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDecoderInstrumentation.java b/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDecoderInstrumentation.java new file mode 100644 index 000000000000..77e9220c5d30 --- /dev/null +++ b/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDecoderInstrumentation.java @@ -0,0 +1,50 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.redisson.v3_0; + +import static io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.RedissonErrorCapture.currentServerErrorType; +import static io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.RedissonErrorCapture.parseServerErrorType; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import io.netty.buffer.ByteBuf; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import javax.annotation.Nullable; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +class RedisCommandDecoderInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named("org.redisson.client.handler.CommandDecoder"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + named("decode") + .and(takesArgument(0, named("io.netty.buffer.ByteBuf"))) + .and(takesArgument(1, named("org.redisson.client.protocol.CommandData"))), + getClass().getName() + "$DecodeAdvice"); + } + + @SuppressWarnings("unused") + public static class DecodeAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + public static String onEnter(@Advice.Argument(0) ByteBuf input) { + return currentServerErrorType().set(parseServerErrorType(input)); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false) + public static void onExit(@Advice.Enter @Nullable String previousErrorType) { + currentServerErrorType().restore(previousErrorType); + } + } +} diff --git a/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedissonInstrumentationModule.java b/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedissonInstrumentationModule.java index 6acbe22829e6..b00eae9fe7ac 100644 --- a/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedissonInstrumentationModule.java +++ b/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedissonInstrumentationModule.java @@ -35,6 +35,7 @@ public List typeInstrumentations() { new ConnectionManagerConnectionFutureInstrumentation(), new MasterSlaveConnectionManagerInstrumentation(), new RedisConnectionInstrumentation(), + new RedisCommandDecoderInstrumentation(), new RedisCommandDataInstrumentation()); } diff --git a/instrumentation/redisson/redisson-3.17/javaagent/build.gradle.kts b/instrumentation/redisson/redisson-3.17/javaagent/build.gradle.kts index 0dfb3b955637..d0d34a5c7ba4 100644 --- a/instrumentation/redisson/redisson-3.17/javaagent/build.gradle.kts +++ b/instrumentation/redisson/redisson-3.17/javaagent/build.gradle.kts @@ -13,6 +13,7 @@ muzzle { dependencies { library("org.redisson:redisson:3.17.0") + latestDepTestLibrary("org.redisson:redisson:3.+") implementation(project(":instrumentation:redisson:redisson-common-3.0:javaagent")) diff --git a/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDataInstrumentation.java b/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDataInstrumentation.java index 46ffda2f4853..153a70df457f 100644 --- a/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDataInstrumentation.java +++ b/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDataInstrumentation.java @@ -6,12 +6,14 @@ package io.opentelemetry.javaagent.instrumentation.redisson.v3_17; import static net.bytebuddy.matcher.ElementMatchers.isConstructor; +import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.CompletableFutureWrapper; +import io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.RedissonErrorCapture; import java.util.concurrent.CompletableFuture; import net.bytebuddy.asm.Advice; import net.bytebuddy.asm.Advice.AssignReturned; @@ -32,6 +34,9 @@ public void transform(TypeTransformer transformer) { transformer.applyAdviceToMethod( isConstructor().and(takesArgument(0, CompletableFuture.class)), getClass().getName() + "$WrapCompletableFutureAdvice"); + transformer.applyAdviceToMethod( + named("tryFailure").and(takesArgument(0, Throwable.class)), + getClass().getName() + "$CaptureErrorAdvice"); } @SuppressWarnings("unused") @@ -44,4 +49,13 @@ public static CompletableFuture onEnter( return CompletableFutureWrapper.wrap(completableFuture); } } + + @SuppressWarnings("unused") + public static class CaptureErrorAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + public static void onEnter(@Advice.Argument(0) Throwable error) { + RedissonErrorCapture.captureServerError(error); + } + } } diff --git a/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDecoderInstrumentation.java b/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDecoderInstrumentation.java new file mode 100644 index 000000000000..ebd9faa76787 --- /dev/null +++ b/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDecoderInstrumentation.java @@ -0,0 +1,50 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.redisson.v3_17; + +import static io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.RedissonErrorCapture.currentServerErrorType; +import static io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.RedissonErrorCapture.parseServerErrorType; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import io.netty.buffer.ByteBuf; +import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import javax.annotation.Nullable; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +class RedisCommandDecoderInstrumentation implements TypeInstrumentation { + + @Override + public ElementMatcher typeMatcher() { + return named("org.redisson.client.handler.CommandDecoder"); + } + + @Override + public void transform(TypeTransformer transformer) { + transformer.applyAdviceToMethod( + named("decode") + .and(takesArgument(0, named("io.netty.buffer.ByteBuf"))) + .and(takesArgument(1, named("org.redisson.client.protocol.CommandData"))), + getClass().getName() + "$DecodeAdvice"); + } + + @SuppressWarnings("unused") + public static class DecodeAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) + public static String onEnter(@Advice.Argument(0) ByteBuf input) { + return currentServerErrorType().set(parseServerErrorType(input)); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false) + public static void onExit(@Advice.Enter @Nullable String previousErrorType) { + currentServerErrorType().restore(previousErrorType); + } + } +} diff --git a/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedissonInstrumentationModule.java b/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedissonInstrumentationModule.java index 6ca9070d4c33..494de26f5c86 100644 --- a/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedissonInstrumentationModule.java +++ b/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedissonInstrumentationModule.java @@ -34,6 +34,7 @@ public List typeInstrumentations() { new MasterSlaveConnectionManagerInstrumentation(), new RedisExecutorConnectionFutureInstrumentation(), new RedisConnectionInstrumentation(), + new RedisCommandDecoderInstrumentation(), new RedisCommandDataInstrumentation()); } diff --git a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/EndOperationListener.java b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/EndOperationListener.java index 6d252a995098..937b1374e3d4 100644 --- a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/EndOperationListener.java +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/EndOperationListener.java @@ -24,6 +24,10 @@ public EndOperationListener( @Override public void accept(@Nullable T unused, @Nullable Throwable error) { - instrumenter.end(context, request, null, error); + try { + instrumenter.end(context, request, null, error); + } finally { + RedissonErrorCapture.clearServerError(error); + } } } diff --git a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetter.java b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetter.java index 4649656fe696..7493376cfc30 100644 --- a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetter.java +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetter.java @@ -10,7 +10,6 @@ import io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DbSystemNameIncubatingValues; import java.net.InetSocketAddress; import javax.annotation.Nullable; -import org.redisson.client.RedisException; final class RedissonDbAttributesGetter implements DbClientAttributesGetter { @@ -48,51 +47,7 @@ public Long getDbOperationBatchSize(RedissonRequest request) { @Nullable public String getErrorType( RedissonRequest request, @Nullable Void response, @Nullable Throwable error) { - if (error == null || error.getClass() != RedisException.class) { - return null; - } - - String message = error.getMessage(); - if (message == null) { - return null; - } - - int serverErrorMarker = message.indexOf(". channel:"); - if (serverErrorMarker < 0) { - return null; - } - - int separator = message.indexOf(' '); - if (separator <= 0 || separator >= serverErrorMarker) { - return null; - } - - String errorType = message.substring(0, separator); - switch (errorType) { - case "ERR": - case "WRONGTYPE": - case "EXECABORT": - case "CROSSSLOT": - case "MASTERDOWN": - case "NOREPLICAS": - case "READONLY": - case "MISCONF": - case "NOPERM": - case "NOSCRIPT": - case "NOAUTH": - case "WRONGPASS": - case "MOVED": - case "ASK": - case "TRYAGAIN": - case "LOADING": - case "OOM": - case "CLUSTERDOWN": - case "BUSY": - case "WAIT": - return errorType; - default: - return null; - } + return RedissonErrorCapture.getServerErrorType(error); } @Nullable diff --git a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java new file mode 100644 index 000000000000..9ff7d9d5009e --- /dev/null +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java @@ -0,0 +1,107 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0; + +import static java.nio.charset.StandardCharsets.US_ASCII; + +import io.netty.buffer.ByteBuf; +import io.opentelemetry.instrumentation.api.internal.ScopedThreadValue; +import io.opentelemetry.instrumentation.api.util.VirtualField; +import javax.annotation.Nullable; +import org.redisson.client.RedisException; + +/** + * Captures the first token from RESP error replies before Redisson completes the command promise. + * + *

Redisson's {@code CommandDecoder} calls {@code CommandData.tryFailure} while decoding a {@code + * -} reply, so the token is scoped to that decoder frame and attached to the resulting generic + * exception before completion callbacks can run. The operation listener clears it after extracting + * telemetry; all other errors use the shared exception-class fallback. + * + * @see RESP + * specification + * @see Redisson + * CommandDecoder + */ +public final class RedissonErrorCapture { + + private static final ScopedThreadValue currentServerErrorType = new ScopedThreadValue<>(); + + private static final VirtualField SERVER_ERROR = + VirtualField.find(RedisException.class, RedisServerError.class); + + public static ScopedThreadValue currentServerErrorType() { + return currentServerErrorType; + } + + @Nullable + public static String parseServerErrorType(ByteBuf input) { + try { + int readerIndex = input.readerIndex(); + if (input.getByte(readerIndex) != '-') { + return null; + } + + int start = readerIndex + 1; + int end = start; + byte next; + do { + next = input.getByte(end++); + } while (next != ' ' && next != '\t' && next != '\r' && next != '\n'); + + int length = end - start - 1; + if (length == 0) { + return null; + } + + byte[] token = new byte[length]; + input.getBytes(start, token); + return new String(token, US_ASCII); + } catch (Throwable ignored) { + // ReplayingDecoder retries incomplete replies; leave buffer handling to Redisson. + return null; + } + } + + public static void captureServerError(@Nullable Throwable error) { + if (error == null || error.getClass() != RedisException.class) { + return; + } + + String errorType = currentServerErrorType.get(); + if (errorType != null) { + SERVER_ERROR.set((RedisException) error, new RedisServerError(errorType)); + } + } + + @Nullable + public static String getServerErrorType(@Nullable Throwable error) { + if (error == null || error.getClass() != RedisException.class) { + return null; + } + + RedisServerError redisServerError = SERVER_ERROR.get((RedisException) error); + return redisServerError != null ? redisServerError.errorType : null; + } + + public static void clearServerError(@Nullable Throwable error) { + if (error != null && error.getClass() == RedisException.class) { + SERVER_ERROR.set((RedisException) error, null); + } + } + + private static final class RedisServerError { + private final String errorType; + + private RedisServerError(String errorType) { + this.errorType = errorType; + } + } + + private RedissonErrorCapture() {} +} diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java index 9abf6eff74d9..169395decb1b 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java @@ -14,13 +14,16 @@ import static io.opentelemetry.semconv.DbAttributes.DB_OPERATION_NAME; import static io.opentelemetry.semconv.DbAttributes.DB_QUERY_TEXT; import static io.opentelemetry.semconv.DbAttributes.DB_SYSTEM_NAME; +import static io.opentelemetry.semconv.ErrorAttributes.ERROR_TYPE; import static io.opentelemetry.semconv.NetworkAttributes.NETWORK_PEER_ADDRESS; import static io.opentelemetry.semconv.NetworkAttributes.NETWORK_PEER_PORT; import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS; import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DbSystemNameIncubatingValues.REDIS; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.awaitility.Awaitility.await; import io.opentelemetry.api.trace.Span; @@ -37,6 +40,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assumptions; @@ -54,8 +58,10 @@ import org.redisson.api.RFuture; import org.redisson.api.RScheduledExecutorService; import org.redisson.api.RScheduledFuture; +import org.redisson.api.RScript; import org.redisson.api.RSet; import org.redisson.api.RedissonClient; +import org.redisson.client.RedisException; import org.redisson.client.codec.StringCodec; import org.redisson.config.Config; import org.redisson.config.SingleServerConfig; @@ -103,6 +109,12 @@ void setup(TestInfo testInfo) throws InvocationTargetException, IllegalAccessExc newAddress = "redis://" + address; } Config config = new Config(); + try { + // script cache is enabled by default in 3.46.0 + Config.class.getMethod("setUseScriptCache", boolean.class).invoke(config, false); + } catch (NoSuchMethodException ignored) { + // ignored + } SingleServerConfig singleServerConfig = config.useSingleServer(); singleServerConfig.setAddress(newAddress); singleServerConfig.setTimeout(30_000); @@ -164,6 +176,45 @@ void futureSet() { equalTo(DB_OPERATION_NAME, "SET")))); } + @Test + void asyncCustomServerErrorIsRecordedInSpanAndMetrics() { + RFuture future = + redisson + .getScript() + .evalAsync( + RScript.Mode.READ_WRITE, + "return redis.error_reply('CUSTOM diagnostic details')", + RScript.ReturnType.VALUE); + assertThatThrownBy(() -> future.toCompletableFuture().get(TIMEOUT.toMillis(), MILLISECONDS)) + .hasCauseInstanceOf(RedisException.class); + + AtomicReference instrumentationName = new AtomicReference<>(); + testing.waitAndAssertTraces( + trace -> { + instrumentationName.set(trace.getSpan(0).getInstrumentationScopeInfo().getName()); + trace.hasSpansSatisfyingExactly( + span -> + span.hasKind(CLIENT) + .hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()).containsEntry(ERROR_TYPE, "CUSTOM"))); + }); + + testing.waitAndAssertMetrics( + instrumentationName.get(), + metrics -> + metrics + .hasName("db.client.operation.duration") + .hasHistogramSatisfying( + histogram -> + histogram.hasPointsSatisfying( + point -> + point.hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()) + .containsEntry(ERROR_TYPE, "CUSTOM"))))); + } + @Test @Tag(TEST_RECONNECT) void futureCallback() { diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index f974cff7243d..8231d097d391 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -71,6 +71,8 @@ import org.redisson.api.RSet; import org.redisson.api.RedissonClient; import org.redisson.client.RedisException; +import org.redisson.client.codec.StringCodec; +import org.redisson.client.protocol.Decoder; import org.redisson.config.Config; import org.redisson.config.SingleServerConfig; import org.slf4j.Logger; @@ -285,6 +287,63 @@ void wrongTypeErrorIsRecordedInSpanAndMetrics() throws IOException, InterruptedE .containsEntry(ERROR_TYPE, "WRONGTYPE"))))); } + @Test + void clientSideDecodeFailureKeepsExceptionClassFallback() + throws IOException, InterruptedException { + assertThat( + redisServer + .execInContainer("redis-cli", "set", "client-side-decode-error", "value") + .getExitCode()) + .isZero(); + RBucket bucket = + redisson.getBucket( + "client-side-decode-error", + new StringCodec() { + @Override + public Decoder getValueDecoder() { + return (buffer, state) -> { + throw new RedisException("ERR client-side decode failure"); + }; + } + }); + assertThatThrownBy(bucket::get).isInstanceOf(RedisException.class); + + AtomicReference instrumentationName = new AtomicReference<>(); + testing.waitAndAssertTraces( + trace -> { + instrumentationName.set(trace.getSpan(0).getInstrumentationScopeInfo().getName()); + trace.hasSpansSatisfyingExactly( + span -> + span.hasName("GET " + address) + .hasKind(CLIENT) + .hasAttributesSatisfyingExactly( + equalTo(NETWORK_PEER_ADDRESS, ip), + equalTo(NETWORK_PEER_PORT, port), + equalTo(SERVER_ADDRESS, host), + equalTo(SERVER_PORT, port), + equalTo(DB_SYSTEM_NAME, REDIS), + equalTo(DB_NAMESPACE, dbNamespace("0")), + equalTo(DB_QUERY_TEXT, "GET client-side-decode-error"), + equalTo(DB_OPERATION_NAME, "GET"), + equalTo(ERROR_TYPE, RedisException.class.getName()))); + }); + + testing.waitAndAssertMetrics( + instrumentationName.get(), + metrics -> + metrics + .hasName("db.client.operation.duration") + .hasHistogramSatisfying( + histogram -> + histogram.hasPointsSatisfying( + point -> + point.hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()) + .containsEntry( + ERROR_TYPE, RedisException.class.getName()))))); + } + @Test void configuredDatabaseIndex() throws InvocationTargetException, IllegalAccessException { RedissonClient databaseOne = Redisson.create(createConfig(1, null)); From d88c5979d57b004f41e25d4e56ba9dfd6ec4ef3e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 20 Oct 2026 23:14:51 +0000 Subject: [PATCH 04/22] Document Redisson latest test range Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../redisson/redisson-3.17/javaagent/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/redisson/redisson-3.17/javaagent/build.gradle.kts b/instrumentation/redisson/redisson-3.17/javaagent/build.gradle.kts index d0d34a5c7ba4..ab79f71a9a95 100644 --- a/instrumentation/redisson/redisson-3.17/javaagent/build.gradle.kts +++ b/instrumentation/redisson/redisson-3.17/javaagent/build.gradle.kts @@ -13,7 +13,7 @@ muzzle { dependencies { library("org.redisson:redisson:3.17.0") - latestDepTestLibrary("org.redisson:redisson:3.+") + latestDepTestLibrary("org.redisson:redisson:3.+") // documented limitation: Redisson 4.x is not covered by this test implementation(project(":instrumentation:redisson:redisson-common-3.0:javaagent")) From 8ea80793ae18a59e319e246f25870b71e5d330ed Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Oct 2026 02:02:28 +0000 Subject: [PATCH 05/22] Restore Redisson latest release coverage Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .github/config/latest-dep-versions.json | 1 - .../redisson/redisson-3.17/javaagent/build.gradle.kts | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/config/latest-dep-versions.json b/.github/config/latest-dep-versions.json index f7f0ddc46497..b5edef4501a6 100644 --- a/.github/config/latest-dep-versions.json +++ b/.github/config/latest-dep-versions.json @@ -478,7 +478,6 @@ "org.playframework:play_3#+": "3.0.12", "org.quartz-scheduler:quartz#+": "2.5.2", "org.redisson:redisson#+": "4.8.0", - "org.redisson:redisson#3.+": "3.52.0", "org.redisson:redisson#3.11.+": "3.11.6", "org.redisson:redisson#3.16.+": "3.16.8", "org.redisson:redisson#3.17.+": "3.17.7", diff --git a/instrumentation/redisson/redisson-3.17/javaagent/build.gradle.kts b/instrumentation/redisson/redisson-3.17/javaagent/build.gradle.kts index ab79f71a9a95..0dfb3b955637 100644 --- a/instrumentation/redisson/redisson-3.17/javaagent/build.gradle.kts +++ b/instrumentation/redisson/redisson-3.17/javaagent/build.gradle.kts @@ -13,7 +13,6 @@ muzzle { dependencies { library("org.redisson:redisson:3.17.0") - latestDepTestLibrary("org.redisson:redisson:3.+") // documented limitation: Redisson 4.x is not covered by this test implementation(project(":instrumentation:redisson:redisson-common-3.0:javaagent")) From 748681d46d1a05782cdd2ee41ef1b0f4c0aafd25 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Oct 2026 04:20:04 +0000 Subject: [PATCH 06/22] Handle dedicated Redisson server errors Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../v3_0/RedissonDbAttributesGetterTest.java | 61 +++++++- .../common/v3_0/RedissonErrorCapture.java | 32 ++-- .../AbstractRedissonAsyncClientTest.java | 137 ++++++++++++++++++ .../redisson/AbstractRedissonClientTest.java | 102 +++++++++++++ 4 files changed, 320 insertions(+), 12 deletions(-) diff --git a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java index 8caeefc5ae13..592912a148fe 100644 --- a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java +++ b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java @@ -16,6 +16,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientAttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import java.util.Collections; import org.junit.jupiter.api.Test; import org.redisson.client.RedisConnectionException; import org.redisson.client.RedisException; @@ -75,6 +76,39 @@ void malformedAndNonErrorRepliesDoNotProduceServerErrorType() { } finally { incomplete.release(); } + + ByteBuf incompleteAfterToken = Unpooled.copiedBuffer("-CUSTOM details", US_ASCII); + try { + assertThat(RedissonErrorCapture.parseServerErrorType(incompleteAfterToken)).isNull(); + } finally { + incompleteAfterToken.release(); + } + + ByteBuf emptyReply = Unpooled.copiedBuffer("-\r\n", US_ASCII); + try { + assertThat(RedissonErrorCapture.parseServerErrorType(emptyReply)).isNull(); + } finally { + emptyReply.release(); + } + + ByteBuf emptyToken = Unpooled.copiedBuffer("- \r\n", US_ASCII); + try { + assertThat(RedissonErrorCapture.parseServerErrorType(emptyToken)).isNull(); + } finally { + emptyToken.release(); + } + } + + @Test + void parsesFirstNonemptyAsciiSpaceDelimitedTokenVerbatim() { + assertThat(parse("-Custom.Code! diagnostic details\r\n")).isEqualTo("Custom.Code!"); + assertThat(parse("-lowercase\r\n")).isEqualTo("lowercase"); + assertThat(parse("-prefix-only\r\n")).isEqualTo("prefix-only"); + assertThat(parse("- Leading.Code! diagnostic\r\n")).isEqualTo("Leading.Code!"); + assertThat(parse("-Tab.Code!\tdiagnostic\r\n")).isEqualTo("Tab.Code!\tdiagnostic"); + + String longPrefix = String.join("", Collections.nCopies(80, "x")); + assertThat(parse("-" + longPrefix + " diagnostic\r\n")).isEqualTo(longPrefix); } @Test @@ -94,11 +128,25 @@ void missingMessageHasNoErrorType() { } @Test - void dedicatedExceptionKeepsItsExceptionClassFallback() { + void dedicatedExceptionUsesCapturedServerPrefixAndClearsIt() { RedisLoadingException error = new RedisLoadingException("LOADING Redis is loading"); + ByteBuf response = Unpooled.copiedBuffer("-LOADING Redis is loading\r\n", US_ASCII); + String previous = + RedissonErrorCapture.currentServerErrorType() + .set(RedissonErrorCapture.parseServerErrorType(response)); + try { + RedissonErrorCapture.captureServerError(error); + } finally { + RedissonErrorCapture.currentServerErrorType().restore(previous); + response.release(); + } - assertThat(GETTER.getErrorType(REQUEST, null, error)).isNull(); + assertThat(GETTER.getErrorType(REQUEST, null, error)).isEqualTo("LOADING"); + assertThat(extractedErrorType(error)).isEqualTo("LOADING"); + + RedissonErrorCapture.clearServerError(error); assertThat(extractedErrorType(error)).isEqualTo(RedisLoadingException.class.getName()); + assertThat(RedissonErrorCapture.currentServerErrorType().get()).isNull(); } @Test @@ -114,4 +162,13 @@ private static String extractedErrorType(Throwable error) { EXTRACTOR.onEnd(attributes, Context.root(), REQUEST, null, error); return attributes.build().get(ERROR_TYPE); } + + private static String parse(String reply) { + ByteBuf response = Unpooled.copiedBuffer(reply, US_ASCII); + try { + return RedissonErrorCapture.parseServerErrorType(response); + } finally { + response.release(); + } + } } diff --git a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java index 9ff7d9d5009e..46a3986581d8 100644 --- a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java @@ -48,17 +48,29 @@ public static String parseServerErrorType(ByteBuf input) { } int start = readerIndex + 1; - int end = start; - byte next; - do { - next = input.getByte(end++); - } while (next != ' ' && next != '\t' && next != '\r' && next != '\n'); + int writerIndex = input.writerIndex(); + int replyEnd = start; + while (replyEnd + 1 < writerIndex + && (input.getByte(replyEnd) != '\r' || input.getByte(replyEnd + 1) != '\n')) { + replyEnd++; + } + if (replyEnd + 1 >= writerIndex) { + return null; + } - int length = end - start - 1; - if (length == 0) { + while (start < replyEnd && input.getByte(start) == ' ') { + start++; + } + if (start == replyEnd) { return null; } + int end = start; + while (end < replyEnd && input.getByte(end) != ' ') { + end++; + } + + int length = end - start; byte[] token = new byte[length]; input.getBytes(start, token); return new String(token, US_ASCII); @@ -69,7 +81,7 @@ public static String parseServerErrorType(ByteBuf input) { } public static void captureServerError(@Nullable Throwable error) { - if (error == null || error.getClass() != RedisException.class) { + if (!(error instanceof RedisException)) { return; } @@ -81,7 +93,7 @@ public static void captureServerError(@Nullable Throwable error) { @Nullable public static String getServerErrorType(@Nullable Throwable error) { - if (error == null || error.getClass() != RedisException.class) { + if (!(error instanceof RedisException)) { return null; } @@ -90,7 +102,7 @@ public static String getServerErrorType(@Nullable Throwable error) { } public static void clearServerError(@Nullable Throwable error) { - if (error != null && error.getClass() == RedisException.class) { + if (error instanceof RedisException) { SERVER_ERROR.set((RedisException) error, null); } } diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java index 169395decb1b..bfd370fd08ac 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java @@ -20,6 +20,7 @@ import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS; import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DbSystemNameIncubatingValues.REDIS; +import static java.util.Collections.nCopies; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; @@ -62,7 +63,9 @@ import org.redisson.api.RSet; import org.redisson.api.RedissonClient; import org.redisson.client.RedisException; +import org.redisson.client.RedisOutOfMemoryException; import org.redisson.client.codec.StringCodec; +import org.redisson.client.protocol.Decoder; import org.redisson.config.Config; import org.redisson.config.SingleServerConfig; import org.testcontainers.containers.GenericContainer; @@ -215,6 +218,85 @@ void asyncCustomServerErrorIsRecordedInSpanAndMetrics() { .containsEntry(ERROR_TYPE, "CUSTOM"))))); } + @Test + void asyncServerErrorPrefixContract() throws IOException, InterruptedException { + assertThat( + redisServer + .execInContainer("redis-cli", "lpush", "async-wrong-type", "value") + .getExitCode()) + .isZero(); + assertThatThrownBy(() -> redisson.getBucket("async-wrong-type").getAsync().join()) + .hasRootCauseInstanceOf(RedisException.class); + assertErrorTypeOnLastOperation("WRONGTYPE"); + + assertScriptErrorType("OOM command not allowed", "OOM", RedisOutOfMemoryException.class); + assertScriptErrorType("Custom.Code! diagnostic details", "Custom.Code!", null); + assertScriptErrorType("lowercase diagnostic", "lowercase", null); + assertScriptErrorType("Tab.Code!\tdiagnostic", "Tab.Code!\tdiagnostic", null); + String longPrefix = String.join("", nCopies(80, "x")); + assertScriptErrorType(longPrefix + " diagnostic", longPrefix, null); + assertScriptErrorType("prefix-only", "prefix-only", null); + assertScriptErrorType(" Leading.Code! diagnostic", "Leading.Code!", null); + assertScriptErrorType("", RedisException.class.getName(), null); + assertScriptErrorType(" ", RedisException.class.getName(), null); + + assertThat( + redisson + .getScript() + .evalAsync(RScript.Mode.READ_ONLY, "return 1", RScript.ReturnType.INTEGER) + .toCompletableFuture() + .join()) + .isEqualTo(1L); + assertErrorTypeOnLastOperation(null); + } + + @Test + void asyncFailedBatchErrorPrefixIsRecordedInSpanAndMetrics() { + try { + Class.forName("org.redisson.api.BatchOptions"); + } catch (ClassNotFoundException ignored) { + Assumptions.abort(); + } + + RBatch batch = redisson.createBatch(BatchOptions.defaults()); + batch + .getScript() + .evalAsync( + RScript.Mode.READ_WRITE, + "return redis.error_reply('Batch.Code! diagnostic')", + RScript.ReturnType.VALUE); + + assertThatThrownBy(() -> batch.executeAsync().toCompletableFuture().join()) + .hasRootCauseInstanceOf(RedisException.class); + + assertErrorTypeOnLastOperation("Batch.Code!"); + } + + @Test + void asyncClientSideDecodeFailureKeepsExceptionClassFallback() + throws IOException, InterruptedException { + assertThat( + redisServer + .execInContainer("redis-cli", "set", "async-client-side-decode-error", "value") + .getExitCode()) + .isZero(); + RBucket bucket = + redisson.getBucket( + "async-client-side-decode-error", + new StringCodec() { + @Override + public Decoder getValueDecoder() { + return (buffer, state) -> { + throw new RedisException("ERR client-side decode failure"); + }; + } + }); + assertThatThrownBy(() -> bucket.getAsync().toCompletableFuture().join()) + .hasRootCauseInstanceOf(RedisException.class); + + assertErrorTypeOnLastOperation(RedisException.class.getName()); + } + @Test @Tag(TEST_RECONNECT) void futureCallback() { @@ -253,6 +335,61 @@ void futureCallback() { span -> span.hasName("callback").hasKind(INTERNAL).hasParent(trace.getSpan(0)))); } + private void assertScriptErrorType( + String serverError, String expectedErrorType, Class expectedCause) { + String script = "return redis.error_reply('" + serverError + "')"; + RFuture future = + redisson.getScript().evalAsync(RScript.Mode.READ_WRITE, script, RScript.ReturnType.VALUE); + assertThatThrownBy(() -> future.toCompletableFuture().join()) + .satisfies( + error -> { + if (expectedCause != null) { + assertThat(error).hasRootCauseInstanceOf(expectedCause); + } + }); + assertErrorTypeOnLastOperation(expectedErrorType); + } + + private static void assertErrorTypeOnLastOperation(String expectedErrorType) { + AtomicReference instrumentationName = new AtomicReference<>(); + testing.waitAndAssertTraces( + trace -> { + instrumentationName.set(trace.getSpan(0).getInstrumentationScopeInfo().getName()); + trace.hasSpansSatisfyingExactly( + span -> + span.hasKind(CLIENT) + .hasAttributesSatisfying( + attributes -> { + if (expectedErrorType == null) { + assertThat(attributes.asMap()).doesNotContainKey(ERROR_TYPE); + } else { + assertThat(attributes.asMap()) + .containsEntry(ERROR_TYPE, expectedErrorType); + } + })); + }); + testing.waitAndAssertMetrics( + instrumentationName.get(), + metrics -> + metrics + .hasName("db.client.operation.duration") + .hasHistogramSatisfying( + histogram -> + histogram.hasPointsSatisfying( + point -> + point.hasAttributesSatisfying( + attributes -> { + if (expectedErrorType == null) { + assertThat(attributes.asMap()) + .doesNotContainKey(ERROR_TYPE); + } else { + assertThat(attributes.asMap()) + .containsEntry(ERROR_TYPE, expectedErrorType); + } + })))); + testing.clearData(); + } + // regression test for // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/19332 @Test diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index 8231d097d391..c9f975d28e6f 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -68,9 +68,11 @@ import org.redisson.api.RLock; import org.redisson.api.RMap; import org.redisson.api.RScoredSortedSet; +import org.redisson.api.RScript; import org.redisson.api.RSet; import org.redisson.api.RedissonClient; import org.redisson.client.RedisException; +import org.redisson.client.RedisOutOfMemoryException; import org.redisson.client.codec.StringCodec; import org.redisson.client.protocol.Decoder; import org.redisson.config.Config; @@ -287,6 +289,46 @@ void wrongTypeErrorIsRecordedInSpanAndMetrics() throws IOException, InterruptedE .containsEntry(ERROR_TYPE, "WRONGTYPE"))))); } + @Test + void serverErrorPrefixContract() { + assertScriptErrorType("OOM command not allowed", "OOM", RedisOutOfMemoryException.class); + assertScriptErrorType("Custom.Code! diagnostic details", "Custom.Code!", null); + assertScriptErrorType("lowercase diagnostic", "lowercase", null); + assertScriptErrorType("Tab.Code!\tdiagnostic", "Tab.Code!\tdiagnostic", null); + String longPrefix = String.join("", nCopies(80, "x")); + assertScriptErrorType(longPrefix + " diagnostic", longPrefix, null); + assertScriptErrorType("prefix-only", "prefix-only", null); + assertScriptErrorType(" Leading.Code! diagnostic", "Leading.Code!", null); + assertScriptErrorType("", RedisException.class.getName(), null); + assertScriptErrorType(" ", RedisException.class.getName(), null); + + Object result = + redisson.getScript().eval(RScript.Mode.READ_ONLY, "return 1", RScript.ReturnType.INTEGER); + assertThat(result).isEqualTo(1L); + assertErrorTypeOnLastOperation(null); + } + + @Test + void failedBatchErrorPrefixIsRecordedInSpanAndMetrics() throws ReflectiveOperationException { + try { + Class.forName("org.redisson.api.BatchOptions"); + } catch (ClassNotFoundException ignored) { + Assumptions.abort(); + } + + RBatch batch = createBatch(redisson); + batch + .getScript() + .evalAsync( + RScript.Mode.READ_WRITE, + "return redis.error_reply('Batch.Code! diagnostic')", + RScript.ReturnType.VALUE); + + assertThatThrownBy(() -> invokeExecute(batch)).hasRootCauseInstanceOf(RedisException.class); + + assertErrorTypeOnLastOperation("Batch.Code!"); + } + @Test void clientSideDecodeFailureKeepsExceptionClassFallback() throws IOException, InterruptedException { @@ -344,6 +386,66 @@ public Decoder getValueDecoder() { ERROR_TYPE, RedisException.class.getName()))))); } + private void assertScriptErrorType( + String serverError, String expectedErrorType, Class expectedCause) { + String script = "return redis.error_reply('" + serverError + "')"; + assertThatThrownBy( + () -> + redisson + .getScript() + .eval(RScript.Mode.READ_WRITE, script, RScript.ReturnType.VALUE)) + .satisfies( + error -> { + if (expectedCause != null) { + assertThat(error) + .satisfiesAnyOf( + e -> assertThat(e).isInstanceOf(expectedCause), + e -> assertThat(e).hasRootCauseInstanceOf(expectedCause)); + } + }); + assertErrorTypeOnLastOperation(expectedErrorType); + } + + private static void assertErrorTypeOnLastOperation(String expectedErrorType) { + AtomicReference instrumentationName = new AtomicReference<>(); + testing.waitAndAssertTraces( + trace -> { + instrumentationName.set(trace.getSpan(0).getInstrumentationScopeInfo().getName()); + trace.hasSpansSatisfyingExactly( + span -> + span.hasKind(CLIENT) + .hasAttributesSatisfying( + attributes -> { + if (expectedErrorType == null) { + assertThat(attributes.asMap()).doesNotContainKey(ERROR_TYPE); + } else { + assertThat(attributes.asMap()) + .containsEntry(ERROR_TYPE, expectedErrorType); + } + })); + }); + testing.waitAndAssertMetrics( + instrumentationName.get(), + metrics -> + metrics + .hasName("db.client.operation.duration") + .hasHistogramSatisfying( + histogram -> + histogram.hasPointsSatisfying( + point -> + point.hasAttributesSatisfying( + attributes -> { + if (expectedErrorType == null) { + assertThat(attributes.asMap()) + .doesNotContainKey(ERROR_TYPE); + } else { + assertThat(attributes.asMap()) + .containsEntry(ERROR_TYPE, expectedErrorType); + } + })))); + testing.clearData(); + } + @Test void configuredDatabaseIndex() throws InvocationTargetException, IllegalAccessException { RedissonClient databaseOne = Redisson.create(createConfig(1, null)); From c66ccc70cc8ccc3c3449804652e10cdcbae66538 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 20 Oct 2026 23:06:30 -0700 Subject: [PATCH 07/22] Preserve Redisson error prefixes across supported versions Exercise failed batches on baseline clients and avoid removed APIs in latest dependency tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../v3_0/RedissonDbAttributesGetterTest.java | 17 ++++++- .../v3_0/RedissonAsyncClientTest.java | 9 +++- .../common/v3_0/RedissonErrorCapture.java | 6 +-- .../AbstractRedissonAsyncClientTest.java | 45 +++++++++++-------- .../redisson/AbstractRedissonClientTest.java | 31 ++++++------- 5 files changed, 67 insertions(+), 41 deletions(-) diff --git a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java index 592912a148fe..2f24e89b1fe9 100644 --- a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java +++ b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java @@ -7,6 +7,7 @@ import static io.opentelemetry.semconv.ErrorAttributes.ERROR_TYPE; import static java.nio.charset.StandardCharsets.US_ASCII; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; import io.netty.buffer.ByteBuf; @@ -102,6 +103,7 @@ void malformedAndNonErrorRepliesDoNotProduceServerErrorType() { @Test void parsesFirstNonemptyAsciiSpaceDelimitedTokenVerbatim() { assertThat(parse("-Custom.Code! diagnostic details\r\n")).isEqualTo("Custom.Code!"); + assertThat(parse("-Pr\u00e9fix diagnostic\r\n")).isEqualTo("Pr\u00e9fix"); assertThat(parse("-lowercase\r\n")).isEqualTo("lowercase"); assertThat(parse("-prefix-only\r\n")).isEqualTo("prefix-only"); assertThat(parse("- Leading.Code! diagnostic\r\n")).isEqualTo("Leading.Code!"); @@ -111,6 +113,19 @@ void parsesFirstNonemptyAsciiSpaceDelimitedTokenVerbatim() { assertThat(parse("-" + longPrefix + " diagnostic\r\n")).isEqualTo(longPrefix); } + @Test + void parsingStopsAtTheReplyTerminatorWithoutConsumingTheBuffer() { + ByteBuf response = + Unpooled.copiedBuffer("ignored:-PrefixOnly\r\n-SECOND diagnostic\r\n", UTF_8); + response.readerIndex(8); + try { + assertThat(RedissonErrorCapture.parseServerErrorType(response)).isEqualTo("PrefixOnly"); + assertThat(response.readerIndex()).isEqualTo(8); + } finally { + response.release(); + } + } + @Test void clientExceptionMessageIsNotClassifiedAsServerReply() { RedisException error = new RedisException("ERR connection refused"); @@ -164,7 +179,7 @@ private static String extractedErrorType(Throwable error) { } private static String parse(String reply) { - ByteBuf response = Unpooled.copiedBuffer(reply, US_ASCII); + ByteBuf response = Unpooled.copiedBuffer(reply, UTF_8); try { return RedissonErrorCapture.parseServerErrorType(response); } finally { diff --git a/instrumentation/redisson/redisson-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedissonAsyncClientTest.java b/instrumentation/redisson/redisson-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedissonAsyncClientTest.java index 19412ef7b152..a647bc576645 100644 --- a/instrumentation/redisson/redisson-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedissonAsyncClientTest.java +++ b/instrumentation/redisson/redisson-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedissonAsyncClientTest.java @@ -6,5 +6,12 @@ package io.opentelemetry.javaagent.instrumentation.redisson.v3_0; import io.opentelemetry.javaagent.instrumentation.redisson.AbstractRedissonAsyncClientTest; +import org.redisson.api.RBatch; +import org.redisson.api.RedissonClient; -class RedissonAsyncClientTest extends AbstractRedissonAsyncClientTest {} +class RedissonAsyncClientTest extends AbstractRedissonAsyncClientTest { + @Override + protected RBatch createBatch(RedissonClient redisson) { + return redisson.createBatch(); + } +} diff --git a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java index 46a3986581d8..b42f30607163 100644 --- a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java @@ -5,7 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0; -import static java.nio.charset.StandardCharsets.US_ASCII; +import static java.nio.charset.StandardCharsets.UTF_8; import io.netty.buffer.ByteBuf; import io.opentelemetry.instrumentation.api.internal.ScopedThreadValue; @@ -17,7 +17,7 @@ * Captures the first token from RESP error replies before Redisson completes the command promise. * *

Redisson's {@code CommandDecoder} calls {@code CommandData.tryFailure} while decoding a {@code - * -} reply, so the token is scoped to that decoder frame and attached to the resulting generic + * -} reply, so the token is scoped to that decoder frame and attached to the resulting Redisson * exception before completion callbacks can run. The operation listener clears it after extracting * telemetry; all other errors use the shared exception-class fallback. * @@ -73,7 +73,7 @@ public static String parseServerErrorType(ByteBuf input) { int length = end - start; byte[] token = new byte[length]; input.getBytes(start, token); - return new String(token, US_ASCII); + return new String(token, UTF_8); } catch (Throwable ignored) { // ReplayingDecoder retries incomplete replies; leave buffer handling to Redisson. return null; diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java index bfd370fd08ac..60a6253842d6 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java @@ -225,12 +225,19 @@ void asyncServerErrorPrefixContract() throws IOException, InterruptedException { .execInContainer("redis-cli", "lpush", "async-wrong-type", "value") .getExitCode()) .isZero(); - assertThatThrownBy(() -> redisson.getBucket("async-wrong-type").getAsync().join()) + assertThatThrownBy( + () -> + redisson + .getBucket("async-wrong-type") + .getAsync() + .toCompletableFuture() + .join()) .hasRootCauseInstanceOf(RedisException.class); assertErrorTypeOnLastOperation("WRONGTYPE"); assertScriptErrorType("OOM command not allowed", "OOM", RedisOutOfMemoryException.class); assertScriptErrorType("Custom.Code! diagnostic details", "Custom.Code!", null); + assertScriptErrorType("Pr\u00e9fix diagnostic", "Pr\u00e9fix", null); assertScriptErrorType("lowercase diagnostic", "lowercase", null); assertScriptErrorType("Tab.Code!\tdiagnostic", "Tab.Code!\tdiagnostic", null); String longPrefix = String.join("", nCopies(80, "x")); @@ -243,7 +250,7 @@ void asyncServerErrorPrefixContract() throws IOException, InterruptedException { assertThat( redisson .getScript() - .evalAsync(RScript.Mode.READ_ONLY, "return 1", RScript.ReturnType.INTEGER) + .evalAsync(RScript.Mode.READ_ONLY, "return 1", RScript.ReturnType.VALUE) .toCompletableFuture() .join()) .isEqualTo(1L); @@ -252,13 +259,7 @@ void asyncServerErrorPrefixContract() throws IOException, InterruptedException { @Test void asyncFailedBatchErrorPrefixIsRecordedInSpanAndMetrics() { - try { - Class.forName("org.redisson.api.BatchOptions"); - } catch (ClassNotFoundException ignored) { - Assumptions.abort(); - } - - RBatch batch = redisson.createBatch(BatchOptions.defaults()); + RBatch batch = createBatch(redisson); batch .getScript() .evalAsync( @@ -377,16 +378,18 @@ private static void assertErrorTypeOnLastOperation(String expectedErrorType) { histogram -> histogram.hasPointsSatisfying( point -> - point.hasAttributesSatisfying( - attributes -> { - if (expectedErrorType == null) { - assertThat(attributes.asMap()) - .doesNotContainKey(ERROR_TYPE); - } else { - assertThat(attributes.asMap()) - .containsEntry(ERROR_TYPE, expectedErrorType); - } - })))); + point + .hasCount(1) + .hasAttributesSatisfying( + attributes -> { + if (expectedErrorType == null) { + assertThat(attributes.asMap()) + .doesNotContainKey(ERROR_TYPE); + } else { + assertThat(attributes.asMap()) + .containsEntry(ERROR_TYPE, expectedErrorType); + } + })))); testing.clearData(); } @@ -550,6 +553,10 @@ private String dbNamespace(String databaseIndex) { return hasDatabaseIndex() ? databaseIndex : null; } + protected RBatch createBatch(RedissonClient redisson) { + return redisson.createBatch(BatchOptions.defaults()); + } + private static class MyCallable implements Serializable, Callable { private static final long serialVersionUID = 1L; diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index c9f975d28e6f..ebd984292717 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -293,6 +293,7 @@ void wrongTypeErrorIsRecordedInSpanAndMetrics() throws IOException, InterruptedE void serverErrorPrefixContract() { assertScriptErrorType("OOM command not allowed", "OOM", RedisOutOfMemoryException.class); assertScriptErrorType("Custom.Code! diagnostic details", "Custom.Code!", null); + assertScriptErrorType("Pr\u00e9fix diagnostic", "Pr\u00e9fix", null); assertScriptErrorType("lowercase diagnostic", "lowercase", null); assertScriptErrorType("Tab.Code!\tdiagnostic", "Tab.Code!\tdiagnostic", null); String longPrefix = String.join("", nCopies(80, "x")); @@ -303,19 +304,13 @@ void serverErrorPrefixContract() { assertScriptErrorType(" ", RedisException.class.getName(), null); Object result = - redisson.getScript().eval(RScript.Mode.READ_ONLY, "return 1", RScript.ReturnType.INTEGER); + redisson.getScript().eval(RScript.Mode.READ_ONLY, "return 1", RScript.ReturnType.VALUE); assertThat(result).isEqualTo(1L); assertErrorTypeOnLastOperation(null); } @Test void failedBatchErrorPrefixIsRecordedInSpanAndMetrics() throws ReflectiveOperationException { - try { - Class.forName("org.redisson.api.BatchOptions"); - } catch (ClassNotFoundException ignored) { - Assumptions.abort(); - } - RBatch batch = createBatch(redisson); batch .getScript() @@ -433,16 +428,18 @@ private static void assertErrorTypeOnLastOperation(String expectedErrorType) { histogram -> histogram.hasPointsSatisfying( point -> - point.hasAttributesSatisfying( - attributes -> { - if (expectedErrorType == null) { - assertThat(attributes.asMap()) - .doesNotContainKey(ERROR_TYPE); - } else { - assertThat(attributes.asMap()) - .containsEntry(ERROR_TYPE, expectedErrorType); - } - })))); + point + .hasCount(1) + .hasAttributesSatisfying( + attributes -> { + if (expectedErrorType == null) { + assertThat(attributes.asMap()) + .doesNotContainKey(ERROR_TYPE); + } else { + assertThat(attributes.asMap()) + .containsEntry(ERROR_TYPE, expectedErrorType); + } + })))); testing.clearData(); } From ffdcff142073f71f2f9e125335e962570e3ad0dd Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Tue, 20 Oct 2026 23:25:06 -0700 Subject: [PATCH 08/22] Use literal Unicode in Redis prefix tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../redisson/common/v3_0/RedissonDbAttributesGetterTest.java | 2 +- .../redisson/AbstractRedissonAsyncClientTest.java | 2 +- .../instrumentation/redisson/AbstractRedissonClientTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java index 2f24e89b1fe9..831ec0acb1a6 100644 --- a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java +++ b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java @@ -103,7 +103,7 @@ void malformedAndNonErrorRepliesDoNotProduceServerErrorType() { @Test void parsesFirstNonemptyAsciiSpaceDelimitedTokenVerbatim() { assertThat(parse("-Custom.Code! diagnostic details\r\n")).isEqualTo("Custom.Code!"); - assertThat(parse("-Pr\u00e9fix diagnostic\r\n")).isEqualTo("Pr\u00e9fix"); + assertThat(parse("-Préfix diagnostic\r\n")).isEqualTo("Préfix"); assertThat(parse("-lowercase\r\n")).isEqualTo("lowercase"); assertThat(parse("-prefix-only\r\n")).isEqualTo("prefix-only"); assertThat(parse("- Leading.Code! diagnostic\r\n")).isEqualTo("Leading.Code!"); diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java index 60a6253842d6..03245d47a601 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java @@ -237,7 +237,7 @@ void asyncServerErrorPrefixContract() throws IOException, InterruptedException { assertScriptErrorType("OOM command not allowed", "OOM", RedisOutOfMemoryException.class); assertScriptErrorType("Custom.Code! diagnostic details", "Custom.Code!", null); - assertScriptErrorType("Pr\u00e9fix diagnostic", "Pr\u00e9fix", null); + assertScriptErrorType("Préfix diagnostic", "Préfix", null); assertScriptErrorType("lowercase diagnostic", "lowercase", null); assertScriptErrorType("Tab.Code!\tdiagnostic", "Tab.Code!\tdiagnostic", null); String longPrefix = String.join("", nCopies(80, "x")); diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index ebd984292717..7f2ebdbfba27 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -293,7 +293,7 @@ void wrongTypeErrorIsRecordedInSpanAndMetrics() throws IOException, InterruptedE void serverErrorPrefixContract() { assertScriptErrorType("OOM command not allowed", "OOM", RedisOutOfMemoryException.class); assertScriptErrorType("Custom.Code! diagnostic details", "Custom.Code!", null); - assertScriptErrorType("Pr\u00e9fix diagnostic", "Pr\u00e9fix", null); + assertScriptErrorType("Préfix diagnostic", "Préfix", null); assertScriptErrorType("lowercase diagnostic", "lowercase", null); assertScriptErrorType("Tab.Code!\tdiagnostic", "Tab.Code!\tdiagnostic", null); String longPrefix = String.join("", nCopies(80, "x")); From 775a3873b1bbde5b7fc549c41ed420697bebb888 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Oct 2026 16:23:57 +0000 Subject: [PATCH 09/22] Use shared Redis prefix extraction Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../semconv/db/internal/RedisErrorPrefix.java | 30 +++++++++ .../db/internal/RedisErrorPrefixTest.java | 67 ++++++++++++++++++- .../v3_0/RedissonDbAttributesGetterTest.java | 27 +------- .../common/v3_0/RedissonErrorCapture.java | 20 +----- 4 files changed, 100 insertions(+), 44 deletions(-) diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefix.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefix.java index 07f73c610f05..441922c38960 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefix.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefix.java @@ -5,6 +5,9 @@ package io.opentelemetry.instrumentation.api.incubator.semconv.db.internal; +import static java.nio.charset.StandardCharsets.UTF_8; + +import java.nio.ByteBuffer; import javax.annotation.Nullable; /** @@ -38,5 +41,32 @@ public static String extract(@Nullable String serverErrorLine) { return end < 0 ? serverErrorLine.substring(start) : serverErrorLine.substring(start, end); } + /** Returns the first token from a bounded UTF-8 server error line, or {@code null} if absent. */ + @Nullable + public static String extract(@Nullable ByteBuffer errorLine) { + if (errorLine == null) { + return null; + } + + int start = errorLine.position(); + int limit = errorLine.limit(); + while (start < limit && errorLine.get(start) == ' ') { + start++; + } + if (start == limit) { + return null; + } + + int end = start; + while (end < limit && errorLine.get(end) != ' ') { + end++; + } + + ByteBuffer token = errorLine.asReadOnlyBuffer(); + token.position(start); + token.limit(end); + return UTF_8.decode(token).toString(); + } + private RedisErrorPrefix() {} } diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefixTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefixTest.java index d4fe0f0166b9..d1ee17aa48a1 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefixTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefixTest.java @@ -5,10 +5,13 @@ package io.opentelemetry.instrumentation.api.incubator.semconv.db.internal; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.params.provider.Arguments.argumentSet; +import java.nio.ByteBuffer; import java.util.stream.Stream; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -21,6 +24,44 @@ void extractsFirstToken(String serverErrorLine, String expected) { assertThat(RedisErrorPrefix.extract(serverErrorLine)).isEqualTo(expected); } + @ParameterizedTest + @MethodSource("serverErrorLines") + void extractsFirstTokenFromByteBuffer(String serverErrorLine, String expected) { + ByteBuffer errorLine = + serverErrorLine == null ? null : ByteBuffer.wrap(serverErrorLine.getBytes(UTF_8)); + assertThat(RedisErrorPrefix.extract(errorLine)).isEqualTo(expected); + } + + @Test + void extractsFromByteBufferViewsWithoutChangingState() { + String errorLine = "excluded-before ERRé中! detail excluded-after"; + byte[] contents = errorLine.getBytes(UTF_8); + int position = "excluded-before".length(); + int limit = position + " ERRé中!".getBytes(UTF_8).length; + + ByteBuffer heap = ByteBuffer.wrap(contents); + assertExtractsWithoutChangingState(heap, position, limit); + + ByteBuffer direct = ByteBuffer.allocateDirect(contents.length); + direct.put(contents).flip(); + assertExtractsWithoutChangingState(direct, position, limit); + + ByteBuffer readOnly = ByteBuffer.wrap(contents).asReadOnlyBuffer(); + assertExtractsWithoutChangingState(readOnly, position, limit); + + byte[] slicedContents = ("outside" + errorLine + "outside").getBytes(UTF_8); + ByteBuffer parent = ByteBuffer.wrap(slicedContents); + parent.position("outside".length()); + parent.limit("outside".length() + contents.length); + ByteBuffer slice = parent.slice(); + assertExtractsWithoutChangingState(slice, position, limit); + } + + @Test + void handlesNullByteBuffer() { + assertThat(RedisErrorPrefix.extract((ByteBuffer) null)).isNull(); + } + private static Stream serverErrorLines() { return Stream.of( argumentSet("wrong type", "WRONGTYPE Operation against a key", "WRONGTYPE"), @@ -42,7 +83,31 @@ private static Stream serverErrorLines() { argumentSet("leading tab", "\tERR detail", "\tERR"), argumentSet("tab only", "\t", "\t"), argumentSet("spaces before tab", " \tERR detail", "\tERR"), - argumentSet("Unicode space", "ERR\u2003custom detail", "ERR\u2003custom"), + argumentSet("Unicode space", "ERR custom detail", "ERR custom"), argumentSet("Unicode text", "ERRé中 detail", "ERRé中")); } + + private static void assertExtractsWithoutChangingState( + ByteBuffer input, int position, int limit) { + byte[] originalContents = contents(input); + input.position(position); + input.limit(limit); + input.position(position); + input.mark(); + + assertThat(RedisErrorPrefix.extract(input)).isEqualTo("ERRé中!"); + assertThat(input.position()).isEqualTo(position); + assertThat(input.limit()).isEqualTo(limit); + input.reset(); + assertThat(input.position()).isEqualTo(position); + assertThat(contents(input)).containsExactly(originalContents); + } + + private static byte[] contents(ByteBuffer input) { + ByteBuffer duplicate = input.duplicate(); + duplicate.clear(); + byte[] contents = new byte[duplicate.remaining()]; + duplicate.get(contents); + return contents; + } } diff --git a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java index 831ec0acb1a6..6a4db0a6e9dd 100644 --- a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java +++ b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java @@ -17,7 +17,6 @@ import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientAttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; -import java.util.Collections; import org.junit.jupiter.api.Test; import org.redisson.client.RedisConnectionException; import org.redisson.client.RedisException; @@ -100,26 +99,13 @@ void malformedAndNonErrorRepliesDoNotProduceServerErrorType() { } } - @Test - void parsesFirstNonemptyAsciiSpaceDelimitedTokenVerbatim() { - assertThat(parse("-Custom.Code! diagnostic details\r\n")).isEqualTo("Custom.Code!"); - assertThat(parse("-Préfix diagnostic\r\n")).isEqualTo("Préfix"); - assertThat(parse("-lowercase\r\n")).isEqualTo("lowercase"); - assertThat(parse("-prefix-only\r\n")).isEqualTo("prefix-only"); - assertThat(parse("- Leading.Code! diagnostic\r\n")).isEqualTo("Leading.Code!"); - assertThat(parse("-Tab.Code!\tdiagnostic\r\n")).isEqualTo("Tab.Code!\tdiagnostic"); - - String longPrefix = String.join("", Collections.nCopies(80, "x")); - assertThat(parse("-" + longPrefix + " diagnostic\r\n")).isEqualTo(longPrefix); - } - @Test void parsingStopsAtTheReplyTerminatorWithoutConsumingTheBuffer() { ByteBuf response = - Unpooled.copiedBuffer("ignored:-PrefixOnly\r\n-SECOND diagnostic\r\n", UTF_8); + Unpooled.copiedBuffer("ignored:-PréfixOnly\r\n-SECOND diagnostic\r\n", UTF_8); response.readerIndex(8); try { - assertThat(RedissonErrorCapture.parseServerErrorType(response)).isEqualTo("PrefixOnly"); + assertThat(RedissonErrorCapture.parseServerErrorType(response)).isEqualTo("PréfixOnly"); assertThat(response.readerIndex()).isEqualTo(8); } finally { response.release(); @@ -177,13 +163,4 @@ private static String extractedErrorType(Throwable error) { EXTRACTOR.onEnd(attributes, Context.root(), REQUEST, null, error); return attributes.build().get(ERROR_TYPE); } - - private static String parse(String reply) { - ByteBuf response = Unpooled.copiedBuffer(reply, UTF_8); - try { - return RedissonErrorCapture.parseServerErrorType(response); - } finally { - response.release(); - } - } } diff --git a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java index b42f30607163..7b2171f47f68 100644 --- a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java @@ -5,9 +5,8 @@ package io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0; -import static java.nio.charset.StandardCharsets.UTF_8; - import io.netty.buffer.ByteBuf; +import io.opentelemetry.instrumentation.api.incubator.semconv.db.internal.RedisErrorPrefix; import io.opentelemetry.instrumentation.api.internal.ScopedThreadValue; import io.opentelemetry.instrumentation.api.util.VirtualField; import javax.annotation.Nullable; @@ -58,22 +57,7 @@ public static String parseServerErrorType(ByteBuf input) { return null; } - while (start < replyEnd && input.getByte(start) == ' ') { - start++; - } - if (start == replyEnd) { - return null; - } - - int end = start; - while (end < replyEnd && input.getByte(end) != ' ') { - end++; - } - - int length = end - start; - byte[] token = new byte[length]; - input.getBytes(start, token); - return new String(token, UTF_8); + return RedisErrorPrefix.extract(input.nioBuffer(start, replyEnd - start)); } catch (Throwable ignored) { // ReplayingDecoder retries incomplete replies; leave buffer handling to Redisson. return null; From 67ca1b0a1a9dd519c50c3a0aab0a8b3a5183e93e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Oct 2026 17:13:34 +0000 Subject: [PATCH 10/22] Remove redundant Redisson getter test Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../common/v3_0/RedissonDbAttributesGetterTest.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java index 6a4db0a6e9dd..f357a715fde6 100644 --- a/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java +++ b/instrumentation/redisson/redisson-3.0/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java @@ -122,12 +122,6 @@ void clientExceptionMessageIsNotClassifiedAsServerReply() { assertThat(extractedErrorType(error)).isEqualTo(RedisException.class.getName()); } - @Test - void missingMessageHasNoErrorType() { - assertThat(GETTER.getErrorType(REQUEST, null, new RedisException())).isNull(); - assertThat(GETTER.getErrorType(REQUEST, null, null)).isNull(); - } - @Test void dedicatedExceptionUsesCapturedServerPrefixAndClearsIt() { RedisLoadingException error = new RedisLoadingException("LOADING Redis is loading"); From c4e9ffa21eea0c59bfda8e44df0271d8bf5fc401 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 21 Oct 2026 19:30:23 +0000 Subject: [PATCH 11/22] Capture Redisson batch failure prefixes Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../v3_0/RedisCommandDataInstrumentation.java | 4 +- .../RedisCommandDataInstrumentation.java | 4 +- .../redisson/AbstractRedissonClientTest.java | 121 ++++++++++++++++-- 3 files changed, 114 insertions(+), 15 deletions(-) diff --git a/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDataInstrumentation.java b/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDataInstrumentation.java index baece4e3daa7..deb513424f07 100644 --- a/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDataInstrumentation.java +++ b/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDataInstrumentation.java @@ -27,7 +27,9 @@ class RedisCommandDataInstrumentation implements TypeInstrumentation { @Override public ElementMatcher typeMatcher() { return namedOneOf( - "org.redisson.client.protocol.CommandData", "org.redisson.client.protocol.CommandsData"); + "org.redisson.client.protocol.CommandData", + "org.redisson.client.protocol.CommandsData", + "org.redisson.client.protocol.BatchCommandData"); } @Override diff --git a/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDataInstrumentation.java b/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDataInstrumentation.java index 153a70df457f..e54b955a85b6 100644 --- a/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDataInstrumentation.java +++ b/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDataInstrumentation.java @@ -26,7 +26,9 @@ class RedisCommandDataInstrumentation implements TypeInstrumentation { @Override public ElementMatcher typeMatcher() { return namedOneOf( - "org.redisson.client.protocol.CommandData", "org.redisson.client.protocol.CommandsData"); + "org.redisson.client.protocol.CommandData", + "org.redisson.client.protocol.CommandsData", + "org.redisson.client.protocol.BatchCommandData"); } @Override diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index 7f2ebdbfba27..db201d82674f 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -43,6 +43,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.UUID; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.stream.Stream; @@ -324,17 +325,92 @@ void failedBatchErrorPrefixIsRecordedInSpanAndMetrics() throws ReflectiveOperati assertErrorTypeOnLastOperation("Batch.Code!"); } + @Test + void batchRedirectIsRetriedAndPreservesBatchResult() + throws IOException, InterruptedException, ReflectiveOperationException { + String key = "batch-redirect-" + UUID.randomUUID(); + String script = + "local count = redis.call('INCR', '" + + key + + "'); if count == 1 then return redis.error_reply('MOVED 1234 " + + host + + ":" + + port + + "'); end; return 'redirected'"; + RBatch batch = createBatch(redisson); + batch.getScript().evalAsync(RScript.Mode.READ_WRITE, script, RScript.ReturnType.VALUE); + + assertThat(batchResponses(invokeExecute(batch))).isEqualTo(asList("redirected")); + assertErrorTypeOnLastOperation(null); + assertFollowingOperationsHaveNoLeakedServerError(); + } + + @Test + void batchRetryErrorPrefixIsCapturedBeforeCompletion() + throws IOException, InterruptedException, ReflectiveOperationException { + Assumptions.assumeTrue(isClassAvailable("org.redisson.api.BatchOptions")); + Assumptions.assumeTrue(isClassAvailable("org.redisson.client.RedisRetryException")); + Class redisRetryException; + try { + redisRetryException = Class.forName("org.redisson.client.RedisRetryException"); + } catch (ClassNotFoundException ignored) { + Assumptions.abort(); + return; + } + assertBatchServerErrorPrefix("LOADING Redis is loading", "LOADING", redisRetryException); + assertFollowingOperationsHaveNoLeakedServerError(); + } + @Test void clientSideDecodeFailureKeepsExceptionClassFallback() throws IOException, InterruptedException { - assertThat( - redisServer - .execInContainer("redis-cli", "set", "client-side-decode-error", "value") - .getExitCode()) + assertClientSideDecodeFailure("client-side-decode-error"); + } + + private void assertBatchServerErrorPrefix( + String serverError, String expectedErrorType, Class expectedFailureType) + throws ReflectiveOperationException { + RBatch batch = createBatchWithoutRetries(); + batch + .getScript() + .evalAsync( + RScript.Mode.READ_WRITE, + "return redis.error_reply('" + serverError + "')", + RScript.ReturnType.VALUE); + + assertThatThrownBy(() -> invokeExecute(batch)) + .satisfies(error -> assertThat(rootCause(error)).isInstanceOf(expectedFailureType)); + assertErrorTypeOnLastOperation(expectedErrorType); + } + + private RBatch createBatchWithoutRetries() throws ReflectiveOperationException { + Class batchOptions = Class.forName("org.redisson.api.BatchOptions"); + Object options = batchOptions.getMethod("defaults").invoke(null); + batchOptions.getMethod("retryAttempts", int.class).invoke(options, 0); + return (RBatch) + redisson.getClass().getMethod("createBatch", batchOptions).invoke(redisson, options); + } + + private static List batchResponses(Object result) throws ReflectiveOperationException { + if (result instanceof List) { + return (List) result; + } + return (List) result.getClass().getMethod("getResponses").invoke(result); + } + + private void assertFollowingOperationsHaveNoLeakedServerError() + throws IOException, InterruptedException { + redisson.getBucket("after-batch-server-error").set("success"); + assertErrorTypeOnLastOperation(null); + assertClientSideDecodeFailure("client-side-after-batch-error"); + } + + private void assertClientSideDecodeFailure(String key) throws IOException, InterruptedException { + assertThat(redisServer.execInContainer("redis-cli", "set", key, "value").getExitCode()) .isZero(); RBucket bucket = redisson.getBucket( - "client-side-decode-error", + key, new StringCodec() { @Override public Decoder getValueDecoder() { @@ -360,7 +436,7 @@ public Decoder getValueDecoder() { equalTo(SERVER_PORT, port), equalTo(DB_SYSTEM_NAME, REDIS), equalTo(DB_NAMESPACE, dbNamespace("0")), - equalTo(DB_QUERY_TEXT, "GET client-side-decode-error"), + equalTo(DB_QUERY_TEXT, "GET " + key), equalTo(DB_OPERATION_NAME, "GET"), equalTo(ERROR_TYPE, RedisException.class.getName()))); }); @@ -374,11 +450,30 @@ public Decoder getValueDecoder() { histogram -> histogram.hasPointsSatisfying( point -> - point.hasAttributesSatisfying( - attributes -> - assertThat(attributes.asMap()) - .containsEntry( - ERROR_TYPE, RedisException.class.getName()))))); + point + .hasCount(1) + .hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()) + .containsEntry( + ERROR_TYPE, RedisException.class.getName()))))); + testing.clearData(); + } + + private static Throwable rootCause(Throwable error) { + while (error.getCause() != null && error.getCause() != error) { + error = error.getCause(); + } + return error; + } + + private static boolean isClassAvailable(String name) { + try { + Class.forName(name, false, AbstractRedissonClientTest.class.getClassLoader()); + return true; + } catch (ClassNotFoundException ignored) { + return false; + } } private void assertScriptErrorType( @@ -638,8 +733,8 @@ private static Stream batchScenarios() { .build())); } - private static void invokeExecute(RBatch batch) throws ReflectiveOperationException { - batch.getClass().getMethod("execute").invoke(batch); + private static Object invokeExecute(RBatch batch) throws ReflectiveOperationException { + return batch.getClass().getMethod("execute").invoke(batch); } @Test From f581f150631967b43a15ec110f0ad40a1dee1434 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 22 Oct 2026 20:39:44 +0000 Subject: [PATCH 12/22] Simplify Redisson decoder capture Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../semconv/db/internal/RedisErrorPrefix.java | 30 ------- .../db/internal/RedisErrorPrefixTest.java | 65 -------------- .../v3_0/RedissonInstrumentationModule.java | 1 + .../RedisCommandDecoderInstrumentation.java | 50 ----------- .../v3_17/RedissonInstrumentationModule.java | 1 + .../RedisCommandDecoderInstrumentation.java | 4 +- .../common/v3_0/RedissonErrorCapture.java | 4 +- .../AbstractRedissonAsyncClientTest.java | 31 +------ .../redisson/AbstractRedissonClientTest.java | 90 ++++++++++++++----- 9 files changed, 76 insertions(+), 200 deletions(-) delete mode 100644 instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDecoderInstrumentation.java rename instrumentation/redisson/{redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson => redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common}/v3_0/RedisCommandDecoderInstrumentation.java (92%) diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefix.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefix.java index e4b46532f054..236d7ae60bb4 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefix.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefix.java @@ -5,9 +5,6 @@ package io.opentelemetry.instrumentation.api.incubator.semconv.db.internal; -import static java.nio.charset.StandardCharsets.UTF_8; - -import java.nio.ByteBuffer; import javax.annotation.Nullable; /** @@ -59,32 +56,5 @@ public static String extract(@Nullable String serverErrorLine) { return end < 0 ? serverErrorLine.substring(start) : serverErrorLine.substring(start, end); } - /** Returns the first token from a bounded UTF-8 server error line, or {@code null} if absent. */ - @Nullable - public static String extract(@Nullable ByteBuffer errorLine) { - if (errorLine == null) { - return null; - } - - int start = errorLine.position(); - int limit = errorLine.limit(); - while (start < limit && errorLine.get(start) == ' ') { - start++; - } - if (start == limit) { - return null; - } - - int end = start; - while (end < limit && errorLine.get(end) != ' ') { - end++; - } - - ByteBuffer token = errorLine.asReadOnlyBuffer(); - token.position(start); - token.limit(end); - return UTF_8.decode(token).toString(); - } - private RedisErrorPrefix() {} } diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefixTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefixTest.java index 61eec0cb8744..6e40e1c786f3 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefixTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefixTest.java @@ -5,13 +5,10 @@ package io.opentelemetry.instrumentation.api.incubator.semconv.db.internal; -import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.params.provider.Arguments.argumentSet; -import java.nio.ByteBuffer; import java.util.stream.Stream; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -30,44 +27,6 @@ void extractsAsciiPrefix(String serverErrorLine, String expected) { assertThat(RedisErrorPrefix.extractAscii(serverErrorLine)).isEqualTo(expected); } - @ParameterizedTest - @MethodSource("serverErrorLines") - void extractsFirstTokenFromByteBuffer(String serverErrorLine, String expected) { - ByteBuffer errorLine = - serverErrorLine == null ? null : ByteBuffer.wrap(serverErrorLine.getBytes(UTF_8)); - assertThat(RedisErrorPrefix.extract(errorLine)).isEqualTo(expected); - } - - @Test - void extractsFromByteBufferViewsWithoutChangingState() { - String errorLine = "excluded-before ERRé中! detail excluded-after"; - byte[] contents = errorLine.getBytes(UTF_8); - int position = "excluded-before".length(); - int limit = position + " ERRé中!".getBytes(UTF_8).length; - - ByteBuffer heap = ByteBuffer.wrap(contents); - assertExtractsWithoutChangingState(heap, position, limit); - - ByteBuffer direct = ByteBuffer.allocateDirect(contents.length); - direct.put(contents).flip(); - assertExtractsWithoutChangingState(direct, position, limit); - - ByteBuffer readOnly = ByteBuffer.wrap(contents).asReadOnlyBuffer(); - assertExtractsWithoutChangingState(readOnly, position, limit); - - byte[] slicedContents = ("outside" + errorLine + "outside").getBytes(UTF_8); - ByteBuffer parent = ByteBuffer.wrap(slicedContents); - parent.position("outside".length()); - parent.limit("outside".length() + contents.length); - ByteBuffer slice = parent.slice(); - assertExtractsWithoutChangingState(slice, position, limit); - } - - @Test - void handlesNullByteBuffer() { - assertThat(RedisErrorPrefix.extract((ByteBuffer) null)).isNull(); - } - private static Stream serverErrorLines() { return Stream.of( argumentSet("wrong type", "WRONGTYPE Operation against a key", "WRONGTYPE"), @@ -121,28 +80,4 @@ private static Stream asciiServerErrorLines() { argumentSet("empty", "", null), argumentSet("spaces only", " ", null)); } - - private static void assertExtractsWithoutChangingState( - ByteBuffer input, int position, int limit) { - byte[] originalContents = contents(input); - input.position(position); - input.limit(limit); - input.position(position); - input.mark(); - - assertThat(RedisErrorPrefix.extract(input)).isEqualTo("ERRé中!"); - assertThat(input.position()).isEqualTo(position); - assertThat(input.limit()).isEqualTo(limit); - input.reset(); - assertThat(input.position()).isEqualTo(position); - assertThat(contents(input)).containsExactly(originalContents); - } - - private static byte[] contents(ByteBuffer input) { - ByteBuffer duplicate = input.duplicate(); - duplicate.clear(); - byte[] contents = new byte[duplicate.remaining()]; - duplicate.get(contents); - return contents; - } } diff --git a/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedissonInstrumentationModule.java b/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedissonInstrumentationModule.java index b00eae9fe7ac..171c13ab2b5d 100644 --- a/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedissonInstrumentationModule.java +++ b/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedissonInstrumentationModule.java @@ -13,6 +13,7 @@ import com.google.auto.service.AutoService; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.RedisCommandDecoderInstrumentation; import java.util.List; import net.bytebuddy.matcher.ElementMatcher; diff --git a/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDecoderInstrumentation.java b/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDecoderInstrumentation.java deleted file mode 100644 index ebd9faa76787..000000000000 --- a/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedisCommandDecoderInstrumentation.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.redisson.v3_17; - -import static io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.RedissonErrorCapture.currentServerErrorType; -import static io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.RedissonErrorCapture.parseServerErrorType; -import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; - -import io.netty.buffer.ByteBuf; -import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; -import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import javax.annotation.Nullable; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; - -class RedisCommandDecoderInstrumentation implements TypeInstrumentation { - - @Override - public ElementMatcher typeMatcher() { - return named("org.redisson.client.handler.CommandDecoder"); - } - - @Override - public void transform(TypeTransformer transformer) { - transformer.applyAdviceToMethod( - named("decode") - .and(takesArgument(0, named("io.netty.buffer.ByteBuf"))) - .and(takesArgument(1, named("org.redisson.client.protocol.CommandData"))), - getClass().getName() + "$DecodeAdvice"); - } - - @SuppressWarnings("unused") - public static class DecodeAdvice { - - @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) - public static String onEnter(@Advice.Argument(0) ByteBuf input) { - return currentServerErrorType().set(parseServerErrorType(input)); - } - - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false) - public static void onExit(@Advice.Enter @Nullable String previousErrorType) { - currentServerErrorType().restore(previousErrorType); - } - } -} diff --git a/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedissonInstrumentationModule.java b/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedissonInstrumentationModule.java index 494de26f5c86..46e76c5b4738 100644 --- a/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedissonInstrumentationModule.java +++ b/instrumentation/redisson/redisson-3.17/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_17/RedissonInstrumentationModule.java @@ -12,6 +12,7 @@ import com.google.auto.service.AutoService; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; +import io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.RedisCommandDecoderInstrumentation; import java.util.List; import net.bytebuddy.matcher.ElementMatcher; diff --git a/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDecoderInstrumentation.java b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedisCommandDecoderInstrumentation.java similarity index 92% rename from instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDecoderInstrumentation.java rename to instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedisCommandDecoderInstrumentation.java index 77e9220c5d30..40b865acd111 100644 --- a/instrumentation/redisson/redisson-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/v3_0/RedisCommandDecoderInstrumentation.java +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedisCommandDecoderInstrumentation.java @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -package io.opentelemetry.javaagent.instrumentation.redisson.v3_0; +package io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0; import static io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.RedissonErrorCapture.currentServerErrorType; import static io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0.RedissonErrorCapture.parseServerErrorType; @@ -18,7 +18,7 @@ import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; -class RedisCommandDecoderInstrumentation implements TypeInstrumentation { +public class RedisCommandDecoderInstrumentation implements TypeInstrumentation { @Override public ElementMatcher typeMatcher() { diff --git a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java index 7b2171f47f68..00253cc8299e 100644 --- a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java @@ -5,6 +5,8 @@ package io.opentelemetry.javaagent.instrumentation.redisson.common.v3_0; +import static java.nio.charset.StandardCharsets.UTF_8; + import io.netty.buffer.ByteBuf; import io.opentelemetry.instrumentation.api.incubator.semconv.db.internal.RedisErrorPrefix; import io.opentelemetry.instrumentation.api.internal.ScopedThreadValue; @@ -57,7 +59,7 @@ public static String parseServerErrorType(ByteBuf input) { return null; } - return RedisErrorPrefix.extract(input.nioBuffer(start, replyEnd - start)); + return RedisErrorPrefix.extract(input.toString(start, replyEnd - start, UTF_8)); } catch (Throwable ignored) { // ReplayingDecoder retries incomplete replies; leave buffer handling to Redisson. return null; diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java index 03245d47a601..b833ef87d5ff 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java @@ -20,7 +20,6 @@ import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS; import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DbSystemNameIncubatingValues.REDIS; -import static java.util.Collections.nCopies; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; @@ -63,7 +62,6 @@ import org.redisson.api.RSet; import org.redisson.api.RedissonClient; import org.redisson.client.RedisException; -import org.redisson.client.RedisOutOfMemoryException; import org.redisson.client.codec.StringCodec; import org.redisson.client.protocol.Decoder; import org.redisson.config.Config; @@ -219,7 +217,7 @@ void asyncCustomServerErrorIsRecordedInSpanAndMetrics() { } @Test - void asyncServerErrorPrefixContract() throws IOException, InterruptedException { + void asyncWrongTypeErrorIsRecordedInSpanAndMetrics() throws IOException, InterruptedException { assertThat( redisServer .execInContainer("redis-cli", "lpush", "async-wrong-type", "value") @@ -235,18 +233,6 @@ void asyncServerErrorPrefixContract() throws IOException, InterruptedException { .hasRootCauseInstanceOf(RedisException.class); assertErrorTypeOnLastOperation("WRONGTYPE"); - assertScriptErrorType("OOM command not allowed", "OOM", RedisOutOfMemoryException.class); - assertScriptErrorType("Custom.Code! diagnostic details", "Custom.Code!", null); - assertScriptErrorType("Préfix diagnostic", "Préfix", null); - assertScriptErrorType("lowercase diagnostic", "lowercase", null); - assertScriptErrorType("Tab.Code!\tdiagnostic", "Tab.Code!\tdiagnostic", null); - String longPrefix = String.join("", nCopies(80, "x")); - assertScriptErrorType(longPrefix + " diagnostic", longPrefix, null); - assertScriptErrorType("prefix-only", "prefix-only", null); - assertScriptErrorType(" Leading.Code! diagnostic", "Leading.Code!", null); - assertScriptErrorType("", RedisException.class.getName(), null); - assertScriptErrorType(" ", RedisException.class.getName(), null); - assertThat( redisson .getScript() @@ -336,21 +322,6 @@ void futureCallback() { span -> span.hasName("callback").hasKind(INTERNAL).hasParent(trace.getSpan(0)))); } - private void assertScriptErrorType( - String serverError, String expectedErrorType, Class expectedCause) { - String script = "return redis.error_reply('" + serverError + "')"; - RFuture future = - redisson.getScript().evalAsync(RScript.Mode.READ_WRITE, script, RScript.ReturnType.VALUE); - assertThatThrownBy(() -> future.toCompletableFuture().join()) - .satisfies( - error -> { - if (expectedCause != null) { - assertThat(error).hasRootCauseInstanceOf(expectedCause); - } - }); - assertErrorTypeOnLastOperation(expectedErrorType); - } - private static void assertErrorTypeOnLastOperation(String expectedErrorType) { AtomicReference instrumentationName = new AtomicReference<>(); testing.waitAndAssertTraces( diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index db201d82674f..bcb59ef8ba99 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -295,14 +295,7 @@ void serverErrorPrefixContract() { assertScriptErrorType("OOM command not allowed", "OOM", RedisOutOfMemoryException.class); assertScriptErrorType("Custom.Code! diagnostic details", "Custom.Code!", null); assertScriptErrorType("Préfix diagnostic", "Préfix", null); - assertScriptErrorType("lowercase diagnostic", "lowercase", null); - assertScriptErrorType("Tab.Code!\tdiagnostic", "Tab.Code!\tdiagnostic", null); - String longPrefix = String.join("", nCopies(80, "x")); - assertScriptErrorType(longPrefix + " diagnostic", longPrefix, null); - assertScriptErrorType("prefix-only", "prefix-only", null); - assertScriptErrorType(" Leading.Code! diagnostic", "Leading.Code!", null); assertScriptErrorType("", RedisException.class.getName(), null); - assertScriptErrorType(" ", RedisException.class.getName(), null); Object result = redisson.getScript().eval(RScript.Mode.READ_ONLY, "return 1", RScript.ReturnType.VALUE); @@ -328,21 +321,30 @@ void failedBatchErrorPrefixIsRecordedInSpanAndMetrics() throws ReflectiveOperati @Test void batchRedirectIsRetriedAndPreservesBatchResult() throws IOException, InterruptedException, ReflectiveOperationException { - String key = "batch-redirect-" + UUID.randomUUID(); - String script = - "local count = redis.call('INCR', '" - + key - + "'); if count == 1 then return redis.error_reply('MOVED 1234 " - + host - + ":" - + port - + "'); end; return 'redirected'"; - RBatch batch = createBatch(redisson); - batch.getScript().evalAsync(RScript.Mode.READ_WRITE, script, RScript.ReturnType.VALUE); - - assertThat(batchResponses(invokeExecute(batch))).isEqualTo(asList("redirected")); - assertErrorTypeOnLastOperation(null); - assertFollowingOperationsHaveNoLeakedServerError(); + Assumptions.assumeTrue(isClassAvailable("org.redisson.api.BatchOptions")); + GenericContainer redirectServer = + new GenericContainer<>("redis:6.2.3-alpine").withExposedPorts(6379); + redirectServer.start(); + try { + String key = "batch-redirect-" + UUID.randomUUID(); + assertThat(redirectServer.execInContainer("redis-cli", "incr", key).getExitCode()).isZero(); + String script = + "local count = redis.call('INCR', '" + + key + + "'); if count == 1 then return redis.error_reply('MOVED 1234 " + + redirectServer.getHost() + + ":" + + redirectServer.getMappedPort(6379) + + "'); end; return count"; + RBatch batch = createBatch(redisson); + batch.getScript().evalAsync(RScript.Mode.READ_WRITE, script, RScript.ReturnType.INTEGER); + + assertThat(batchResponses(invokeExecute(batch))).isEqualTo(asList(2L)); + assertRedirectErrorTypeOnAttempts(); + assertFollowingOperationsHaveNoLeakedServerError(); + } finally { + redirectServer.stop(); + } } @Test @@ -538,6 +540,50 @@ private static void assertErrorTypeOnLastOperation(String expectedErrorType) { testing.clearData(); } + private static void assertRedirectErrorTypeOnAttempts() { + AtomicReference instrumentationName = new AtomicReference<>(); + testing.waitAndAssertTraces( + trace -> { + instrumentationName.set(trace.getSpan(0).getInstrumentationScopeInfo().getName()); + trace.hasSpansSatisfyingExactly( + span -> + span.hasKind(CLIENT) + .hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()).containsEntry(ERROR_TYPE, "MOVED"))); + }, + trace -> + trace.hasSpansSatisfyingExactly( + span -> + span.hasKind(CLIENT) + .hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()).doesNotContainKey(ERROR_TYPE)))); + testing.waitAndAssertMetrics( + instrumentationName.get(), + metrics -> + metrics + .hasName("db.client.operation.duration") + .hasHistogramSatisfying( + histogram -> + histogram.hasPointsSatisfying( + point -> + point + .hasCount(1) + .hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()) + .containsEntry(ERROR_TYPE, "MOVED")), + point -> + point + .hasCount(1) + .hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()) + .doesNotContainKey(ERROR_TYPE))))); + testing.clearData(); + } + @Test void configuredDatabaseIndex() throws InvocationTargetException, IllegalAccessException { RedissonClient databaseOne = Redisson.create(createConfig(1, null)); From baf04368644aeac8d0cd558b6f30605dfc38509c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:03:08 +0000 Subject: [PATCH 13/22] Consume captured Redisson server error prefix before completion callbacks Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../common/v3_0/RedissonErrorCapture.java | 2 ++ .../v3_0/RedissonDbAttributesGetterTest.java | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java index 00253cc8299e..0e2933faffac 100644 --- a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java @@ -74,6 +74,8 @@ public static void captureServerError(@Nullable Throwable error) { String errorType = currentServerErrorType.get(); if (errorType != null) { SERVER_ERROR.set((RedisException) error, new RedisServerError(errorType)); + // consume so completion callbacks run by tryFailure cannot attribute it to another command + currentServerErrorType.set(null); } } diff --git a/instrumentation/redisson/redisson-common-3.0/javaagent/src/unitTests/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java b/instrumentation/redisson/redisson-common-3.0/javaagent/src/unitTests/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java index f357a715fde6..d84a79d9c290 100644 --- a/instrumentation/redisson/redisson-common-3.0/javaagent/src/unitTests/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/src/unitTests/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonDbAttributesGetterTest.java @@ -144,6 +144,26 @@ void dedicatedExceptionUsesCapturedServerPrefixAndClearsIt() { assertThat(RedissonErrorCapture.currentServerErrorType().get()).isNull(); } + @Test + void capturedPrefixIsConsumedBeforeCompletionCallbacks() { + RedisException serverError = new RedisException("CUSTOM diagnostic details"); + RedisException clientError = new RedisException("client-side failure"); + String previous = RedissonErrorCapture.currentServerErrorType().set("CUSTOM"); + try { + RedissonErrorCapture.captureServerError(serverError); + // a callback issuing a new, client-side failing command within the decoder frame + RedissonErrorCapture.captureServerError(clientError); + assertThat(RedissonErrorCapture.currentServerErrorType().get()).isNull(); + // a repeated capture, as with BatchCommandData calling super.tryFailure, keeps the prefix + RedissonErrorCapture.captureServerError(serverError); + } finally { + RedissonErrorCapture.currentServerErrorType().restore(previous); + } + + assertThat(extractedErrorType(serverError)).isEqualTo("CUSTOM"); + assertThat(extractedErrorType(clientError)).isEqualTo(RedisException.class.getName()); + } + @Test void connectionExceptionIsNotClassifiedAsServerError() { RedisConnectionException error = new RedisConnectionException("ERR connection refused"); From 7955b9238c9d679bb0b09b5849fe21b187a66e71 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:29:18 +0000 Subject: [PATCH 14/22] Test reentrant Redisson completion callbacks Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../redisson/AbstractRedissonClientTest.java | 176 ++++++++++++++++++ 1 file changed, 176 insertions(+) diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index bcb59ef8ba99..eb9dd204b241 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -27,6 +27,7 @@ import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DbSystemNameIncubatingValues.REDIS; import static java.util.Arrays.asList; import static java.util.Collections.nCopies; +import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.params.provider.Arguments.argumentSet; @@ -37,6 +38,7 @@ import io.opentelemetry.sdk.testing.assertj.TraceAssert; import java.io.IOException; import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Proxy; import java.net.InetAddress; import java.net.UnknownHostException; import java.util.ArrayList; @@ -44,6 +46,8 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.stream.Stream; @@ -65,6 +69,7 @@ import org.redisson.api.RAtomicLong; import org.redisson.api.RBatch; import org.redisson.api.RBucket; +import org.redisson.api.RFuture; import org.redisson.api.RList; import org.redisson.api.RLock; import org.redisson.api.RMap; @@ -303,6 +308,177 @@ void serverErrorPrefixContract() { assertErrorTypeOnLastOperation(null); } + @Test + void reentrantFailureCallbackKeepsClientErrorFallback() throws InterruptedException { + Assumptions.assumeTrue(isClassAvailable("org.redisson.config.CommandMapper")); + + Class commandMapperType = loadCommandMapperType(); + Config config; + try { + config = createConfig(0, null); + } catch (ReflectiveOperationException e) { + throw new AssertionError(e); + } + config.useSingleServer().setConnectionPoolSize(1).setConnectionMinimumIdleSize(1); + + CountDownLatch evalMappingStarted = new CountDownLatch(1); + CountDownLatch allowEvalMapping = new CountDownLatch(1); + AtomicBoolean failureCallbackActive = new AtomicBoolean(); + AtomicBoolean getMappedInFailureCallback = new AtomicBoolean(); + Object commandMapper = + Proxy.newProxyInstance( + commandMapperType.getClassLoader(), + new Class[] {commandMapperType}, + (proxy, method, args) -> { + if (method.getName().equals("map")) { + String command = (String) args[0]; + if (command.equals("EVAL") || command.equals("EVALSHA")) { + evalMappingStarted.countDown(); + try { + if (!allowEvalMapping.await(30, SECONDS)) { + throw new RedisException("timed out waiting to release command mapping"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RedisException("interrupted while mapping command", e); + } + } else if (command.equals("GET")) { + getMappedInFailureCallback.set(failureCallbackActive.get()); + throw new RedisException("client-side command mapper failure"); + } + return command; + } + return null; + }); + setCommandMapper(config, commandMapperType, commandMapper); + + RedissonClient callbackClient = Redisson.create(config); + testing.clearData(); + try { + testing.runWithSpan( + "parent", + () -> { + RFuture serverError = + callbackClient + .getScript() + .evalAsync( + RScript.Mode.READ_WRITE, + "return redis.error_reply('Custom.Callback! diagnostic details')", + RScript.ReturnType.VALUE); + assertThat(evalMappingStarted.await(30, SECONDS)).isTrue(); + + AtomicBoolean failureCallbackCalled = new AtomicBoolean(); + AtomicReference> clientError = new AtomicReference<>(); + serverError.whenComplete( + (result, error) -> { + failureCallbackCalled.set(true); + failureCallbackActive.set(true); + try { + clientError.set(callbackClient.getBucket("callback-client-error").getAsync()); + } finally { + failureCallbackActive.set(false); + } + }); + allowEvalMapping.countDown(); + + assertThatThrownBy(serverError::join).hasRootCauseInstanceOf(RedisException.class); + assertThat(failureCallbackCalled).isTrue(); + assertThat(getMappedInFailureCallback).isTrue(); + assertThat(clientError.get()).isNotNull(); + assertThatThrownBy(() -> clientError.get().join()) + .hasRootCauseInstanceOf(RedisException.class); + callbackClient.getBucket("callback-success").set("success"); + }); + } finally { + allowEvalMapping.countDown(); + callbackClient.shutdown(); + } + + AtomicReference instrumentationName = new AtomicReference<>(); + testing.waitAndAssertTraces( + trace -> { + instrumentationName.set(trace.getSpan(1).getInstrumentationScopeInfo().getName()); + trace.hasSpansSatisfyingExactly( + span -> span.hasName("parent").hasKind(INTERNAL).hasNoParent(), + span -> + span.hasName("EVAL " + address) + .hasKind(CLIENT) + .hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()) + .containsEntry(ERROR_TYPE, "Custom.Callback!")), + span -> + span.hasName("GET " + address) + .hasKind(CLIENT) + .hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()) + .containsEntry(ERROR_TYPE, RedisException.class.getName())), + span -> + span.hasName("SET " + address) + .hasKind(CLIENT) + .hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()).doesNotContainKey(ERROR_TYPE))); + }); + testing.waitAndAssertMetrics( + instrumentationName.get(), + metrics -> + metrics + .hasName("db.client.operation.duration") + .hasHistogramSatisfying( + histogram -> + histogram + .satisfies(data -> assertThat(data.getPoints()).hasSize(3)) + .hasPointsSatisfying( + point -> + point + .hasCount(1) + .hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()) + .containsEntry(ERROR_TYPE, "Custom.Callback!")), + point -> + point + .hasCount(1) + .hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()) + .containsEntry( + ERROR_TYPE, + RedisException.class.getName())), + point -> + point + .hasCount(1) + .hasAttributesSatisfying( + attributes -> + assertThat(attributes.asMap()) + .doesNotContainKey(ERROR_TYPE))))); + } + + private static Class loadCommandMapperType() { + try { + return Class.forName( + "org.redisson.config.CommandMapper", + false, + AbstractRedissonClientTest.class.getClassLoader()); + } catch (ClassNotFoundException e) { + throw new AssertionError(e); + } + } + + private static void setCommandMapper( + Config config, Class commandMapperType, Object commandMapper) { + try { + config + .getClass() + .getMethod("setCommandMapper", commandMapperType) + .invoke(config, commandMapper); + } catch (ReflectiveOperationException e) { + throw new AssertionError(e); + } + } + @Test void failedBatchErrorPrefixIsRecordedInSpanAndMetrics() throws ReflectiveOperationException { RBatch batch = createBatch(redisson); From 110a764404c7baa243e0a1086717eda7903651ca Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 22 Oct 2026 18:03:48 -0700 Subject: [PATCH 15/22] Apply suggestion from @trask --- .../api/incubator/semconv/db/internal/RedisErrorPrefixTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefixTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefixTest.java index 6e40e1c786f3..43b0510e50d3 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefixTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/RedisErrorPrefixTest.java @@ -55,7 +55,7 @@ private static Stream serverErrorLines() { " Custom.Code!\r\nsuffix details", "Custom.Code!\r\nsuffix"), argumentSet("CRLF-only prefix", "\r\n", "\r\n"), - argumentSet("Unicode space", "ERR custom detail", "ERR custom"), + argumentSet("Unicode space", "ERR\u2003custom detail", "ERR\u2003custom"), argumentSet("leading non-breaking space", "\u00a0CODE detail", "\u00a0CODE"), argumentSet("Unicode text", "ERRé中 detail", "ERRé中")); } From 281ede5153ee15f74ebd34f20e0e31690b5063e0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:15:29 +0000 Subject: [PATCH 16/22] Refactor Redisson CRLF scanning Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../common/v3_0/RedissonErrorCapture.java | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java index 0e2933faffac..cc3ccdf3d910 100644 --- a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java @@ -43,29 +43,34 @@ public static ScopedThreadValue currentServerErrorType() { @Nullable public static String parseServerErrorType(ByteBuf input) { try { - int readerIndex = input.readerIndex(); - if (input.getByte(readerIndex) != '-') { + int replyStart = input.readerIndex(); + if (input.getByte(replyStart) != '-') { return null; } - int start = readerIndex + 1; - int writerIndex = input.writerIndex(); - int replyEnd = start; - while (replyEnd + 1 < writerIndex - && (input.getByte(replyEnd) != '\r' || input.getByte(replyEnd + 1) != '\n')) { - replyEnd++; - } - if (replyEnd + 1 >= writerIndex) { + int lineStart = replyStart + 1; + int lineEnd = findCrlf(input, lineStart); + if (lineEnd == -1) { return null; } - return RedisErrorPrefix.extract(input.toString(start, replyEnd - start, UTF_8)); + return RedisErrorPrefix.extract(input.toString(lineStart, lineEnd - lineStart, UTF_8)); } catch (Throwable ignored) { // ReplayingDecoder retries incomplete replies; leave buffer handling to Redisson. return null; } } + private static int findCrlf(ByteBuf input, int start) { + int end = input.writerIndex(); + for (int i = start; i + 1 < end; i++) { + if (input.getByte(i) == '\r' && input.getByte(i + 1) == '\n') { + return i; + } + } + return -1; + } + public static void captureServerError(@Nullable Throwable error) { if (!(error instanceof RedisException)) { return; From e1db51def9d0f730a8d353df1d1ad9030f58069e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:57:51 +0000 Subject: [PATCH 17/22] Use named Redis error virtual field Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../common/v3_0/RedissonErrorCapture.java | 17 ++++------------- .../redisson/AbstractRedissonClientTest.java | 6 +++--- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java index cc3ccdf3d910..cf9c19bdbb67 100644 --- a/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/common/v3_0/RedissonErrorCapture.java @@ -33,8 +33,8 @@ public final class RedissonErrorCapture { private static final ScopedThreadValue currentServerErrorType = new ScopedThreadValue<>(); - private static final VirtualField SERVER_ERROR = - VirtualField.find(RedisException.class, RedisServerError.class); + private static final VirtualField SERVER_ERROR = + VirtualField.find(RedisException.class, String.class, "redissonServerErrorType"); public static ScopedThreadValue currentServerErrorType() { return currentServerErrorType; @@ -78,7 +78,7 @@ public static void captureServerError(@Nullable Throwable error) { String errorType = currentServerErrorType.get(); if (errorType != null) { - SERVER_ERROR.set((RedisException) error, new RedisServerError(errorType)); + SERVER_ERROR.set((RedisException) error, errorType); // consume so completion callbacks run by tryFailure cannot attribute it to another command currentServerErrorType.set(null); } @@ -90,8 +90,7 @@ public static String getServerErrorType(@Nullable Throwable error) { return null; } - RedisServerError redisServerError = SERVER_ERROR.get((RedisException) error); - return redisServerError != null ? redisServerError.errorType : null; + return SERVER_ERROR.get((RedisException) error); } public static void clearServerError(@Nullable Throwable error) { @@ -100,13 +99,5 @@ public static void clearServerError(@Nullable Throwable error) { } } - private static final class RedisServerError { - private final String errorType; - - private RedisServerError(String errorType) { - this.errorType = errorType; - } - } - private RedissonErrorCapture() {} } diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index eb9dd204b241..1518670e6d02 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -317,7 +317,7 @@ void reentrantFailureCallbackKeepsClientErrorFallback() throws InterruptedExcept try { config = createConfig(0, null); } catch (ReflectiveOperationException e) { - throw new AssertionError(e); + throw new LinkageError(e.getMessage(), e); } config.useSingleServer().setConnectionPoolSize(1).setConnectionMinimumIdleSize(1); @@ -384,7 +384,7 @@ void reentrantFailureCallbackKeepsClientErrorFallback() throws InterruptedExcept assertThatThrownBy(serverError::join).hasRootCauseInstanceOf(RedisException.class); assertThat(failureCallbackCalled).isTrue(); assertThat(getMappedInFailureCallback).isTrue(); - assertThat(clientError.get()).isNotNull(); + assertThat((Object) clientError.get()).isNotNull(); assertThatThrownBy(() -> clientError.get().join()) .hasRootCauseInstanceOf(RedisException.class); callbackClient.getBucket("callback-success").set("success"); @@ -475,7 +475,7 @@ private static void setCommandMapper( .getMethod("setCommandMapper", commandMapperType) .invoke(config, commandMapper); } catch (ReflectiveOperationException e) { - throw new AssertionError(e); + throw new LinkageError(e.getMessage(), e); } } From 75b55b6f551e99ffb247f5aa8f0de232a4d268fc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 23 Oct 2026 03:26:45 +0000 Subject: [PATCH 18/22] Pin Redisson helper unit-test baseline Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../redisson/redisson-common-3.0/javaagent/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/redisson/redisson-common-3.0/javaagent/build.gradle.kts b/instrumentation/redisson/redisson-common-3.0/javaagent/build.gradle.kts index ab4afa58ddfd..744f2d586adb 100644 --- a/instrumentation/redisson/redisson-common-3.0/javaagent/build.gradle.kts +++ b/instrumentation/redisson/redisson-common-3.0/javaagent/build.gradle.kts @@ -15,7 +15,7 @@ testing { dependencies { implementation(project()) implementation(project(":instrumentation-api-incubator")) - implementation("org.redisson:redisson:${baseVersion("3.0.0").orLatest("3.16.+")}") + implementation("org.redisson:redisson:3.0.0") } } } From 7670cc9ce2d36f1e37d01717f11eec76ed4914b2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 23 Oct 2026 04:14:17 +0000 Subject: [PATCH 19/22] Simplify checked exceptions in Redisson tests Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../redisson/AbstractRedissonClientTest.java | 39 +++++++------------ 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index 1518670e6d02..2e840967f464 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -309,16 +309,12 @@ void serverErrorPrefixContract() { } @Test - void reentrantFailureCallbackKeepsClientErrorFallback() throws InterruptedException { + void reentrantFailureCallbackKeepsClientErrorFallback() + throws InterruptedException, ReflectiveOperationException { Assumptions.assumeTrue(isClassAvailable("org.redisson.config.CommandMapper")); Class commandMapperType = loadCommandMapperType(); - Config config; - try { - config = createConfig(0, null); - } catch (ReflectiveOperationException e) { - throw new LinkageError(e.getMessage(), e); - } + Config config = createConfig(0, null); config.useSingleServer().setConnectionPoolSize(1).setConnectionMinimumIdleSize(1); CountDownLatch evalMappingStarted = new CountDownLatch(1); @@ -456,27 +452,20 @@ void reentrantFailureCallbackKeepsClientErrorFallback() throws InterruptedExcept .doesNotContainKey(ERROR_TYPE))))); } - private static Class loadCommandMapperType() { - try { - return Class.forName( - "org.redisson.config.CommandMapper", - false, - AbstractRedissonClientTest.class.getClassLoader()); - } catch (ClassNotFoundException e) { - throw new AssertionError(e); - } + private static Class loadCommandMapperType() throws ClassNotFoundException { + return Class.forName( + "org.redisson.config.CommandMapper", + false, + AbstractRedissonClientTest.class.getClassLoader()); } private static void setCommandMapper( - Config config, Class commandMapperType, Object commandMapper) { - try { - config - .getClass() - .getMethod("setCommandMapper", commandMapperType) - .invoke(config, commandMapper); - } catch (ReflectiveOperationException e) { - throw new LinkageError(e.getMessage(), e); - } + Config config, Class commandMapperType, Object commandMapper) + throws ReflectiveOperationException { + config + .getClass() + .getMethod("setCommandMapper", commandMapperType) + .invoke(config, commandMapper); } @Test From 693bf82ded860dfffd577ea7296c6a6e835d3125 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 23 Oct 2026 04:32:12 +0000 Subject: [PATCH 20/22] Rename Redisson error assertion helper Co-authored-by: trask <218610+trask@users.noreply.github.com> --- .../redisson/AbstractRedissonAsyncClientTest.java | 10 +++++----- .../redisson/AbstractRedissonClientTest.java | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java index b833ef87d5ff..058b982dc189 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonAsyncClientTest.java @@ -231,7 +231,7 @@ void asyncWrongTypeErrorIsRecordedInSpanAndMetrics() throws IOException, Interru .toCompletableFuture() .join()) .hasRootCauseInstanceOf(RedisException.class); - assertErrorTypeOnLastOperation("WRONGTYPE"); + assertErrorTypeOnSpanAndMetric("WRONGTYPE"); assertThat( redisson @@ -240,7 +240,7 @@ void asyncWrongTypeErrorIsRecordedInSpanAndMetrics() throws IOException, Interru .toCompletableFuture() .join()) .isEqualTo(1L); - assertErrorTypeOnLastOperation(null); + assertErrorTypeOnSpanAndMetric(null); } @Test @@ -256,7 +256,7 @@ void asyncFailedBatchErrorPrefixIsRecordedInSpanAndMetrics() { assertThatThrownBy(() -> batch.executeAsync().toCompletableFuture().join()) .hasRootCauseInstanceOf(RedisException.class); - assertErrorTypeOnLastOperation("Batch.Code!"); + assertErrorTypeOnSpanAndMetric("Batch.Code!"); } @Test @@ -281,7 +281,7 @@ public Decoder getValueDecoder() { assertThatThrownBy(() -> bucket.getAsync().toCompletableFuture().join()) .hasRootCauseInstanceOf(RedisException.class); - assertErrorTypeOnLastOperation(RedisException.class.getName()); + assertErrorTypeOnSpanAndMetric(RedisException.class.getName()); } @Test @@ -322,7 +322,7 @@ void futureCallback() { span -> span.hasName("callback").hasKind(INTERNAL).hasParent(trace.getSpan(0)))); } - private static void assertErrorTypeOnLastOperation(String expectedErrorType) { + private static void assertErrorTypeOnSpanAndMetric(String expectedErrorType) { AtomicReference instrumentationName = new AtomicReference<>(); testing.waitAndAssertTraces( trace -> { diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index 2e840967f464..d01feda2482a 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -305,7 +305,7 @@ void serverErrorPrefixContract() { Object result = redisson.getScript().eval(RScript.Mode.READ_ONLY, "return 1", RScript.ReturnType.VALUE); assertThat(result).isEqualTo(1L); - assertErrorTypeOnLastOperation(null); + assertErrorTypeOnSpanAndMetric(null); } @Test @@ -480,7 +480,7 @@ void failedBatchErrorPrefixIsRecordedInSpanAndMetrics() throws ReflectiveOperati assertThatThrownBy(() -> invokeExecute(batch)).hasRootCauseInstanceOf(RedisException.class); - assertErrorTypeOnLastOperation("Batch.Code!"); + assertErrorTypeOnSpanAndMetric("Batch.Code!"); } @Test @@ -547,7 +547,7 @@ private void assertBatchServerErrorPrefix( assertThatThrownBy(() -> invokeExecute(batch)) .satisfies(error -> assertThat(rootCause(error)).isInstanceOf(expectedFailureType)); - assertErrorTypeOnLastOperation(expectedErrorType); + assertErrorTypeOnSpanAndMetric(expectedErrorType); } private RBatch createBatchWithoutRetries() throws ReflectiveOperationException { @@ -568,7 +568,7 @@ private static List batchResponses(Object result) throws ReflectiveOperationE private void assertFollowingOperationsHaveNoLeakedServerError() throws IOException, InterruptedException { redisson.getBucket("after-batch-server-error").set("success"); - assertErrorTypeOnLastOperation(null); + assertErrorTypeOnSpanAndMetric(null); assertClientSideDecodeFailure("client-side-after-batch-error"); } @@ -660,10 +660,10 @@ private void assertScriptErrorType( e -> assertThat(e).hasRootCauseInstanceOf(expectedCause)); } }); - assertErrorTypeOnLastOperation(expectedErrorType); + assertErrorTypeOnSpanAndMetric(expectedErrorType); } - private static void assertErrorTypeOnLastOperation(String expectedErrorType) { + private static void assertErrorTypeOnSpanAndMetric(String expectedErrorType) { AtomicReference instrumentationName = new AtomicReference<>(); testing.waitAndAssertTraces( trace -> { From 7dd02deaa389031e0fbaab91e51b2921624f84c9 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker <218610+trask@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:07:03 +0000 Subject: [PATCH 21/22] Keep Redisson error tests compatible with 4.x Use the shared VALUE script return type and wait through CompletableFuture instead of the removed RFuture.join API. Await callback completion before checking its effects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../redisson/AbstractRedissonClientTest.java | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index d01feda2482a..e6291cb1ea98 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -46,6 +46,7 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.concurrent.CompletionStage; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -365,23 +366,26 @@ void reentrantFailureCallbackKeepsClientErrorFallback() AtomicBoolean failureCallbackCalled = new AtomicBoolean(); AtomicReference> clientError = new AtomicReference<>(); - serverError.whenComplete( - (result, error) -> { - failureCallbackCalled.set(true); - failureCallbackActive.set(true); - try { - clientError.set(callbackClient.getBucket("callback-client-error").getAsync()); - } finally { - failureCallbackActive.set(false); - } - }); + CompletionStage failureCallback = + serverError.whenComplete( + (result, error) -> { + failureCallbackCalled.set(true); + failureCallbackActive.set(true); + try { + clientError.set( + callbackClient.getBucket("callback-client-error").getAsync()); + } finally { + failureCallbackActive.set(false); + } + }); allowEvalMapping.countDown(); - assertThatThrownBy(serverError::join).hasRootCauseInstanceOf(RedisException.class); + assertThatThrownBy(() -> failureCallback.toCompletableFuture().join()) + .hasRootCauseInstanceOf(RedisException.class); assertThat(failureCallbackCalled).isTrue(); assertThat(getMappedInFailureCallback).isTrue(); assertThat((Object) clientError.get()).isNotNull(); - assertThatThrownBy(() -> clientError.get().join()) + assertThatThrownBy(() -> clientError.get().toCompletableFuture().join()) .hasRootCauseInstanceOf(RedisException.class); callbackClient.getBucket("callback-success").set("success"); }); @@ -502,7 +506,7 @@ void batchRedirectIsRetriedAndPreservesBatchResult() + redirectServer.getMappedPort(6379) + "'); end; return count"; RBatch batch = createBatch(redisson); - batch.getScript().evalAsync(RScript.Mode.READ_WRITE, script, RScript.ReturnType.INTEGER); + batch.getScript().evalAsync(RScript.Mode.READ_WRITE, script, RScript.ReturnType.VALUE); assertThat(batchResponses(invokeExecute(batch))).isEqualTo(asList(2L)); assertRedirectErrorTypeOnAttempts(); From 89d22f5ea980e6e86791a2a09fd17596968cc59e Mon Sep 17 00:00:00 2001 From: Trask Stalnaker <218610+trask@users.noreply.github.com> Date: Fri, 23 Oct 2026 17:21:43 +0000 Subject: [PATCH 22/22] Avoid intercepting Redisson startup commands in callback test Leave startup command mapping unchanged until Redisson.create completes, then enable the callback test's EVAL gate and client-side GET failure. This prevents the Redisson 4.8 cluster-detection EVAL from timing out and exporting an unexpected trace. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../instrumentation/redisson/AbstractRedissonClientTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java index e6291cb1ea98..225ac4f0cfc8 100644 --- a/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java +++ b/instrumentation/redisson/redisson-common-3.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/redisson/AbstractRedissonClientTest.java @@ -320,6 +320,7 @@ void reentrantFailureCallbackKeepsClientErrorFallback() CountDownLatch evalMappingStarted = new CountDownLatch(1); CountDownLatch allowEvalMapping = new CountDownLatch(1); + AtomicBoolean interceptCommands = new AtomicBoolean(); AtomicBoolean failureCallbackActive = new AtomicBoolean(); AtomicBoolean getMappedInFailureCallback = new AtomicBoolean(); Object commandMapper = @@ -329,6 +330,9 @@ void reentrantFailureCallbackKeepsClientErrorFallback() (proxy, method, args) -> { if (method.getName().equals("map")) { String command = (String) args[0]; + if (!interceptCommands.get()) { + return command; + } if (command.equals("EVAL") || command.equals("EVALSHA")) { evalMappingStarted.countDown(); try { @@ -350,6 +354,7 @@ void reentrantFailureCallbackKeepsClientErrorFallback() setCommandMapper(config, commandMapperType, commandMapper); RedissonClient callbackClient = Redisson.create(config); + interceptCommands.set(true); testing.clearData(); try { testing.runWithSpan(