Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Http spans minor fixes #24954

Merged
merged 3 commits into from
Oct 22, 2021
Merged

Http spans minor fixes #24954

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c819e06
Add OpenTelemtery HTTP policy test for retries
lmolkova Oct 21, 2021
eeececc
Fix minor inconsistencies in span attribute and name
lmolkova Oct 21, 2021
9701265
fix build
lmolkova Oct 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions ...telemetry/src/main/java/com/azure/core/tracing/opentelemetry/OpenTelemetryHttpPolicy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,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 +95,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 +130,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 +189,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
107 changes: 99 additions & 8 deletions ...etry/src/test/java/com/azure/core/tracing/opentelemetry/OpenTelemetryHttpPolicyTests.java
View file
Open in desktop
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 @@ -26,13 +28,18 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInfo;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
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 +50,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 +86,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 +99,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 +201,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 +215,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