Skip to content

Commit 84cb7cd

Browse files
committed
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.
1 parent 19615e6 commit 84cb7cd

8 files changed

Lines changed: 256 additions & 15 deletions

File tree

‎CHANGES.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ Release Notes.
77

88
* Fix the `NullPointerException` thrown by the `postgresql-8.x` plugin when executing statements prepared with
99
`Statement.RETURN_GENERATED_KEYS` or column names (apache/skywalking#14138).
10+
* Fix the `NullPointerException` thrown by the Lettuce plugin and the lost trace context of reactive commands when
11+
Lettuce tracing is enabled, and when a reactive command is cancelled before it is written (apache/skywalking#14178).
1012

1113
All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/267?closed=1)
1214

‎apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisChannelWriterInterceptor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ protected String getCommandName(ProtocolKeyword protocol) {
154154
return "UNKNOWN";
155155
}
156156

157-
private static RedisCommand<?, ?, ?> getSpanCarrierCommand(Object o) {
157+
static RedisCommand<?, ?, ?> getSpanCarrierCommand(Object o) {
158158
RedisCommand<?, ?, ?> command = null;
159159
if (o instanceof RedisCommand) {
160160
command = (RedisCommand<?, ?, ?>) o;

‎apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCancelMethodInterceptor.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
3636

3737
@Override
3838
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes, Object ret) {
39-
if (objInst.getSkyWalkingDynamicField() != null) {
40-
AbstractSpan span = ((RedisCommandEnhanceInfo) objInst.getSkyWalkingDynamicField()).getSpan();
39+
AbstractSpan span = RedisCommandEnhanceInfo.getSpan(objInst);
40+
if (span != null) {
4141
span.errorOccurred();
4242
span.tag(new StringTag(CANCEL_SIGNAL_TAG), COMMAND_CANCEL_VALUE);
4343
span.asyncFinish();
@@ -48,8 +48,8 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA
4848

4949
@Override
5050
public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes, Throwable t) {
51-
if (objInst.getSkyWalkingDynamicField() != null) {
52-
AbstractSpan span = ((RedisCommandEnhanceInfo) objInst.getSkyWalkingDynamicField()).getSpan();
51+
AbstractSpan span = RedisCommandEnhanceInfo.getSpan(objInst);
52+
if (span != null) {
5353
span.log(t);
5454
}
5555
}

‎apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCompleteExceptionallyMethodInterceptor.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
3333

3434
@Override
3535
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes, Object ret) {
36-
if (objInst.getSkyWalkingDynamicField() != null) {
36+
AbstractSpan span = RedisCommandEnhanceInfo.getSpan(objInst);
37+
if (span != null) {
3738
Throwable t = (Throwable) allArguments[0];
38-
AbstractSpan span = ((RedisCommandEnhanceInfo) objInst.getSkyWalkingDynamicField()).getSpan();
3939
span.log(t);
4040
span.asyncFinish();
4141
objInst.setSkyWalkingDynamicField(null);
@@ -45,8 +45,8 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA
4545

4646
@Override
4747
public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes, Throwable t) {
48-
if (objInst.getSkyWalkingDynamicField() != null) {
49-
AbstractSpan span = ((RedisCommandEnhanceInfo) objInst.getSkyWalkingDynamicField()).getSpan();
48+
AbstractSpan span = RedisCommandEnhanceInfo.getSpan(objInst);
49+
if (span != null) {
5050
span.log(t);
5151
}
5252
}

‎apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandCompleteMethodInterceptor.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
3333

3434
@Override
3535
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes, Object ret) {
36-
if (objInst.getSkyWalkingDynamicField() != null) {
37-
AbstractSpan span = ((RedisCommandEnhanceInfo) objInst.getSkyWalkingDynamicField()).getSpan();
36+
AbstractSpan span = RedisCommandEnhanceInfo.getSpan(objInst);
37+
if (span != null) {
3838
span.asyncFinish();
3939
objInst.setSkyWalkingDynamicField(null);
4040
}
@@ -44,8 +44,8 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA
4444
@Override
4545
public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
4646
Class<?>[] argumentsTypes, Throwable t) {
47-
if (objInst.getSkyWalkingDynamicField() != null) {
48-
AbstractSpan span = ((RedisCommandEnhanceInfo) objInst.getSkyWalkingDynamicField()).getSpan();
47+
AbstractSpan span = RedisCommandEnhanceInfo.getSpan(objInst);
48+
if (span != null) {
4949
span.log(t);
5050
}
5151
}

‎apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisCommandEnhanceInfo.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import org.apache.skywalking.apm.agent.core.context.ContextSnapshot;
2222
import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan;
23+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
2324

2425
/**
2526
* RedisCommandEnhanceInfo holds SkyWalking tracing data for Lettuce commands
@@ -43,6 +44,15 @@ public AbstractSpan getSpan() {
4344
return span;
4445
}
4546

47+
/**
48+
* @return the span of the command, or null if the command has no span. A reactive command only carries the
49+
* snapshot until it is written, so a command cancelled before that has no span to finish.
50+
*/
51+
static AbstractSpan getSpan(EnhancedInstance command) {
52+
RedisCommandEnhanceInfo enhanceInfo = (RedisCommandEnhanceInfo) command.getSkyWalkingDynamicField();
53+
return enhanceInfo == null ? null : enhanceInfo.getSpan();
54+
}
55+
4656
public RedisCommandEnhanceInfo setSpan(AbstractSpan span) {
4757
this.span = span;
4858
return this;

‎apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/org/apache/skywalking/apm/plugin/lettuce/common/RedisSubscriptionConstructorInterceptor.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,17 @@
2525
* Interceptor for RedisSubscription constructor.
2626
* <p>
2727
* This interceptor captures the {@link io.lettuce.core.protocol.RedisCommand} instance
28-
* at subscription construction time and stores it into SkyWalking dynamic field.
28+
* at subscription construction time and stores it into SkyWalking dynamic field. A decorated
29+
* command is unwrapped to the command that carries the span, so that the snapshot is found
30+
* by {@link RedisChannelWriterInterceptor}.
2931
* </p>
3032
*/
3133
public class RedisSubscriptionConstructorInterceptor implements InstanceConstructorInterceptor {
3234

3335
@Override
3436
public void onConstruct(EnhancedInstance objInst, Object[] allArguments) {
3537
// allArguments[1] is the RedisCommand passed to the RedisSubscription constructor
36-
objInst.setSkyWalkingDynamicField(allArguments[1]);
38+
objInst.setSkyWalkingDynamicField(RedisChannelWriterInterceptor.getSpanCarrierCommand(allArguments[1]));
3739
}
3840

3941
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,227 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package org.apache.skywalking.apm.plugin.lettuce.common;
20+
21+
import static org.hamcrest.CoreMatchers.is;
22+
import static org.hamcrest.CoreMatchers.nullValue;
23+
import static org.hamcrest.CoreMatchers.sameInstance;
24+
import static org.hamcrest.MatcherAssert.assertThat;
25+
26+
import io.lettuce.core.protocol.Command;
27+
import io.lettuce.core.protocol.CommandType;
28+
import io.lettuce.core.protocol.RedisCommand;
29+
import io.lettuce.core.protocol.TracedCommand;
30+
import io.lettuce.core.tracing.TraceContext;
31+
import java.util.List;
32+
import org.apache.skywalking.apm.agent.core.context.ContextManager;
33+
import org.apache.skywalking.apm.agent.core.context.ContextSnapshot;
34+
import org.apache.skywalking.apm.agent.core.context.trace.AbstractTracingSpan;
35+
import org.apache.skywalking.apm.agent.core.context.trace.TraceSegment;
36+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
37+
import org.apache.skywalking.apm.agent.test.helper.SegmentHelper;
38+
import org.apache.skywalking.apm.agent.test.tools.AgentServiceRule;
39+
import org.apache.skywalking.apm.agent.test.tools.SegmentStorage;
40+
import org.apache.skywalking.apm.agent.test.tools.SegmentStoragePoint;
41+
import org.apache.skywalking.apm.agent.test.tools.TracingSegmentRunner;
42+
import org.junit.Before;
43+
import org.junit.Rule;
44+
import org.junit.Test;
45+
import org.junit.runner.RunWith;
46+
import org.reactivestreams.Subscription;
47+
import reactor.core.CoreSubscriber;
48+
import reactor.util.context.Context;
49+
50+
@RunWith(TracingSegmentRunner.class)
51+
public class RedisReactiveCommandSpanTest {
52+
53+
private static final String PEER = "192.168.1.12:6379";
54+
55+
@SegmentStoragePoint
56+
private SegmentStorage segmentStorage;
57+
58+
@Rule
59+
public AgentServiceRule serviceRule = new AgentServiceRule();
60+
61+
private final RedisCommandCompleteMethodInterceptor completeInterceptor = new RedisCommandCompleteMethodInterceptor();
62+
private final RedisCommandCancelMethodInterceptor cancelInterceptor = new RedisCommandCancelMethodInterceptor();
63+
private final RedisCommandCompleteExceptionallyMethodInterceptor completeExceptionallyInterceptor =
64+
new RedisCommandCompleteExceptionallyMethodInterceptor();
65+
66+
private MockCommand command;
67+
private MockTracedCommand tracedCommand;
68+
private MockInstance channelWriter;
69+
private MockInstance subscription;
70+
71+
private static class MockInstance implements EnhancedInstance {
72+
private Object object;
73+
74+
@Override
75+
public Object getSkyWalkingDynamicField() {
76+
return object;
77+
}
78+
79+
@Override
80+
public void setSkyWalkingDynamicField(Object value) {
81+
this.object = value;
82+
}
83+
}
84+
85+
private static class MockCommand extends Command<String, String, String> implements EnhancedInstance {
86+
private Object object;
87+
88+
MockCommand() {
89+
super(CommandType.SET, null);
90+
}
91+
92+
@Override
93+
public Object getSkyWalkingDynamicField() {
94+
return object;
95+
}
96+
97+
@Override
98+
public void setSkyWalkingDynamicField(Object value) {
99+
this.object = value;
100+
}
101+
}
102+
103+
private static class MockTracedCommand extends TracedCommand<String, String, String> implements EnhancedInstance {
104+
private Object object;
105+
106+
MockTracedCommand(MockCommand command) {
107+
super(command, TraceContext.EMPTY);
108+
}
109+
110+
@Override
111+
public Object getSkyWalkingDynamicField() {
112+
return object;
113+
}
114+
115+
@Override
116+
public void setSkyWalkingDynamicField(Object value) {
117+
this.object = value;
118+
}
119+
}
120+
121+
private static class MockSubscriber implements CoreSubscriber<Object> {
122+
private final ContextSnapshot snapshot;
123+
124+
MockSubscriber(ContextSnapshot snapshot) {
125+
this.snapshot = snapshot;
126+
}
127+
128+
@Override
129+
public Context currentContext() {
130+
return Context.of("SKYWALKING_CONTEXT_SNAPSHOT", snapshot);
131+
}
132+
133+
@Override
134+
public void onSubscribe(Subscription s) {
135+
}
136+
137+
@Override
138+
public void onNext(Object o) {
139+
}
140+
141+
@Override
142+
public void onError(Throwable t) {
143+
}
144+
145+
@Override
146+
public void onComplete() {
147+
}
148+
}
149+
150+
@Before
151+
public void setUp() {
152+
command = new MockCommand();
153+
tracedCommand = new MockTracedCommand(command);
154+
channelWriter = new MockInstance();
155+
channelWriter.setSkyWalkingDynamicField(PEER);
156+
subscription = new MockInstance();
157+
}
158+
159+
@Test
160+
public void snapshotOfDecoratedCommandIsContinuedOnSpanCarrierCommand() {
161+
subscribe(tracedCommand);
162+
write(tracedCommand);
163+
164+
assertThat(tracedCommand.getSkyWalkingDynamicField(), nullValue());
165+
166+
completeInterceptor.afterMethod(command, null, null, null, null);
167+
// Mono#next cancels the subscription after the command has been completed.
168+
completeInterceptor.afterMethod(tracedCommand, null, null, null, null);
169+
cancelInterceptor.afterMethod(tracedCommand, null, null, null, null);
170+
cancelInterceptor.afterMethod(command, null, null, null, null);
171+
172+
assertThat(command.getSkyWalkingDynamicField(), nullValue());
173+
assertSpansOfContinuedCommand();
174+
}
175+
176+
@Test
177+
public void commandWithoutDecoratorStillContinuesSnapshot() {
178+
subscribe(command);
179+
assertThat(subscription.getSkyWalkingDynamicField(), sameInstance((Object) command));
180+
181+
write(command);
182+
completeInterceptor.afterMethod(command, null, null, null, null);
183+
184+
assertSpansOfContinuedCommand();
185+
}
186+
187+
@Test
188+
public void commandCancelledBeforeWriteHasNoSpanToFinish() {
189+
subscribe(command);
190+
Throwable error = new IllegalStateException("cancelled");
191+
192+
cancelInterceptor.afterMethod(command, null, null, null, null);
193+
cancelInterceptor.handleMethodException(command, null, null, null, error);
194+
completeInterceptor.afterMethod(command, null, null, null, null);
195+
completeInterceptor.handleMethodException(command, null, null, null, error);
196+
completeExceptionallyInterceptor.afterMethod(command, null, new Object[]{error}, null, null);
197+
completeExceptionallyInterceptor.handleMethodException(command, null, null, null, error);
198+
199+
assertThat(segmentStorage.getTraceSegments().size(), is(1));
200+
assertThat(SegmentHelper.getSpans(segmentStorage.getTraceSegments().get(0)).size(), is(1));
201+
}
202+
203+
private void subscribe(RedisCommand<?, ?, ?> redisCommand) {
204+
ContextManager.createEntrySpan("/redis", null);
205+
ContextSnapshot snapshot = ContextManager.capture();
206+
ContextManager.stopSpan();
207+
208+
new RedisSubscriptionConstructorInterceptor().onConstruct(subscription, new Object[]{null, redisCommand});
209+
new RedisSubscriptionSubscribeMethodInterceptor().beforeMethod(
210+
subscription, null, new Object[]{new MockSubscriber(snapshot)}, null, null);
211+
}
212+
213+
private void write(Object redisCommand) {
214+
new RedisChannelWriterInterceptor().beforeMethod(channelWriter, null, new Object[]{redisCommand}, null, null);
215+
}
216+
217+
private void assertSpansOfContinuedCommand() {
218+
List<TraceSegment> segments = segmentStorage.getTraceSegments();
219+
assertThat(segments.size(), is(2));
220+
List<AbstractTracingSpan> spans = SegmentHelper.getSpans(segments.get(1));
221+
assertThat(spans.size(), is(2));
222+
assertThat(spans.get(0).getOperationName(), is("Lettuce/UNKNOWN"));
223+
assertThat(spans.get(0).isExit(), is(true));
224+
assertThat(spans.get(1).getOperationName(), is("RedisReactive/local"));
225+
assertThat(segments.get(1).getRelatedGlobalTrace().getId(), is(segments.get(0).getRelatedGlobalTrace().getId()));
226+
}
227+
}

0 commit comments

Comments
 (0)