diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpServerParser.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpServerParser.java index 417fe01a9..6f9f5b276 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpServerParser.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpServerParser.java @@ -47,7 +47,7 @@ class SleuthHttpServerParser extends SleuthHttpClientParser return; // already parsed the error } - if (httpStatus == HttpServletResponse.SC_OK && response.error() == null) { + if (httpStatus == HttpServletResponse.SC_OK && response.error() != null) { // Filter chain threw exception but the response status may not have been set // yet, so we have to guess. span.tag(STATUS_CODE_KEY, diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpServerParserTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpServerParserTests.java new file mode 100644 index 000000000..6f9259ca9 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpServerParserTests.java @@ -0,0 +1,95 @@ +/* + * Copyright 2013-2019 the original author or authors. + * + * Licensed 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 + * + * https://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.springframework.cloud.sleuth.instrument.web; + +import brave.Span; +import brave.Tracing; +import brave.http.HttpResponse; +import brave.sampler.Sampler; +import org.assertj.core.api.BDDAssertions; +import org.junit.Test; + +import org.springframework.cloud.sleuth.util.ArrayListSpanReporter; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +public class SleuthHttpServerParserTests { + + ArrayListSpanReporter reporter = new ArrayListSpanReporter(); + + @Test + public void should_tag_span_with_error_when_response_has_error_and_status_is_ok() { + HttpResponse response = errorResponseWithOkStatus(); + + whenSpanGetsParsed(response, newSpan()); + + thenReportedSpanContainsErrorTag(); + } + + @Test + public void should_not_tag_span_with_error_when_response_has_no_error_and_status_is_ok() { + HttpResponse response = responseWithOkStatus(); + + whenSpanGetsParsed(response, newSpan()); + + thenReportedSpanDoesNotContainErrorTag(); + } + + private void whenSpanGetsParsed(HttpResponse response, Span span) { + try { + new SleuthHttpServerParser(null).parse(response, null, span); + } + finally { + span.finish(); + } + } + + private void thenReportedSpanContainsErrorTag() { + BDDAssertions + .then(this.reporter.getSpans().stream() + .flatMap(s -> s.tags().keySet().stream())) + .contains("http.status_code"); + } + + private void thenReportedSpanDoesNotContainErrorTag() { + BDDAssertions + .then(this.reporter.getSpans().stream() + .flatMap(s -> s.tags().keySet().stream())) + .doesNotContain("http.status_code"); + } + + private HttpResponse errorResponseWithOkStatus() { + HttpResponse response = mock(HttpResponse.class); + given(response.statusCode()).willReturn(200); + given(response.error()).willReturn(new RuntimeException("hello")); + return response; + } + + private HttpResponse responseWithOkStatus() { + HttpResponse response = mock(HttpResponse.class); + given(response.statusCode()).willReturn(200); + return response; + } + + private Span newSpan() { + return Tracing.newBuilder().spanReporter(this.reporter) + .sampler(Sampler.ALWAYS_SAMPLE).build().tracer().nextSpan().name("span") + .start(); + } + +}