Skip to content

Commit

Permalink
Http spans minor fixes (#24954)
Browse files Browse the repository at this point in the history 
* Add OpenTelemtery HTTP policy test for retries

* Fix minor inconsistencies in span attribute and name

* fix build
  • Loading branch information
@lmolkova
lmolkova authored Oct 22, 2021
1 parent 7fce5e6 commit 7a5d4ab
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 15 deletions.
19 changes: 12 additions & 7 deletions ...telemetry/src/main/java/com/azure/core/tracing/opentelemetry/OpenTelemetryHttpPolicy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import com.azure.core.http.policy.HttpPipelinePolicy;
import com.azure.core.tracing.opentelemetry.implementation.HttpTraceUtil;
import com.azure.core.util.CoreUtils;
import com.azure.core.util.UrlBuilder;
import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.trace.Span;
Expand Down Expand Up @@ -73,7 +72,12 @@ public OpenTelemetryHttpPolicy() {
private static final String HTTP_METHOD = "http.method";
private static final String HTTP_URL = "http.url";
private static final String HTTP_STATUS_CODE = "http.status_code";
private static final String REQUEST_ID = "x-ms-request-id";
private static final String SERVICE_REQUEST_ID_HEADER = "x-ms-request-id";
private static final String SERVICE_REQUEST_ID_ATTRIBUTE = "serviceRequestId";

private static final String CLIENT_REQUEST_ID_HEADER = "x-ms-client-request-id";
private static final String CLIENT_REQUEST_ID_ATTRIBUTE = "requestId";


// This helper class implements W3C distributed tracing protocol and injects SpanContext into the outgoing http
// request
Expand All @@ -90,9 +94,7 @@ public Mono<HttpResponse> process(HttpPipelineCallContext context, HttpPipelineN
HttpRequest request = context.getHttpRequest();

// Build new child span representing this outgoing request.
final UrlBuilder urlBuilder = UrlBuilder.parse(context.getHttpRequest().getUrl());

SpanBuilder spanBuilder = tracer.spanBuilder(urlBuilder.getPath())
SpanBuilder spanBuilder = tracer.spanBuilder("HTTP " + request.getHttpMethod().toString())
.setParent(currentContext.with(parentSpan));

// A span's kind can be SERVER (incoming request) or CLIENT (outgoing request);
Expand Down Expand Up @@ -127,6 +129,9 @@ private static void addSpanRequestAttributes(Span span, HttpRequest request,
Optional<Object> tracingNamespace = context.getData(AZ_TRACING_NAMESPACE_KEY);
tracingNamespace.ifPresent(o -> putAttributeIfNotEmptyOrNull(span, OpenTelemetryTracer.AZ_NAMESPACE_KEY,
o.toString()));

String requestId = request.getHeaders().getValue(CLIENT_REQUEST_ID_HEADER);
putAttributeIfNotEmptyOrNull(span, CLIENT_REQUEST_ID_ATTRIBUTE, requestId);
}

private static void putAttributeIfNotEmptyOrNull(Span span, String key, String value) {
Expand Down Expand Up @@ -183,10 +188,10 @@ private static void spanEnd(Span span, HttpResponse response, Throwable error) {
String requestId = null;
if (response != null) {
statusCode = response.getStatusCode();
requestId = response.getHeaderValue(REQUEST_ID);
requestId = response.getHeaderValue(SERVICE_REQUEST_ID_HEADER);
}

putAttributeIfNotEmptyOrNull(span, REQUEST_ID, requestId);
putAttributeIfNotEmptyOrNull(span, SERVICE_REQUEST_ID_ATTRIBUTE, requestId);
span.setAttribute(HTTP_STATUS_CODE, statusCode);
span = HttpTraceUtil.setSpanStatus(span, statusCode, error);
}
Expand Down
105 changes: 97 additions & 8 deletions ...etry/src/test/java/com/azure/core/tracing/opentelemetry/OpenTelemetryHttpPolicyTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import com.azure.core.http.HttpResponse;
import com.azure.core.http.policy.HttpPipelinePolicy;
import com.azure.core.http.policy.HttpPolicyProviders;
import com.azure.core.http.policy.RequestIdPolicy;
import com.azure.core.http.policy.RetryPolicy;
import com.azure.core.test.http.MockHttpResponse;
import com.azure.core.util.Context;
import io.opentelemetry.api.trace.Span;
Expand All @@ -27,12 +29,15 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;

import static com.azure.core.util.tracing.Tracer.AZ_TRACING_NAMESPACE_KEY;
import static com.azure.core.util.tracing.Tracer.PARENT_SPAN_KEY;
Expand All @@ -43,7 +48,8 @@
*/
public class OpenTelemetryHttpPolicyTests {

private static final String X_MS_REQUEST_ID = "response id";
private static final String X_MS_REQUEST_ID_1 = "response id 1";
private static final String X_MS_REQUEST_ID_2 = "response id 2";
private static final int RESPONSE_STATUS_CODE = 201;
private TestExporter exporter;
private Tracer tracer;
Expand Down Expand Up @@ -78,7 +84,7 @@ public void openTelemetryHttpPolicyTest() {
.addData(AZ_TRACING_NAMESPACE_KEY, "foo");

// Act
HttpRequest request = new HttpRequest(HttpMethod.GET, "https://httpbin.org/hello?there#otel");
HttpRequest request = new HttpRequest(HttpMethod.POST, "https://httpbin.org/hello?there#otel");
request.setHeader("User-Agent", "user-agent");
HttpResponse response = createHttpPipeline(tracer).send(request, tracingContext).block();

Expand All @@ -91,17 +97,99 @@ public void openTelemetryHttpPolicyTest() {

assertEquals(request.getHeaders().getValue("Traceparent"), String.format("00-%s-%s-01", httpSpan.getTraceId(), httpSpan.getSpanId()));
assertEquals(((ReadableSpan) parentSpan).getSpanContext().getSpanId(), httpSpan.getParentSpanId());
assertEquals("/hello", httpSpan.getName());
assertEquals("HTTP POST", httpSpan.getName());

Map<String, Object> httpAttributes = getAttributes(httpSpan);

assertEquals(6, httpAttributes.size());
assertEquals("https://httpbin.org/hello?there#otel", httpAttributes.get("http.url"));
assertEquals("GET", httpAttributes.get("http.method"));
assertEquals("POST", httpAttributes.get("http.method"));
assertEquals("user-agent", httpAttributes.get("http.user_agent"));
assertEquals("foo", httpAttributes.get(AZ_TRACING_NAMESPACE_KEY));
assertEquals(Long.valueOf(RESPONSE_STATUS_CODE), httpAttributes.get("http.status_code"));
assertEquals(X_MS_REQUEST_ID, httpAttributes.get("x-ms-request-id"));
assertEquals(X_MS_REQUEST_ID_1, httpAttributes.get("serviceRequestId"));
}

@Test
public void clientRequestIdIsStamped() {
HttpRequest request = new HttpRequest(HttpMethod.PUT, "https://httpbin.org/hello?there#otel");
HttpResponse response = createHttpPipeline(tracer, new RequestIdPolicy()).send(request).block();

// Assert
List<SpanData> exportedSpans = exporter.getSpans();
assertEquals(1, exportedSpans.size());

assertEquals("HTTP PUT", exportedSpans.get(0).getName());

Map<String, Object> httpAttributes = getAttributes(exportedSpans.get(0));
assertEquals(5, httpAttributes.size());

assertEquals(response.getRequest().getHeaders().getValue("x-ms-client-request-id"), httpAttributes.get("requestId"));
assertEquals(X_MS_REQUEST_ID_1, httpAttributes.get("serviceRequestId"));

assertEquals("https://httpbin.org/hello?there#otel", httpAttributes.get("http.url"));
assertEquals("PUT", httpAttributes.get("http.method"));
assertEquals(Long.valueOf(RESPONSE_STATUS_CODE), httpAttributes.get("http.status_code"));
}

@Test
public void everyTryIsTraced() {
AtomicInteger attemptCount = new AtomicInteger();
AtomicReference<String> traceparentTry503 = new AtomicReference<>();
AtomicReference<String> traceparentTry200 = new AtomicReference<>();

HttpPipeline pipeline = new HttpPipelineBuilder()
.policies(new RetryPolicy())
.policies(new OpenTelemetryHttpPolicy(tracer))
.httpClient(request -> {
HttpHeaders headers = new HttpHeaders();

int count = attemptCount.getAndIncrement();
if (count == 0) {
traceparentTry503.set(request.getHeaders().getValue("traceparent"));
headers.set("x-ms-request-id", X_MS_REQUEST_ID_1);
return Mono.just(new MockHttpResponse(request, 503, headers));
} else if (count == 1) {
traceparentTry200.set(request.getHeaders().getValue("traceparent"));
headers.set("x-ms-request-id", X_MS_REQUEST_ID_2);
return Mono.just(new MockHttpResponse(request, 200, headers));
} else {
// Too many requests have been made.
return Mono.just(new MockHttpResponse(request, 400, headers));
}
})
.build();

// Start user parent span and populate context.
Span parentSpan = tracer.spanBuilder(PARENT_SPAN_KEY).startSpan();

Context tracingContext = new Context(PARENT_SPAN_KEY, parentSpan)
.addData(AZ_TRACING_NAMESPACE_KEY, "foo");

StepVerifier.create(pipeline.send(new HttpRequest(HttpMethod.GET, "http://localhost/hello"), tracingContext))
.assertNext(response -> assertEquals(200, response.getStatusCode()))
.verifyComplete();

List<SpanData> exportedSpans = exporter.getSpans();
assertEquals(2, exportedSpans.size());

SpanData try503 = exportedSpans.get(0);
SpanData try200 = exportedSpans.get(1);

assertEquals(traceparentTry503.get(), String.format("00-%s-%s-01", try503.getTraceId(), try503.getSpanId()));
assertEquals(traceparentTry200.get(), String.format("00-%s-%s-01", try200.getTraceId(), try200.getSpanId()));

assertEquals("HTTP GET", try503.getName());
Map<String, Object> httpAttributes503 = getAttributes(try503);
assertEquals(5, httpAttributes503.size());
assertEquals(Long.valueOf(503), httpAttributes503.get("http.status_code"));
assertEquals(X_MS_REQUEST_ID_1, httpAttributes503.get("serviceRequestId"));

assertEquals("HTTP GET", try503.getName());
Map<String, Object> httpAttributes200 = getAttributes(try200);
assertEquals(5, httpAttributes200.size());
assertEquals(Long.valueOf(200), httpAttributes200.get("http.status_code"));
assertEquals(X_MS_REQUEST_ID_2, httpAttributes200.get("serviceRequestId"));
}

private Map<String, Object> getAttributes(SpanData span) {
Expand All @@ -111,10 +199,11 @@ private Map<String, Object> getAttributes(SpanData span) {
return attributes;
}

private static HttpPipeline createHttpPipeline(Tracer tracer) {
private static HttpPipeline createHttpPipeline(Tracer tracer, HttpPipelinePolicy... beforeRetryPolicies) {
final HttpPipeline httpPipeline = new HttpPipelineBuilder()
.httpClient(new SimpleMockHttpClient())
.policies(beforeRetryPolicies)
.policies(new OpenTelemetryHttpPolicy(tracer))
.httpClient(new SimpleMockHttpClient())
.build();
return httpPipeline;
}
Expand All @@ -124,7 +213,7 @@ private static class SimpleMockHttpClient implements HttpClient {
@Override
public Mono<HttpResponse> send(HttpRequest request) {
HttpHeaders headers = new HttpHeaders()
.set("x-ms-request-id", X_MS_REQUEST_ID);
.set("x-ms-request-id", X_MS_REQUEST_ID_1);

return Mono.just(new MockHttpResponse(request, RESPONSE_STATUS_CODE, headers));
}
Expand Down

0 comments on commit 7a5d4ab

Please sign in to comment.