diff --git a/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java b/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java index cfeda65b7..1ab0dfeb1 100644 --- a/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java @@ -218,6 +218,9 @@ static String executeAndGetValueOfSomeCustomHeader(HttpRequest request) { /** OpenCensus tracing component. */ private Tracer tracer = OpenCensusUtils.getTracer(); + /** Prefix for tracing span name. */ + private static final String traceSpanNamePrefix = "Sent." + HttpRequest.class.getName() + "."; + /** * @param transport HTTP transport * @param requestMethod HTTP request method or {@code null} for none @@ -862,12 +865,9 @@ public HttpResponse execute() throws IOException { Preconditions.checkNotNull(requestMethod); Preconditions.checkNotNull(url); - Span span = tracer - .spanBuilder(OpenCensusUtils.SPAN_NAME_HTTP_REQUEST_EXECUTE) - .setRecordEvents(OpenCensusUtils.isRecordEvent()) - .startSpan(); + Span span = tracer.spanBuilder(traceSpanNamePrefix + "execute").startSpan(); do { - span.addAnnotation("retry #" + (numRetries - retriesRemaining)); + span.addAnnotation("retry #" + numRetries); // Cleanup any unneeded response from a previous iteration if (response != null) { response.ignore(); @@ -911,7 +911,7 @@ public HttpResponse execute() throws IOException { headers.setUserAgent(originalUserAgent + " " + USER_AGENT_SUFFIX); } } - OpenCensusUtils.propagateTracingContext(span, headers); + OpenCensusUtils.propagateTracingContext(headers); // headers HttpHeaders.serializeHeaders(headers, logbuf, curlbuf, logger, lowLevelHttpRequest); @@ -995,12 +995,8 @@ public HttpResponse execute() throws IOException { // switch tracing scope to current span @SuppressWarnings("MustBeClosedChecker") Scope ws = tracer.withSpan(span); - OpenCensusUtils.recordSentMessageEvent(span, lowLevelHttpRequest.getContentLength()); try { LowLevelHttpResponse lowLevelHttpResponse = lowLevelHttpRequest.execute(); - if (lowLevelHttpResponse != null) { - OpenCensusUtils.recordReceivedMessageEvent(span, lowLevelHttpResponse.getContentLength()); - } // Flag used to indicate if an exception is thrown before the response is constructed. boolean responseConstructed = false; try { diff --git a/google-http-client/src/main/java/com/google/api/client/util/OpenCensusUtils.java b/google-http-client/src/main/java/com/google/api/client/util/OpenCensusUtils.java index 112612711..836706021 100644 --- a/google-http-client/src/main/java/com/google/api/client/util/OpenCensusUtils.java +++ b/google-http-client/src/main/java/com/google/api/client/util/OpenCensusUtils.java @@ -15,22 +15,19 @@ package com.google.api.client.util; import com.google.api.client.http.HttpHeaders; -import com.google.api.client.http.HttpRequest; +import com.google.api.client.http.HttpResponse; import com.google.api.client.http.HttpStatusCodes; -import com.google.common.annotations.VisibleForTesting; import io.opencensus.contrib.http.util.HttpPropagationUtil; import io.opencensus.trace.BlankSpan; import io.opencensus.trace.EndSpanOptions; -import io.opencensus.trace.NetworkEvent; -import io.opencensus.trace.NetworkEvent.Type; import io.opencensus.trace.Span; import io.opencensus.trace.Status; import io.opencensus.trace.Tracer; import io.opencensus.trace.Tracing; import io.opencensus.trace.propagation.TextFormat; -import java.util.Collections; +import java.io.IOException; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -38,56 +35,33 @@ /** * Utilities for Census monitoring and tracing. * - * @author Hailong Wen * @since 1.24 + * @author Hailong Wen */ public class OpenCensusUtils { - private static final Logger logger = Logger.getLogger(OpenCensusUtils.class.getName()); - - /** - * Span name for tracing {@link HttpRequest#execute()}. - */ - public static final String SPAN_NAME_HTTP_REQUEST_EXECUTE = - "Sent." + HttpRequest.class.getName() + ".execute"; - - /** - * OpenCensus tracing component. When no OpenCensus implementation is provided, it will return a - * no-op tracer. - */ - private static Tracer tracer = Tracing.getTracer(); - - /** - * Sequence id generator for message event. - */ - private static AtomicLong idGenerator = new AtomicLong(); + private static final Logger LOGGER = Logger.getLogger(OpenCensusUtils.class.getName()); /** - * Whether spans should be recorded locally. Defaults to true. + * OpenCensus tracing component. + * When no OpenCensus implementation is provided, it will return a no-op tracer. */ - private static volatile boolean isRecordEvent = true; + static Tracer tracer = Tracing.getTracer(); /** * {@link TextFormat} used in tracing context propagation. */ @Nullable - @VisibleForTesting - static volatile TextFormat propagationTextFormat = null; + static TextFormat propagationTextFormat = null; /** - * {@link TextFormat.Setter} for {@link #propagationTextFormat}. + * {@link TextFormat.Setter} for {@link activeTextFormat}. */ @Nullable - @VisibleForTesting - static volatile TextFormat.Setter propagationTextFormatSetter = null; + static TextFormat.Setter propagationTextFormatSetter = null; /** * Sets the {@link TextFormat} used in context propagation. - * - *

This API allows users of google-http-client to specify other text format, or disable context - * propagation by setting it to {@code null}. It should be used along with {@link - * #setPropagationTextFormatSetter} for setting purpose.

- * * @param textFormat the text format. */ public static void setPropagationTextFormat(@Nullable TextFormat textFormat) { @@ -96,28 +70,12 @@ public static void setPropagationTextFormat(@Nullable TextFormat textFormat) { /** * Sets the {@link TextFormat.Setter} used in context propagation. - * - *

This API allows users of google-http-client to specify other text format setter, or disable - * context propagation by setting it to {@code null}. It should be used along with {@link - * #setPropagationTextFormat} for setting purpose.

- * * @param textFormatSetter the {@code TextFormat.Setter} for the text format. */ public static void setPropagationTextFormatSetter(@Nullable TextFormat.Setter textFormatSetter) { propagationTextFormatSetter = textFormatSetter; } - /** - * Sets whether spans should be recorded locally. - * - *

This API allows users of google-http-client to turn on/off local span collection.

- * - * @param recordEvent record span locally if true. - */ - public static void setIsRecordEvent(boolean recordEvent) { - isRecordEvent = recordEvent; - } - /** * Returns the tracing component of OpenCensus. * @@ -127,27 +85,15 @@ public static Tracer getTracer() { return tracer; } - /** - * Returns whether spans should be recorded locally. - * - * @return whether spans should be recorded locally. - */ - public static boolean isRecordEvent() { - return isRecordEvent; - } - /** * Propagate information of current tracing context. This information will be injected into HTTP * header. - * - * @param span the span to be propagated. - * @param headers the headers used in propagation. */ - public static void propagateTracingContext(Span span, HttpHeaders headers) { - Preconditions.checkArgument(span != null, "span should not be null."); - Preconditions.checkArgument(headers != null, "headers should not be null."); + public static void propagateTracingContext(HttpHeaders headers) { + Preconditions.checkNotNull(headers); if (propagationTextFormat != null && propagationTextFormatSetter != null) { - if (!span.equals(BlankSpan.INSTANCE)) { + Span span = tracer.getCurrentSpan(); + if (span != null && !span.equals(BlankSpan.INSTANCE)) { propagationTextFormat.inject(span.getContext(), headers, propagationTextFormatSetter); } } @@ -161,7 +107,7 @@ public static void propagateTracingContext(Span span, HttpHeaders headers) { */ public static EndSpanOptions getEndSpanOptions(@Nullable Integer statusCode) { // Always sample the span, but optionally export it. - EndSpanOptions.Builder builder = EndSpanOptions.builder(); + EndSpanOptions.Builder builder = EndSpanOptions.builder().setSampleToLocalSpanStore(true); if (statusCode == null) { builder.setStatus(Status.UNKNOWN); } else if (!HttpStatusCodes.isSuccess(statusCode)) { @@ -193,49 +139,6 @@ public static EndSpanOptions getEndSpanOptions(@Nullable Integer statusCode) { return builder.build(); } - /** - * Records a new message event which contains the size of the request content. Note that the size - * represents the message size in application layer, i.e., content-length. - * - * @param span The {@code span} in which the send event occurs. - * @param size Size of the request. - */ - public static void recordSentMessageEvent(Span span, long size) { - recordMessageEvent(span, size, Type.SENT); - } - - /** - * Records a new message event which contains the size of the response content. Note that the size - * represents the message size in application layer, i.e., content-length. - * - * @param span The {@code span} in which the receive event occurs. - * @param size Size of the response. - */ - public static void recordReceivedMessageEvent(Span span, long size) { - recordMessageEvent(span, size, Type.RECV); - } - - /** - * Records a message event of a certain {@link NetowrkEvent.Type}. This method is package - * protected since {@link NetworkEvent} might be deprecated in future releases. - * - * @param span The {@code span} in which the event occurs. - * @param size Size of the message. - * @param eventType The {@code NetworkEvent.Type} of the message event. - */ - @VisibleForTesting - static void recordMessageEvent(Span span, long size, Type eventType) { - Preconditions.checkArgument(span != null, "span should not be null."); - if (size < 0) { - size = 0; - } - NetworkEvent event = NetworkEvent - .builder(eventType, idGenerator.getAndIncrement()) - .setUncompressedMessageSize(size) - .build(); - span.addNetworkEvent(event); - } - static { try { propagationTextFormat = HttpPropagationUtil.getCloudTraceFormat(); @@ -246,16 +149,7 @@ public void put(HttpHeaders carrier, String key, String value) { } }; } catch (Exception e) { - logger.log( - Level.WARNING, "Cannot initialize default OpenCensus HTTP propagation text format.", e); - } - - try { - Tracing.getExportComponent().getSampledSpanStore().registerSpanNamesForCollection( - Collections.singletonList(SPAN_NAME_HTTP_REQUEST_EXECUTE)); - } catch (Exception e) { - logger.log( - Level.WARNING, "Cannot register default OpenCensus span names for collection.", e); + LOGGER.log(Level.WARNING, "Cannot initiate OpenCensus modules, tracing disabled", e); } } diff --git a/google-http-client/src/test/java/com/google/api/client/util/OpenCensusUtilsTest.java b/google-http-client/src/test/java/com/google/api/client/util/OpenCensusUtilsTest.java index 069b051ec..7b75c0f01 100644 --- a/google-http-client/src/test/java/com/google/api/client/util/OpenCensusUtilsTest.java +++ b/google-http-client/src/test/java/com/google/api/client/util/OpenCensusUtilsTest.java @@ -16,22 +16,18 @@ import com.google.api.client.http.HttpHeaders; -import io.opencensus.trace.BlankSpan; +import io.opencensus.common.Scope; import io.opencensus.trace.EndSpanOptions; -import io.opencensus.trace.Annotation; -import io.opencensus.trace.AttributeValue; -import io.opencensus.trace.Link; -import io.opencensus.trace.NetworkEvent; import io.opencensus.trace.Span; import io.opencensus.trace.SpanContext; import io.opencensus.trace.Status; import io.opencensus.trace.Tracer; import io.opencensus.trace.propagation.TextFormat; import java.util.List; -import java.util.Map; import junit.framework.TestCase; import org.junit.Assert; import org.junit.Rule; +import org.junit.rules.ExpectedException; /** * Tests {@link OpenCensusUtils}. @@ -40,11 +36,9 @@ */ public class OpenCensusUtilsTest extends TestCase { + @Rule public ExpectedException thrown = ExpectedException.none(); TextFormat mockTextFormat; TextFormat.Setter mockTextFormatSetter; - TextFormat originTextFormat; - TextFormat.Setter originTextFormatSetter; - Span mockSpan; HttpHeaders headers; Tracer tracer; @@ -54,7 +48,7 @@ public OpenCensusUtilsTest(String testName) { @Override public void setUp() { - mockTextFormat = new TextFormat() { + TextFormat mockTextFormat = new TextFormat() { @Override public List fields() { throw new UnsupportedOperationException("TextFormat.fields"); @@ -70,7 +64,7 @@ public SpanContext extract(C carrier, Getter getter) { throw new UnsupportedOperationException("TextFormat.extract"); } }; - mockTextFormatSetter = new TextFormat.Setter() { + TextFormat.Setter mockTextFormatSetter = new TextFormat.Setter() { @Override public void put(HttpHeaders carrier, String key, String value) { throw new UnsupportedOperationException("TextFormat.Setter.put"); @@ -78,33 +72,6 @@ public void put(HttpHeaders carrier, String key, String value) { }; headers = new HttpHeaders(); tracer = OpenCensusUtils.getTracer(); - mockSpan = new Span(tracer.getCurrentSpan().getContext(), null) { - - @Override - public void addAnnotation(String description, Map attributes) {} - - @Override - public void addAnnotation(Annotation annotation) {} - - @Override - public void addNetworkEvent(NetworkEvent event) { - throw new UnsupportedOperationException("Span.addNetworkEvent"); - } - - @Override - public void addLink(Link link) {} - - @Override - public void end(EndSpanOptions options) {} - }; - originTextFormat = OpenCensusUtils.propagationTextFormat; - originTextFormatSetter = OpenCensusUtils.propagationTextFormatSetter; - } - - @Override - public void tearDown() { - OpenCensusUtils.setPropagationTextFormat(originTextFormat); - OpenCensusUtils.setPropagationTextFormatSetter(originTextFormatSetter); } public void testInitializatoin() { @@ -125,115 +92,108 @@ public void testSetPropagationTextFormatSetter() { public void testPropagateTracingContextInjection() { OpenCensusUtils.setPropagationTextFormat(mockTextFormat); - try { - OpenCensusUtils.propagateTracingContext(mockSpan, headers); - fail("expected " + UnsupportedOperationException.class); - } catch (UnsupportedOperationException e) { - assertEquals(e.getMessage(), "TextFormat.inject"); - } + Span span = OpenCensusUtils.getTracer().spanBuilder("test").startSpan(); + Scope scope = OpenCensusUtils.getTracer().withSpan(span); + thrown.expect(UnsupportedOperationException.class); + thrown.expectMessage("TextFormat.inject"); + OpenCensusUtils.propagateTracingContext(headers); + scope.close(); + span.end(); } public void testPropagateTracingContextHeader() { OpenCensusUtils.setPropagationTextFormatSetter(mockTextFormatSetter); - try { - OpenCensusUtils.propagateTracingContext(mockSpan, headers); - fail("expected " + UnsupportedOperationException.class); - } catch (UnsupportedOperationException e) { - assertEquals(e.getMessage(), "TextFormat.Setter.put"); - } - } - - public void testPropagateTracingContextNullSpan() { - OpenCensusUtils.setPropagationTextFormat(mockTextFormat); - try { - OpenCensusUtils.propagateTracingContext(null, headers); - fail("expected " + IllegalArgumentException.class); - } catch (IllegalArgumentException e) { - assertEquals(e.getMessage(), "span should not be null."); - } - } - - public void testPropagateTracingContextNullHeaders() { - OpenCensusUtils.setPropagationTextFormat(mockTextFormat); - try { - OpenCensusUtils.propagateTracingContext(mockSpan, null); - fail("expected " + IllegalArgumentException.class); - } catch (IllegalArgumentException e) { - assertEquals(e.getMessage(), "headers should not be null."); - } + Span span = OpenCensusUtils.getTracer().spanBuilder("test").startSpan(); + Scope scope = OpenCensusUtils.getTracer().withSpan(span); + thrown.expect(UnsupportedOperationException.class); + thrown.expectMessage("TextFormat.Setter.put"); + OpenCensusUtils.propagateTracingContext(headers); + scope.close(); + span.end(); } public void testPropagateTracingContextInvalidSpan() { OpenCensusUtils.setPropagationTextFormat(mockTextFormat); // No injection. No exceptions should be thrown. - OpenCensusUtils.propagateTracingContext(BlankSpan.INSTANCE, headers); + OpenCensusUtils.propagateTracingContext(headers); } public void testGetEndSpanOptionsNoResponse() { - EndSpanOptions expected = EndSpanOptions.builder().setStatus(Status.UNKNOWN).build(); + EndSpanOptions expected = + EndSpanOptions.builder().setSampleToLocalSpanStore(true).setStatus(Status.UNKNOWN).build(); assertEquals(expected, OpenCensusUtils.getEndSpanOptions(null)); } public void testGetEndSpanOptionsSuccess() { - EndSpanOptions expected = EndSpanOptions.builder().setStatus(Status.OK).build(); + EndSpanOptions expected = + EndSpanOptions.builder().setSampleToLocalSpanStore(true).setStatus(Status.OK).build(); assertEquals(expected, OpenCensusUtils.getEndSpanOptions(200)); assertEquals(expected, OpenCensusUtils.getEndSpanOptions(201)); assertEquals(expected, OpenCensusUtils.getEndSpanOptions(202)); } public void testGetEndSpanOptionsBadRequest() { - EndSpanOptions expected = EndSpanOptions.builder().setStatus(Status.INVALID_ARGUMENT).build(); + EndSpanOptions expected = EndSpanOptions + .builder() + .setSampleToLocalSpanStore(true) + .setStatus(Status.INVALID_ARGUMENT) + .build(); assertEquals(expected, OpenCensusUtils.getEndSpanOptions(400)); } public void testGetEndSpanOptionsUnauthorized() { - EndSpanOptions expected = EndSpanOptions.builder().setStatus(Status.UNAUTHENTICATED).build(); + EndSpanOptions expected = EndSpanOptions + .builder() + .setSampleToLocalSpanStore(true) + .setStatus(Status.UNAUTHENTICATED) + .build(); assertEquals(expected, OpenCensusUtils.getEndSpanOptions(401)); } public void testGetEndSpanOptionsForbidden() { - EndSpanOptions expected = EndSpanOptions.builder().setStatus(Status.PERMISSION_DENIED).build(); + EndSpanOptions expected = EndSpanOptions + .builder() + .setSampleToLocalSpanStore(true) + .setStatus(Status.PERMISSION_DENIED) + .build(); assertEquals(expected, OpenCensusUtils.getEndSpanOptions(403)); } public void testGetEndSpanOptionsNotFound() { - EndSpanOptions expected = EndSpanOptions.builder().setStatus(Status.NOT_FOUND).build(); + EndSpanOptions expected = EndSpanOptions + .builder() + .setSampleToLocalSpanStore(true) + .setStatus(Status.NOT_FOUND) + .build(); assertEquals(expected, OpenCensusUtils.getEndSpanOptions(404)); } public void testGetEndSpanOptionsPreconditionFailed() { - EndSpanOptions expected = EndSpanOptions.builder().setStatus(Status.FAILED_PRECONDITION).build(); + EndSpanOptions expected = EndSpanOptions + .builder() + .setSampleToLocalSpanStore(true) + .setStatus(Status.FAILED_PRECONDITION) + .build(); assertEquals(expected, OpenCensusUtils.getEndSpanOptions(412)); } public void testGetEndSpanOptionsServerError() { - EndSpanOptions expected = EndSpanOptions.builder().setStatus(Status.UNAVAILABLE).build(); + EndSpanOptions expected = EndSpanOptions + .builder() + .setSampleToLocalSpanStore(true) + .setStatus(Status.UNAVAILABLE) + .build(); assertEquals(expected, OpenCensusUtils.getEndSpanOptions(500)); } public void testGetEndSpanOptionsOther() { - EndSpanOptions expected = EndSpanOptions.builder().setStatus(Status.UNKNOWN).build(); + EndSpanOptions expected = EndSpanOptions.builder() + .setSampleToLocalSpanStore(true) + .setStatus(Status.UNKNOWN) + .build(); // test some random unsupported statuses assertEquals(expected, OpenCensusUtils.getEndSpanOptions(301)); assertEquals(expected, OpenCensusUtils.getEndSpanOptions(402)); assertEquals(expected, OpenCensusUtils.getEndSpanOptions(501)); } - - public void testRecordMessageEventInNullSpan() { - try { - OpenCensusUtils.recordMessageEvent(null, 0, NetworkEvent.Type.SENT); - fail("expected " + IllegalArgumentException.class); - } catch (IllegalArgumentException e) { - assertEquals(e.getMessage(), "span should not be null."); - } - } - - public void testRecordMessageEvent() { - try { - OpenCensusUtils.recordMessageEvent(mockSpan, 0, NetworkEvent.Type.SENT); - fail("expected " + UnsupportedOperationException.class); - } catch (UnsupportedOperationException e) { - assertEquals(e.getMessage(), "Span.addNetworkEvent"); - } - } }