Skip to content

Commit ba83918

Browse files
marcingrzejszczakmarcingrzejszczak
committed
Merge branch 'null-spantag-parameter' of https://github.com/bedla/spring-cloud-sleuth into bedla-null-spantag-parameter
2 parents 916e52c + 62225e6 commit ba83918

File tree

3 files changed

+182
-6
lines changed

3 files changed

+182
-6
lines changed

‎spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SpanTagAnnotationHandler.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -146,19 +146,20 @@ private String resolveTagKey(SleuthAnnotatedParameter container) {
146146
}
147147

148148
String resolveTagValue(SpanTag annotation, Object argument) {
149-
if (argument == null) {
150-
return "";
151-
}
149+
String value = null;
152150
if (annotation.resolver() != NoOpTagValueResolver.class) {
153151
TagValueResolver tagValueResolver = this.beanFactory
154152
.getBean(annotation.resolver());
155-
return tagValueResolver.resolve(argument);
153+
value = tagValueResolver.resolve(argument);
156154
}
157155
else if (StringUtils.hasText(annotation.expression())) {
158-
return this.beanFactory.getBean(TagValueExpressionResolver.class)
156+
value = this.beanFactory.getBean(TagValueExpressionResolver.class)
159157
.resolve(annotation.expression(), argument);
160158
}
161-
return argument.toString();
159+
else if (argument != null) {
160+
value = argument.toString();
161+
}
162+
return value == null ? "" : value;
162163
}
163164

164165
}

‎spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/NullSpanTagAnnotationHandlerTests.java‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
/*
2+
* Copyright 2013-2019 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.cloud.sleuth.annotation;
18+
19+
import java.lang.annotation.Annotation;
20+
import java.lang.reflect.Method;
21+
22+
import brave.sampler.Sampler;
23+
import org.junit.jupiter.api.BeforeEach;
24+
import org.junit.jupiter.api.Test;
25+
26+
import org.springframework.beans.factory.BeanFactory;
27+
import org.springframework.beans.factory.annotation.Autowired;
28+
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
29+
import org.springframework.boot.test.context.SpringBootTest;
30+
import org.springframework.context.annotation.Bean;
31+
import org.springframework.context.annotation.Configuration;
32+
33+
import static org.assertj.core.api.Assertions.assertThat;
34+
import static org.assertj.core.api.Assertions.fail;
35+
36+
@SpringBootTest(classes = NullSpanTagAnnotationHandlerTests.TestConfiguration.class)
37+
38+
public class NullSpanTagAnnotationHandlerTests {
39+
40+
@Autowired
41+
BeanFactory beanFactory;
42+
43+
@Autowired
44+
TagValueResolver tagValueResolver;
45+
46+
SpanTagAnnotationHandler handler;
47+
48+
@BeforeEach
49+
public void setup() {
50+
this.handler = new SpanTagAnnotationHandler(this.beanFactory);
51+
}
52+
53+
@Test
54+
public void shouldUseEmptyStringWheCustomTagValueResolverReturnsNull()
55+
throws NoSuchMethodException, SecurityException {
56+
Method method = AnnotationMockClass.class
57+
.getMethod("getAnnotationForTagValueResolver", String.class);
58+
Annotation annotation = method.getParameterAnnotations()[0][0];
59+
if (annotation instanceof SpanTag) {
60+
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation,
61+
"test");
62+
assertThat(resolvedValue).isEqualTo("");
63+
}
64+
else {
65+
fail("Annotation was not SleuthSpanTag");
66+
}
67+
}
68+
69+
@Test
70+
public void shouldUseEmptyStringWhenTagValueExpressionReturnNull()
71+
throws NoSuchMethodException, SecurityException {
72+
Method method = AnnotationMockClass.class
73+
.getMethod("getAnnotationForTagValueExpression", String.class);
74+
Annotation annotation = method.getParameterAnnotations()[0][0];
75+
if (annotation instanceof SpanTag) {
76+
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation,
77+
"test");
78+
79+
assertThat(resolvedValue).isEqualTo("");
80+
}
81+
else {
82+
fail("Annotation was not SleuthSpanTag");
83+
}
84+
}
85+
86+
@Test
87+
public void shouldUseEmptyStringWhenArgumentIsNull()
88+
throws NoSuchMethodException, SecurityException {
89+
Method method = AnnotationMockClass.class
90+
.getMethod("getAnnotationForArgumentToString", Long.class);
91+
Annotation annotation = method.getParameterAnnotations()[0][0];
92+
if (annotation instanceof SpanTag) {
93+
String resolvedValue = this.handler.resolveTagValue((SpanTag) annotation,
94+
null);
95+
assertThat(resolvedValue).isEqualTo("");
96+
}
97+
else {
98+
fail("Annotation was not SleuthSpanTag");
99+
}
100+
}
101+
102+
@Configuration
103+
@EnableAutoConfiguration
104+
protected static class TestConfiguration {
105+
106+
@Bean
107+
public TagValueResolver tagValueResolver() {
108+
return parameter -> null;
109+
}
110+
111+
@Bean
112+
Sampler alwaysSampler() {
113+
return Sampler.ALWAYS_SAMPLE;
114+
}
115+
116+
}
117+
118+
protected class AnnotationMockClass {
119+
120+
@NewSpan
121+
public void getAnnotationForTagValueResolver(
122+
@SpanTag(key = "test", resolver = TagValueResolver.class) String test) {
123+
}
124+
125+
@NewSpan
126+
public void getAnnotationForTagValueExpression(
127+
@SpanTag(key = "test", expression = "null") String test) {
128+
}
129+
130+
@NewSpan
131+
public void getAnnotationForArgumentToString(@SpanTag("test") Long param) {
132+
}
133+
134+
}
135+
136+
}

‎tests/spring-cloud-sleuth-instrumentation-mvc-tests/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,16 @@
4747
import org.springframework.boot.test.context.SpringBootTest;
4848
import org.springframework.boot.test.system.CapturedOutput;
4949
import org.springframework.boot.test.system.OutputCaptureExtension;
50+
import org.springframework.cloud.sleuth.annotation.ContinueSpan;
51+
import org.springframework.cloud.sleuth.annotation.SpanTag;
5052
import org.springframework.context.annotation.Bean;
5153
import org.springframework.context.annotation.Configuration;
5254
import org.springframework.core.env.Environment;
5355
import org.springframework.http.ResponseEntity;
5456
import org.springframework.http.client.ClientHttpResponse;
5557
import org.springframework.web.bind.annotation.RequestMapping;
5658
import org.springframework.web.bind.annotation.RequestMethod;
59+
import org.springframework.web.bind.annotation.RequestParam;
5760
import org.springframework.web.bind.annotation.RestController;
5861
import org.springframework.web.client.DefaultResponseErrorHandler;
5962
import org.springframework.web.client.HttpClientErrorException;
@@ -95,6 +98,24 @@ public void should_tag_url() {
9598
then(spanHandler.takeRemoteSpan(Kind.SERVER).tags()).containsKey("http.url");
9699
}
97100

101+
@Test
102+
public void should_tag_with_value_from_null_expression() {
103+
new RestTemplate().getForObject("http://localhost:" + port() + "/null-parameter",
104+
String.class);
105+
106+
then(this.currentTraceContext.get()).isNull();
107+
then(spanHandler.takeRemoteSpan(Kind.SERVER).tags()).containsEntry("foo", "1001");
108+
}
109+
110+
@Test
111+
public void should_tag_with_value_from_non_null_expression() {
112+
new RestTemplate().getForObject(
113+
"http://localhost:" + port() + "/null-parameter?bar=10", String.class);
114+
115+
then(this.currentTraceContext.get()).isNull();
116+
then(spanHandler.takeRemoteSpan(Kind.SERVER).tags()).containsEntry("foo", "11");
117+
}
118+
98119
@Test
99120
public void exception_logging_span_handler_logs_synchronous_exceptions(
100121
CapturedOutput capture) {
@@ -164,6 +185,11 @@ GoodController goodController() {
164185
return new GoodController();
165186
}
166187

188+
@Bean
189+
NullParameterController nullParameterController() {
190+
return new NullParameterController();
191+
}
192+
167193
@Bean
168194
ExceptionThrowingController badController() {
169195
return new ExceptionThrowingController();
@@ -258,6 +284,19 @@ public String beGood() {
258284

259285
}
260286

287+
@RestController
288+
public static class NullParameterController {
289+
290+
@RequestMapping("/null-parameter")
291+
@ContinueSpan
292+
public String nullParameter(
293+
@SpanTag(key = "foo", expression = "(#root?:1000)+1") @RequestParam(
294+
value = "bar", required = false) Integer param) {
295+
return "ok param=" + param;
296+
}
297+
298+
}
299+
261300
@RestController
262301
public static class ExceptionThrowingController {
263302

0 commit comments

Comments
 (0)