Repository navigation
Add Jedis pipeline span batch support - #19054
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Jedis pipeline span aggregation to the javaagent instrumentation for Jedis 1.4, 3.0, and 4.0. Instead of dropping or individually tracing commands queued in a pipeline, the queued commands are captured and merged into a single CLIENT span emitted when the pipeline is synced. A new shared helper (JedisPipelineContext, in the jedis-common-1.4 module) holds the per-thread "current pipeline" and the captured requests, and each JedisRequest is extended with operation-name/query-text overrides plus a batch size so the aggregate span can expose db.operation.batch.size and a combined statement.
Changes:
- Suppress per-command spans while a pipeline is active and emit one aggregated span on
sync/syncAndReturnAll(orgetAllfor the legacy Jedis 1.4 callback pipeline). - Compute aggregate operation name (
SET,PIPELINE SET, orPIPELINE),;-joined sanitized statement, and batch size; surface batch size viagetDbOperationBatchSize. - Add parameterized pipeline tests (empty, single, same-operation, mixed-operation) across all three versions, including a reflective/callback runner for Jedis 1.4.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| jedis-common-1.4/.../JedisPipelineContext.java | New shared per-thread pipeline state and request capture store |
| jedis-4.0/.../JedisRequest.java | Adds override/batch-size fields and pipeline aggregation factory |
| jedis-4.0/.../JedisPipelineInstrumentation.java | New advice capturing queued commands and emitting span on sync |
| jedis-4.0/.../JedisConnectionInstrumentation.java | Captures requests into pipeline context; threads socket through |
| jedis-4.0/.../JedisDbAttributesGetter.java | Exposes db.operation.batch.size |
| jedis-4.0/.../JedisInstrumentationModule.java | Registers pipeline instrumentation |
| jedis-4.0/.../Jedis40ClientTest.java | Parameterized pipeline tests |
| jedis-3.0/.../JedisRequest.java | Same aggregation logic (with a redundant double loop) |
| jedis-3.0/.../JedisPipelineInstrumentation.java | New pipeline advice for Jedis 3.0 |
| jedis-3.0/.../JedisConnectionInstrumentation.java | Captures requests into pipeline context |
| jedis-3.0/.../JedisDbAttributesGetter.java | Exposes db.operation.batch.size |
| jedis-3.0/.../JedisInstrumentationModule.java | Registers pipeline instrumentation |
| jedis-3.0/.../Jedis30ClientTest.java | Parameterized pipeline tests |
| jedis-1.4/.../JedisRequest.java | Adds sanitizer + aggregation logic |
| jedis-1.4/.../JedisPipelineInstrumentation.java | New advice covering callback and no-arg pipelines |
| jedis-1.4/.../JedisConnectionInstrumentation.java | Capture + getAll advice for legacy callback pipeline |
| jedis-1.4/.../JedisDbAttributesGetter.java | Delegates query/operation/batch size to JedisRequest |
| jedis-1.4/.../JedisInstrumentationModule.java | Registers pipeline instrumentation |
| jedis-1.4/testing/.../AbstractJedisTest.java | Shared parameterized pipeline tests + reflective runner |
| jedis-1.4/testing/.../Jedis14PipelineRunner.java | Callback-based pipeline runner for Jedis 1.x |
…ponse) boundary jedis 1.4.0's callback pipeline class (JedisPipeline) was removed in 1.5 and the object-style Response/sync pipeline only arrived in 2.0, so callback-pipeline batching cannot span the [1.4.0,2.0.0) range. Remove the batching feature from jedis-1.4 and gate the module on not(Response) so base instrumentation covers the whole pre-2.0 line (including 1.5.x). Retarget jedis-2.0's classLoaderMatcher to the real 2.0 marker (Response) instead of Pipeline (1.5).
| @@ -5,6 +5,161 @@ | |||
|
|
|||
There was a problem hiding this comment.
Copied from instrumentation/jedis/jedis-1.4/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/AbstractJedisTest.java
--- a/instrumentation/jedis/jedis-1.4/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/AbstractJedisTest.java
+++ b/instrumentation/jedis/jedis-1.4/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jedis/v1_4/JedisClientTest.java
@@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.javaagent.instrumentation.jedis;
+package io.opentelemetry.javaagent.instrumentation.jedis.v1_4;
import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableDatabaseSemconv;
import static io.opentelemetry.instrumentation.testing.junit.db.DbClientMetricsTestUtil.assertDurationMetric;
@@ -32,7 +32,7 @@ import org.testcontainers.containers.GenericContainer;
import redis.clients.jedis.Jedis;
@SuppressWarnings("deprecation") // using deprecated semconv
-public abstract class AbstractJedisTest {
+class JedisClientTest {
@RegisterExtension
private static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
| @@ -0,0 +1,49 @@ | |||
| plugins { | |||
There was a problem hiding this comment.
Copied from instrumentation/jedis/jedis-3.0/javaagent/build.gradle.kts
--- a/instrumentation/jedis/jedis-3.0/javaagent/build.gradle.kts
+++ b/instrumentation/jedis/jedis-2.0/javaagent/build.gradle.kts
@@ -6,25 +6,27 @@ muzzle {
pass {
group.set("redis.clients")
module.set("jedis")
- versions.set("[3.0.0,4)")
- skip("jedis-3.6.2")
+ versions.set("[2.0.0,3.0.0)")
assertInverse.set(true)
}
}
dependencies {
- library("redis.clients:jedis:3.0.0")
+ library("redis.clients:jedis:2.0.0")
compileOnly("com.google.auto.value:auto-value-annotations")
annotationProcessor("com.google.auto.value:auto-value")
implementation(project(":instrumentation:jedis:jedis-common-1.4:javaagent"))
+ testImplementation("io.opentelemetry.javaagent:opentelemetry-testing-common")
+ testImplementation("org.testcontainers:testcontainers")
+
testInstrumentation(project(":instrumentation:jedis:jedis-1.4:javaagent"))
- testInstrumentation(project(":instrumentation:jedis:jedis-2.0:javaagent"))
+ testInstrumentation(project(":instrumentation:jedis:jedis-3.0:javaagent"))
testInstrumentation(project(":instrumentation:jedis:jedis-4.0:javaagent"))
- latestDepTestLibrary("redis.clients:jedis:3.+") // see jedis-4.0 module
+ latestDepTestLibrary("redis.clients:jedis:2.+") // see jedis-3.0 module
}
tasks {| @@ -0,0 +1,142 @@ | |||
| /* | |||
There was a problem hiding this comment.
Copied from instrumentation/jedis/jedis-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v1_4/JedisConnectionInstrumentation.java
--- a/instrumentation/jedis/jedis-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v1_4/JedisConnectionInstrumentation.java
+++ b/instrumentation/jedis/jedis-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v2_0/JedisConnectionInstrumentation.java
@@ -3,10 +3,10 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.javaagent.instrumentation.jedis.v1_4;
+package io.opentelemetry.javaagent.instrumentation.jedis.v2_0;
import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext;
-import static io.opentelemetry.javaagent.instrumentation.jedis.v1_4.JedisSingletons.instrumenter;
+import static io.opentelemetry.javaagent.instrumentation.jedis.v2_0.JedisSingletons.instrumenter;
import static java.util.Arrays.asList;
import static net.bytebuddy.matcher.ElementMatchers.is;
import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -71,7 +71,17 @@ class JedisConnectionInstrumentation implements TypeInstrumentation {
@Nullable
public static AdviceScope start(JedisRequest request) {
+ if (JedisPipelineContext.inTransactionFraming()) {
+ // MULTI/EXEC/DISCARD frame a batched transaction; they are represented by the MULTI batch
+ // span rather than getting their own spans.
+ return null;
+ }
Context parentContext = currentContext();
+ if (JedisPipelineContext.capture(request)) {
+ // A pipeline or transaction is active, so this command is captured and aggregated into the
+ // batch span created at sync()/exec() rather than getting its own span.
+ return null;
+ }
if (!instrumenter().shouldStart(parentContext, request)) {
return null;
}| @@ -0,0 +1,50 @@ | |||
| /* | |||
There was a problem hiding this comment.
Copied from instrumentation/jedis/jedis-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/JedisDbAttributesGetter.java
--- a/instrumentation/jedis/jedis-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/JedisDbAttributesGetter.java
+++ b/instrumentation/jedis/jedis-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v2_0/JedisDbAttributesGetter.java
@@ -3,12 +3,10 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.javaagent.instrumentation.jedis.v3_0;
+package io.opentelemetry.javaagent.instrumentation.jedis.v2_0;
import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientAttributesGetter;
import io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DbSystemNameIncubatingValues;
-import java.net.InetSocketAddress;
-import java.net.Socket;
import javax.annotation.Nullable;
final class JedisDbAttributesGetter implements DbClientAttributesGetter<JedisRequest, Void> {
@@ -49,15 +47,4 @@ final class JedisDbAttributesGetter implements DbClientAttributesGetter<JedisReq
public Integer getServerPort(JedisRequest request) {
return request.getConnection().getPort();
}
-
- @Override
- @Nullable
- public InetSocketAddress getNetworkPeerInetSocketAddress(
- JedisRequest request, @Nullable Void unused) {
- Socket socket = request.getConnection().getSocket();
- if (socket != null && socket.getRemoteSocketAddress() instanceof InetSocketAddress) {
- return (InetSocketAddress) socket.getRemoteSocketAddress();
- }
- return null;
- }
}| @@ -0,0 +1,86 @@ | |||
| /* | |||
There was a problem hiding this comment.
Copied from instrumentation/jedis/jedis-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/JedisInstrumentation.java
--- a/instrumentation/jedis/jedis-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/JedisInstrumentation.java
+++ b/instrumentation/jedis/jedis-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v2_0/JedisInstrumentation.java
@@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.javaagent.instrumentation.jedis.v3_0;
+package io.opentelemetry.javaagent.instrumentation.jedis.v2_0;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
@@ -44,10 +44,7 @@ class JedisInstrumentation implements TypeInstrumentation {
"resetState",
"getClient",
"disconnect",
- "getConnection",
- "isBroken",
- "multi",
- "toString"))),
+ "multi"))),
getClass().getName() + "$JedisMethodAdvice");
transformer.applyAdviceToMethod(
named("multi").and(takesArguments(0)), getClass().getName() + "$MultiAdvice");| @@ -0,0 +1,109 @@ | |||
| /* | |||
There was a problem hiding this comment.
Copied from instrumentation/jedis/jedis-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/JedisTransactionInstrumentation.java
--- a/instrumentation/jedis/jedis-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/JedisTransactionInstrumentation.java
+++ b/instrumentation/jedis/jedis-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v2_0/JedisTransactionInstrumentation.java
@@ -3,10 +3,10 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.javaagent.instrumentation.jedis.v3_0;
+package io.opentelemetry.javaagent.instrumentation.jedis.v2_0;
import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext;
-import static io.opentelemetry.javaagent.instrumentation.jedis.v3_0.JedisSingletons.instrumenter;
+import static io.opentelemetry.javaagent.instrumentation.jedis.v2_0.JedisSingletons.instrumenter;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.namedOneOf;
@@ -19,18 +19,18 @@ import javax.annotation.Nullable;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
-import redis.clients.jedis.Transaction;
class JedisTransactionInstrumentation implements TypeInstrumentation {
@Override
public ElementMatcher<TypeDescription> typeMatcher() {
- return named("redis.clients.jedis.Transaction");
+ // exec()/discard() are declared on BinaryTransaction in jedis 2.0.x and on Transaction in later
+ // 2.x; match both so they are instrumented across the range.
+ return namedOneOf("redis.clients.jedis.Transaction", "redis.clients.jedis.BinaryTransaction");
}
@Override
public void transform(TypeTransformer transformer) {
- transformer.applyAdviceToMethod(
- namedOneOf("exec", "execGetResponse"), getClass().getName() + "$ExecAdvice");
+ transformer.applyAdviceToMethod(named("exec"), getClass().getName() + "$ExecAdvice");
transformer.applyAdviceToMethod(named("discard"), getClass().getName() + "$DiscardAdvice");
}
@@ -49,7 +49,7 @@ class JedisTransactionInstrumentation implements TypeInstrumentation {
this.request = request;
}
- public static AdviceScope start(Transaction transaction) {
+ public static AdviceScope start(Object transaction) {
List<JedisRequest> requests = JedisPipelineContext.getAndClearCapturedRequests(transaction);
// Suppress the EXEC framing command's own span; the transaction is reported as a single
// batch span here.
@@ -79,7 +79,7 @@ class JedisTransactionInstrumentation implements TypeInstrumentation {
}
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
- public static AdviceScope onEnter(@Advice.This Transaction transaction) {
+ public static AdviceScope onEnter(@Advice.This Object transaction) {
return AdviceScope.start(transaction);
}
@@ -94,7 +94,7 @@ class JedisTransactionInstrumentation implements TypeInstrumentation {
public static class DiscardAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
- public static void onEnter(@Advice.This Transaction transaction) {
+ public static void onEnter(@Advice.This Object transaction) {
// A discarded transaction is abandoned, so drop its captured commands without reporting a
// batch span, and suppress the DISCARD framing command's own span.
JedisPipelineContext.getAndClearCapturedRequests(transaction);| @@ -0,0 +1,365 @@ | |||
| /* | |||
There was a problem hiding this comment.
Copied from instrumentation/jedis/jedis-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/Jedis30ClientTest.java
--- a/instrumentation/jedis/jedis-3.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/Jedis30ClientTest.java
+++ b/instrumentation/jedis/jedis-2.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jedis/v2_0/JedisClientTest.java
@@ -3,27 +3,21 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.javaagent.instrumentation.jedis.v3_0;
+package io.opentelemetry.javaagent.instrumentation.jedis.v2_0;
-import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitOldDatabaseSemconv;
import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableDatabaseSemconv;
import static io.opentelemetry.instrumentation.testing.junit.db.DbClientMetricsTestUtil.assertDurationMetric;
import static io.opentelemetry.instrumentation.testing.junit.db.SemconvStabilityUtil.maybeStable;
import static io.opentelemetry.instrumentation.testing.junit.service.SemconvServiceStabilityUtil.maybeStablePeerService;
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
-import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies;
import static io.opentelemetry.semconv.DbAttributes.DB_OPERATION_BATCH_SIZE;
-import static io.opentelemetry.semconv.DbAttributes.DB_OPERATION_NAME;
-import static io.opentelemetry.semconv.DbAttributes.DB_SYSTEM_NAME;
-import static io.opentelemetry.semconv.NetworkAttributes.NETWORK_PEER_ADDRESS;
-import static io.opentelemetry.semconv.NetworkAttributes.NETWORK_PEER_PORT;
-import static io.opentelemetry.semconv.NetworkAttributes.NETWORK_TYPE;
-import static io.opentelemetry.semconv.NetworkAttributes.NetworkTypeValues.IPV4;
import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS;
import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT;
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION;
+import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION_NAME;
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_STATEMENT;
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SYSTEM;
+import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SYSTEM_NAME;
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DbSystemNameIncubatingValues.REDIS;
import static org.assertj.core.api.Assertions.assertThat;
@@ -31,11 +25,8 @@ import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
-import java.net.InetAddress;
-import java.net.UnknownHostException;
import java.util.function.Consumer;
import java.util.stream.Stream;
-import org.assertj.core.api.AbstractLongAssert;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -49,7 +40,7 @@ import redis.clients.jedis.Pipeline;
import redis.clients.jedis.Transaction;
@SuppressWarnings("deprecation") // using deprecated semconv
-class Jedis30ClientTest {
+class JedisClientTest {
@RegisterExtension
private static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
@@ -61,21 +52,18 @@ class Jedis30ClientTest {
private static String host;
- private static String ip;
-
private static int port;
private static Jedis jedis;
@BeforeAll
- static void setup() throws UnknownHostException {
+ static void setup() {
redisServer.start();
cleanup.deferAfterAll(redisServer::stop);
host = redisServer.getHost();
- ip = InetAddress.getByName(host).getHostAddress();
port = redisServer.getMappedPort(6379);
jedis = new Jedis(host, port);
- cleanup.deferAfterAll(jedis);
+ cleanup.deferAfterAll(jedis::disconnect);
}
@BeforeEach
@@ -101,20 +89,15 @@ class Jedis30ClientTest {
equalTo(maybeStable(DB_OPERATION), "SET"),
equalTo(maybeStablePeerService(), "test-peer-service"),
equalTo(SERVER_ADDRESS, host),
- equalTo(SERVER_PORT, port),
- equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
- equalTo(NETWORK_PEER_ADDRESS, ip),
- satisfies(NETWORK_PEER_PORT, AbstractLongAssert::isNotNegative))));
+ equalTo(SERVER_PORT, port))));
assertDurationMetric(
testing,
- "io.opentelemetry.jedis-3.0",
- DB_OPERATION_NAME,
+ "io.opentelemetry.jedis-2.0",
DB_SYSTEM_NAME,
+ DB_OPERATION_NAME,
SERVER_ADDRESS,
- SERVER_PORT,
- NETWORK_PEER_ADDRESS,
- NETWORK_PEER_PORT);
+ SERVER_PORT);
}
@Test
@@ -136,10 +119,7 @@ class Jedis30ClientTest {
equalTo(maybeStable(DB_OPERATION), "SET"),
equalTo(maybeStablePeerService(), "test-peer-service"),
equalTo(SERVER_ADDRESS, host),
- equalTo(SERVER_PORT, port),
- equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
- equalTo(NETWORK_PEER_ADDRESS, ip),
- satisfies(NETWORK_PEER_PORT, AbstractLongAssert::isNotNegative))),
+ equalTo(SERVER_PORT, port))),
trace ->
trace.hasSpansSatisfyingExactly(
span ->
@@ -151,10 +131,7 @@ class Jedis30ClientTest {
equalTo(maybeStable(DB_OPERATION), "GET"),
equalTo(maybeStablePeerService(), "test-peer-service"),
equalTo(SERVER_ADDRESS, host),
- equalTo(SERVER_PORT, port),
- equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
- equalTo(NETWORK_PEER_ADDRESS, ip),
- satisfies(NETWORK_PEER_PORT, AbstractLongAssert::isNotNegative))));
+ equalTo(SERVER_PORT, port))));
}
@Test
@@ -176,10 +153,7 @@ class Jedis30ClientTest {
equalTo(maybeStable(DB_OPERATION), "SET"),
equalTo(maybeStablePeerService(), "test-peer-service"),
equalTo(SERVER_ADDRESS, host),
- equalTo(SERVER_PORT, port),
- equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
- equalTo(NETWORK_PEER_ADDRESS, ip),
- satisfies(NETWORK_PEER_PORT, AbstractLongAssert::isNotNegative))),
+ equalTo(SERVER_PORT, port))),
trace ->
trace.hasSpansSatisfyingExactly(
span ->
@@ -194,10 +168,7 @@ class Jedis30ClientTest {
equalTo(maybeStable(DB_OPERATION), "RANDOMKEY"),
equalTo(maybeStablePeerService(), "test-peer-service"),
equalTo(SERVER_ADDRESS, host),
- equalTo(SERVER_PORT, port),
- equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
- equalTo(NETWORK_PEER_ADDRESS, ip),
- satisfies(NETWORK_PEER_PORT, AbstractLongAssert::isNotNegative))));
+ equalTo(SERVER_PORT, port))));
}
@ParameterizedTest
@@ -230,10 +201,7 @@ class Jedis30ClientTest {
emitStableDatabaseSemconv() ? scenario.batchSize : null),
equalTo(maybeStablePeerService(), "test-peer-service"),
equalTo(SERVER_ADDRESS, host),
- equalTo(SERVER_PORT, port),
- equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
- equalTo(NETWORK_PEER_ADDRESS, ip),
- satisfies(NETWORK_PEER_PORT, AbstractLongAssert::isNotNegative))));
+ equalTo(SERVER_PORT, port))));
}
@ParameterizedTest
@@ -269,10 +237,7 @@ class Jedis30ClientTest {
emitStableDatabaseSemconv() ? scenario.batchSize : null),
equalTo(maybeStablePeerService(), "test-peer-service"),
equalTo(SERVER_ADDRESS, host),
- equalTo(SERVER_PORT, port),
- equalTo(NETWORK_TYPE, emitOldDatabaseSemconv() ? IPV4 : null),
- equalTo(NETWORK_PEER_ADDRESS, ip),
- satisfies(NETWORK_PEER_PORT, AbstractLongAssert::isNotNegative))));
+ equalTo(SERVER_PORT, port))));
}
@Test| @@ -0,0 +1,82 @@ | |||
| /* | |||
There was a problem hiding this comment.
Copied from instrumentation/jedis/jedis-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v4_0/JedisPipelineContext.java
--- a/instrumentation/jedis/jedis-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v4_0/JedisPipelineContext.java
+++ b/instrumentation/jedis/jedis-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/JedisPipelineContext.java
@@ -3,38 +3,35 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.javaagent.instrumentation.jedis.v4_0;
+package io.opentelemetry.javaagent.instrumentation.jedis.v3_0;
import static java.util.Collections.emptyList;
import io.opentelemetry.instrumentation.api.util.VirtualField;
import java.util.ArrayList;
import java.util.List;
-import javax.annotation.Nullable;
import redis.clients.jedis.Pipeline;
+import redis.clients.jedis.PipelineBase;
import redis.clients.jedis.Transaction;
public final class JedisPipelineContext {
- // Pipeline and Transaction have no common supertype across the supported jedis range (Queable was
- // removed in 5.0), so captured requests are keyed on each concrete type separately.
- private static final ThreadLocal<Object> currentBatch = new ThreadLocal<>();
+ private static final ThreadLocal<PipelineBase> currentPipeline = new ThreadLocal<>();
private static final ThreadLocal<Boolean> inTransactionFraming = new ThreadLocal<>();
- private static final VirtualField<Pipeline, CapturedRequests> PIPELINE_REQUESTS =
- VirtualField.find(Pipeline.class, CapturedRequests.class);
- private static final VirtualField<Transaction, CapturedRequests> TRANSACTION_REQUESTS =
- VirtualField.find(Transaction.class, CapturedRequests.class);
-
- public static void enter(Object batch) {
- // Pipeline aggregates at sync() and Transaction at exec(); both capture their queued commands
- // here. Other batch subtypes have no flush point, so leaving them uncaptured keeps their
- // per-command spans.
- if (batch instanceof Pipeline || batch instanceof Transaction) {
- currentBatch.set(batch);
+ private static final VirtualField<PipelineBase, CapturedRequests> CAPTURED_REQUESTS =
+ VirtualField.find(PipelineBase.class, CapturedRequests.class);
+
+ public static void enter(PipelineBase pipeline) {
+ // Pipeline aggregates at sync(), Transaction at exec(); both capture their queued commands
+ // here.
+ // Other MultiKeyPipelineBase subtypes have no flush point, so leaving them uncaptured keeps
+ // their per-command spans.
+ if (pipeline instanceof Pipeline || pipeline instanceof Transaction) {
+ currentPipeline.set(pipeline);
}
}
public static void exit() {
- currentBatch.remove();
+ currentPipeline.remove();
}
public static void enterTransactionFraming() {
@@ -50,51 +47,23 @@ public final class JedisPipelineContext {
}
public static boolean capture(JedisRequest request) {
- CapturedRequests requests = getOrCreateCapturedRequests(currentBatch.get());
- if (requests == null) {
+ PipelineBase pipeline = currentPipeline.get();
+ if (pipeline == null) {
return false;
}
+ CapturedRequests requests = CAPTURED_REQUESTS.get(pipeline);
+ if (requests == null) {
+ requests = new CapturedRequests();
+ CAPTURED_REQUESTS.set(pipeline, requests);
+ }
requests.add(request);
return true;
}
- @Nullable
- private static CapturedRequests getOrCreateCapturedRequests(@Nullable Object batch) {
- if (batch instanceof Pipeline) {
- Pipeline pipeline = (Pipeline) batch;
- CapturedRequests requests = PIPELINE_REQUESTS.get(pipeline);
- if (requests == null) {
- requests = new CapturedRequests();
- PIPELINE_REQUESTS.set(pipeline, requests);
- }
- return requests;
- }
- if (batch instanceof Transaction) {
- Transaction transaction = (Transaction) batch;
- CapturedRequests requests = TRANSACTION_REQUESTS.get(transaction);
- if (requests == null) {
- requests = new CapturedRequests();
- TRANSACTION_REQUESTS.set(transaction, requests);
- }
- return requests;
- }
- return null;
- }
-
- public static List<JedisRequest> getAndClearCapturedRequests(Object batch) {
- if (batch instanceof Pipeline) {
- Pipeline pipeline = (Pipeline) batch;
- CapturedRequests requests = PIPELINE_REQUESTS.get(pipeline);
- PIPELINE_REQUESTS.set(pipeline, null);
- return requests != null ? requests.requests() : emptyList();
- }
- if (batch instanceof Transaction) {
- Transaction transaction = (Transaction) batch;
- CapturedRequests requests = TRANSACTION_REQUESTS.get(transaction);
- TRANSACTION_REQUESTS.set(transaction, null);
- return requests != null ? requests.requests() : emptyList();
- }
- return emptyList();
+ public static List<JedisRequest> getAndClearCapturedRequests(PipelineBase pipeline) {
+ CapturedRequests requests = CAPTURED_REQUESTS.get(pipeline);
+ CAPTURED_REQUESTS.set(pipeline, null);
+ return requests != null ? requests.requests() : emptyList();
}
private static class CapturedRequests {| @@ -0,0 +1,114 @@ | |||
| /* | |||
There was a problem hiding this comment.
Copied from instrumentation/jedis/jedis-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v4_0/JedisPipelineInstrumentation.java
--- a/instrumentation/jedis/jedis-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v4_0/JedisPipelineInstrumentation.java
+++ b/instrumentation/jedis/jedis-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/JedisPipelineInstrumentation.java
@@ -3,12 +3,15 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.javaagent.instrumentation.jedis.v4_0;
+package io.opentelemetry.javaagent.instrumentation.jedis.v3_0;
import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext;
-import static io.opentelemetry.javaagent.instrumentation.jedis.v4_0.JedisSingletons.instrumenter;
+import static io.opentelemetry.javaagent.instrumentation.jedis.v3_0.JedisSingletons.instrumenter;
+import static net.bytebuddy.matcher.ElementMatchers.isMethod;
+import static net.bytebuddy.matcher.ElementMatchers.isPublic;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.namedOneOf;
+import static net.bytebuddy.matcher.ElementMatchers.returns;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
@@ -19,26 +22,23 @@ import javax.annotation.Nullable;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
+import redis.clients.jedis.PipelineBase;
class JedisPipelineInstrumentation implements TypeInstrumentation {
@Override
public ElementMatcher<TypeDescription> typeMatcher() {
- // appendCommand is declared on different pipeline/transaction base types across the supported
- // jedis range (the names churn between 4.x, 5.x, and 7.x); match all of them so both pipelines
- // and transactions are captured.
return namedOneOf(
"redis.clients.jedis.Pipeline",
"redis.clients.jedis.PipelineBase",
- "redis.clients.jedis.Transaction",
- "redis.clients.jedis.PipelinedTransactionBase",
- "redis.clients.jedis.TransactionBase",
- "redis.clients.jedis.PipeliningBase");
+ "redis.clients.jedis.MultiKeyPipelineBase");
}
@Override
public void transform(TypeTransformer transformer) {
+ // Queued pipeline command methods return Response.
transformer.applyAdviceToMethod(
- named("appendCommand"), getClass().getName() + "$QueueCommandAdvice");
+ isPublic().and(isMethod()).and(returns(named("redis.clients.jedis.Response"))),
+ getClass().getName() + "$QueueCommandAdvice");
transformer.applyAdviceToMethod(
namedOneOf("sync", "syncAndReturnAll"), getClass().getName() + "$SyncAdvice");
}
@@ -47,7 +47,7 @@ class JedisPipelineInstrumentation implements TypeInstrumentation {
public static class QueueCommandAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
- public static void onEnter(@Advice.This Object pipeline) {
+ public static void onEnter(@Advice.This PipelineBase pipeline) {
// Attaches a thread-local pipeline that the nested Connection.sendCommand advice uses to
// collect captured requests; sync() then consumes them to build the batch span.
JedisPipelineContext.enter(pipeline);
@@ -74,7 +74,7 @@ class JedisPipelineInstrumentation implements TypeInstrumentation {
}
@Nullable
- public static AdviceScope start(Object pipeline) {
+ public static AdviceScope start(PipelineBase pipeline) {
List<JedisRequest> requests = JedisPipelineContext.getAndClearCapturedRequests(pipeline);
if (requests.isEmpty()) {
// An empty pipeline sends nothing to the server, and with no captured request there is no
@@ -98,7 +98,7 @@ class JedisPipelineInstrumentation implements TypeInstrumentation {
@Nullable
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
- public static AdviceScope onEnter(@Advice.This Object pipeline) {
+ public static AdviceScope onEnter(@Advice.This PipelineBase pipeline) {
return AdviceScope.start(pipeline);
}
| @@ -0,0 +1,109 @@ | |||
| /* | |||
There was a problem hiding this comment.
Copied from instrumentation/jedis/jedis-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v4_0/JedisTransactionInstrumentation.java
--- a/instrumentation/jedis/jedis-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v4_0/JedisTransactionInstrumentation.java
+++ b/instrumentation/jedis/jedis-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/JedisTransactionInstrumentation.java
@@ -3,10 +3,10 @@
* SPDX-License-Identifier: Apache-2.0
*/
-package io.opentelemetry.javaagent.instrumentation.jedis.v4_0;
+package io.opentelemetry.javaagent.instrumentation.jedis.v3_0;
import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext;
-import static io.opentelemetry.javaagent.instrumentation.jedis.v4_0.JedisSingletons.instrumenter;
+import static io.opentelemetry.javaagent.instrumentation.jedis.v3_0.JedisSingletons.instrumenter;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.namedOneOf;
@@ -19,21 +19,18 @@ import javax.annotation.Nullable;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
+import redis.clients.jedis.Transaction;
class JedisTransactionInstrumentation implements TypeInstrumentation {
@Override
public ElementMatcher<TypeDescription> typeMatcher() {
- // exec()/discard() are declared on different transaction base types across the jedis range.
- return namedOneOf(
- "redis.clients.jedis.Transaction",
- "redis.clients.jedis.PipelinedTransactionBase",
- "redis.clients.jedis.TransactionBase",
- "redis.clients.jedis.AbstractTransaction");
+ return named("redis.clients.jedis.Transaction");
}
@Override
public void transform(TypeTransformer transformer) {
- transformer.applyAdviceToMethod(named("exec"), getClass().getName() + "$ExecAdvice");
+ transformer.applyAdviceToMethod(
+ namedOneOf("exec", "execGetResponse"), getClass().getName() + "$ExecAdvice");
transformer.applyAdviceToMethod(named("discard"), getClass().getName() + "$DiscardAdvice");
}
@@ -52,7 +49,7 @@ class JedisTransactionInstrumentation implements TypeInstrumentation {
this.request = request;
}
- public static AdviceScope start(Object transaction) {
+ public static AdviceScope start(Transaction transaction) {
List<JedisRequest> requests = JedisPipelineContext.getAndClearCapturedRequests(transaction);
// Suppress the EXEC framing command's own span; the transaction is reported as a single
// batch span here.
@@ -82,7 +79,7 @@ class JedisTransactionInstrumentation implements TypeInstrumentation {
}
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
- public static AdviceScope onEnter(@Advice.This Object transaction) {
+ public static AdviceScope onEnter(@Advice.This Transaction transaction) {
return AdviceScope.start(transaction);
}
@@ -97,7 +94,7 @@ class JedisTransactionInstrumentation implements TypeInstrumentation {
public static class DiscardAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
- public static void onEnter(@Advice.This Object transaction) {
+ public static void onEnter(@Advice.This Transaction transaction) {
// A discarded transaction is abandoned, so drop its captured commands without reporting a
// batch span, and suppress the DISCARD framing command's own span.
JedisPipelineContext.getAndClearCapturedRequests(transaction);# Conflicts: # instrumentation/jedis/jedis-1.4/javaagent/build.gradle.kts
Implementing batch support for 1.4 would have required splitting out 1.5 module as well, which didn't seem worth it given that 2.0 was released in 2011. In fact, given how long ago 2.0 was released, I went ahead and disabled 1.4 instrumentation behind the v3preview flag, and we can drop it in the Java agent 3.0 release.
jedis-2.0javaagent module and narrow the existingjedis-1.4muzzle/classloader range from[1.4.0, 3.0.0)to[1.4.0, 2.0.0)so each module covers a distinct version rangejedis-1.4module and disable it under v3-preview