diff --git a/CHANGES.md b/CHANGES.md index c6e9e5862e..2dd610a5cd 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,6 +5,9 @@ Release Notes. 9.8.0 ------------------ +* Fix `jedis-4.x-plugin`'s `AbstractConnectionInterceptor` double-stopping the span stack on any + Redis-level exception (or a null dynamic field on a pooled/recycled `Connection`), which corrupted + the parent trace for the rest of the request (apache/skywalking#14085). * Add Spring LDAP 3.3.x-4.x plugin. * Exclude macOS metadata files from source and binary release archives (apache/skywalking#14080). * Fix `NoSuchMethodError: org.apache.skywalking.apm.plugin.spring.webflux.v6.DispatcherHandlerHandleMethodInterceptor` diff --git a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/AbstractConnectionInterceptor.java b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/AbstractConnectionInterceptor.java index a1a8a16225..b4b399d83c 100644 --- a/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/AbstractConnectionInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/jedis-plugins/jedis-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jedis/v4/AbstractConnectionInterceptor.java @@ -53,14 +53,22 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr // Refer to `plugin.jedis.operation_mapping_read`, `plugin.jedis.operation_mapping_write` config item in agent.config String cmd = protocolCommand == null ? UNKNOWN : protocolCommand.toLowerCase(); ConnectionInformation connectionData = (ConnectionInformation) objInst.getSkyWalkingDynamicField(); + // connectionData can be null when this Connection instance wasn't captured by the constructor + // interceptor (e.g. a pooled/recycled connection created through a code path the constructor + // interceptor doesn't cover). Fall back to UNKNOWN instead of throwing here: an exception in + // this method, before createExitSpan() runs, would leave no exit span pushed for this call, + // so afterMethod()'s unconditional stopSpan() would incorrectly pop and close whatever span + // is already on the stack (typically the caller's entry/local span). + String actualTarget = connectionData == null ? UNKNOWN : connectionData.getActualTarget(); + String clusterNodes = connectionData == null ? null : connectionData.getClusterNodes(); // Use cluster information to adapt Virtual Cache if exists, otherwise use real server host - String peer = StringUtil.isBlank(connectionData.getClusterNodes()) ? connectionData.getActualTarget() : connectionData.getClusterNodes(); + String peer = StringUtil.isBlank(clusterNodes) ? actualTarget : clusterNodes; AbstractSpan span = ContextManager.createExitSpan("Jedis/" + cmd, peer); span.setComponent(ComponentsDefine.JEDIS); readKeyIfNecessary(iterator).ifPresent(key -> Tags.CACHE_KEY.set(span, key)); Tags.CACHE_CMD.set(span, cmd); Tags.CACHE_TYPE.set(span, CACHE_TYPE); - TAG_ARGS.set(span, connectionData.getActualTarget()); + TAG_ARGS.set(span, actualTarget); parseOperation(cmd).ifPresent(op -> Tags.CACHE_OP.set(span, op)); SpanLayer.asCache(span); } @@ -84,8 +92,12 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA @Override public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Throwable t) { - AbstractSpan span = ContextManager.activeSpan().log(t).errorOccurred(); - ContextManager.stopSpan(span); + // Do not call ContextManager.stopSpan() here: afterMethod() below always runs afterwards + // (InstMethodsInter invokes it in a finally block, on every path including exceptions) and + // already pops this span. Stopping it a second time here pops one extra span - typically the + // caller's entry/local span - corrupting the trace for the rest of the request. Only log the + // exception on the still-active span, matching the jedis-2.x-3.x-plugin's safe behavior. + ContextManager.activeSpan().log(t).errorOccurred(); } private Optional parseOperation(String cmd) {