diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/trace/sleuth/AzureSleuthAutoConfiguration.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/trace/sleuth/AzureSleuthAutoConfiguration.java index 94a684e01bfde..27874c87f9537 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/trace/sleuth/AzureSleuthAutoConfiguration.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/trace/sleuth/AzureSleuthAutoConfiguration.java @@ -11,7 +11,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.sleuth.Tracer; -import org.springframework.cloud.sleuth.propagation.Propagator; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Role; @@ -30,13 +29,12 @@ public class AzureSleuthAutoConfiguration { /** * Autoconfigure the {@link HttpPipelinePolicy} for sleuth usage. * @param tracer the sleuth {@link Tracer}. - * @param propagator the sleuth {@link Propagator} * @return the http pipeline policy */ @Bean(name = DEFAULT_SLEUTH_HTTP_POLICY_BEAN_NAME) @ConditionalOnMissingBean(name = DEFAULT_SLEUTH_HTTP_POLICY_BEAN_NAME) - public HttpPipelinePolicy azureSleuthHttpPolicy(Tracer tracer, Propagator propagator) { - return new SleuthHttpPolicy(tracer, propagator); + public HttpPipelinePolicy azureSleuthHttpPolicy(Tracer tracer) { + return new SleuthHttpPolicy(tracer); } @Role(BeanDefinition.ROLE_INFRASTRUCTURE) diff --git a/sdk/spring/spring-cloud-azure-trace-sleuth/pom.xml b/sdk/spring/spring-cloud-azure-trace-sleuth/pom.xml index a13360a261fce..c8db5b1193342 100644 --- a/sdk/spring/spring-cloud-azure-trace-sleuth/pom.xml +++ b/sdk/spring/spring-cloud-azure-trace-sleuth/pom.xml @@ -49,6 +49,12 @@ 4.0.0 test + + com.azure + azure-core-test + 1.7.8 + test + org.junit.jupiter junit-jupiter diff --git a/sdk/spring/spring-cloud-azure-trace-sleuth/src/main/java/com/azure/spring/cloud/trace/sleuth/SleuthHttpPolicy.java b/sdk/spring/spring-cloud-azure-trace-sleuth/src/main/java/com/azure/spring/cloud/trace/sleuth/SleuthHttpPolicy.java index e8df3c450b968..fe6f6615e34b4 100644 --- a/sdk/spring/spring-cloud-azure-trace-sleuth/src/main/java/com/azure/spring/cloud/trace/sleuth/SleuthHttpPolicy.java +++ b/sdk/spring/spring-cloud-azure-trace-sleuth/src/main/java/com/azure/spring/cloud/trace/sleuth/SleuthHttpPolicy.java @@ -14,9 +14,7 @@ import com.azure.core.util.UrlBuilder; import com.azure.spring.cloud.trace.sleuth.implementation.HttpTraceUtil; import org.springframework.cloud.sleuth.Span; -import org.springframework.cloud.sleuth.TraceContext; import org.springframework.cloud.sleuth.Tracer; -import org.springframework.cloud.sleuth.propagation.Propagator; import org.springframework.util.Assert; import reactor.core.publisher.Mono; import reactor.core.publisher.Signal; @@ -28,7 +26,6 @@ import static com.azure.core.util.tracing.Tracer.AZ_TRACING_NAMESPACE_KEY; import static com.azure.core.util.tracing.Tracer.DISABLE_TRACING_KEY; import static com.azure.core.util.tracing.Tracer.PARENT_TRACE_CONTEXT_KEY; -import static com.azure.spring.cloud.trace.sleuth.implementation.TraceContextUtil.isValid; /** * Pipeline policy that creates a Sleuth span which traces the service request, @@ -38,7 +35,6 @@ public class SleuthHttpPolicy implements HttpPipelinePolicy { // Singleton Sleuth tracer capable of starting and exporting spans. private final Tracer tracer; - private final Propagator propagator; // standard attributes with http call information private static final String HTTP_USER_AGENT = "http.user_agent"; @@ -51,13 +47,10 @@ public class SleuthHttpPolicy implements HttpPipelinePolicy { /** * Creates a new instance of {@link SleuthHttpPolicy}. * @param tracer the tracer - * @param propagator the propagator */ - public SleuthHttpPolicy(Tracer tracer, Propagator propagator) { + public SleuthHttpPolicy(Tracer tracer) { Assert.notNull(tracer, "tracer must not be null!"); - Assert.notNull(propagator, "propagator must not be null!"); this.tracer = tracer; - this.propagator = propagator; } @Override @@ -71,9 +64,10 @@ public Mono process(HttpPipelineCallContext context, HttpPipelineN // Build new child span representing this outgoing request. final UrlBuilder urlBuilder = UrlBuilder.parse(context.getHttpRequest().getUrl()); - - Span.Builder spanBuilder = tracer.spanBuilder().name(urlBuilder.getPath()) - .setParent(parentSpan.context()); + Span.Builder spanBuilder = tracer.spanBuilder().name(urlBuilder.getPath()); + if (parentSpan != null) { + spanBuilder.setParent(parentSpan.context()); + } // A span's kind can be SERVER (incoming request) or CLIENT (outgoing request); spanBuilder.kind(Span.Kind.CLIENT); @@ -86,12 +80,6 @@ public Mono process(HttpPipelineCallContext context, HttpPipelineN addSpanRequestAttributes(span, request, context); // Adds HTTP method, URL, & user-agent } - // For no-op tracer, SpanContext is INVALID; inject valid span headers onto outgoing request - TraceContext traceContext = span.context(); - if (isValid(traceContext)) { - propagator.inject(traceContext, request, contextSetter); - } - // run the next policy and handle success and error return next.process() .doOnEach(SleuthHttpPolicy::handleResponse) @@ -175,8 +163,4 @@ private static void spanEnd(Span span, HttpResponse response, Throwable error) { // Ending the span schedules it for export if sampled in or just ignores it if sampled out. span.end(); } - - // lambda that actually injects arbitrary header into the request - private final Propagator.Setter contextSetter = - (request, key, value) -> request.getHeaders().set(key, value); } diff --git a/sdk/spring/spring-cloud-azure-trace-sleuth/src/main/java/com/azure/spring/cloud/trace/sleuth/implementation/SpanId.java b/sdk/spring/spring-cloud-azure-trace-sleuth/src/main/java/com/azure/spring/cloud/trace/sleuth/implementation/SpanId.java deleted file mode 100644 index 24f308179222d..0000000000000 --- a/sdk/spring/spring-cloud-azure-trace-sleuth/src/main/java/com/azure/spring/cloud/trace/sleuth/implementation/SpanId.java +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package com.azure.spring.cloud.trace.sleuth.implementation; - -import javax.annotation.concurrent.Immutable; - -import static com.azure.spring.cloud.trace.sleuth.implementation.TraceContextUtil.isValidBase16String; - -/** - * Helper methods for dealing with a span identifier. A valid span identifier is a 16 character lowercase hex (base16) - * String, where at least one of the characters is not a "0". - * - *

There are two more other representation that this class helps with: - * - *

    - *
  • Bytes: a 8-byte array, where valid means that at least one of the bytes is not `\0`. - *
  • Long: a {@code long} value, where valid means that the value is non-zero. - *
- */ -@Immutable -public final class SpanId { - private static final ThreadLocal CHAR_BUFFER = new ThreadLocal<>(); - - private static final int BYTES_LENGTH = 8; - private static final int HEX_LENGTH = 2 * BYTES_LENGTH; - private static final String INVALID = "0000000000000000"; - - /** - * Returns the length of the lowercase hex (base16) representation of the {@code SpanId}. - * - * @return the length of the lowercase hex (base16) representation of the {@code SpanId}. - */ - public static int getLength() { - return HEX_LENGTH; - } - - /** - * Returns the invalid {@code SpanId} in lowercase hex (base16) representation. All characters are "0". - * - * @return the invalid {@code SpanId} lowercase in hex (base16) representation. - */ - public static String getInvalid() { - return INVALID; - } - - /** - * Returns whether the span identifier is valid. A valid span identifier is a 16 character hex String, where at - * least one of the characters is not a '0'. - * - * @return {@code true} if the span identifier is valid. - */ - public static boolean isValid(CharSequence spanId) { - return spanId != null - && spanId.length() == HEX_LENGTH - && !INVALID.contentEquals(spanId) - && isValidBase16String(spanId); - } -} diff --git a/sdk/spring/spring-cloud-azure-trace-sleuth/src/main/java/com/azure/spring/cloud/trace/sleuth/implementation/TraceContextUtil.java b/sdk/spring/spring-cloud-azure-trace-sleuth/src/main/java/com/azure/spring/cloud/trace/sleuth/implementation/TraceContextUtil.java deleted file mode 100644 index b463cc670085e..0000000000000 --- a/sdk/spring/spring-cloud-azure-trace-sleuth/src/main/java/com/azure/spring/cloud/trace/sleuth/implementation/TraceContextUtil.java +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package com.azure.spring.cloud.trace.sleuth.implementation; - -import org.springframework.cloud.sleuth.TraceContext; - -public class TraceContextUtil { - - public static boolean isValid(TraceContext context) { - return TraceId.isValid(context.traceId()) && SpanId.isValid(context.spanId()); - } - - /** - * Returns whether the {@link CharSequence} is a valid hex string. - */ - public static boolean isValidBase16String(CharSequence value) { - for (int i = 0; i < value.length(); i++) { - char b = value.charAt(i); - // 48..57 && 97..102 are valid - if (!Character.isDigit(b) && !isLowercaseHexCharacter(b)) { - return false; - } - } - return true; - } - - private static boolean isLowercaseHexCharacter(char b) { - return 97 <= b && b <= 102; - } -} diff --git a/sdk/spring/spring-cloud-azure-trace-sleuth/src/main/java/com/azure/spring/cloud/trace/sleuth/implementation/TraceId.java b/sdk/spring/spring-cloud-azure-trace-sleuth/src/main/java/com/azure/spring/cloud/trace/sleuth/implementation/TraceId.java deleted file mode 100644 index e4bcc82cf5364..0000000000000 --- a/sdk/spring/spring-cloud-azure-trace-sleuth/src/main/java/com/azure/spring/cloud/trace/sleuth/implementation/TraceId.java +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package com.azure.spring.cloud.trace.sleuth.implementation; - -import javax.annotation.concurrent.Immutable; - -import static com.azure.spring.cloud.trace.sleuth.implementation.TraceContextUtil.isValidBase16String; - -@Immutable -public final class TraceId { - private static final ThreadLocal CHAR_BUFFER = new ThreadLocal<>(); - - private static final int BYTES_LENGTH = 16; - private static final int HEX_LENGTH = 2 * BYTES_LENGTH; - private static final String INVALID = "00000000000000000000000000000000"; - - /** - * Returns the length of the lowercase hex (base16) representation of the {@code TraceId}. - * - * @return the length of the lowercase hex (base16) representation of the {@code TraceId}. - */ - public static int getLength() { - return HEX_LENGTH; - } - - /** - * Returns the invalid {@code TraceId} in lowercase hex (base16) representation. All characters are "0". - * - * @return the invalid {@code TraceId} in lowercase hex (base16) representation. - */ - public static String getInvalid() { - return INVALID; - } - - /** - * Returns whether the {@code TraceId} is valid. A valid trace identifier is a 32 character hex String, where at - * least one of the characters is not a '0'. - * - * @return {@code true} if the {@code TraceId} is valid. - */ - public static boolean isValid(CharSequence traceId) { - return traceId != null - && traceId.length() == HEX_LENGTH - && !INVALID.contentEquals(traceId) - && isValidBase16String(traceId); - } -} diff --git a/sdk/spring/spring-cloud-azure-trace-sleuth/src/test/java/com/azure/spring/cloud/trace/sleuth/SleuthHttpPolicyTests.java b/sdk/spring/spring-cloud-azure-trace-sleuth/src/test/java/com/azure/spring/cloud/trace/sleuth/SleuthHttpPolicyTests.java index 96d753ca3bba3..0cede4de7dcb9 100644 --- a/sdk/spring/spring-cloud-azure-trace-sleuth/src/test/java/com/azure/spring/cloud/trace/sleuth/SleuthHttpPolicyTests.java +++ b/sdk/spring/spring-cloud-azure-trace-sleuth/src/test/java/com/azure/spring/cloud/trace/sleuth/SleuthHttpPolicyTests.java @@ -5,9 +5,15 @@ import com.azure.core.credential.TokenCredential; import com.azure.core.http.HttpClient; +import com.azure.core.http.HttpHeaders; +import com.azure.core.http.HttpMethod; import com.azure.core.http.HttpPipeline; import com.azure.core.http.HttpPipelineBuilder; +import com.azure.core.http.HttpPipelineCallContext; +import com.azure.core.http.HttpPipelineNextPolicy; +import com.azure.core.http.HttpRequest; import com.azure.core.http.policy.HttpPipelinePolicy; +import com.azure.core.test.http.MockHttpResponse; import com.azure.identity.ClientSecretCredentialBuilder; import com.azure.storage.blob.BlobServiceClient; import com.azure.storage.blob.BlobServiceClientBuilder; @@ -17,13 +23,24 @@ import org.junit.jupiter.api.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.Tracer; -import org.springframework.cloud.sleuth.propagation.Propagator; +import reactor.core.publisher.Mono; +import java.net.MalformedURLException; +import java.net.URL; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; class SleuthHttpPolicyTests { @@ -31,7 +48,12 @@ class SleuthHttpPolicyTests { private Tracer tracer; @Mock - private Propagator propagator; + private HttpPipelineCallContext httpPipelineCallContext; + + @Mock + private HttpPipelineNextPolicy httpPipelineNextPolicy; + + private MockHttpResponse successResponse = spy(new MockHttpResponse(mock(HttpRequest.class), 200)); @BeforeEach void setup() { @@ -45,7 +67,7 @@ void cleanup() throws Exception { @Test void addPolicyForBlobServiceClientBuilder() { - SleuthHttpPolicy sleuthHttpPolicy = new SleuthHttpPolicy(tracer, propagator); + SleuthHttpPolicy sleuthHttpPolicy = new SleuthHttpPolicy(tracer); // key is test-key CustomerProvidedKey providedKey = new CustomerProvidedKey("dGVzdC1rZXk="); TokenCredential tokenCredential = new ClientSecretCredentialBuilder() @@ -65,6 +87,92 @@ void addPolicyForBlobServiceClientBuilder() { assertEquals(SleuthHttpPolicy.class, pipeline.getPolicy(6).getClass()); } + @Test + void processWhenDisableTracingKey() { + SleuthHttpPolicy sleuthHttpPolicy = spy(new SleuthHttpPolicy(tracer)); + when(httpPipelineCallContext.getData(com.azure.core.util.tracing.Tracer.DISABLE_TRACING_KEY)) + .thenReturn(Optional.of(true)); + sleuthHttpPolicy.process(httpPipelineCallContext, httpPipelineNextPolicy); + verify(httpPipelineCallContext, times(1)) + .getData(com.azure.core.util.tracing.Tracer.DISABLE_TRACING_KEY); + verify(httpPipelineCallContext, times(0)) + .getData(com.azure.core.util.tracing.Tracer.PARENT_TRACE_CONTEXT_KEY); + } + + @Test + void processAndSetParent() throws MalformedURLException { + SleuthHttpPolicy sleuthHttpPolicy = spy(new SleuthHttpPolicy(tracer)); + Span parentSpan = mock(Span.class); + Span.Builder spanBuilder = mock(Span.Builder.class); + HttpRequest request = mock(HttpRequest.class); + commonProcess(parentSpan, spanBuilder, request); + + Span span = mock(Span.class); + when(spanBuilder.start()).thenReturn(span); + when(span.isNoop()).thenReturn(true); + + when(httpPipelineNextPolicy.process()).thenReturn(Mono.just(successResponse)); + sleuthHttpPolicy.process(httpPipelineCallContext, httpPipelineNextPolicy); + verify(spanBuilder, times(1)).setParent(any()); + verify(spanBuilder, times(1)).kind(Span.Kind.CLIENT); + } + + @Test + void processAndPutTag() throws MalformedURLException { + SleuthHttpPolicy sleuthHttpPolicy = spy(new SleuthHttpPolicy(tracer)); + Span parentSpan = mock(Span.class); + Span.Builder spanBuilder = mock(Span.Builder.class); + HttpRequest request = mock(HttpRequest.class); + commonProcess(parentSpan, spanBuilder, request); + + Span span = mock(Span.class); + when(spanBuilder.start()).thenReturn(span); + + when(httpPipelineNextPolicy.process()).thenReturn(Mono.just(successResponse)); + + setSpanHeaders(request); + + sleuthHttpPolicy.process(httpPipelineCallContext, httpPipelineNextPolicy); + verify(span, times(3)).tag(any(), any()); + } + + private void setSpanHeaders(HttpRequest request) { + HttpHeaders headers = new HttpHeaders(); + headers.add("User-Agent", "test"); + headers.add("x-ms-request-id", "test-request-id"); + when(request.getHeaders()).thenReturn(headers); + when(request.getHttpMethod()).thenReturn(HttpMethod.POST); + } + + @Test + void processAndHandleResponse() throws MalformedURLException { + SleuthHttpPolicy sleuthHttpPolicy = spy(new SleuthHttpPolicy(tracer)); + Span parentSpan = mock(Span.class); + Span.Builder spanBuilder = mock(Span.Builder.class); + HttpRequest request = mock(HttpRequest.class); + commonProcess(parentSpan, spanBuilder, request); + + Span span = mock(Span.class); + when(spanBuilder.start()).thenReturn(span); + + setSpanHeaders(request); + when(httpPipelineNextPolicy.process()).thenReturn(Mono.just(successResponse)); + when(successResponse.getHeaderValue("x-ms-request-id")).thenReturn("test-request-id"); + + sleuthHttpPolicy.process(httpPipelineCallContext, httpPipelineNextPolicy).block(); + verify(span, times(1)).end(); + verify(span, times(1)).tag("x-ms-request-id", "test-request-id"); + verify(span, times(1)).tag("http.status_code", "200"); + } + + private void commonProcess(Span parentSpan, Span.Builder spanBuilder, HttpRequest request) throws MalformedURLException { + when(tracer.currentSpan()).thenReturn(parentSpan); + when(tracer.spanBuilder()).thenReturn(spanBuilder); + when(httpPipelineCallContext.getHttpRequest()).thenReturn(request); + when(request.getUrl()).thenReturn(new URL("https://test.blob.core.windows.net/")); + when(spanBuilder.name(anyString())).thenReturn(spanBuilder); + } + @Test void addAfterPolicyForHttpPipeline() { final HttpPipeline pipeline = createHttpPipeline(); @@ -75,7 +183,7 @@ void addAfterPolicyForHttpPipeline() { private HttpPipeline createHttpPipeline() { final HttpClient httpClient = HttpClient.createDefault(); final List policies = new ArrayList<>(); - policies.add(new SleuthHttpPolicy(tracer, propagator)); + policies.add(new SleuthHttpPolicy(tracer)); final HttpPipeline httpPipeline = new HttpPipelineBuilder() .httpClient(httpClient) .policies(policies.toArray(new HttpPipelinePolicy[0])) diff --git a/sdk/spring/spring-cloud-azure-trace-sleuth/src/test/java/com/azure/spring/cloud/trace/sleuth/implementation/SpanIdTests.java b/sdk/spring/spring-cloud-azure-trace-sleuth/src/test/java/com/azure/spring/cloud/trace/sleuth/implementation/SpanIdTests.java deleted file mode 100644 index 6ee68326ab404..0000000000000 --- a/sdk/spring/spring-cloud-azure-trace-sleuth/src/test/java/com/azure/spring/cloud/trace/sleuth/implementation/SpanIdTests.java +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package com.azure.spring.cloud.trace.sleuth.implementation; - -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - -class SpanIdTests { - - @Test - void validateSpanId() { - String validSpanId = "62974557992089f2"; - String invalidSpanId = "0000000000000000"; - String isNotBase16String = "this---not-valid"; - assertFalse(SpanId.isValid(null)); - assertFalse(SpanId.isValid(validSpanId + "extra")); - assertFalse(SpanId.isValid(invalidSpanId)); - assertFalse(SpanId.isValid(isNotBase16String)); - assertTrue(SpanId.isValid(validSpanId)); - } -}