From 84cb7cd72e8b8d8d31a0d3438c8528173d99b8cc Mon Sep 17 00:00:00 2001 From: Carter <236375267@qq.com> Date: Fri, 30 Oct 2026 11:51:57 -0400 Subject: [PATCH 1/3] Fix NPE and lost trace context of reactive Lettuce commands when tracing is enabled Put the snapshot on the command that carries the span instead of the TracedCommand wrapper, and skip the command interceptors for a command without a span. --- CHANGES.md | 2 + .../common/RedisChannelWriterInterceptor.java | 2 +- .../RedisCommandCancelMethodInterceptor.java | 8 +- ...ompleteExceptionallyMethodInterceptor.java | 8 +- ...RedisCommandCompleteMethodInterceptor.java | 8 +- .../common/RedisCommandEnhanceInfo.java | 10 + ...disSubscriptionConstructorInterceptor.java | 6 +- .../common/RedisReactiveCommandSpanTest.java | 227 ++++++++++++++++++ 8 files changed, 256 insertions(+), 15 deletions(-) create mode 100644 apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/test/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisReactiveCommandSpanTest.java diff --git a/CHANGES.md b/CHANGES.md index d6426e015b..b5bcc6ad61 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -7,6 +7,8 @@ Release Notes. * Fix the `NullPointerException` thrown by the `postgresql-8.x` plugin when executing statements prepared with `Statement.RETURN_GENERATED_KEYS` or column names (apache/skywalking#14138). +* Fix the `NullPointerException` thrown by the Lettuce plugin and the lost trace context of reactive commands when + Lettuce tracing is enabled, and when a reactive command is cancelled before it is written (apache/skywalking#14178). All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/267?closed=1) diff --git a/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisChannelWriterInterceptor.java b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisChannelWriterInterceptor.java index 9a94e186d8..19f9ab2a5e 100644 --- a/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisChannelWriterInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisChannelWriterInterceptor.java @@ -154,7 +154,7 @@ protected String getCommandName(ProtocolKeyword protocol) { return "UNKNOWN"; } - private static RedisCommand, ?, ?> getSpanCarrierCommand(Object o) { + static RedisCommand, ?, ?> getSpanCarrierCommand(Object o) { RedisCommand, ?, ?> command = null; if (o instanceof RedisCommand) { command = (RedisCommand, ?, ?>) o; diff --git a/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCancelMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCancelMethodInterceptor.java index 82f4d6e55d..fd29897ca2 100644 --- a/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCancelMethodInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCancelMethodInterceptor.java @@ -36,8 +36,8 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr @Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class>[] argumentsTypes, Object ret) { - if (objInst.getSkyWalkingDynamicField() != null) { - AbstractSpan span = ((RedisCommandEnhanceInfo) objInst.getSkyWalkingDynamicField()).getSpan(); + AbstractSpan span = RedisCommandEnhanceInfo.getSpan(objInst); + if (span != null) { span.errorOccurred(); span.tag(new StringTag(CANCEL_SIGNAL_TAG), COMMAND_CANCEL_VALUE); span.asyncFinish(); @@ -48,8 +48,8 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA @Override public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, Class>[] argumentsTypes, Throwable t) { - if (objInst.getSkyWalkingDynamicField() != null) { - AbstractSpan span = ((RedisCommandEnhanceInfo) objInst.getSkyWalkingDynamicField()).getSpan(); + AbstractSpan span = RedisCommandEnhanceInfo.getSpan(objInst); + if (span != null) { span.log(t); } } diff --git a/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCompleteExceptionallyMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCompleteExceptionallyMethodInterceptor.java index 022c2622ee..d00c950ef0 100644 --- a/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCompleteExceptionallyMethodInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCompleteExceptionallyMethodInterceptor.java @@ -33,9 +33,9 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr @Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class>[] argumentsTypes, Object ret) { - if (objInst.getSkyWalkingDynamicField() != null) { + AbstractSpan span = RedisCommandEnhanceInfo.getSpan(objInst); + if (span != null) { Throwable t = (Throwable) allArguments[0]; - AbstractSpan span = ((RedisCommandEnhanceInfo) objInst.getSkyWalkingDynamicField()).getSpan(); span.log(t); span.asyncFinish(); objInst.setSkyWalkingDynamicField(null); @@ -45,8 +45,8 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA @Override public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, Class>[] argumentsTypes, Throwable t) { - if (objInst.getSkyWalkingDynamicField() != null) { - AbstractSpan span = ((RedisCommandEnhanceInfo) objInst.getSkyWalkingDynamicField()).getSpan(); + AbstractSpan span = RedisCommandEnhanceInfo.getSpan(objInst); + if (span != null) { span.log(t); } } diff --git a/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCompleteMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCompleteMethodInterceptor.java index 725f2742bd..9ea73190b0 100644 --- a/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCompleteMethodInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCompleteMethodInterceptor.java @@ -33,8 +33,8 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr @Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class>[] argumentsTypes, Object ret) { - if (objInst.getSkyWalkingDynamicField() != null) { - AbstractSpan span = ((RedisCommandEnhanceInfo) objInst.getSkyWalkingDynamicField()).getSpan(); + AbstractSpan span = RedisCommandEnhanceInfo.getSpan(objInst); + if (span != null) { span.asyncFinish(); objInst.setSkyWalkingDynamicField(null); } @@ -44,8 +44,8 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA @Override public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, Class>[] argumentsTypes, Throwable t) { - if (objInst.getSkyWalkingDynamicField() != null) { - AbstractSpan span = ((RedisCommandEnhanceInfo) objInst.getSkyWalkingDynamicField()).getSpan(); + AbstractSpan span = RedisCommandEnhanceInfo.getSpan(objInst); + if (span != null) { span.log(t); } } diff --git a/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandEnhanceInfo.java b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandEnhanceInfo.java index 942d5e18f4..e797d31692 100644 --- a/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandEnhanceInfo.java +++ b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandEnhanceInfo.java @@ -20,6 +20,7 @@ import org.apache.skywalking.apm.agent.core.context.ContextSnapshot; import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance; /** * RedisCommandEnhanceInfo holds SkyWalking tracing data for Lettuce commands @@ -43,6 +44,15 @@ public AbstractSpan getSpan() { return span; } + /** + * @return the span of the command, or null if the command has no span. A reactive command only carries the + * snapshot until it is written, so a command cancelled before that has no span to finish. + */ + static AbstractSpan getSpan(EnhancedInstance command) { + RedisCommandEnhanceInfo enhanceInfo = (RedisCommandEnhanceInfo) command.getSkyWalkingDynamicField(); + return enhanceInfo == null ? null : enhanceInfo.getSpan(); + } + public RedisCommandEnhanceInfo setSpan(AbstractSpan span) { this.span = span; return this; diff --git a/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisSubscriptionConstructorInterceptor.java b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisSubscriptionConstructorInterceptor.java index 7c79ebde93..31f4d07078 100644 --- a/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisSubscriptionConstructorInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisSubscriptionConstructorInterceptor.java @@ -25,7 +25,9 @@ * Interceptor for RedisSubscription constructor. *
* This interceptor captures the {@link io.lettuce.core.protocol.RedisCommand} instance - * at subscription construction time and stores it into SkyWalking dynamic field. + * at subscription construction time and stores it into SkyWalking dynamic field. A decorated + * command is unwrapped to the command that carries the span, so that the snapshot is found + * by {@link RedisChannelWriterInterceptor}. *
*/ public class RedisSubscriptionConstructorInterceptor implements InstanceConstructorInterceptor { @@ -33,7 +35,7 @@ public class RedisSubscriptionConstructorInterceptor implements InstanceConstruc @Override public void onConstruct(EnhancedInstance objInst, Object[] allArguments) { // allArguments[1] is the RedisCommand passed to the RedisSubscription constructor - objInst.setSkyWalkingDynamicField(allArguments[1]); + objInst.setSkyWalkingDynamicField(RedisChannelWriterInterceptor.getSpanCarrierCommand(allArguments[1])); } } diff --git a/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/test/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisReactiveCommandSpanTest.java b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/test/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisReactiveCommandSpanTest.java new file mode 100644 index 0000000000..857b5d8f9b --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/test/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisReactiveCommandSpanTest.java @@ -0,0 +1,227 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.apm.plugin.lettuce.common; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.CoreMatchers.sameInstance; +import static org.hamcrest.MatcherAssert.assertThat; + +import io.lettuce.core.protocol.Command; +import io.lettuce.core.protocol.CommandType; +import io.lettuce.core.protocol.RedisCommand; +import io.lettuce.core.protocol.TracedCommand; +import io.lettuce.core.tracing.TraceContext; +import java.util.List; +import org.apache.skywalking.apm.agent.core.context.ContextManager; +import org.apache.skywalking.apm.agent.core.context.ContextSnapshot; +import org.apache.skywalking.apm.agent.core.context.trace.AbstractTracingSpan; +import org.apache.skywalking.apm.agent.core.context.trace.TraceSegment; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance; +import org.apache.skywalking.apm.agent.test.helper.SegmentHelper; +import org.apache.skywalking.apm.agent.test.tools.AgentServiceRule; +import org.apache.skywalking.apm.agent.test.tools.SegmentStorage; +import org.apache.skywalking.apm.agent.test.tools.SegmentStoragePoint; +import org.apache.skywalking.apm.agent.test.tools.TracingSegmentRunner; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.reactivestreams.Subscription; +import reactor.core.CoreSubscriber; +import reactor.util.context.Context; + +@RunWith(TracingSegmentRunner.class) +public class RedisReactiveCommandSpanTest { + + private static final String PEER = "192.168.1.12:6379"; + + @SegmentStoragePoint + private SegmentStorage segmentStorage; + + @Rule + public AgentServiceRule serviceRule = new AgentServiceRule(); + + private final RedisCommandCompleteMethodInterceptor completeInterceptor = new RedisCommandCompleteMethodInterceptor(); + private final RedisCommandCancelMethodInterceptor cancelInterceptor = new RedisCommandCancelMethodInterceptor(); + private final RedisCommandCompleteExceptionallyMethodInterceptor completeExceptionallyInterceptor = + new RedisCommandCompleteExceptionallyMethodInterceptor(); + + private MockCommand command; + private MockTracedCommand tracedCommand; + private MockInstance channelWriter; + private MockInstance subscription; + + private static class MockInstance implements EnhancedInstance { + private Object object; + + @Override + public Object getSkyWalkingDynamicField() { + return object; + } + + @Override + public void setSkyWalkingDynamicField(Object value) { + this.object = value; + } + } + + private static class MockCommand extends Command