Skip to content

Commit 1a64ef2

Browse files
HailongWenmattwhisenhunt
authored andcommitted
Address Bogdan's comments.
* Use Mockito in tests. * Use HttpRequest local id generator for message event. * Use Attribute in Annotation to record number of retry.
1 parent d93eb99 commit 1a64ef2

File tree

3 files changed

+105
-128
lines changed

3 files changed

+105
-128
lines changed

google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,14 @@
2525
import com.google.api.client.util.StringUtils;
2626

2727
import io.opencensus.common.Scope;
28+
import io.opencensus.trace.Annotation;
29+
import io.opencensus.trace.AttributeValue;
2830
import io.opencensus.trace.Span;
2931
import io.opencensus.trace.Tracer;
3032

3133
import java.io.IOException;
3234
import java.io.InputStream;
35+
import java.util.Collections;
3336
import java.util.concurrent.Callable;
3437
import java.util.concurrent.Executor;
3538
import java.util.concurrent.Executors;
@@ -866,8 +869,15 @@ public HttpResponse execute() throws IOException {
866869
.spanBuilder(OpenCensusUtils.SPAN_NAME_HTTP_REQUEST_EXECUTE)
867870
.setRecordEvents(OpenCensusUtils.isRecordEvent())
868871
.startSpan();
872+
long idGenerator = 0L;
873+
869874
do {
870-
span.addAnnotation("retry #" + (numRetries - retriesRemaining));
875+
span.addAnnotation(
876+
Annotation.fromDescriptionAndAttributes(
877+
"retry",
878+
Collections.<String, AttributeValue>singletonMap(
879+
"number of retry",
880+
AttributeValue.longAttributeValue(numRetries - retriesRemaining))));
871881
// Cleanup any unneeded response from a previous iteration
872882
if (response != null) {
873883
response.ignore();
@@ -911,7 +921,7 @@ public HttpResponse execute() throws IOException {
911921
headers.setUserAgent(originalUserAgent + " " + USER_AGENT_SUFFIX);
912922
}
913923
}
914-
OpenCensusUtils.propagateTracingContext(span, headers);
924+
OpenCensusUtils.propagateTracingContext(span.getContext(), headers);
915925

916926
// headers
917927
HttpHeaders.serializeHeaders(headers, logbuf, curlbuf, logger, lowLevelHttpRequest);
@@ -994,11 +1004,13 @@ public HttpResponse execute() throws IOException {
9941004
lowLevelHttpRequest.setTimeout(connectTimeout, readTimeout);
9951005
// switch tracing scope to current span
9961006
Scope ws = tracer.withSpan(span);
997-
OpenCensusUtils.recordSentMessageEvent(span, lowLevelHttpRequest.getContentLength());
1007+
OpenCensusUtils.recordSentMessageEvent(
1008+
span, idGenerator++, lowLevelHttpRequest.getContentLength());
9981009
try {
9991010
LowLevelHttpResponse lowLevelHttpResponse = lowLevelHttpRequest.execute();
10001011
if (lowLevelHttpResponse != null) {
1001-
OpenCensusUtils.recordReceivedMessageEvent(span, lowLevelHttpResponse.getContentLength());
1012+
OpenCensusUtils.recordReceivedMessageEvent(
1013+
span, idGenerator++, lowLevelHttpResponse.getContentLength());
10021014
}
10031015
// Flag used to indicate if an exception is thrown before the response is constructed.
10041016
boolean responseConstructed = false;

google-http-client/src/main/java/com/google/api/client/util/OpenCensusUtils.java

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@
2121
import com.google.common.annotations.VisibleForTesting;
2222

2323
import io.opencensus.contrib.http.util.HttpPropagationUtil;
24-
import io.opencensus.trace.BlankSpan;
2524
import io.opencensus.trace.EndSpanOptions;
2625
import io.opencensus.trace.NetworkEvent;
2726
import io.opencensus.trace.NetworkEvent.Type;
2827
import io.opencensus.trace.Span;
28+
import io.opencensus.trace.SpanContext;
2929
import io.opencensus.trace.Status;
3030
import io.opencensus.trace.Tracer;
3131
import io.opencensus.trace.Tracing;
@@ -60,11 +60,6 @@ public class OpenCensusUtils {
6060
*/
6161
private static Tracer tracer = Tracing.getTracer();
6262

63-
/**
64-
* Sequence id generator for message event.
65-
*/
66-
private static AtomicLong idGenerator = new AtomicLong();
67-
6863
/**
6964
* Whether spans should be recorded locally. Defaults to true.
7065
*/
@@ -140,18 +135,18 @@ public static boolean isRecordEvent() {
140135
}
141136

142137
/**
143-
* Propagate information of current tracing context. This information will be injected into HTTP
138+
* Propagate information of a given tracing context. This information will be injected into HTTP
144139
* header.
145140
*
146-
* @param span the span to be propagated.
141+
* @param spanContext the spanContext to be propagated.
147142
* @param headers the headers used in propagation.
148143
*/
149-
public static void propagateTracingContext(Span span, HttpHeaders headers) {
150-
Preconditions.checkArgument(span != null, "span should not be null.");
144+
public static void propagateTracingContext(SpanContext spanContext, HttpHeaders headers) {
145+
Preconditions.checkArgument(spanContext != null, "spanContext should not be null.");
151146
Preconditions.checkArgument(headers != null, "headers should not be null.");
152147
if (propagationTextFormat != null && propagationTextFormatSetter != null) {
153-
if (!span.equals(BlankSpan.INSTANCE)) {
154-
propagationTextFormat.inject(span.getContext(), headers, propagationTextFormatSetter);
148+
if (!spanContext.equals(SpanContext.INVALID)) {
149+
propagationTextFormat.inject(spanContext, headers, propagationTextFormatSetter);
155150
}
156151
}
157152
}
@@ -201,21 +196,23 @@ public static EndSpanOptions getEndSpanOptions(@Nullable Integer statusCode) {
201196
* Note that the size represents the message size in application layer, i.e., content-length.
202197
*
203198
* @param span The {@code span} in which the send event occurs.
199+
* @param id The id for the message, It is unique within an {@link HttpRequest}.
204200
* @param size Size of the request.
205201
*/
206-
public static void recordSentMessageEvent(Span span, long size) {
207-
recordMessageEvent(span, size, Type.SENT);
202+
public static void recordSentMessageEvent(Span span, long id, long size) {
203+
recordMessageEvent(span, id, size, Type.SENT);
208204
}
209205

210206
/**
211207
* Records a new message event which contains the size of the response content.
212208
* Note that the size represents the message size in application layer, i.e., content-length.
213209
*
214210
* @param span The {@code span} in which the receive event occurs.
211+
* @param id The id for the message. It is unique within an {@link HttpRequest}.
215212
* @param size Size of the response.
216213
*/
217-
public static void recordReceivedMessageEvent(Span span, long size) {
218-
recordMessageEvent(span, size, Type.RECV);
214+
public static void recordReceivedMessageEvent(Span span, long id, long size) {
215+
recordMessageEvent(span, id, size, Type.RECV);
219216
}
220217

221218
/**
@@ -224,15 +221,16 @@ public static void recordReceivedMessageEvent(Span span, long size) {
224221
* releases.
225222
*
226223
* @param span The {@code span} in which the event occurs.
224+
* @param id The id for the message.
227225
* @param size Size of the message.
228226
* @param eventType The {@code NetworkEvent.Type} of the message event.
229227
*/
230228
@VisibleForTesting
231-
static void recordMessageEvent(Span span, long size, Type eventType) {
229+
static void recordMessageEvent(Span span, long id, long size, Type eventType) {
232230
Preconditions.checkArgument(span != null, "span should not be null.");
233231
if (size < 0) size = 0;
234232
NetworkEvent event = NetworkEvent
235-
.builder(eventType, idGenerator.getAndIncrement())
233+
.builder(eventType, id)
236234
.setUncompressedMessageSize(size)
237235
.build();
238236
span.addNetworkEvent(event);

0 commit comments

Comments
 (0)