diff --git a/instrumentation/benchmarks/src/main/java/brave/grpc/GrpcPropagationBenchmarks.java b/instrumentation/benchmarks/src/main/java/brave/grpc/GrpcPropagationBenchmarks.java index 0009d49a17..76d80d9210 100644 --- a/instrumentation/benchmarks/src/main/java/brave/grpc/GrpcPropagationBenchmarks.java +++ b/instrumentation/benchmarks/src/main/java/brave/grpc/GrpcPropagationBenchmarks.java @@ -66,13 +66,16 @@ public class GrpcPropagationBenchmarks { .build(); static final Propagation b3 = B3Propagation.FACTORY.get(); - static final Injector b3Injector = b3.injector(GrpcClientRequest.SETTER); - static final Extractor b3Extractor = b3.extractor(GrpcServerRequest.GETTER); + static final Injector b3Injector = + b3.injector(GrpcClientRequest::propagationField); + static final Extractor b3Extractor = + b3.extractor(GrpcServerRequest::propagationField); static final Propagation both = GrpcPropagation.create(B3Propagation.get()); - static final Injector bothInjector = both.injector(GrpcClientRequest.SETTER); + static final Injector bothInjector = + both.injector(GrpcClientRequest::propagationField); static final Extractor bothExtractor = - both.extractor(GrpcServerRequest.GETTER); + both.extractor(GrpcServerRequest::propagationField); static final TraceContext context = TraceContext.newBuilder() .traceIdHigh(HexCodec.lowerHexToUnsignedLong("67891233abcdef01")) diff --git a/instrumentation/dubbo-rpc/README.md b/instrumentation/dubbo-rpc/README.md index a94e7f9810..8876386200 100644 --- a/instrumentation/dubbo-rpc/README.md +++ b/instrumentation/dubbo-rpc/README.md @@ -85,3 +85,38 @@ Make sure the following line is in `META-INF/dubbo/com.alibaba.dubbo.common.exte ``` tracing=com.yourcompany.dubbo.TracingExtensionFactory ``` + +## Sampling and data policy + +Please read the [RPC documentation](../rpc/README.md) before proceeding, as it +covers important topics such as which tags are added to spans, and how traces +are sampled. + +### RPC model mapping + +As mentioned above, the RPC model types `RpcRequest` and `RpcResponse` allow +portable sampling decisions and tag parsing. + +Dubbo maps to this model as follows: +* `RpcRequest.service()` - `Invoker.url.serviceInterface` + * Ex. "GreeterService" for a URL "dubbo://localhost:9090?interface=brave.dubbo.GreeterService" +* `RpcRequest.method()` - `Invocation.methodName` + * When absent, this falls back to the string arg[0] to the "$invoke" method. +* `RpcResponse.errorCode()` - The constant name for `RpcException.code`. + * Ex. "FORBIDDEN_EXCEPTION" when `RpcException.code == 4` + +### Dubbo-specific model + +The `DubboRequest` and `DubboResponse` are available for custom sampling and +tag parsing. + +Here is an example that adds default tags, and if Dubbo, Java arguments: +```java +rpcTracing = rpcTracingBuilder + .clientRequestParser((req, context, span) -> { + RpcRequestParser.DEFAULT.parse(req, context, span); + if (req instanceof DubboRequest) { + tagArguments(((DubboRequest) req).invocation().getArguments()); + } + }).build(); +``` diff --git a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboClientRequest.java b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboClientRequest.java index e6d32fed92..34bce9e22d 100644 --- a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboClientRequest.java +++ b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboClientRequest.java @@ -14,26 +14,13 @@ package brave.dubbo.rpc; import brave.Span; -import brave.propagation.Propagation.Setter; import brave.rpc.RpcClientRequest; import com.alibaba.dubbo.common.URL; import com.alibaba.dubbo.rpc.Invocation; import com.alibaba.dubbo.rpc.Invoker; import java.util.Map; -// intentionally not yet public until we add tag parsing functionality final class DubboClientRequest extends RpcClientRequest implements DubboRequest { - static final Setter SETTER = - new Setter() { - @Override public void put(DubboClientRequest request, String key, String value) { - request.propagationField(key, value); - } - - @Override public String toString() { - return "DubboClientRequest::propagationField"; - } - }; - final Invoker invoker; final Invocation invocation; final Map attachments; @@ -61,25 +48,24 @@ final class DubboClientRequest extends RpcClientRequest implements DubboRequest } /** - * Returns the method name of the invocation or the first string arg of an "$invoke" or - * "$invokeAsync" method. + * Returns the method name of the invocation or the first string arg of an "$invoke" method. */ @Override public String method() { return DubboParser.method(invocation); } /** - * Returns the {@link URL#getServiceInterface() service interface} of the invocation. + * Returns the {@link URL#getServiceInterface() service interface} of the invoker. */ @Override public String service() { - return DubboParser.service(invocation); + return DubboParser.service(invoker); } - boolean parseRemoteIpAndPort(Span span) { + @Override public boolean parseRemoteIpAndPort(Span span) { return DubboParser.parseRemoteIpAndPort(span); } - void propagationField(String keyName, String value) { + @Override protected void propagationField(String keyName, String value) { attachments.put(keyName, value); } } diff --git a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboClientResponse.java b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboClientResponse.java index 49bef803b5..b05da97dce 100644 --- a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboClientResponse.java +++ b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboClientResponse.java @@ -13,13 +13,12 @@ */ package brave.dubbo.rpc; -import brave.Response; -import brave.Span; import brave.internal.Nullable; +import brave.rpc.RpcClientResponse; import com.alibaba.dubbo.rpc.Result; import com.alibaba.dubbo.rpc.RpcException; -final class DubboClientResponse extends Response implements DubboResponse { +final class DubboClientResponse extends RpcClientResponse implements DubboResponse { final DubboClientRequest request; @Nullable final Result result; @Nullable final Throwable error; @@ -40,10 +39,6 @@ final class DubboClientResponse extends Response implements DubboResponse { return result; } - @Override public Span.Kind spanKind() { - return Span.Kind.CLIENT; - } - @Override public DubboClientRequest request() { return request; } @@ -53,7 +48,7 @@ final class DubboClientResponse extends Response implements DubboResponse { } /** Returns the string form of the {@link RpcException#getCode()} */ - String errorCode() { + @Override public String errorCode() { return DubboParser.errorCode(error); } } diff --git a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboParser.java b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboParser.java index 9d3d5ee908..1df35f24db 100644 --- a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboParser.java +++ b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboParser.java @@ -45,13 +45,11 @@ final class DubboParser { } /** - * Returns the {@link URL#getServiceInterface() service interface} of the invocation. + * Returns the {@link URL#getServiceInterface() service interface} of the invoker. * *

This was chosen as the {@link URL#getServiceName() service name} is deprecated for it. */ - static @Nullable String service(Invocation invocation) { - Invoker invoker = invocation.getInvoker(); - if (invoker == null) return null; + @Nullable static String service(Invoker invoker) { URL url = invoker.getUrl(); if (url == null) return null; String service = url.getServiceInterface(); @@ -63,8 +61,8 @@ static boolean parseRemoteIpAndPort(Span span) { InetSocketAddress remoteAddress = rpcContext.getRemoteAddress(); if (remoteAddress == null) return false; return span.remoteIpAndPort( - Platform.get().getHostString(remoteAddress), - remoteAddress.getPort() + Platform.get().getHostString(remoteAddress), + remoteAddress.getPort() ); } diff --git a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboRequest.java b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboRequest.java index 7f5c03cd09..4d0d88fd20 100644 --- a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboRequest.java +++ b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboRequest.java @@ -15,6 +15,7 @@ import brave.rpc.RpcClientRequest; import brave.rpc.RpcServerRequest; +import brave.rpc.RpcTracing; import com.alibaba.dubbo.rpc.Invocation; import com.alibaba.dubbo.rpc.Invoker; @@ -35,12 +36,15 @@ *

Note: Do not implement this type directly. An implementation will be * either as {@link RpcClientRequest} or an {@link RpcServerRequest}. * + * @see RpcTracing#clientRequestParser() + * @see RpcTracing#serverRequestParser() + * @see DubboResponse * @since 5.12 */ // Note: Unlike Apache Dubbo, Alibaba Dubbo is Java 1.6+. // This means we cannot add default methods later. However, Alibaba Dubbo is // deprecated, so there should not be cause to add methods later. -interface DubboRequest { // TODO: make public after #999 +public interface DubboRequest { Invoker invoker(); Invocation invocation(); diff --git a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboResponse.java b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboResponse.java index ae2efce9e6..dfbc064105 100644 --- a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboResponse.java +++ b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboResponse.java @@ -14,6 +14,9 @@ package brave.dubbo.rpc; import brave.internal.Nullable; +import brave.rpc.RpcClientResponse; +import brave.rpc.RpcServerResponse; +import brave.rpc.RpcTracing; import com.alibaba.dubbo.rpc.Result; /** @@ -33,9 +36,15 @@ * }).build(); * } * + *

Note: Do not implement this type directly. An implementation will be + * either as {@link RpcClientResponse} or an {@link RpcServerResponse}. + * + * @see RpcTracing#clientResponseParser() + * @see RpcTracing#serverResponseParser() + * @see DubboResponse * @since 5.12 */ -interface DubboResponse { // TODO: make public after #999 +public interface DubboResponse { DubboRequest request(); @Nullable Result result(); diff --git a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboServerRequest.java b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboServerRequest.java index 38108ab25e..0679ddc10d 100644 --- a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboServerRequest.java +++ b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboServerRequest.java @@ -14,25 +14,12 @@ package brave.dubbo.rpc; import brave.Span; -import brave.propagation.Propagation.Getter; import brave.rpc.RpcServerRequest; import com.alibaba.dubbo.common.URL; import com.alibaba.dubbo.rpc.Invocation; import com.alibaba.dubbo.rpc.Invoker; -// intentionally not yet public until we add tag parsing functionality final class DubboServerRequest extends RpcServerRequest implements DubboRequest { - static final Getter GETTER = - new Getter() { - @Override public String get(DubboServerRequest request, String key) { - return request.propagationField(key); - } - - @Override public String toString() { - return "DubboServerRequest::propagationField"; - } - }; - final Invoker invoker; final Invocation invocation; @@ -64,17 +51,17 @@ final class DubboServerRequest extends RpcServerRequest implements DubboRequest } /** - * Returns the {@link URL#getServiceInterface() service interface} of the invocation. + * Returns the {@link URL#getServiceInterface() service interface} of the invoker. */ @Override public String service() { - return DubboParser.service(invocation); + return DubboParser.service(invoker); } - boolean parseRemoteIpAndPort(Span span) { + @Override public boolean parseRemoteIpAndPort(Span span) { return DubboParser.parseRemoteIpAndPort(span); } - String propagationField(String keyName) { + @Override protected String propagationField(String keyName) { return invocation.getAttachment(keyName); } } diff --git a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboServerResponse.java b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboServerResponse.java index 6cc8c01527..a7702a0c79 100644 --- a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboServerResponse.java +++ b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/DubboServerResponse.java @@ -13,13 +13,12 @@ */ package brave.dubbo.rpc; -import brave.Response; -import brave.Span; import brave.internal.Nullable; +import brave.rpc.RpcServerResponse; import com.alibaba.dubbo.rpc.Result; import com.alibaba.dubbo.rpc.RpcException; -final class DubboServerResponse extends Response implements DubboResponse { +final class DubboServerResponse extends RpcServerResponse implements DubboResponse { final DubboServerRequest request; @Nullable final Result result; @Nullable final Throwable error; @@ -32,10 +31,6 @@ final class DubboServerResponse extends Response implements DubboResponse { this.error = error; } - @Override public Span.Kind spanKind() { - return Span.Kind.SERVER; - } - @Override public Result result() { return result; } @@ -53,7 +48,7 @@ final class DubboServerResponse extends Response implements DubboResponse { } /** Returns the string form of the {@link RpcException#getCode()} */ - String errorCode() { + @Override public String errorCode() { return DubboParser.errorCode(error); } } diff --git a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/FinishSpan.java b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/FinishSpan.java new file mode 100644 index 0000000000..f051522148 --- /dev/null +++ b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/FinishSpan.java @@ -0,0 +1,89 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.dubbo.rpc; + +import brave.Span; +import brave.internal.Nullable; +import brave.rpc.RpcClientHandler; +import brave.rpc.RpcClientRequest; +import brave.rpc.RpcServerHandler; +import com.alibaba.dubbo.rpc.Result; + +// Intentionally the same as Apache Dubbo, even though we don't use the first arg. +// When the signature is the same, it reduces work porting bug fixes or tests +abstract class FinishSpan { // implements BiConsumer except Java 6 + static void finish(TracingFilter filter, + DubboRequest request, @Nullable Result result, @Nullable Throwable error, Span span) { + if (request instanceof RpcClientRequest) { + filter.clientHandler.handleReceive( + new DubboClientResponse((DubboClientRequest) request, result, error), span); + } else { + filter.serverHandler.handleSend( + new DubboServerResponse((DubboServerRequest) request, result, error), span); + } + } + + static FinishSpan create(TracingFilter filter, DubboRequest request, Result result, Span span) { + if (request instanceof DubboClientRequest) { + return new FinishClientSpan( + span, result, filter.clientHandler, (DubboClientRequest) request); + } + return new FinishServerSpan(span, result, filter.serverHandler, (DubboServerRequest) request); + } + + final Span span; + final Result result; + + FinishSpan(Span span, Result result) { + if (span == null) throw new NullPointerException("span == null"); + if (result == null) throw new NullPointerException("result == null"); + this.span = span; + this.result = result; + } + + /** One, but not both parameters can be {@code null}. */ + public abstract void accept(@Nullable Object unused, @Nullable Throwable error); + + static final class FinishClientSpan extends FinishSpan { + final RpcClientHandler clientHandler; + final DubboClientRequest request; + + FinishClientSpan( + Span span, Result result, RpcClientHandler clientHandler, DubboClientRequest request) { + super(span, result); + this.clientHandler = clientHandler; + this.request = request; + } + + @Override public void accept(@Nullable Object unused, @Nullable Throwable error) { + clientHandler.handleReceive(new DubboClientResponse(request, result, error), span); + } + } + + static final class FinishServerSpan extends FinishSpan { + final RpcServerHandler serverHandler; + final DubboServerRequest request; + + FinishServerSpan( + Span span, Result result, RpcServerHandler serverHandler, DubboServerRequest request) { + super(span, result); + this.serverHandler = serverHandler; + this.request = request; + } + + @Override public void accept(@Nullable Object unused, @Nullable Throwable error) { + serverHandler.handleSend(new DubboServerResponse(request, result, error), span); + } + } +} diff --git a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/FinishSpanResponseFuture.java b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/FinishSpanResponseFuture.java index 42470d3a38..747775f462 100644 --- a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/FinishSpanResponseFuture.java +++ b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/FinishSpanResponseFuture.java @@ -20,18 +20,20 @@ import com.alibaba.dubbo.remoting.RemotingException; import com.alibaba.dubbo.remoting.exchange.ResponseCallback; import com.alibaba.dubbo.remoting.exchange.ResponseFuture; +import com.alibaba.dubbo.rpc.Result; /** Ensures any callbacks finish the span. */ final class FinishSpanResponseFuture implements ResponseFuture { final ResponseFuture delegate; - final Span span; + final FinishSpan finishSpan; final CurrentTraceContext currentTraceContext; final @Nullable TraceContext callbackContext; - FinishSpanResponseFuture(ResponseFuture delegate, TracingFilter filter, Span span, + FinishSpanResponseFuture( + ResponseFuture delegate, TracingFilter filter, DubboRequest request, Result result, Span span, @Nullable TraceContext callbackContext) { this.delegate = delegate; - this.span = span; + this.finishSpan = FinishSpan.create(filter, request, result, span); this.currentTraceContext = filter.currentTraceContext; this.callbackContext = callbackContext; // Ensures even if no callback added later, for example when a consumer, we finish the span @@ -48,7 +50,7 @@ final class FinishSpanResponseFuture implements ResponseFuture { @Override public void setCallback(ResponseCallback callback) { delegate.setCallback( - TracingResponseCallback.create(callback, span, currentTraceContext, callbackContext) + TracingResponseCallback.create(callback, finishSpan, currentTraceContext, callbackContext) ); } diff --git a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingFilter.java b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingFilter.java index 4f0f41d8f2..c4870f274a 100644 --- a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingFilter.java +++ b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingFilter.java @@ -15,18 +15,17 @@ import brave.Span; import brave.Span.Kind; +import brave.SpanCustomizer; import brave.Tag; -import brave.Tracer; import brave.Tracing; import brave.propagation.CurrentTraceContext; import brave.propagation.CurrentTraceContext.Scope; import brave.propagation.TraceContext; -import brave.propagation.TraceContext.Extractor; -import brave.propagation.TraceContext.Injector; -import brave.propagation.TraceContextOrSamplingFlags; -import brave.rpc.RpcRequest; +import brave.rpc.RpcClientHandler; +import brave.rpc.RpcResponse; +import brave.rpc.RpcResponseParser; +import brave.rpc.RpcServerHandler; import brave.rpc.RpcTracing; -import brave.sampler.SamplerFunction; import com.alibaba.dubbo.common.Constants; import com.alibaba.dubbo.common.extension.Activate; import com.alibaba.dubbo.common.extension.ExtensionLoader; @@ -42,10 +41,7 @@ import java.util.Map; import java.util.concurrent.Future; -import static brave.dubbo.rpc.DubboClientRequest.SETTER; -import static brave.dubbo.rpc.DubboServerRequest.GETTER; import static brave.internal.Throwables.propagateIfFatal; -import static brave.sampler.SamplerFunctions.deferDecision; @Activate(group = {Constants.PROVIDER, Constants.CONSUMER}, value = "tracing") // http://dubbo.apache.org/en-us/docs/dev/impls/filter.html @@ -57,12 +53,15 @@ public final class TracingFilter implements Filter { return String.valueOf(((RpcException) input).getCode()); } }; + static final RpcResponseParser LEGACY_RESPONSE_PARSER = new RpcResponseParser() { + @Override public void parse(RpcResponse response, TraceContext context, SpanCustomizer span) { + DUBBO_ERROR_CODE.tag(response.error(), span); + } + }; CurrentTraceContext currentTraceContext; - Tracer tracer; - Extractor extractor; - Injector injector; - SamplerFunction clientSampler = deferDecision(), serverSampler = deferDecision(); + RpcClientHandler clientHandler; + RpcServerHandler serverHandler; volatile boolean isInit = false; /** @@ -74,23 +73,28 @@ public final class TracingFilter implements Filter { */ @Deprecated public void setTracing(Tracing tracing) { if (tracing == null) throw new NullPointerException("rpcTracing == null"); - setRpcTracing(RpcTracing.create(tracing)); + setRpcTracing(RpcTracing.newBuilder(tracing) + .clientResponseParser(LEGACY_RESPONSE_PARSER) + .serverResponseParser(LEGACY_RESPONSE_PARSER) + .build()); } /** * {@link ExtensionLoader} supplies the tracing implementation which must be named "rpcTracing". * For example, if using the {@link SpringExtensionFactory}, only a bean named "rpcTracing" will * be injected. + * + *

Custom parsing

+ * Custom parsers, such as {@link RpcTracing#clientRequestParser()}, can use Dubbo-specific types + * {@link DubboRequest} and {@link DubboResponse} to get access such as the Java invocation or + * result. */ public void setRpcTracing(RpcTracing rpcTracing) { if (rpcTracing == null) throw new NullPointerException("rpcTracing == null"); // we don't guard on init because we intentionally want to overwrite any call to setTracing currentTraceContext = rpcTracing.tracing().currentTraceContext(); - extractor = rpcTracing.tracing().propagation().extractor(GETTER); - injector = rpcTracing.tracing().propagation().injector(SETTER); - tracer = rpcTracing.tracing().tracer(); - clientSampler = rpcTracing.clientSampler(); - serverSampler = rpcTracing.serverSampler(); + clientHandler = RpcClientHandler.create(rpcTracing); + serverHandler = RpcServerHandler.create(rpcTracing); isInit = true; } @@ -101,6 +105,7 @@ public void setRpcTracing(RpcTracing rpcTracing) { RpcContext rpcContext = RpcContext.getContext(); Kind kind = rpcContext.isProviderSide() ? Kind.SERVER : Kind.CLIENT; Span span; + DubboRequest request; if (kind.equals(Kind.CLIENT)) { // When A service invoke B service, then B service then invoke C service, the parentId of the // C service span is A when read from invocation.getAttachments(). This is because @@ -108,37 +113,33 @@ public void setRpcTracing(RpcTracing rpcTracing) { // See com.alibaba.dubbo.rpc.protocol.AbstractInvoker(line 138) from v2.6.7 Map attachments = RpcContext.getContext().getAttachments(); DubboClientRequest clientRequest = new DubboClientRequest(invoker, invocation, attachments); - span = tracer.nextSpan(clientSampler, clientRequest); - injector.inject(span.context(), clientRequest); + request = clientRequest; + span = clientHandler.handleSendWithParent(clientRequest, invocationContext); } else { DubboServerRequest serverRequest = new DubboServerRequest(invoker, invocation); - TraceContextOrSamplingFlags extracted = extractor.extract(serverRequest); - span = nextSpan(extracted, serverRequest); - } - - if (!span.isNoop()) { - span.kind(kind); - String service = DubboParser.service(invocation); - String method = DubboParser.method(invocation); - span.name(service + "/" + method); - DubboParser.parseRemoteIpAndPort(span); - span.start(); + request = serverRequest; + span = serverHandler.handleReceive(serverRequest); } - boolean deferFinish = false; + boolean isSynchronous = true; Scope scope = currentTraceContext.newScope(span.context()); + Result result = null; Throwable error = null; try { - Result result = invoker.invoke(invocation); + result = invoker.invoke(invocation); error = result.getException(); Future future = rpcContext.getFuture(); // the case on async client invocation - if (future instanceof FutureAdapter) { - deferFinish = true; + if (future != null) { + if (!(future instanceof FutureAdapter)) { + assert false : "we can't defer the span finish unless we can access the ResponseFuture"; + return result; + } + isSynchronous = false; ResponseFuture original = ((FutureAdapter) future).getFuture(); // See instrumentation/RATIONALE.md for why the below response callbacks are invocation context TraceContext callbackContext = kind == Kind.CLIENT ? invocationContext : span.context(); ResponseFuture wrapped = - new FinishSpanResponseFuture(original, this, span, callbackContext); + new FinishSpanResponseFuture(original, this, request, result, span, callbackContext); RpcContext.getContext().setFuture(new FutureAdapter<>(wrapped)); } return result; @@ -147,28 +148,8 @@ public void setRpcTracing(RpcTracing rpcTracing) { error = e; throw e; } finally { - if (error != null) onError(error, span); - if (!deferFinish) span.finish(); + if (isSynchronous) FinishSpan.finish(this, request, result, error, span); scope.close(); } } - - /** Creates a potentially noop span representing this request */ - // This is the same code as HttpServerHandler.nextSpan - // TODO: pull this into RpcServerHandler when stable https://github.com/openzipkin/brave/pull/999 - Span nextSpan(TraceContextOrSamplingFlags extracted, DubboServerRequest request) { - Boolean sampled = extracted.sampled(); - // only recreate the context if the sampler made a decision - if (sampled == null && (sampled = serverSampler.trySample(request)) != null) { - extracted = extracted.sampled(sampled.booleanValue()); - } - return extracted.context() != null - ? tracer.joinSpan(extracted.context()) - : tracer.nextSpan(extracted); - } - - static void onError(Throwable error, Span span) { - span.error(error); - DUBBO_ERROR_CODE.tag(error, span); - } } diff --git a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingResponseCallback.java b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingResponseCallback.java index 827c111914..bcd3b1e42f 100644 --- a/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingResponseCallback.java +++ b/instrumentation/dubbo-rpc/src/main/java/brave/dubbo/rpc/TracingResponseCallback.java @@ -13,40 +13,37 @@ */ package brave.dubbo.rpc; -import brave.Span; import brave.internal.Nullable; import brave.propagation.CurrentTraceContext; import brave.propagation.CurrentTraceContext.Scope; import brave.propagation.TraceContext; import com.alibaba.dubbo.remoting.exchange.ResponseCallback; -import static brave.dubbo.rpc.TracingFilter.onError; - /** * Ensures deferred async calls complete a span upon success or failure callback. * *

This was originally a copy of {@code brave.kafka.clients.TracingCallback}. */ class TracingResponseCallback implements ResponseCallback { - static ResponseCallback create(@Nullable ResponseCallback delegate, Span span, + static ResponseCallback create( + @Nullable ResponseCallback delegate, FinishSpan finishSpan, CurrentTraceContext currentTraceContext, @Nullable TraceContext context) { - if (delegate == null) return new TracingResponseCallback(span); - return new DelegateAndFinishSpan(span, delegate, currentTraceContext, context); + if (delegate == null) return new TracingResponseCallback(finishSpan); + return new DelegateAndFinishSpan(finishSpan, delegate, currentTraceContext, context); } - final Span span; + final FinishSpan finishSpan; - TracingResponseCallback(Span span) { - this.span = span; + TracingResponseCallback(FinishSpan finishSpan) { + this.finishSpan = finishSpan; } @Override public void done(Object response) { - span.finish(); + finishSpan.accept(response, null); } @Override public void caught(Throwable exception) { - onError(exception, span); - span.finish(); + finishSpan.accept(null, exception); } static final class DelegateAndFinishSpan extends TracingResponseCallback { @@ -54,9 +51,9 @@ static final class DelegateAndFinishSpan extends TracingResponseCallback { final CurrentTraceContext current; @Nullable final TraceContext context; - DelegateAndFinishSpan(Span span, ResponseCallback delegate, + DelegateAndFinishSpan(FinishSpan finishSpan, ResponseCallback delegate, CurrentTraceContext currentTraceContext, @Nullable TraceContext context) { - super(span); + super(finishSpan); this.delegate = delegate; this.current = currentTraceContext; this.context = context; diff --git a/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/DubboParserTest.java b/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/DubboParserTest.java index 7b8f658dc6..a462acb9d5 100644 --- a/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/DubboParserTest.java +++ b/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/DubboParserTest.java @@ -35,7 +35,7 @@ public class DubboParserTest { when(invocation.getMethodName()).thenReturn("sayHello"); assertThat(DubboParser.method(invocation)) - .isEqualTo("sayHello"); + .isEqualTo("sayHello"); } @Test public void method_malformed() { @@ -49,7 +49,7 @@ public class DubboParserTest { when(invocation.getArguments()).thenReturn(new Object[] {"sayHello"}); assertThat(DubboParser.method(invocation)) - .isEqualTo("sayHello"); + .isEqualTo("sayHello"); } @Test public void method_invoke_nullArgs() { @@ -73,58 +73,49 @@ public class DubboParserTest { } @Test public void service() { - when(invocation.getInvoker()).thenReturn(invoker); - URL url = URL.valueOf("http://localhost:9000?interface=brave.dubbo.GreeterService"); + URL url = URL.valueOf("dubbo://localhost:9090?interface=brave.dubbo.GreeterService"); when(invoker.getUrl()).thenReturn(url); - assertThat(DubboParser.service(invocation)) - .isEqualTo("brave.dubbo.GreeterService"); - } - - @Test public void service_nullInvoker() { - assertThat(DubboParser.service(invocation)).isNull(); + assertThat(DubboParser.service(invoker)) + .isEqualTo("brave.dubbo.GreeterService"); } @Test public void service_nullUrl() { - when(invocation.getInvoker()).thenReturn(invoker); - - assertThat(DubboParser.service(invocation)).isNull(); + assertThat(DubboParser.service(invoker)).isNull(); } @Test public void service_nullServiceInterface() { - when(invocation.getInvoker()).thenReturn(invoker); - URL url = URL.valueOf("http://localhost:9000"); + URL url = URL.valueOf("dubbo://localhost:9090"); when(invoker.getUrl()).thenReturn(url); - assertThat(DubboParser.service(invocation)).isNull(); + assertThat(DubboParser.service(invoker)).isNull(); } @Test public void service_malformed() { - when(invocation.getInvoker()).thenReturn(invoker); - URL url = URL.valueOf("http://localhost:9000?interface="); + URL url = URL.valueOf("dubbo://localhost:9090?interface="); when(invoker.getUrl()).thenReturn(url); - assertThat(DubboParser.service(invocation)).isNull(); + assertThat(DubboParser.service(invoker)).isNull(); } @Test public void errorCodes() { assertThat(DubboParser.errorCode(null)) - .isEqualTo(DubboParser.errorCode(new IOException("timeout"))) - .isNull(); + .isEqualTo(DubboParser.errorCode(new IOException("timeout"))) + .isNull(); assertThat(DubboParser.errorCode(new RpcException(0))) - .isEqualTo("UNKNOWN_EXCEPTION"); + .isEqualTo("UNKNOWN_EXCEPTION"); assertThat(DubboParser.errorCode(new RpcException(1))) - .isEqualTo("NETWORK_EXCEPTION"); + .isEqualTo("NETWORK_EXCEPTION"); assertThat(DubboParser.errorCode(new RpcException(2))) - .isEqualTo("TIMEOUT_EXCEPTION"); + .isEqualTo("TIMEOUT_EXCEPTION"); assertThat(DubboParser.errorCode(new RpcException(3))) - .isEqualTo("BIZ_EXCEPTION"); + .isEqualTo("BIZ_EXCEPTION"); assertThat(DubboParser.errorCode(new RpcException(4))) - .isEqualTo("FORBIDDEN_EXCEPTION"); + .isEqualTo("FORBIDDEN_EXCEPTION"); assertThat(DubboParser.errorCode(new RpcException(5))) - .isEqualTo("SERIALIZATION_EXCEPTION"); + .isEqualTo("SERIALIZATION_EXCEPTION"); assertThat(DubboParser.errorCode(new RpcException(6))) - .isEqualTo("6"); // this will catch drift if Dubbo adds another code + .isEqualTo("6"); // this will catch drift if Dubbo adds another code } } diff --git a/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/FinishSpanTest.java b/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/FinishSpanTest.java new file mode 100644 index 0000000000..4b627940f4 --- /dev/null +++ b/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/FinishSpanTest.java @@ -0,0 +1,143 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.dubbo.rpc; + +import brave.Span; +import com.alibaba.dubbo.rpc.Invocation; +import com.alibaba.dubbo.rpc.Invoker; +import com.alibaba.dubbo.rpc.Result; +import java.util.Collections; +import org.junit.Before; +import org.junit.Test; +import zipkin2.Span.Kind; + +import static org.mockito.Mockito.mock; + +public class FinishSpanTest extends ITTracingFilter { + DubboClientRequest clientRequest = + new DubboClientRequest(mock(Invoker.class), mock(Invocation.class), Collections.emptyMap()); + DubboServerRequest serverRequest = + new DubboServerRequest(mock(Invoker.class), mock(Invocation.class)); + TracingFilter filter; + + @Before public void setup() { + filter = init(); + } + + @Test public void finish_null_result_and_error_DubboClientRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.CLIENT).start(); + + FinishSpan.finish(filter, clientRequest, null, null, span); + + reporter.takeRemoteSpan(Kind.CLIENT); + } + + @Test public void finish_null_result_and_error_DubboServerRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.SERVER).start(); + + FinishSpan.finish(filter, serverRequest, null, null, span); + + reporter.takeRemoteSpan(Kind.SERVER); + } + + @Test public void finish_result_but_null_error_DubboClientRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.CLIENT).start(); + + FinishSpan.finish(filter, clientRequest, mock(Result.class), null, span); + + reporter.takeRemoteSpan(Kind.CLIENT); + } + + @Test public void finish_result_but_null_error_DubboServerRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.SERVER).start(); + + FinishSpan.finish(filter, serverRequest, mock(Result.class), null, span); + + reporter.takeRemoteSpan(Kind.SERVER); + } + + @Test public void finish_error_but_null_result_DubboClientRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.CLIENT).start(); + + Throwable error = new RuntimeException("melted"); + FinishSpan.finish(filter, clientRequest, null, error, span); + + reporter.takeRemoteSpanWithError(Kind.CLIENT, error.getMessage()); + } + + @Test public void finish_error_but_null_result_DubboServerRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.SERVER).start(); + + Throwable error = new RuntimeException("melted"); + FinishSpan.finish(filter, serverRequest, null, error, span); + + reporter.takeRemoteSpanWithError(Kind.SERVER, error.getMessage()); + } + + @Test public void create_null_result_value_and_error_DubboClientRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.CLIENT).start(); + + FinishSpan.create(filter, clientRequest, mock(Result.class), span) + .accept(null, null); + + reporter.takeRemoteSpan(Kind.CLIENT); + } + + @Test public void create_null_result_value_and_error_DubboServerRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.SERVER).start(); + + FinishSpan.create(filter, serverRequest, mock(Result.class), span) + .accept(null, null); + + reporter.takeRemoteSpan(Kind.SERVER); + } + + @Test public void create_result_value_but_null_error_DubboClientRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.CLIENT).start(); + + FinishSpan.create(filter, clientRequest, mock(Result.class), span) + .accept(new Object(), null); + + reporter.takeRemoteSpan(Kind.CLIENT); + } + + @Test public void create_result_value_but_null_error_DubboServerRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.SERVER).start(); + + FinishSpan.create(filter, serverRequest, mock(Result.class), span) + .accept(new Object(), null); + + reporter.takeRemoteSpan(Kind.SERVER); + } + + @Test public void create_error_but_null_result_value_DubboClientRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.CLIENT).start(); + + Throwable error = new RuntimeException("melted"); + FinishSpan.create(filter, clientRequest, mock(Result.class), span) + .accept(null, error); + + reporter.takeRemoteSpanWithError(Kind.CLIENT, error.getMessage()); + } + + @Test public void create_error_but_null_result_value_DubboServerRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.SERVER).start(); + + Throwable error = new RuntimeException("melted"); + FinishSpan.create(filter, serverRequest, mock(Result.class), span) + .accept(null, error); + + reporter.takeRemoteSpanWithError(Kind.SERVER, error.getMessage()); + } +} diff --git a/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/ITTracingFilter_Consumer.java b/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/ITTracingFilter_Consumer.java index 95394a0c8d..c707740c2f 100644 --- a/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/ITTracingFilter_Consumer.java +++ b/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/ITTracingFilter_Consumer.java @@ -14,19 +14,29 @@ package brave.dubbo.rpc; import brave.Clock; +import brave.Tag; import brave.propagation.CurrentTraceContext.Scope; import brave.propagation.SamplingFlags; import brave.propagation.TraceContext; +import brave.rpc.RpcResponseParser; +import brave.rpc.RpcRuleSampler; +import brave.rpc.RpcTracing; +import com.alibaba.dubbo.common.beanutil.JavaBeanDescriptor; import com.alibaba.dubbo.common.extension.ExtensionLoader; import com.alibaba.dubbo.config.ApplicationConfig; import com.alibaba.dubbo.config.ReferenceConfig; import com.alibaba.dubbo.rpc.Filter; +import com.alibaba.dubbo.rpc.Result; import com.alibaba.dubbo.rpc.RpcContext; import com.alibaba.dubbo.rpc.RpcException; import org.junit.Before; import org.junit.Test; import zipkin2.Span; +import static brave.rpc.RpcRequestMatchers.methodEquals; +import static brave.rpc.RpcRequestMatchers.serviceEquals; +import static brave.sampler.Sampler.ALWAYS_SAMPLE; +import static brave.sampler.Sampler.NEVER_SAMPLE; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -159,14 +169,14 @@ public class ITTracingFilter_Consumer extends ITTracingFilter { client.get().sayHello("jorge"); assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).name()) - .isEqualTo("brave.dubbo.rpc.greeterservice/sayhello"); + .isEqualTo("brave.dubbo.rpc.greeterservice/sayhello"); } @Test public void onTransportException_addsErrorTag() { server.stop(); assertThatThrownBy(() -> client.get().sayHello("jorge")) - .isInstanceOf(RpcException.class); + .isInstanceOf(RpcException.class); reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, ".*RemotingException.*"); } @@ -189,30 +199,30 @@ public class ITTracingFilter_Consumer extends ITTracingFilter { @Test public void addsErrorTag_onUnimplemented() { assertThatThrownBy(() -> wrongClient.get().sayHello("jorge")) - .isInstanceOf(RpcException.class); + .isInstanceOf(RpcException.class); Span span = - reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, ".*Not found exported service.*"); + reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, ".*Not found exported service.*"); assertThat(span.tags()) - .containsEntry("dubbo.error_code", "1"); + .containsEntry("dubbo.error_code", "1"); } /** Shows if you aren't using RpcTracing, the old "dubbo.error_code" works */ @Test public void addsErrorTag_onUnimplemented_legacy() { ((TracingFilter) ExtensionLoader.getExtensionLoader(Filter.class) - .getExtension("tracing")).isInit = false; + .getExtension("tracing")).isInit = false; ((TracingFilter) ExtensionLoader.getExtensionLoader(Filter.class) - .getExtension("tracing")) - .setTracing(tracing); + .getExtension("tracing")) + .setTracing(tracing); assertThatThrownBy(() -> wrongClient.get().sayHello("jorge")) - .isInstanceOf(RpcException.class); + .isInstanceOf(RpcException.class); Span span = - reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, ".*Not found exported service.*"); + reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, ".*Not found exported service.*"); assertThat(span.tags()) - .containsEntry("dubbo.error_code", "1"); + .containsEntry("dubbo.error_code", "1"); } /** Ensures the span completes on asynchronous invocation. */ @@ -225,4 +235,51 @@ public class ITTracingFilter_Consumer extends ITTracingFilter { reporter.takeRemoteSpan(Span.Kind.CLIENT); } + + /* RpcTracing-specific feature tests */ + + @Test public void customSampler() { + RpcTracing rpcTracing = RpcTracing.newBuilder(tracing).clientSampler(RpcRuleSampler.newBuilder() + .putRule(methodEquals("sayGoodbye"), NEVER_SAMPLE) + .putRule(serviceEquals("brave.dubbo"), ALWAYS_SAMPLE) + .build()).build(); + init().setRpcTracing(rpcTracing); + + // unsampled + client.get().sayGoodbye("jorge"); + + // sampled + client.get().sayHello("jorge"); + + assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).name()).endsWith("sayhello"); + // @After will also check that sayGoodbye was not sampled + } + + @Test public void customParser() { + Tag javaValue = new Tag("dubbo.result_value") { + @Override protected String parseValue(DubboResponse input, TraceContext context) { + Result result = input.result(); + if (result == null) return null; + Object value = result.getValue(); + if (value instanceof JavaBeanDescriptor) { + return String.valueOf(((JavaBeanDescriptor) value).getProperty("value")); + } + return null; + } + }; + + RpcTracing rpcTracing = RpcTracing.newBuilder(tracing) + .clientResponseParser((res, context, span) -> { + RpcResponseParser.DEFAULT.parse(res, context, span); + if (res instanceof DubboResponse) { + javaValue.tag((DubboResponse) res, span); + } + }).build(); + init().setRpcTracing(rpcTracing); + + String javaResult = client.get().sayHello("jorge"); + + assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).tags()) + .containsEntry("dubbo.result_value", javaResult); + } } diff --git a/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/ITTracingFilter_Provider.java b/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/ITTracingFilter_Provider.java index f69209fe37..900b20400f 100644 --- a/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/ITTracingFilter_Provider.java +++ b/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/ITTracingFilter_Provider.java @@ -13,14 +13,17 @@ */ package brave.dubbo.rpc; +import brave.Tag; import brave.propagation.B3SingleFormat; import brave.propagation.SamplingFlags; import brave.propagation.TraceContext; +import brave.rpc.RpcResponseParser; import brave.rpc.RpcRuleSampler; import brave.rpc.RpcTracing; import com.alibaba.dubbo.common.beanutil.JavaBeanDescriptor; import com.alibaba.dubbo.config.ApplicationConfig; import com.alibaba.dubbo.config.ReferenceConfig; +import com.alibaba.dubbo.rpc.Result; import com.alibaba.dubbo.rpc.RpcContext; import org.junit.Before; import org.junit.Test; @@ -43,8 +46,8 @@ public class ITTracingFilter_Provider extends ITTracingFilter { throw new IllegalArgumentException(); } String value = currentTraceContext.get() != null - ? currentTraceContext.get().traceIdString() - : ""; + ? currentTraceContext.get().traceIdString() + : ""; arg.setProperty("value", value); return args[0]; }); @@ -91,7 +94,7 @@ public class ITTracingFilter_Provider extends ITTracingFilter { @Test public void currentSpanVisibleToImpl() { assertThat(client.get().sayHello("jorge")) - .isNotEmpty(); + .isNotEmpty(); reporter.takeRemoteSpan(Span.Kind.SERVER); } @@ -100,28 +103,30 @@ public class ITTracingFilter_Provider extends ITTracingFilter { client.get().sayHello("jorge"); assertThat(reporter.takeRemoteSpan(Span.Kind.SERVER).kind()) - .isEqualTo(Span.Kind.SERVER); + .isEqualTo(Span.Kind.SERVER); } @Test public void defaultSpanNameIsMethodName() { client.get().sayHello("jorge"); assertThat(reporter.takeRemoteSpan(Span.Kind.SERVER).name()) - .isEqualTo("brave.dubbo.rpc.greeterservice/sayhello"); + .isEqualTo("brave.dubbo.rpc.greeterservice/sayhello"); } @Test public void addsErrorTagOnException() { assertThatThrownBy(() -> client.get().sayHello("bad")) - .isInstanceOf(IllegalArgumentException.class); + .isInstanceOf(IllegalArgumentException.class); reporter.takeRemoteSpanWithError(Span.Kind.SERVER, "IllegalArgumentException"); } + /* RpcTracing-specific feature tests */ + @Test public void customSampler() { RpcTracing rpcTracing = RpcTracing.newBuilder(tracing).serverSampler(RpcRuleSampler.newBuilder() - .putRule(methodEquals("sayGoodbye"), NEVER_SAMPLE) - .putRule(serviceEquals("brave.dubbo"), ALWAYS_SAMPLE) - .build()).build(); + .putRule(methodEquals("sayGoodbye"), NEVER_SAMPLE) + .putRule(serviceEquals("brave.dubbo"), ALWAYS_SAMPLE) + .build()).build(); init().setRpcTracing(rpcTracing); // unsampled @@ -133,4 +138,33 @@ public class ITTracingFilter_Provider extends ITTracingFilter { assertThat(reporter.takeRemoteSpan(Span.Kind.SERVER).name()).endsWith("sayhello"); // @After will also check that sayGoodbye was not sampled } + + @Test public void customParser() { + Tag javaValue = new Tag("dubbo.result_value") { + @Override protected String parseValue(DubboResponse input, TraceContext context) { + Result result = input.result(); + if (result == null) return null; + Object value = result.getValue(); + if (value instanceof JavaBeanDescriptor) { + return String.valueOf(((JavaBeanDescriptor) value).getProperty("value")); + } + return null; + } + }; + + RpcTracing rpcTracing = RpcTracing.newBuilder(tracing) + .serverResponseParser((res, context, span) -> { + RpcResponseParser.DEFAULT.parse(res, context, span); + if (res instanceof DubboResponse) { + javaValue.tag((DubboResponse) res, span); + } + }) + .build(); + init().setRpcTracing(rpcTracing); + + String javaResult = client.get().sayHello("jorge"); + + assertThat(reporter.takeRemoteSpan(Span.Kind.SERVER).tags()) + .containsEntry("dubbo.result_value", javaResult); + } } diff --git a/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/TracingResponseCallbackTest.java b/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/TracingResponseCallbackTest.java index b37c08ae40..568b484fb8 100644 --- a/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/TracingResponseCallbackTest.java +++ b/instrumentation/dubbo-rpc/src/test/java/brave/dubbo/rpc/TracingResponseCallbackTest.java @@ -13,7 +13,6 @@ */ package brave.dubbo.rpc; -import brave.Span; import brave.propagation.CurrentTraceContext; import brave.propagation.ThreadLocalCurrentTraceContext; import brave.propagation.TraceContext; @@ -25,41 +24,40 @@ import static org.mockito.Mockito.verify; public class TracingResponseCallbackTest { - Span span = mock(Span.class); + FinishSpan finishSpan = mock(FinishSpan.class); TraceContext invocationContext = TraceContext.newBuilder().traceId(1).spanId(2).build(); CurrentTraceContext currentTraceContext = ThreadLocalCurrentTraceContext.create(); @Test public void done_should_finish_span() { ResponseCallback callback = - TracingResponseCallback.create(null, span, currentTraceContext, invocationContext); + TracingResponseCallback.create(null, finishSpan, currentTraceContext, invocationContext); callback.done(null); - verify(span).finish(); + verify(finishSpan).accept(null, null); } @Test public void done_should_finish_span_caught() { ResponseCallback callback = - TracingResponseCallback.create(null, span, currentTraceContext, invocationContext); + TracingResponseCallback.create(null, finishSpan, currentTraceContext, invocationContext); Throwable error = new Exception("Test exception"); callback.caught(error); - verify(span).error(error); - verify(span).finish(); + verify(finishSpan).accept(null, error); } @Test public void done_should_forward_then_finish_span() { ResponseCallback delegate = mock(ResponseCallback.class); ResponseCallback callback = - TracingResponseCallback.create(delegate, span, currentTraceContext, invocationContext); + TracingResponseCallback.create(delegate, finishSpan, currentTraceContext, invocationContext); Object result = new Object(); callback.done(result); verify(delegate).done(result); - verify(span).finish(); + verify(finishSpan).accept(result, null); } @Test public void done_should_have_span_in_scope() { @@ -74,12 +72,12 @@ public class TracingResponseCallbackTest { }; ResponseCallback callback = - TracingResponseCallback.create(delegate, span, currentTraceContext, invocationContext); + TracingResponseCallback.create(delegate, finishSpan, currentTraceContext, invocationContext); Object result = new Object(); callback.done(result); - verify(span).finish(); + verify(finishSpan).accept(result, null); } @Test public void done_should_have_span_in_scope_caught() { @@ -94,12 +92,11 @@ public class TracingResponseCallbackTest { }; ResponseCallback callback = - TracingResponseCallback.create(delegate, span, currentTraceContext, invocationContext); + TracingResponseCallback.create(delegate, finishSpan, currentTraceContext, invocationContext); Throwable error = new Exception("Test exception"); callback.caught(error); - verify(span).error(error); - verify(span).finish(); + verify(finishSpan).accept(null, error); } } diff --git a/instrumentation/dubbo/RATIONALE.md b/instrumentation/dubbo/RATIONALE.md index e18e6470b1..6209547dcf 100644 --- a/instrumentation/dubbo/RATIONALE.md +++ b/instrumentation/dubbo/RATIONALE.md @@ -1,25 +1,7 @@ # brave-instrumentation-dubbo rationale +See [RPC](../rpc/RATIONALE.md) for general RPC rationale. ## Error code words not numbers -Similar to [RPC](../rpc/RATIONALE.md), we use error code names, not numbers in -Dubbo. - -HTTP status codes are grouped by classification, and have existed so long that -support teams can usually identify meaning by looking at a number like 401. -Being triple digits, it is relatively easy to search for what an HTTP status -means. - -Dubbo code numbers are more like enum ordinals. There's no grouping and it is -not easy to identify a problem quickly by seeing a number like 2 vs the code -name "TIMEOUT_EXCEPTION". There is no generic documentation on Dubbo errors. If -given only the number 2, a user unfamiliar with how Dubbo works internally will -have a hard time. For example, searching Dubbo's code base for "2" will return -less relevant results than searching for "TIMEOUT_EXCEPTION". - -It may seem that exception messages can overcome this problem, and they -certainly can when present. Also, exception messages can be localized. However, -exception messages are not good for trace search because they are long and -contain variables. - -For all these reasons, we use code names, not numbers, for Dubbo, as defined in -`RpcException`'s constants (so that they are easy to search). +As per normal rationale, we use code names, not numbers, for Dubbo, as defined +in `RpcException`'s constants. In short, this makes trace data more intuitive +and easier to search. diff --git a/instrumentation/dubbo/README.md b/instrumentation/dubbo/README.md index baa0588ace..385911cc8c 100644 --- a/instrumentation/dubbo/README.md +++ b/instrumentation/dubbo/README.md @@ -85,3 +85,38 @@ Make sure the following line is in `META-INF/dubbo/org.apache.dubbo.common.exten ``` tracing=com.yourcompany.dubbo.TracingExtensionFactory ``` + +## Sampling and data policy + +Please read the [RPC documentation](../rpc/README.md) before proceeding, as it +covers important topics such as which tags are added to spans, and how traces +are sampled. + +### RPC model mapping + +As mentioned above, the RPC model types `RpcRequest` and `RpcResponse` allow +portable sampling decisions and tag parsing. + +Dubbo maps to this model as follows: +* `RpcRequest.service()` - `Invoker.url.serviceInterface` + * Ex. "GreeterService" for a URL "dubbo://localhost:9090?interface=brave.dubbo.GreeterService" +* `RpcRequest.method()` - `Invocation.methodName` + * When absent, this falls back to the string arg[0] to the "$invoke" or "$invokeAsync" methods. +* `RpcResponse.errorCode()` - The constant name for `RpcException.code`. + * Ex. "FORBIDDEN_EXCEPTION" when `RpcException.code == 4` + +### Dubbo-specific model + +The `DubboRequest` and `DubboResponse` are available for custom sampling and +tag parsing. + +Here is an example that adds default tags, and if Dubbo, Java arguments: +```java +rpcTracing = rpcTracingBuilder + .clientRequestParser((req, context, span) -> { + RpcRequestParser.DEFAULT.parse(req, context, span); + if (req instanceof DubboRequest) { + tagArguments(((DubboRequest) req).invocation().getArguments()); + } + }).build(); +``` diff --git a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboClientRequest.java b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboClientRequest.java index f7279dbaf6..6d8b725a2d 100644 --- a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboClientRequest.java +++ b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboClientRequest.java @@ -14,26 +14,13 @@ package brave.dubbo; import brave.Span; -import brave.propagation.Propagation; -import brave.propagation.Propagation.Setter; import brave.rpc.RpcClientRequest; import java.util.Map; import org.apache.dubbo.common.URL; import org.apache.dubbo.rpc.Invocation; import org.apache.dubbo.rpc.Invoker; -// intentionally not yet public until we add tag parsing functionality final class DubboClientRequest extends RpcClientRequest implements DubboRequest { - static final Setter SETTER = new Setter() { - @Override public void put(DubboClientRequest request, String key, String value) { - request.propagationField(key, value); - } - - @Override public String toString() { - return "DubboClientRequest::propagationField"; - } - }; - final Invoker invoker; final Invocation invocation; final Map attachments; @@ -68,17 +55,17 @@ final class DubboClientRequest extends RpcClientRequest implements DubboRequest } /** - * Returns the {@link URL#getServiceInterface() service interface} of the invocation. + * Returns the {@link URL#getServiceInterface() service interface} of the invoker. */ @Override public String service() { - return DubboParser.service(invocation); + return DubboParser.service(invoker); } - boolean parseRemoteIpAndPort(Span span) { + @Override public boolean parseRemoteIpAndPort(Span span) { return DubboParser.parseRemoteIpAndPort(span); } - void propagationField(String keyName, String value) { + @Override protected void propagationField(String keyName, String value) { attachments.put(keyName, value); } } diff --git a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboClientResponse.java b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboClientResponse.java index ed4a2396a7..0016e4554d 100644 --- a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboClientResponse.java +++ b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboClientResponse.java @@ -13,13 +13,12 @@ */ package brave.dubbo; -import brave.Response; -import brave.Span; import brave.internal.Nullable; +import brave.rpc.RpcClientResponse; import org.apache.dubbo.rpc.Result; import org.apache.dubbo.rpc.RpcException; -final class DubboClientResponse extends Response implements DubboResponse { +final class DubboClientResponse extends RpcClientResponse implements DubboResponse { final DubboClientRequest request; @Nullable final Result result; @Nullable final Throwable error; @@ -32,10 +31,6 @@ final class DubboClientResponse extends Response implements DubboResponse { this.error = error; } - @Override public Span.Kind spanKind() { - return Span.Kind.CLIENT; - } - @Override public Result result() { return result; } @@ -53,7 +48,7 @@ final class DubboClientResponse extends Response implements DubboResponse { } /** Returns the string form of the {@link RpcException#getCode()} */ - String errorCode() { + @Override public String errorCode() { return DubboParser.errorCode(error); } } diff --git a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboParser.java b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboParser.java index 5e783062af..87db256666 100644 --- a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboParser.java +++ b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboParser.java @@ -44,9 +44,9 @@ static Map errorCodeNumberToName() { Map result = new LinkedHashMap<>(); for (Field field : RpcException.class.getDeclaredFields()) { if (Modifier.isPublic(field.getModifiers()) - && Modifier.isStatic(field.getModifiers()) - && Modifier.isFinal(field.getModifiers()) - && field.getType() == int.class + && Modifier.isStatic(field.getModifiers()) + && Modifier.isFinal(field.getModifiers()) + && field.getType() == int.class ) { try { result.put((Integer) field.get(null), field.getName()); @@ -84,13 +84,11 @@ static Map errorCodeNumberToName() { } /** - * Returns the {@link URL#getServiceInterface() service interface} of the invocation. + * Returns the {@link URL#getServiceInterface() service interface} of the invoker. * *

This was chosen as the {@link URL#getServiceName() service name} is deprecated for it. */ - @Nullable static String service(Invocation invocation) { - Invoker invoker = invocation.getInvoker(); - if (invoker == null) return null; + @Nullable static String service(Invoker invoker) { URL url = invoker.getUrl(); if (url == null) return null; String service = url.getServiceInterface(); @@ -102,8 +100,8 @@ static boolean parseRemoteIpAndPort(Span span) { InetSocketAddress remoteAddress = rpcContext.getRemoteAddress(); if (remoteAddress == null) return false; return span.remoteIpAndPort( - Platform.get().getHostString(remoteAddress), - remoteAddress.getPort() + Platform.get().getHostString(remoteAddress), + remoteAddress.getPort() ); } diff --git a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboRequest.java b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboRequest.java index 891c2c1040..d90cb64c33 100644 --- a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboRequest.java +++ b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboRequest.java @@ -15,6 +15,7 @@ import brave.rpc.RpcClientRequest; import brave.rpc.RpcServerRequest; +import brave.rpc.RpcTracing; import org.apache.dubbo.rpc.Invocation; import org.apache.dubbo.rpc.Invoker; @@ -35,11 +36,14 @@ *

Note: Do not implement this type directly. An implementation will be * either as {@link RpcClientRequest} or an {@link RpcServerRequest}. * + * @see RpcTracing#clientRequestParser() + * @see RpcTracing#serverRequestParser() + * @see DubboResponse * @since 5.12 */ // Note: Unlike Alibaba Dubbo, Apache Dubbo is Java 8+. // This means we can add default methods later should needs arise. -interface DubboRequest { // TODO: make public after #999 +public interface DubboRequest { Invoker invoker(); Invocation invocation(); diff --git a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboResponse.java b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboResponse.java index 21aa12be9b..4544cb13df 100644 --- a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboResponse.java +++ b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboResponse.java @@ -14,6 +14,9 @@ package brave.dubbo; import brave.internal.Nullable; +import brave.rpc.RpcClientResponse; +import brave.rpc.RpcServerResponse; +import brave.rpc.RpcTracing; import org.apache.dubbo.rpc.Result; /** @@ -33,9 +36,15 @@ * }).build(); * } * + *

Note: Do not implement this type directly. An implementation will be + * either as {@link RpcClientResponse} or an {@link RpcServerResponse}. + * + * @see RpcTracing#clientResponseParser() + * @see RpcTracing#serverResponseParser() + * @see DubboResponse * @since 5.12 */ -interface DubboResponse { // TODO: make public after #999 +public interface DubboResponse { DubboRequest request(); @Nullable Result result(); diff --git a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboServerRequest.java b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboServerRequest.java index e67148bca3..8621a9028b 100644 --- a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboServerRequest.java +++ b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboServerRequest.java @@ -13,24 +13,13 @@ */ package brave.dubbo; -import brave.propagation.Propagation; +import brave.Span; import brave.rpc.RpcServerRequest; import org.apache.dubbo.common.URL; import org.apache.dubbo.rpc.Invocation; import org.apache.dubbo.rpc.Invoker; final class DubboServerRequest extends RpcServerRequest implements DubboRequest { - static final Propagation.Getter GETTER = - new Propagation.Getter() { - @Override public String get(DubboServerRequest request, String key) { - return request.propagationField(key); - } - - @Override public String toString() { - return "DubboServerRequest::propagationField"; - } - }; - final Invoker invoker; final Invocation invocation; @@ -66,10 +55,14 @@ final class DubboServerRequest extends RpcServerRequest implements DubboRequest * Returns the {@link URL#getServiceInterface() service interface} of the invocation. */ @Override public String service() { - return DubboParser.service(invocation); + return DubboParser.service(invoker); + } + + @Override public boolean parseRemoteIpAndPort(Span span) { + return DubboParser.parseRemoteIpAndPort(span); } - String propagationField(String keyName) { + @Override protected String propagationField(String keyName) { return invocation.getAttachment(keyName); } } diff --git a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboServerResponse.java b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboServerResponse.java index 22d6a1ff3a..64ee8bd02c 100644 --- a/instrumentation/dubbo/src/main/java/brave/dubbo/DubboServerResponse.java +++ b/instrumentation/dubbo/src/main/java/brave/dubbo/DubboServerResponse.java @@ -13,13 +13,12 @@ */ package brave.dubbo; -import brave.Response; -import brave.Span; import brave.internal.Nullable; +import brave.rpc.RpcServerResponse; import org.apache.dubbo.rpc.Result; import org.apache.dubbo.rpc.RpcException; -final class DubboServerResponse extends Response implements DubboResponse { +final class DubboServerResponse extends RpcServerResponse implements DubboResponse { final DubboServerRequest request; @Nullable final Result result; @Nullable final Throwable error; @@ -32,10 +31,6 @@ final class DubboServerResponse extends Response implements DubboResponse { this.error = error; } - @Override public Span.Kind spanKind() { - return Span.Kind.SERVER; - } - @Override public Result result() { return result; } @@ -53,7 +48,7 @@ final class DubboServerResponse extends Response implements DubboResponse { } /** Returns the string form of the {@link RpcException#getCode()} */ - String errorCode() { + @Override public String errorCode() { return DubboParser.errorCode(error); } } diff --git a/instrumentation/dubbo/src/main/java/brave/dubbo/FinishSpan.java b/instrumentation/dubbo/src/main/java/brave/dubbo/FinishSpan.java new file mode 100644 index 0000000000..881c74d84e --- /dev/null +++ b/instrumentation/dubbo/src/main/java/brave/dubbo/FinishSpan.java @@ -0,0 +1,85 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.dubbo; + +import brave.Span; +import brave.internal.Nullable; +import brave.rpc.RpcClientHandler; +import brave.rpc.RpcClientRequest; +import brave.rpc.RpcServerHandler; +import java.util.function.BiConsumer; +import org.apache.dubbo.rpc.Result; + +abstract class FinishSpan implements BiConsumer { + static void finish(TracingFilter filter, + DubboRequest request, @Nullable Result result, @Nullable Throwable error, Span span) { + if (request instanceof RpcClientRequest) { + filter.clientHandler.handleReceive( + new DubboClientResponse((DubboClientRequest) request, result, error), span); + } else { + filter.serverHandler.handleSend( + new DubboServerResponse((DubboServerRequest) request, result, error), span); + } + } + + static FinishSpan create(TracingFilter filter, DubboRequest request, Result result, Span span) { + if (request instanceof DubboClientRequest) { + return new FinishClientSpan( + span, result, filter.clientHandler, (DubboClientRequest) request); + } + return new FinishServerSpan(span, result, filter.serverHandler, (DubboServerRequest) request); + } + + final Span span; + final Result result; + + FinishSpan(Span span, Result result) { + if (span == null) throw new NullPointerException("span == null"); + if (result == null) throw new NullPointerException("result == null"); + this.span = span; + this.result = result; + } + + static final class FinishClientSpan extends FinishSpan { + final RpcClientHandler clientHandler; + final DubboClientRequest request; + + FinishClientSpan( + Span span, Result result, RpcClientHandler clientHandler, DubboClientRequest request) { + super(span, result); + this.clientHandler = clientHandler; + this.request = request; + } + + @Override public void accept(@Nullable Object unused, @Nullable Throwable error) { + clientHandler.handleReceive(new DubboClientResponse(request, result, error), span); + } + } + + static final class FinishServerSpan extends FinishSpan { + final RpcServerHandler serverHandler; + final DubboServerRequest request; + + FinishServerSpan( + Span span, Result result, RpcServerHandler serverHandler, DubboServerRequest request) { + super(span, result); + this.serverHandler = serverHandler; + this.request = request; + } + + @Override public void accept(@Nullable Object unused, @Nullable Throwable error) { + serverHandler.handleSend(new DubboServerResponse(request, result, error), span); + } + } +} diff --git a/instrumentation/dubbo/src/main/java/brave/dubbo/TracingFilter.java b/instrumentation/dubbo/src/main/java/brave/dubbo/TracingFilter.java index 48c7190300..6a4a6ed0e6 100644 --- a/instrumentation/dubbo/src/main/java/brave/dubbo/TracingFilter.java +++ b/instrumentation/dubbo/src/main/java/brave/dubbo/TracingFilter.java @@ -15,21 +15,19 @@ import brave.Span; import brave.Span.Kind; +import brave.SpanCustomizer; import brave.Tag; -import brave.Tracer; import brave.Tracing; import brave.propagation.CurrentTraceContext; import brave.propagation.CurrentTraceContext.Scope; import brave.propagation.TraceContext; -import brave.propagation.TraceContext.Extractor; -import brave.propagation.TraceContext.Injector; -import brave.propagation.TraceContextOrSamplingFlags; -import brave.rpc.RpcRequest; +import brave.rpc.RpcClientHandler; +import brave.rpc.RpcResponse; +import brave.rpc.RpcResponseParser; +import brave.rpc.RpcServerHandler; import brave.rpc.RpcTracing; -import brave.sampler.SamplerFunction; import java.util.Map; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.Future; import org.apache.dubbo.common.constants.CommonConstants; import org.apache.dubbo.common.extension.Activate; import org.apache.dubbo.common.extension.ExtensionLoader; @@ -41,10 +39,7 @@ import org.apache.dubbo.rpc.RpcContext; import org.apache.dubbo.rpc.RpcException; -import static brave.dubbo.DubboClientRequest.SETTER; -import static brave.dubbo.DubboServerRequest.GETTER; import static brave.internal.Throwables.propagateIfFatal; -import static brave.sampler.SamplerFunctions.deferDecision; @Activate(group = {CommonConstants.PROVIDER, CommonConstants.CONSUMER}, value = "tracing") // http://dubbo.apache.org/en-us/docs/dev/impls/filter.html @@ -56,12 +51,15 @@ public final class TracingFilter implements Filter { return String.valueOf(((RpcException) input).getCode()); } }; + static final RpcResponseParser LEGACY_RESPONSE_PARSER = new RpcResponseParser() { + @Override public void parse(RpcResponse response, TraceContext context, SpanCustomizer span) { + DUBBO_ERROR_CODE.tag(response.error(), span); + } + }; CurrentTraceContext currentTraceContext; - Extractor extractor; - Injector injector; - Tracer tracer; - SamplerFunction clientSampler = deferDecision(), serverSampler = deferDecision(); + RpcClientHandler clientHandler; + RpcServerHandler serverHandler; volatile boolean isInit = false; /** @@ -73,29 +71,35 @@ public final class TracingFilter implements Filter { */ @Deprecated public void setTracing(Tracing tracing) { if (tracing == null) throw new NullPointerException("rpcTracing == null"); - setRpcTracing(RpcTracing.create(tracing)); + setRpcTracing(RpcTracing.newBuilder(tracing) + .clientResponseParser(LEGACY_RESPONSE_PARSER) + .serverResponseParser(LEGACY_RESPONSE_PARSER) + .build()); } /** * {@link ExtensionLoader} supplies the tracing implementation which must be named "rpcTracing". * For example, if using the {@link SpringExtensionFactory}, only a bean named "rpcTracing" will * be injected. + * + *

Custom parsing

+ * Custom parsers, such as {@link RpcTracing#clientRequestParser()}, can use Dubbo-specific types + * {@link DubboRequest} and {@link DubboResponse} to get access such as the Java invocation or + * result. */ public void setRpcTracing(RpcTracing rpcTracing) { if (rpcTracing == null) throw new NullPointerException("rpcTracing == null"); // we don't guard on init because we intentionally want to overwrite any call to setTracing currentTraceContext = rpcTracing.tracing().currentTraceContext(); - extractor = rpcTracing.tracing().propagation().extractor(GETTER); - injector = rpcTracing.tracing().propagation().injector(SETTER); - tracer = rpcTracing.tracing().tracer(); - clientSampler = rpcTracing.clientSampler(); - serverSampler = rpcTracing.serverSampler(); + clientHandler = RpcClientHandler.create(rpcTracing); + serverHandler = RpcServerHandler.create(rpcTracing); isInit = true; } @Override public Result invoke(Invoker invoker, Invocation invocation) throws RpcException { if (!isInit) return invoker.invoke(invocation); + TraceContext invocationContext = currentTraceContext.get(); RpcContext rpcContext = RpcContext.getContext(); Kind kind = rpcContext.isProviderSide() ? Kind.SERVER : Kind.CLIENT; @@ -109,40 +113,27 @@ public Result invoke(Invoker invoker, Invocation invocation) throws RpcExcept Map attachments = RpcContext.getContext().getAttachments(); DubboClientRequest clientRequest = new DubboClientRequest(invoker, invocation, attachments); request = clientRequest; - span = tracer.nextSpan(clientSampler, clientRequest); - injector.inject(span.context(), clientRequest); + span = clientHandler.handleSendWithParent(clientRequest, invocationContext); } else { DubboServerRequest serverRequest = new DubboServerRequest(invoker, invocation); request = serverRequest; - TraceContextOrSamplingFlags extracted = extractor.extract(serverRequest); - span = nextSpan(extracted, serverRequest); - } - - if (!span.isNoop()) { - span.kind(kind); - String service = DubboParser.service(invocation); - String method = DubboParser.method(invocation); - span.name(service + "/" + method); - DubboParser.parseRemoteIpAndPort(span); - span.start(); + span = serverHandler.handleReceive(serverRequest); } - boolean deferFinish = false; + boolean isSynchronous = true; Scope scope = currentTraceContext.newScope(span.context()); Result result = null; Throwable error = null; try { result = invoker.invoke(invocation); error = result.getException(); - Future future = rpcContext.getFuture(); // the case on async client invocation - if (future instanceof CompletableFuture) { - deferFinish = true; + CompletableFuture future = rpcContext.getCompletableFuture(); + if (future != null) { + isSynchronous = false; // NOTE: We don't currently instrument CompletableFuture, so callbacks will not see the // invocation context unless they use an executor instrumented by CurrentTraceContext - ((CompletableFuture) future).whenComplete((v, t) -> { - if (t != null) onError(t, span); - span.finish(); - }); + // If we later instrument this, take care to use the correct context depending on RPC kind! + future.whenComplete(FinishSpan.create(this, request, result, span)); } return result; } catch (Throwable e) { @@ -150,28 +141,8 @@ public Result invoke(Invoker invoker, Invocation invocation) throws RpcExcept error = e; throw e; } finally { - if (error != null) onError(error, span); - if (!deferFinish) span.finish(); + if (isSynchronous) FinishSpan.finish(this, request, result, error, span); scope.close(); } } - - /** Creates a potentially noop span representing this request */ - // This is the same code as HttpServerHandler.nextSpan - // TODO: pull this into RpcServerHandler when stable https://github.com/openzipkin/brave/pull/999 - Span nextSpan(TraceContextOrSamplingFlags extracted, DubboServerRequest request) { - Boolean sampled = extracted.sampled(); - // only recreate the context if the sampler made a decision - if (sampled == null && (sampled = serverSampler.trySample(request)) != null) { - extracted = extracted.sampled(sampled.booleanValue()); - } - return extracted.context() != null - ? tracer.joinSpan(extracted.context()) - : tracer.nextSpan(extracted); - } - - static void onError(Throwable error, Span span) { - span.error(error); - DUBBO_ERROR_CODE.tag(error, span); - } } diff --git a/instrumentation/dubbo/src/test/java/brave/dubbo/DubboClientRequestTest.java b/instrumentation/dubbo/src/test/java/brave/dubbo/DubboClientRequestTest.java index 437547364f..4d0b29f597 100644 --- a/instrumentation/dubbo/src/test/java/brave/dubbo/DubboClientRequestTest.java +++ b/instrumentation/dubbo/src/test/java/brave/dubbo/DubboClientRequestTest.java @@ -27,14 +27,13 @@ public class DubboClientRequestTest { Invoker invoker = mock(Invoker.class); Invocation invocation = mock(Invocation.class); - URL url = mock(URL.class); + URL url = URL.valueOf("dubbo://localhost:6666?scope=remote&interface=brave.dubbo.GreeterService"); Map attachments = new LinkedHashMap<>(); DubboClientRequest request = new DubboClientRequest(invoker, invocation, attachments); @Test public void service() { when(invocation.getInvoker()).thenReturn(invoker); when(invoker.getUrl()).thenReturn(url); - when(url.getServiceInterface()).thenReturn("brave.dubbo.GreeterService"); assertThat(request.service()) .isEqualTo("brave.dubbo.GreeterService"); diff --git a/instrumentation/dubbo/src/test/java/brave/dubbo/DubboParserTest.java b/instrumentation/dubbo/src/test/java/brave/dubbo/DubboParserTest.java index 6680e41048..3a10b9f041 100644 --- a/instrumentation/dubbo/src/test/java/brave/dubbo/DubboParserTest.java +++ b/instrumentation/dubbo/src/test/java/brave/dubbo/DubboParserTest.java @@ -36,7 +36,7 @@ public class DubboParserTest { when(invocation.getMethodName()).thenReturn("sayHello"); assertThat(DubboParser.method(invocation)) - .isEqualTo("sayHello"); + .isEqualTo("sayHello"); } @Test public void method_malformed() { @@ -50,7 +50,7 @@ public class DubboParserTest { when(invocation.getArguments()).thenReturn(new Object[] {"sayHello"}); assertThat(DubboParser.method(invocation)) - .isEqualTo("sayHello"); + .isEqualTo("sayHello"); } @Test public void method_invoke_nullArgs() { @@ -74,61 +74,52 @@ public class DubboParserTest { } @Test public void service() { - when(invocation.getInvoker()).thenReturn(invoker); when(invoker.getUrl()).thenReturn(url); when(url.getServiceInterface()).thenReturn("brave.dubbo.GreeterService"); - assertThat(DubboParser.service(invocation)) - .isEqualTo("brave.dubbo.GreeterService"); - } - - @Test public void service_nullInvoker() { - assertThat(DubboParser.service(invocation)).isNull(); + assertThat(DubboParser.service(invoker)) + .isEqualTo("brave.dubbo.GreeterService"); } @Test public void service_nullUrl() { - when(invocation.getInvoker()).thenReturn(invoker); - - assertThat(DubboParser.service(invocation)).isNull(); + assertThat(DubboParser.service(invoker)).isNull(); } @Test public void service_nullServiceInterface() { - when(invocation.getInvoker()).thenReturn(invoker); when(invoker.getUrl()).thenReturn(url); - assertThat(DubboParser.service(invocation)).isNull(); + assertThat(DubboParser.service(invoker)).isNull(); } @Test public void service_malformed() { - when(invocation.getInvoker()).thenReturn(invoker); when(invoker.getUrl()).thenReturn(url); when(url.getServiceInterface()).thenReturn(""); - assertThat(DubboParser.service(invocation)).isNull(); + assertThat(DubboParser.service(invoker)).isNull(); } @Test public void errorCodes() { assertThat(DubboParser.errorCode(null)) - .isEqualTo(DubboParser.errorCode(new IOException("timeout"))) - .isNull(); + .isEqualTo(DubboParser.errorCode(new IOException("timeout"))) + .isNull(); assertThat(DubboParser.errorCode(new RpcException(0))) - .isEqualTo("UNKNOWN_EXCEPTION"); + .isEqualTo("UNKNOWN_EXCEPTION"); assertThat(DubboParser.errorCode(new RpcException(1))) - .isEqualTo("NETWORK_EXCEPTION"); + .isEqualTo("NETWORK_EXCEPTION"); assertThat(DubboParser.errorCode(new RpcException(2))) - .isEqualTo("TIMEOUT_EXCEPTION"); + .isEqualTo("TIMEOUT_EXCEPTION"); assertThat(DubboParser.errorCode(new RpcException(3))) - .isEqualTo("BIZ_EXCEPTION"); + .isEqualTo("BIZ_EXCEPTION"); assertThat(DubboParser.errorCode(new RpcException(4))) - .isEqualTo("FORBIDDEN_EXCEPTION"); + .isEqualTo("FORBIDDEN_EXCEPTION"); assertThat(DubboParser.errorCode(new RpcException(5))) - .isEqualTo("SERIALIZATION_EXCEPTION"); + .isEqualTo("SERIALIZATION_EXCEPTION"); assertThat(DubboParser.errorCode(new RpcException(6))) - .isEqualTo("NO_INVOKER_AVAILABLE_AFTER_FILTER"); + .isEqualTo("NO_INVOKER_AVAILABLE_AFTER_FILTER"); assertThat(DubboParser.errorCode(new RpcException(7))) - .isEqualTo("LIMIT_EXCEEDED_EXCEPTION"); + .isEqualTo("LIMIT_EXCEEDED_EXCEPTION"); assertThat(DubboParser.errorCode(new RpcException(8))) - .isNull(); // This test will drift with a new error code name if Dubbo adds one. + .isNull(); // This test will drift with a new error code name if Dubbo adds one. } } diff --git a/instrumentation/dubbo/src/test/java/brave/dubbo/DubboServerRequestTest.java b/instrumentation/dubbo/src/test/java/brave/dubbo/DubboServerRequestTest.java index a2df95d821..22d1b7c35e 100644 --- a/instrumentation/dubbo/src/test/java/brave/dubbo/DubboServerRequestTest.java +++ b/instrumentation/dubbo/src/test/java/brave/dubbo/DubboServerRequestTest.java @@ -25,16 +25,15 @@ public class DubboServerRequestTest { Invoker invoker = mock(Invoker.class); Invocation invocation = mock(Invocation.class); - URL url = mock(URL.class); + URL url = URL.valueOf("dubbo://localhost:6666?scope=remote&interface=brave.dubbo.GreeterService"); DubboServerRequest request = new DubboServerRequest(invoker, invocation); @Test public void service() { when(invocation.getInvoker()).thenReturn(invoker); when(invoker.getUrl()).thenReturn(url); - when(url.getServiceInterface()).thenReturn("brave.dubbo.GreeterService"); assertThat(request.service()) - .isEqualTo("brave.dubbo.GreeterService"); + .isEqualTo("brave.dubbo.GreeterService"); } @Test public void method() { diff --git a/instrumentation/dubbo/src/test/java/brave/dubbo/FinishSpanTest.java b/instrumentation/dubbo/src/test/java/brave/dubbo/FinishSpanTest.java new file mode 100644 index 0000000000..4d82d2068e --- /dev/null +++ b/instrumentation/dubbo/src/test/java/brave/dubbo/FinishSpanTest.java @@ -0,0 +1,143 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.dubbo; + +import brave.Span; +import java.util.Collections; +import org.apache.dubbo.rpc.Invocation; +import org.apache.dubbo.rpc.Invoker; +import org.apache.dubbo.rpc.Result; +import org.junit.Before; +import org.junit.Test; +import zipkin2.Span.Kind; + +import static org.mockito.Mockito.mock; + +public class FinishSpanTest extends ITTracingFilter { + DubboClientRequest clientRequest = + new DubboClientRequest(mock(Invoker.class), mock(Invocation.class), Collections.emptyMap()); + DubboServerRequest serverRequest = + new DubboServerRequest(mock(Invoker.class), mock(Invocation.class)); + TracingFilter filter; + + @Before public void setup() { + filter = init(); + } + + @Test public void finish_null_result_and_error_DubboClientRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.CLIENT).start(); + + FinishSpan.finish(filter, clientRequest, null, null, span); + + reporter.takeRemoteSpan(Kind.CLIENT); + } + + @Test public void finish_null_result_and_error_DubboServerRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.SERVER).start(); + + FinishSpan.finish(filter, serverRequest, null, null, span); + + reporter.takeRemoteSpan(Kind.SERVER); + } + + @Test public void finish_result_but_null_error_DubboClientRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.CLIENT).start(); + + FinishSpan.finish(filter, clientRequest, mock(Result.class), null, span); + + reporter.takeRemoteSpan(Kind.CLIENT); + } + + @Test public void finish_result_but_null_error_DubboServerRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.SERVER).start(); + + FinishSpan.finish(filter, serverRequest, mock(Result.class), null, span); + + reporter.takeRemoteSpan(Kind.SERVER); + } + + @Test public void finish_error_but_null_result_DubboClientRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.CLIENT).start(); + + Throwable error = new RuntimeException("melted"); + FinishSpan.finish(filter, clientRequest, null, error, span); + + reporter.takeRemoteSpanWithError(Kind.CLIENT, error.getMessage()); + } + + @Test public void finish_error_but_null_result_DubboServerRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.SERVER).start(); + + Throwable error = new RuntimeException("melted"); + FinishSpan.finish(filter, serverRequest, null, error, span); + + reporter.takeRemoteSpanWithError(Kind.SERVER, error.getMessage()); + } + + @Test public void create_null_result_value_and_error_DubboClientRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.CLIENT).start(); + + FinishSpan.create(filter, clientRequest, mock(Result.class), span) + .accept(null, null); + + reporter.takeRemoteSpan(Kind.CLIENT); + } + + @Test public void create_null_result_value_and_error_DubboServerRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.SERVER).start(); + + FinishSpan.create(filter, serverRequest, mock(Result.class), span) + .accept(null, null); + + reporter.takeRemoteSpan(Kind.SERVER); + } + + @Test public void create_result_value_but_null_error_DubboClientRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.CLIENT).start(); + + FinishSpan.create(filter, clientRequest, mock(Result.class), span) + .accept(new Object(), null); + + reporter.takeRemoteSpan(Kind.CLIENT); + } + + @Test public void create_result_value_but_null_error_DubboServerRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.SERVER).start(); + + FinishSpan.create(filter, serverRequest, mock(Result.class), span) + .accept(new Object(), null); + + reporter.takeRemoteSpan(Kind.SERVER); + } + + @Test public void create_error_but_null_result_value_DubboClientRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.CLIENT).start(); + + Throwable error = new RuntimeException("melted"); + FinishSpan.create(filter, clientRequest, mock(Result.class), span) + .accept(null, error); + + reporter.takeRemoteSpanWithError(Kind.CLIENT, error.getMessage()); + } + + @Test public void create_error_but_null_result_value_DubboServerRequest() { + Span span = tracing.tracer().nextSpan().kind(Span.Kind.SERVER).start(); + + Throwable error = new RuntimeException("melted"); + FinishSpan.create(filter, serverRequest, mock(Result.class), span) + .accept(null, error); + + reporter.takeRemoteSpanWithError(Kind.SERVER, error.getMessage()); + } +} diff --git a/instrumentation/dubbo/src/test/java/brave/dubbo/ITTracingFilter_Consumer.java b/instrumentation/dubbo/src/test/java/brave/dubbo/ITTracingFilter_Consumer.java index 67f6ee0249..121a5de0a2 100644 --- a/instrumentation/dubbo/src/test/java/brave/dubbo/ITTracingFilter_Consumer.java +++ b/instrumentation/dubbo/src/test/java/brave/dubbo/ITTracingFilter_Consumer.java @@ -14,9 +14,11 @@ package brave.dubbo; import brave.Clock; +import brave.Tag; import brave.propagation.CurrentTraceContext.Scope; import brave.propagation.SamplingFlags; import brave.propagation.TraceContext; +import brave.rpc.RpcResponseParser; import brave.rpc.RpcRuleSampler; import brave.rpc.RpcTracing; import brave.test.util.AssertableCallback; @@ -24,6 +26,7 @@ import org.apache.dubbo.config.ReferenceConfig; import org.apache.dubbo.config.bootstrap.DubboBootstrap; import org.apache.dubbo.rpc.Filter; +import org.apache.dubbo.rpc.Result; import org.apache.dubbo.rpc.RpcContext; import org.apache.dubbo.rpc.RpcException; import org.junit.After; @@ -58,9 +61,9 @@ public class ITTracingFilter_Consumer extends ITTracingFilter { wrongClient.setUrl(url); DubboBootstrap.getInstance().application(application) - .reference(client) - .reference(wrongClient) - .start(); + .reference(client) + .reference(wrongClient) + .start(); init(); @@ -159,9 +162,9 @@ public class ITTracingFilter_Consumer extends ITTracingFilter { TraceContext parent = newTraceContext(SamplingFlags.SAMPLED); try (Scope scope = currentTraceContext.newScope(parent)) { RpcContext.getContext().asyncCall(() -> client.get().sayHello("jorge")) - .whenComplete(items1); + .whenComplete(items1); RpcContext.getContext().asyncCall(() -> client.get().sayHello("romeo")) - .whenComplete(items2); + .whenComplete(items2); } try (Scope scope = currentTraceContext.newScope(null)) { @@ -181,23 +184,6 @@ public class ITTracingFilter_Consumer extends ITTracingFilter { } } - @Test public void customSampler() { - RpcTracing rpcTracing = RpcTracing.newBuilder(tracing).clientSampler(RpcRuleSampler.newBuilder() - .putRule(methodEquals("sayGoodbye"), NEVER_SAMPLE) - .putRule(serviceEquals("brave.dubbo"), ALWAYS_SAMPLE) - .build()).build(); - init().setRpcTracing(rpcTracing); - - // unsampled - client.get().sayGoodbye("jorge"); - - // sampled - client.get().sayHello("jorge"); - - assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).name()).endsWith("sayhello"); - // @After will also check that sayGoodbye was not sampled - } - @Test public void reportsClientKindToZipkin() { client.get().sayHello("jorge"); @@ -208,14 +194,14 @@ public class ITTracingFilter_Consumer extends ITTracingFilter { client.get().sayHello("jorge"); assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).name()) - .isEqualTo("brave.dubbo.greeterservice/sayhello"); + .isEqualTo("brave.dubbo.greeterservice/sayhello"); } @Test public void onTransportException_addsErrorTag() { server.stop(); assertThatThrownBy(() -> client.get().sayHello("jorge")) - .isInstanceOf(RpcException.class); + .isInstanceOf(RpcException.class); reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, ".*RemotingException.*"); } @@ -238,29 +224,72 @@ public class ITTracingFilter_Consumer extends ITTracingFilter { @Test public void addsErrorTag_onUnimplemented() { assertThatThrownBy(() -> wrongClient.get().sayHello("jorge")) - .isInstanceOf(RpcException.class); + .isInstanceOf(RpcException.class); Span span = - reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, ".*Not found exported service.*"); + reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, ".*Not found exported service.*"); assertThat(span.tags()) - .containsEntry("dubbo.error_code", "1"); + .containsEntry("dubbo.error_code", "1"); } /** Shows if you aren't using RpcTracing, the old "dubbo.error_code" works */ @Test public void addsErrorTag_onUnimplemented_legacy() { ((TracingFilter) ExtensionLoader.getExtensionLoader(Filter.class) - .getExtension("tracing")).isInit = false; + .getExtension("tracing")).isInit = false; ((TracingFilter) ExtensionLoader.getExtensionLoader(Filter.class) - .getExtension("tracing")) - .setTracing(tracing); + .getExtension("tracing")) + .setTracing(tracing); assertThatThrownBy(() -> wrongClient.get().sayHello("jorge")) - .isInstanceOf(RpcException.class); + .isInstanceOf(RpcException.class); Span span = - reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, ".*Not found exported service.*"); + reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, ".*Not found exported service.*"); assertThat(span.tags()) - .containsEntry("dubbo.error_code", "1"); + .containsEntry("dubbo.error_code", "1"); + } + + /* RpcTracing-specific feature tests */ + + @Test public void customSampler() { + RpcTracing rpcTracing = RpcTracing.newBuilder(tracing).clientSampler(RpcRuleSampler.newBuilder() + .putRule(methodEquals("sayGoodbye"), NEVER_SAMPLE) + .putRule(serviceEquals("brave.dubbo"), ALWAYS_SAMPLE) + .build()).build(); + init().setRpcTracing(rpcTracing); + + // unsampled + client.get().sayGoodbye("jorge"); + + // sampled + client.get().sayHello("jorge"); + + assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).name()).endsWith("sayhello"); + // @After will also check that sayGoodbye was not sampled + } + + @Test public void customParser() { + Tag javaValue = new Tag("dubbo.result_value") { + @Override protected String parseValue(DubboResponse input, TraceContext context) { + Result result = input.result(); + if (result == null) return null; + return String.valueOf(result.getValue()); + } + }; + + RpcTracing rpcTracing = RpcTracing.newBuilder(tracing) + .clientResponseParser((res, context, span) -> { + RpcResponseParser.DEFAULT.parse(res, context, span); + if (res instanceof DubboResponse) { + javaValue.tag((DubboResponse) res, span); + } + }).build(); + init().setRpcTracing(rpcTracing); + + String javaResult = client.get().sayHello("jorge"); + + assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).tags()) + .containsEntry("dubbo.result_value", javaResult); } } diff --git a/instrumentation/dubbo/src/test/java/brave/dubbo/ITTracingFilter_Provider.java b/instrumentation/dubbo/src/test/java/brave/dubbo/ITTracingFilter_Provider.java index 2a270c7aee..029ac62b24 100644 --- a/instrumentation/dubbo/src/test/java/brave/dubbo/ITTracingFilter_Provider.java +++ b/instrumentation/dubbo/src/test/java/brave/dubbo/ITTracingFilter_Provider.java @@ -13,13 +13,16 @@ */ package brave.dubbo; +import brave.Tag; import brave.propagation.B3SingleFormat; import brave.propagation.SamplingFlags; import brave.propagation.TraceContext; +import brave.rpc.RpcResponseParser; import brave.rpc.RpcRuleSampler; import brave.rpc.RpcTracing; import org.apache.dubbo.config.ReferenceConfig; import org.apache.dubbo.config.bootstrap.DubboBootstrap; +import org.apache.dubbo.rpc.Result; import org.apache.dubbo.rpc.RpcContext; import org.junit.Before; import org.junit.Test; @@ -41,8 +44,8 @@ public class ITTracingFilter_Provider extends ITTracingFilter { String arg = (String) args[0]; if (arg.equals("bad")) throw new IllegalArgumentException(); return currentTraceContext.get() != null - ? currentTraceContext.get().traceIdString() - : ""; + ? currentTraceContext.get().traceIdString() + : ""; }); init(); server.start(); @@ -54,9 +57,9 @@ public class ITTracingFilter_Provider extends ITTracingFilter { client.setUrl(url); DubboBootstrap.getInstance().application(application) - .service(server.service) - .reference(client) - .start(); + .service(server.service) + .reference(client) + .start(); // perform a warmup request to allow CI to fail quicker client.get().sayHello("jorge"); @@ -97,7 +100,7 @@ public class ITTracingFilter_Provider extends ITTracingFilter { @Test public void currentSpanVisibleToImpl() { assertThat(client.get().sayHello("jorge")) - .isNotEmpty(); + .isNotEmpty(); reporter.takeRemoteSpan(Span.Kind.SERVER); } @@ -112,21 +115,23 @@ public class ITTracingFilter_Provider extends ITTracingFilter { client.get().sayHello("jorge"); assertThat(reporter.takeRemoteSpan(Span.Kind.SERVER).name()) - .isEqualTo("brave.dubbo.greeterservice/sayhello"); + .isEqualTo("brave.dubbo.greeterservice/sayhello"); } @Test public void addsErrorTagOnException() { assertThatThrownBy(() -> client.get().sayHello("bad")) - .isInstanceOf(IllegalArgumentException.class); + .isInstanceOf(IllegalArgumentException.class); reporter.takeRemoteSpanWithError(Span.Kind.SERVER, "IllegalArgumentException"); } + /* RpcTracing-specific feature tests */ + @Test public void customSampler() { RpcTracing rpcTracing = RpcTracing.newBuilder(tracing).serverSampler(RpcRuleSampler.newBuilder() - .putRule(methodEquals("sayGoodbye"), NEVER_SAMPLE) - .putRule(serviceEquals("brave.dubbo"), ALWAYS_SAMPLE) - .build()).build(); + .putRule(methodEquals("sayGoodbye"), NEVER_SAMPLE) + .putRule(serviceEquals("brave.dubbo"), ALWAYS_SAMPLE) + .build()).build(); init().setRpcTracing(rpcTracing); // unsampled @@ -138,4 +143,28 @@ public class ITTracingFilter_Provider extends ITTracingFilter { assertThat(reporter.takeRemoteSpan(Span.Kind.SERVER).name()).endsWith("sayhello"); // @After will also check that sayGoodbye was not sampled } + + @Test public void customParser() { + Tag javaValue = new Tag("dubbo.result_value") { + @Override protected String parseValue(DubboResponse input, TraceContext context) { + Result result = input.result(); + if (result == null) return null; + return String.valueOf(result.getValue()); + } + }; + + RpcTracing rpcTracing = RpcTracing.newBuilder(tracing) + .serverResponseParser((res, context, span) -> { + RpcResponseParser.DEFAULT.parse(res, context, span); + if (res instanceof DubboResponse) { + javaValue.tag((DubboResponse) res, span); + } + }).build(); + init().setRpcTracing(rpcTracing); + + String javaResult = client.get().sayHello("jorge"); + + assertThat(reporter.takeRemoteSpan(Span.Kind.SERVER).tags()) + .containsEntry("dubbo.result_value", javaResult); + } } diff --git a/instrumentation/grpc/RATIONALE.md b/instrumentation/grpc/RATIONALE.md index 32e7915c13..c662abd36f 100644 --- a/instrumentation/grpc/RATIONALE.md +++ b/instrumentation/grpc/RATIONALE.md @@ -25,3 +25,14 @@ record the application exception. In short, we choose to not mask the gRPC status with a potential application exception on close. In worst case, if there is no instrumentation for the layer that throws, its trace and span ID could be known via log correlation. + +## Why don't we use RpcClientHandler or RpcServerHandler for unexpected errors? +Some callbacks, such as `onHalfClose()` can throw due to unexpected bugs. In +these cases, there will be no response `Status` or `Trailers`. In some cases, +we won't have a reference to the call either. Ex when an error raises from +`ServerCallHandler.startCall`. In short the data available is sparse, and often +only the `Throwable`. + +Rather than the complicate response parsing by making all fields `@Nullable`, +we decided to leave interceptor and listener bugs special case. When these +occur, we instead call `span.error(error).finish()`. diff --git a/instrumentation/grpc/README.md b/instrumentation/grpc/README.md index 556b195950..e1c8776436 100644 --- a/instrumentation/grpc/README.md +++ b/instrumentation/grpc/README.md @@ -27,23 +27,46 @@ ManagedChannel channel = ManagedChannelBuilder.forAddress("localhost", serverPor .build(); ``` -## Span data policy -By default, the following are added to both gRPC client and server spans: -* Span.name as the full method name: ex "helloworld.greeter/sayhello" -* Tags/binary annotations: - * "grpc.status_code" when the status is not OK. - * "error", when there is an exception or status is not OK. +## Sampling and data policy -If you just want to control span naming policy, override `spanName` in -your client or server parser. +Please read the [RPC documentation](../rpc/README.md) before proceeding, as it +covers important topics such as which tags are added to spans, and how traces +are sampled. -Ex: +### RPC model mapping + +As mentioned above, the RPC model types `RpcRequest` and `RpcResponse` allow +portable sampling decisions and tag parsing. + +gRPC maps to this model as follows: +* `RpcRequest.service()` - text preceding the first slash ('/') in `MethodDescriptor.fullMethodName` + * Ex. "helloworld.Greeter" for a full method name "helloworld.Greeter/SayHello" +* `RpcRequest.method()` - text following the first slash ('/') in `MethodDescriptor.fullMethodName` + * Ex. "SayHello" for a full method name "helloworld.Greeter/SayHello" +* `RpcResponse.errorCode()` - `Status.Code.name()` when not `Status.isOk()` + * Ex. `null` for `Status.Code.OK` and `UNKNOWN` for `Status.Code.UNKNOWN` + +### gRPC-specific model + +The `GrpcRequest` and `GrpcResponse` are available for custom sampling and tag +parsing. + +Here is an example that adds default tags, and if gRPC, the method type (ex "UNARY"): ```java -overrideSpanName = new GrpcClientParser() { - @Override protected String spanName(MethodDescriptor methodDescriptor) { - return methodDescriptor.getType().name(); +Tag methodType = new Tag("grpc.method_type") { + @Override protected String parseValue(GrpcRequest input, TraceContext context) { + return input.methodDescriptor().getType().name(); } }; + +RpcRequestParser addMethodType = (req, context, span) -> { + RpcRequestParser.DEFAULT.parse(req, context, span); + if (req instanceof GrpcRequest) methodType.tag((GrpcRequest) req, span); +}; + +grpcTracing = GrpcTracing.create(RpcTracing.newBuilder(tracing) + .clientRequestParser(addMethodType) + .serverRequestParser(addMethodType).build()); ``` ## Message processing @@ -81,30 +104,6 @@ managedChannelBuilder.intercept(new ClientInterceptor() { }, grpcTracing.newClientInterceptor()); ``` -## Sampling Policy -The default sampling policy is to use the default (trace ID) sampler for -server and client requests. - -You can use an [RpcRuleSampler](../rpc/README.md) to override this based on -gRPC service or method names. - -Ex. Here's a sampler that traces 100 "GetUserToken" requests per second. This -doesn't start new traces for requests to the health check service. Other -requests will use a global rate provided by the tracing component. - -```java -import static brave.rpc.RpcRequestMatchers.methodEquals; -import static brave.rpc.RpcRequestMatchers.serviceEquals; -import static brave.sampler.Matchers.and; - -rpcTracing = rpcTracingBuilder.serverSampler(RpcRuleSampler.newBuilder() - .putRule(serviceEquals("grpc.health.v1.Health"), Sampler.NEVER_SAMPLE) - .putRule(and(serviceEquals("users.UserService"), methodEquals("GetUserToken")), RateLimitingSampler.create(100)) - .build()).build(); - -grpcTracing = GrpcTracing.create(rpcTracing); -``` - ## gRPC Propagation Format (Census interop) gRPC defines a [binary encoded propagation format](https://github.com/census-instrumentation/opencensus-specs/blob/master/encodings/BinaryEncoding.md) which is implemented diff --git a/instrumentation/grpc/src/main/java/brave/grpc/GrpcClientParser.java b/instrumentation/grpc/src/main/java/brave/grpc/GrpcClientParser.java index 4d52b3ddad..5421f158d8 100644 --- a/instrumentation/grpc/src/main/java/brave/grpc/GrpcClientParser.java +++ b/instrumentation/grpc/src/main/java/brave/grpc/GrpcClientParser.java @@ -14,12 +14,32 @@ package brave.grpc; import brave.SpanCustomizer; +import brave.propagation.TraceContext; +import brave.rpc.RpcRequest; +import brave.rpc.RpcRequestParser; +import brave.rpc.RpcTracing; import io.grpc.CallOptions; import io.grpc.ClientCall; import io.grpc.Metadata; import io.grpc.MethodDescriptor; -public class GrpcClientParser extends GrpcParser { +/** + * @see GrpcClientRequest + * @see GrpcClientResponse + * @deprecated Since 5.12 use {@link RpcTracing#clientRequestParser()} or {@link + * RpcTracing#clientResponseParser()}. + */ +@Deprecated +public class GrpcClientParser extends GrpcParser implements RpcRequestParser { + @Override public void parse(RpcRequest request, TraceContext context, SpanCustomizer span) { + if (request instanceof GrpcClientRequest) { + GrpcClientRequest grpcRequest = (GrpcClientRequest) request; + onStart(grpcRequest.methodDescriptor, grpcRequest.callOptions, grpcRequest.headers, span); + } else { + assert false : "expected a GrpcClientRequest: " + request; + } + } + /** Override the customize the span based on the start of a request. */ protected void onStart(MethodDescriptor method, CallOptions options, Metadata headers, SpanCustomizer span) { diff --git a/instrumentation/grpc/src/main/java/brave/grpc/GrpcClientRequest.java b/instrumentation/grpc/src/main/java/brave/grpc/GrpcClientRequest.java index f55fef6167..01dcf99a46 100644 --- a/instrumentation/grpc/src/main/java/brave/grpc/GrpcClientRequest.java +++ b/instrumentation/grpc/src/main/java/brave/grpc/GrpcClientRequest.java @@ -13,7 +13,6 @@ */ package brave.grpc; -import brave.propagation.Propagation.Setter; import brave.rpc.RpcClientRequest; import io.grpc.CallOptions; import io.grpc.Channel; @@ -24,18 +23,14 @@ import io.grpc.MethodDescriptor; import java.util.Map; -// intentionally not yet public until we add tag parsing functionality -final class GrpcClientRequest extends RpcClientRequest { - static final Setter SETTER = new Setter() { - @Override public void put(GrpcClientRequest request, String key, String value) { - request.propagationField(key, value); - } - - @Override public String toString() { - return "GrpcClientRequest::propagationField"; - } - }; - +/** + * Allows access gRPC specific aspects of a client request during sampling and parsing. + * + * @see GrpcClientResponse + * @see GrpcRequest for a parsing example + * @since 5.12 + */ +public final class GrpcClientRequest extends RpcClientRequest implements GrpcRequest { final Map> nameToKey; final MethodDescriptor methodDescriptor; final CallOptions callOptions; @@ -43,7 +38,7 @@ final class GrpcClientRequest extends RpcClientRequest { final Metadata headers; GrpcClientRequest(Map> nameToKey, MethodDescriptor methodDescriptor, - CallOptions callOptions, ClientCall call, Metadata headers) { + CallOptions callOptions, ClientCall call, Metadata headers) { if (nameToKey == null) throw new NullPointerException("nameToKey == null"); if (methodDescriptor == null) throw new NullPointerException("methodDescriptor == null"); if (callOptions == null) throw new NullPointerException("callOptions == null"); @@ -76,7 +71,7 @@ final class GrpcClientRequest extends RpcClientRequest { * * @since 5.12 */ - public MethodDescriptor methodDescriptor() { + @Override public MethodDescriptor methodDescriptor() { return methodDescriptor; } @@ -101,16 +96,17 @@ public CallOptions callOptions() { } /** - * Returns the {@linkplain Metadata headers} passed to {@link ClientCall#start(ClientCall.Listener, + * Returns the {@linkplain Metadata headers} passed to {@link ClientCall#start(ClientCall.Listener, * Metadata)}. * * @since 5.12 */ - public Metadata headers() { + @Override public Metadata headers() { return headers; } - void propagationField(String keyName, String value) { + @Override protected void propagationField(String keyName, String value) { + if (keyName == null) throw new NullPointerException("keyName == null"); if (value == null) throw new NullPointerException("value == null"); Key key = nameToKey.get(keyName); diff --git a/instrumentation/grpc/src/main/java/brave/grpc/GrpcClientResponse.java b/instrumentation/grpc/src/main/java/brave/grpc/GrpcClientResponse.java index 4dc50f1570..6f3f344e5d 100644 --- a/instrumentation/grpc/src/main/java/brave/grpc/GrpcClientResponse.java +++ b/instrumentation/grpc/src/main/java/brave/grpc/GrpcClientResponse.java @@ -13,35 +13,39 @@ */ package brave.grpc; -import brave.Response; -import brave.Span; import brave.internal.Nullable; +import brave.rpc.RpcClientResponse; import io.grpc.ClientCall; import io.grpc.Metadata; import io.grpc.Status; -// intentionally not yet public until we add tag parsing functionality -final class GrpcClientResponse extends Response { +/** + * Allows access gRPC specific aspects of a client response for parsing. + * + * @see GrpcClientRequest + * @see GrpcResponse for a parsing example + * @since 5.12 + */ +public final class GrpcClientResponse extends RpcClientResponse implements GrpcResponse { final GrpcClientRequest request; - @Nullable final Status status; - @Nullable final Metadata trailers; - @Nullable final Throwable error; + final Metadata headers; + final Status status; + final Metadata trailers; - GrpcClientResponse(GrpcClientRequest request, - @Nullable Status status, @Nullable Metadata trailers, @Nullable Throwable error) { + GrpcClientResponse(GrpcClientRequest request, Metadata headers, Status status, + Metadata trailers) { if (request == null) throw new NullPointerException("request == null"); + if (headers == null) throw new NullPointerException("headers == null"); + if (status == null) throw new NullPointerException("status == null"); + if (trailers == null) throw new NullPointerException("trailers == null"); + this.headers = headers; this.request = request; this.status = status; this.trailers = trailers; - this.error = error != null ? error : status != null ? status.getCause() : null; - } - - @Override public Span.Kind spanKind() { - return Span.Kind.CLIENT; } /** Returns the {@link #status()} */ - @Override @Nullable public Status unwrap() { + @Override public Status unwrap() { return status; } @@ -49,36 +53,44 @@ final class GrpcClientResponse extends Response { return request; } + /** Returns {@link Status#getCause()} */ @Override @Nullable public Throwable error() { - return error; + return status.getCause(); } /** * Returns the string form of the {@link Status#getCode()} or {@code null} when not {@link * Status#isOk()} or {@link #error()}. */ - @Nullable public String errorCode() { - if (status == null || status.isOk()) return null; + @Override @Nullable public String errorCode() { + if (status.isOk()) return null; return status.getCode().name(); } /** - * Returns the status passed to {@link ClientCall.Listener#onClose(Status, Metadata)} or {@code - * null} on {@link #error()}. + * Returns a copy of headers passed to {@link ClientCall.Listener#onHeaders(Metadata)}. + * + * @since 5.12 + */ + @Override public Metadata headers() { + return headers; + } + + /** + * Returns the status passed to {@link ClientCall.Listener#onClose(Status, Metadata)}. * * @since 5.12 */ - @Nullable public Status status() { + @Override public Status status() { return status; } /** - * Returns the trailers passed to {@link ClientCall.Listener#onClose(Status, Metadata)} or {@code - * null} on {@link #error()}. + * Returns the trailers passed to {@link ClientCall.Listener#onClose(Status, Metadata)}. * * @since 5.12 */ - @Nullable public Metadata trailers() { + @Override public Metadata trailers() { return trailers; } } diff --git a/instrumentation/grpc/src/main/java/brave/grpc/GrpcParser.java b/instrumentation/grpc/src/main/java/brave/grpc/GrpcParser.java index af3a431d0d..d73bfb5d43 100644 --- a/instrumentation/grpc/src/main/java/brave/grpc/GrpcParser.java +++ b/instrumentation/grpc/src/main/java/brave/grpc/GrpcParser.java @@ -18,11 +18,35 @@ import brave.SpanCustomizer; import brave.Tracing; import brave.internal.Nullable; +import brave.propagation.TraceContext; +import brave.rpc.RpcResponse; +import brave.rpc.RpcResponseParser; +import brave.rpc.RpcTracing; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.Status; -public class GrpcParser extends MessageProcessor { +/** @deprecated Since 5.12 use parsers in {@link RpcTracing}. */ +@Deprecated +public class GrpcParser extends MessageProcessor implements RpcResponseParser { + static final RpcResponseParser LEGACY_RESPONSE_PARSER = new GrpcParser(); + + @Override + public void parse(RpcResponse response, TraceContext context, SpanCustomizer span) { + Status status = null; + Metadata trailers = null; + if (response instanceof GrpcClientResponse) { + status = ((GrpcClientResponse) response).status(); + trailers = ((GrpcClientResponse) response).trailers(); + } else if (response instanceof GrpcServerResponse) { + status = ((GrpcServerResponse) response).status(); + trailers = ((GrpcServerResponse) response).trailers(); + } else { + assert false : "expected a GrpcClientResponse or GrpcServerResponse: " + response; + } + onClose(status, trailers, span); + } + /** * Override when making custom types. Typically, you'll use {@link Tracing#errorParser()} * @@ -51,8 +75,6 @@ protected String spanName(MethodDescriptor methodDesc /** * Override to change what data from the status or trailers are parsed into the span modeling it. - * By default, this tags "grpc.status_code" when it is not OK, and "error" if there was no {@link - * Status#getCause()}. * *

Note: {@link Status#getCause()} will be set as {@link Span#error(Throwable)} by * default. You don't need to parse it here. diff --git a/instrumentation/grpc/src/main/java/brave/grpc/GrpcRequest.java b/instrumentation/grpc/src/main/java/brave/grpc/GrpcRequest.java new file mode 100644 index 0000000000..17d35e8abb --- /dev/null +++ b/instrumentation/grpc/src/main/java/brave/grpc/GrpcRequest.java @@ -0,0 +1,56 @@ +/* + * Copyright 2013-2020 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.grpc; + +import brave.rpc.RpcTracing; +import io.grpc.Metadata; +import io.grpc.MethodDescriptor; + +/** + * Allows access gRPC specific aspects of a client or server request during sampling and parsing. + * + *

Here's an example that adds default tags, and if gRPC, the {@linkplain + * MethodDescriptor#getType() method type}: + * 

{@code
+ * Tag methodType = new Tag("grpc.method_type") {
+ *   @Override protected String parseValue(GrpcRequest input, TraceContext context) {
+ *     return input.methodDescriptor().getType().name();
+ *   }
+ * };
+ *
+ * RpcRequestParser addMethodType = (req, context, span) -> {
+ *   RpcRequestParser.DEFAULT.parse(req, context, span);
+ *   if (req instanceof GrpcRequest) methodType.tag((GrpcRequest) req, span);
+ * };
+ *
+ * grpcTracing = GrpcTracing.create(RpcTracing.newBuilder(tracing)
+ *     .clientRequestParser(addMethodType)
+ *     .serverRequestParser(addMethodType).build());
+ * }
+ * + * @see GrpcResponse + * @see GrpcClientRequest + * @see GrpcServerRequest + * @see RpcTracing#clientRequestParser() + * @see RpcTracing#serverRequestParser() + * @since 5.12 + */ +// NOTE: gRPC is Java 1.7+, so we cannot add methods to this later +public interface GrpcRequest { + // method would be a nicer name, but this is used in instanceof with an RpcRequest + // and RpcRequest.method() has a String result + MethodDescriptor methodDescriptor(); + + Metadata headers(); +} diff --git a/instrumentation/grpc/src/main/java/brave/grpc/GrpcResponse.java b/instrumentation/grpc/src/main/java/brave/grpc/GrpcResponse.java new file mode 100644 index 0000000000..974a0b00d0 --- /dev/null +++ b/instrumentation/grpc/src/main/java/brave/grpc/GrpcResponse.java @@ -0,0 +1,55 @@ +/* + * Copyright 2013-2020 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.grpc; + +import brave.rpc.RpcTracing; +import io.grpc.Metadata; +import io.grpc.Status; + +/** + * Allows access gRPC specific aspects of a client or server response for parsing. + * + *

Here's an example that adds default tags, and if gRPC, the response encoding: + * 

{@code
+ * Tag responseEncoding = new Tag("grpc.response_encoding") {
+ *   @Override protected String parseValue(GrpcResponse input, TraceContext context) {
+ *     return input.headers().get(GrpcUtil.MESSAGE_ENCODING_KEY);
+ *   }
+ * };
+ *
+ * RpcResponseParser addResponseEncoding = (res, context, span) -> {
+ *   RpcResponseParser.DEFAULT.parse(res, context, span);
+ *   if (res instanceof GrpcResponse) responseEncoding.tag((GrpcResponse) res, span);
+ * };
+ *
+ * grpcTracing = GrpcTracing.create(RpcTracing.newBuilder(tracing)
+ *     .clientResponseParser(addResponseEncoding);
+ *     .serverResponseParser(addResponseEncoding).build());
+ * }
+ * + * @see GrpcRequest + * @see GrpcClientResponse + * @see GrpcServerResponse + * @see RpcTracing#clientResponseParser() + * @see RpcTracing#serverResponseParser() + * @since 5.12 + */ +// NOTE: gRPC is Java 1.7+, so we cannot add methods to this later +public interface GrpcResponse { + Metadata headers(); + + Status status(); + + Metadata trailers(); +} diff --git a/instrumentation/grpc/src/main/java/brave/grpc/GrpcServerParser.java b/instrumentation/grpc/src/main/java/brave/grpc/GrpcServerParser.java index 166a0bc251..678ca61c60 100644 --- a/instrumentation/grpc/src/main/java/brave/grpc/GrpcServerParser.java +++ b/instrumentation/grpc/src/main/java/brave/grpc/GrpcServerParser.java @@ -14,10 +14,30 @@ package brave.grpc; import brave.SpanCustomizer; +import brave.propagation.TraceContext; +import brave.rpc.RpcRequest; +import brave.rpc.RpcRequestParser; +import brave.rpc.RpcTracing; import io.grpc.Metadata; import io.grpc.ServerCall; -public class GrpcServerParser extends GrpcParser { +/** + * @see GrpcServerRequest + * @see GrpcServerResponse + * @deprecated Since 5.12 use {@link RpcTracing#serverRequestParser()} or {@link + * RpcTracing#serverResponseParser()}. + */ +@Deprecated +public class GrpcServerParser extends GrpcParser implements RpcRequestParser { + @Override public void parse(RpcRequest request, TraceContext context, SpanCustomizer span) { + if (request instanceof GrpcServerRequest) { + GrpcServerRequest grpcRequest = (GrpcServerRequest) request; + onStart(grpcRequest.call, grpcRequest.headers, span); + } else { + assert false : "expected a GrpcServerRequest: " + request; + } + } + /** Override the customize the span based on the start of a request. */ protected void onStart(ServerCall call, Metadata headers, SpanCustomizer span) { diff --git a/instrumentation/grpc/src/main/java/brave/grpc/GrpcServerRequest.java b/instrumentation/grpc/src/main/java/brave/grpc/GrpcServerRequest.java index 42fc011ffa..8aef475064 100644 --- a/instrumentation/grpc/src/main/java/brave/grpc/GrpcServerRequest.java +++ b/instrumentation/grpc/src/main/java/brave/grpc/GrpcServerRequest.java @@ -13,26 +13,22 @@ */ package brave.grpc; -import brave.propagation.Propagation.Getter; import brave.rpc.RpcServerRequest; import io.grpc.Metadata; import io.grpc.Metadata.Key; +import io.grpc.MethodDescriptor; import io.grpc.ServerCall; import io.grpc.ServerInterceptor; import java.util.Map; -// intentionally not yet public until we add tag parsing functionality -final class GrpcServerRequest extends RpcServerRequest { - static final Getter GETTER = new Getter() { - @Override public String get(GrpcServerRequest request, String key) { - return request.propagationField(key); - } - - @Override public String toString() { - return "GrpcServerRequest::propagationField"; - } - }; - +/** + * Allows access gRPC specific aspects of a server request during sampling and parsing. + * + * @see GrpcServerResponse + * @see GrpcRequest for a parsing example + * @since 5.12 + */ +public class GrpcServerRequest extends RpcServerRequest implements GrpcRequest { final Map> nameToKey; final ServerCall call; final Metadata headers; @@ -70,16 +66,25 @@ final class GrpcServerRequest extends RpcServerRequest { return call; } + /** + * Returns {@linkplain ServerCall#getMethodDescriptor()}} from the {@link #call()}. + * + * @since 5.12 + */ + @Override public MethodDescriptor methodDescriptor() { + return call.getMethodDescriptor(); + } + /** * Returns the {@linkplain Metadata headers} passed to {@link ServerInterceptor#interceptCall}. * * @since 5.12 */ - public Metadata headers() { + @Override public Metadata headers() { return headers; } - String propagationField(String keyName) { + @Override protected String propagationField(String keyName) { if (keyName == null) throw new NullPointerException("keyName == null"); Key key = nameToKey.get(keyName); if (key == null) { diff --git a/instrumentation/grpc/src/main/java/brave/grpc/GrpcServerResponse.java b/instrumentation/grpc/src/main/java/brave/grpc/GrpcServerResponse.java index 7e8551909d..9d83e8fce0 100644 --- a/instrumentation/grpc/src/main/java/brave/grpc/GrpcServerResponse.java +++ b/instrumentation/grpc/src/main/java/brave/grpc/GrpcServerResponse.java @@ -13,72 +13,84 @@ */ package brave.grpc; -import brave.Response; -import brave.Span; import brave.internal.Nullable; +import brave.rpc.RpcServerResponse; import io.grpc.Metadata; import io.grpc.ServerCall; import io.grpc.Status; -// intentionally not yet public until we add tag parsing functionality -final class GrpcServerResponse extends Response { +/** + * Allows access gRPC specific aspects of a server response for parsing. + * + * @see GrpcServerRequest + * @see GrpcResponse for a parsing example + * @since 5.12 + */ +public final class GrpcServerResponse extends RpcServerResponse implements GrpcResponse { final GrpcServerRequest request; - @Nullable final Status status; - @Nullable final Metadata trailers; - @Nullable final Throwable error; + final Metadata headers; + final Status status; + final Metadata trailers; - GrpcServerResponse(GrpcServerRequest request, - @Nullable Status status, @Nullable Metadata trailers, @Nullable Throwable error) { + GrpcServerResponse(GrpcServerRequest request, Metadata headers, Status status, + Metadata trailers) { if (request == null) throw new NullPointerException("request == null"); + if (headers == null) throw new NullPointerException("headers == null"); + if (status == null) throw new NullPointerException("status == null"); + if (trailers == null) throw new NullPointerException("trailers == null"); + this.headers = headers; this.request = request; this.status = status; this.trailers = trailers; - this.error = error != null ? error : status != null ? status.getCause() : null; } /** Returns the {@link #status()} */ - @Override @Nullable public Status unwrap() { + @Override public Status unwrap() { return status; } - @Override public Span.Kind spanKind() { - return Span.Kind.SERVER; - } - @Override public GrpcServerRequest request() { return request; } + /** Returns {@link Status#getCause()} */ @Override @Nullable public Throwable error() { - return error; + return status.getCause(); } /** * Returns the string form of the {@link Status#getCode()} or {@code null} when not {@link * Status#isOk()} or {@link #error()}. */ - @Nullable public String errorCode() { - if (status == null || status.isOk()) return null; + @Override @Nullable public String errorCode() { + if (status.isOk()) return null; return status.getCode().name(); } /** - * Returns the status passed to{@link ServerCall#close(Status, Metadata)} or {@code null} on - * {@link #error()}. + * Returns a copy of headers passed to {@link ServerCall#sendHeaders(Metadata)}. + * + * @since 5.12 + */ + @Override public Metadata headers() { + return headers; + } + + /** + * Returns the status passed to {@link ServerCall#close(Status, Metadata)}. * * @since 5.12 */ - @Nullable public Status status() { + @Override public Status status() { return status; } /** - * Returns the trailers passed to {@link ServerCall#close(Status, Metadata)} or {@code null} on - * {@link #error()}. + * Returns the trailers passed to {@link ServerCall#close(Status, Metadata)}. * * @since 5.12 */ - @Nullable public Metadata trailers() { + @Override public Metadata trailers() { return trailers; } } diff --git a/instrumentation/grpc/src/main/java/brave/grpc/GrpcTracing.java b/instrumentation/grpc/src/main/java/brave/grpc/GrpcTracing.java index aecee09a72..41a3100e7e 100644 --- a/instrumentation/grpc/src/main/java/brave/grpc/GrpcTracing.java +++ b/instrumentation/grpc/src/main/java/brave/grpc/GrpcTracing.java @@ -15,12 +15,16 @@ import brave.Tracing; import brave.propagation.Propagation; +import brave.rpc.RpcRequestParser; +import brave.rpc.RpcResponseParser; import brave.rpc.RpcTracing; import io.grpc.ClientInterceptor; import io.grpc.Metadata; import io.grpc.ServerInterceptor; import java.util.Map; +import static brave.grpc.GrpcParser.LEGACY_RESPONSE_PARSER; + public final class GrpcTracing { public static GrpcTracing create(Tracing tracing) { return newBuilder(tracing).build(); @@ -31,7 +35,10 @@ public static GrpcTracing create(RpcTracing rpcTracing) { } public static Builder newBuilder(Tracing tracing) { - return newBuilder(RpcTracing.create(tracing)); + return newBuilder(RpcTracing.newBuilder(tracing) + .clientResponseParser(LEGACY_RESPONSE_PARSER) + .serverResponseParser(LEGACY_RESPONSE_PARSER) + .build()); } public static Builder newBuilder(RpcTracing rpcTracing) { @@ -39,9 +46,7 @@ public static Builder newBuilder(RpcTracing rpcTracing) { } public static final class Builder { - final RpcTracing rpcTracing; - GrpcClientParser clientParser = new GrpcClientParser(); - GrpcServerParser serverParser = new GrpcServerParser(); + RpcTracing rpcTracing; boolean grpcPropagationFormatEnabled = false; // for interop with old parsers @@ -55,23 +60,39 @@ public static final class Builder { Builder(GrpcTracing grpcTracing) { rpcTracing = grpcTracing.rpcTracing; - clientParser = grpcTracing.clientParser; - serverParser = grpcTracing.serverParser; clientMessageProcessor = grpcTracing.clientMessageProcessor; serverMessageProcessor = grpcTracing.serverMessageProcessor; } + /** + * @see GrpcClientRequest + * @see GrpcClientResponse + * @deprecated Since 5.12 use {@link RpcTracing.Builder#clientRequestParser(RpcRequestParser)} + * or {@link RpcTracing.Builder#clientResponseParser(RpcResponseParser)}. + */ + @Deprecated public Builder clientParser(GrpcClientParser clientParser) { if (clientParser == null) throw new NullPointerException("clientParser == null"); - this.clientParser = clientParser; - this.clientMessageProcessor = clientParser; + clientMessageProcessor = clientParser; + rpcTracing = rpcTracing.toBuilder() + .clientRequestParser(clientParser) + .clientResponseParser(clientParser).build(); return this; } + /** + * @see GrpcServerRequest + * @see GrpcServerResponse + * @deprecated Since 5.12 use {@link RpcTracing.Builder#serverRequestParser(RpcRequestParser)} + * or {@link RpcTracing.Builder#serverResponseParser(RpcResponseParser)}. + */ + @Deprecated public Builder serverParser(GrpcServerParser serverParser) { if (serverParser == null) throw new NullPointerException("serverParser == null"); - this.serverParser = serverParser; - this.serverMessageProcessor = serverParser; + serverMessageProcessor = serverParser; + rpcTracing = rpcTracing.toBuilder() + .serverRequestParser(serverParser) + .serverResponseParser(serverParser).build(); return this; } @@ -101,8 +122,6 @@ public GrpcTracing build() { final RpcTracing rpcTracing; final Propagation propagation; - final GrpcClientParser clientParser; - final GrpcServerParser serverParser; final Map> nameToKey; final boolean grpcPropagationFormatEnabled; @@ -118,8 +137,6 @@ public GrpcTracing build() { propagation = rpcTracing.tracing().propagation(); } nameToKey = GrpcPropagation.nameToKey(propagation); - clientParser = builder.clientParser; - serverParser = builder.serverParser; clientMessageProcessor = builder.clientMessageProcessor; serverMessageProcessor = builder.serverMessageProcessor; } diff --git a/instrumentation/grpc/src/main/java/brave/grpc/TracingClientInterceptor.java b/instrumentation/grpc/src/main/java/brave/grpc/TracingClientInterceptor.java index db4eb92eff..81fbb897df 100644 --- a/instrumentation/grpc/src/main/java/brave/grpc/TracingClientInterceptor.java +++ b/instrumentation/grpc/src/main/java/brave/grpc/TracingClientInterceptor.java @@ -16,14 +16,11 @@ import brave.NoopSpanCustomizer; import brave.Span; import brave.SpanCustomizer; -import brave.Tracer; import brave.internal.Nullable; import brave.propagation.CurrentTraceContext; import brave.propagation.CurrentTraceContext.Scope; import brave.propagation.TraceContext; -import brave.propagation.TraceContext.Injector; -import brave.rpc.RpcRequest; -import brave.sampler.SamplerFunction; +import brave.rpc.RpcClientHandler; import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ClientCall; @@ -38,25 +35,20 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicReference; -import static brave.grpc.GrpcClientRequest.SETTER; +import static brave.internal.Throwables.propagateIfFatal; // not exposed directly as implementation notably changes between versions 1.2 and 1.3 final class TracingClientInterceptor implements ClientInterceptor { final Map> nameToKey; final CurrentTraceContext currentTraceContext; - final Tracer tracer; - final SamplerFunction sampler; - final Injector injector; - final GrpcClientParser parser; + final RpcClientHandler handler; + final MessageProcessor messageProcessor; TracingClientInterceptor(GrpcTracing grpcTracing) { nameToKey = grpcTracing.nameToKey; currentTraceContext = grpcTracing.rpcTracing.tracing().currentTraceContext(); - tracer = grpcTracing.rpcTracing.tracing().tracer(); - sampler = grpcTracing.rpcTracing.clientSampler(); - injector = grpcTracing.propagation.injector(SETTER); - parser = grpcTracing.clientParser; + handler = RpcClientHandler.create(grpcTracing.rpcTracing); messageProcessor = grpcTracing.clientMessageProcessor; } @@ -67,20 +59,6 @@ public ClientCall interceptCall(MethodDescriptor extends SimpleForwardingClientCall { final MethodDescriptor method; final CallOptions callOptions; @@ -99,12 +77,7 @@ final class TracingClientCall extends SimpleForwardingClientCall( @@ -117,9 +90,14 @@ final class TracingClientCall extends SimpleForwardingClientCall extends SimpleForwardingClientCall extends SimpleForwardingClientCallL @Nullable final TraceContext invocationContext; final AtomicReference spanRef; final GrpcClientRequest request; + final Metadata headers = new Metadata(); TracingClientCallListener( Listener delegate, @@ -192,6 +176,8 @@ final class TracingClientCallListener extends SimpleForwardingClientCallL // See instrumentation/RATIONALE.md for why the below response callbacks are invocation context @Override public void onHeaders(Metadata headers) { + // onHeaders() JavaDoc mentions headers are not thread-safe, so we make a safe copy here. + this.headers.merge(headers); try (Scope scope = currentTraceContext.maybeScope(invocationContext)) { delegate().onHeaders(headers); } @@ -208,8 +194,9 @@ final class TracingClientCallListener extends SimpleForwardingClientCallL @Override public void onClose(Status status, Metadata trailers) { // See /instrumentation/grpc/RATIONALE.md for why we don't catch exceptions from the delegate - GrpcClientResponse response = new GrpcClientResponse(request, status, trailers, null); - finish(response, spanRef.getAndSet(null)); + GrpcClientResponse response = new GrpcClientResponse(request, headers, status, trailers); + Span span = spanRef.getAndSet(null); + if (span != null) handler.handleReceive(response, span); try (Scope scope = currentTraceContext.maybeScope(invocationContext)) { delegate().onClose(status, trailers); diff --git a/instrumentation/grpc/src/main/java/brave/grpc/TracingServerInterceptor.java b/instrumentation/grpc/src/main/java/brave/grpc/TracingServerInterceptor.java index c709958684..5f57b10167 100644 --- a/instrumentation/grpc/src/main/java/brave/grpc/TracingServerInterceptor.java +++ b/instrumentation/grpc/src/main/java/brave/grpc/TracingServerInterceptor.java @@ -16,15 +16,10 @@ import brave.NoopSpanCustomizer; import brave.Span; import brave.SpanCustomizer; -import brave.Tracer; -import brave.internal.Nullable; import brave.propagation.CurrentTraceContext; import brave.propagation.CurrentTraceContext.Scope; import brave.propagation.TraceContext; -import brave.propagation.TraceContext.Extractor; -import brave.propagation.TraceContextOrSamplingFlags; -import brave.rpc.RpcRequest; -import brave.sampler.SamplerFunction; +import brave.rpc.RpcServerHandler; import io.grpc.ForwardingServerCall.SimpleForwardingServerCall; import io.grpc.ForwardingServerCallListener.SimpleForwardingServerCallListener; import io.grpc.Metadata; @@ -37,26 +32,18 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicReference; -import static brave.grpc.GrpcServerRequest.GETTER; - // not exposed directly as implementation notably changes between versions 1.2 and 1.3 final class TracingServerInterceptor implements ServerInterceptor { final Map> nameToKey; final CurrentTraceContext currentTraceContext; - final Tracer tracer; - final Extractor extractor; - final SamplerFunction sampler; - final GrpcServerParser parser; + final RpcServerHandler handler; final boolean grpcPropagationFormatEnabled; final MessageProcessor messageProcessor; TracingServerInterceptor(GrpcTracing grpcTracing) { nameToKey = grpcTracing.nameToKey; currentTraceContext = grpcTracing.rpcTracing.tracing().currentTraceContext(); - tracer = grpcTracing.rpcTracing.tracing().tracer(); - extractor = grpcTracing.propagation.extractor(GETTER); - sampler = grpcTracing.rpcTracing.serverSampler(); - parser = grpcTracing.serverParser; + handler = RpcServerHandler.create(grpcTracing.rpcTracing); grpcPropagationFormatEnabled = grpcTracing.grpcPropagationFormatEnabled; messageProcessor = grpcTracing.serverMessageProcessor; } @@ -66,11 +53,7 @@ public Listener interceptCall(ServerCall call, Metadata headers, ServerCallHandler next) { GrpcServerRequest request = new GrpcServerRequest(nameToKey, call, headers); - Span span = nextSpan(extractor.extract(request), request); - if (!span.isNoop()) { - span.kind(Span.Kind.SERVER).start(); - parser.onStart(call, headers, span.customizer()); - } + Span span = handler.handleReceive(request); AtomicReference spanRef = new AtomicReference<>(span); // startCall invokes user interceptors, so we place the span in scope here @@ -80,45 +63,21 @@ public Listener interceptCall(ServerCall call, } catch (Throwable e) { // Another interceptor may throw an exception during startCall, in which case no other // callbacks are called, so go ahead and close the span here. - finishWithError(spanRef.getAndSet(null), e); + // + // See instrumentation/grpc/RATIONALE.md for why we don't use the handler here + spanRef.set(null); + if (span != null) span.error(e).finish(); throw e; } return new TracingServerCallListener<>(result, span, spanRef, request); } - /** Creates a potentially noop span representing this request */ - // This is the same code as HttpServerHandler.nextSpan - // TODO: pull this into RpcServerHandler when stable https://github.com/openzipkin/brave/pull/999 - Span nextSpan(TraceContextOrSamplingFlags extracted, GrpcServerRequest request) { - Boolean sampled = extracted.sampled(); - // only recreate the context if the sampler made a decision - if (sampled == null && (sampled = sampler.trySample(request)) != null) { - extracted = extracted.sampled(sampled.booleanValue()); - } - return extracted.context() != null - ? tracer.joinSpan(extracted.context()) - : tracer.nextSpan(extracted); - } - - void finish(GrpcServerResponse response, @Nullable Span span) { - if (span == null || span.isNoop()) return; - Throwable error = response.error(); - if (error != null) span.error(error); - parser.onClose(response.status, response.trailers, span.customizer()); - span.finish(); - } - - void finishWithError(@Nullable Span span, Throwable error) { - if (span == null || span.isNoop()) return; - if (error != null) span.error(error); - span.finish(); - } - final class TracingServerCall extends SimpleForwardingServerCall { final TraceContext context; final AtomicReference spanRef; final GrpcServerRequest request; + final Metadata headers = new Metadata(); TracingServerCall(ServerCall delegate, Span span, AtomicReference spanRef, GrpcServerRequest request) { @@ -138,6 +97,8 @@ final class TracingServerCall extends SimpleForwardingServerCall extends SimpleForwardingServerCall extends SimpleForwardingServerCallL } catch (Throwable e) { // If there was an exception executing onHalfClose, we don't expect other lifecycle // commands to succeed. Accordingly, we close the span - finishWithError(spanRef.getAndSet(null), e); + // + // See instrumentation/grpc/RATIONALE.md for why we don't use the handler here + Span span = spanRef.getAndSet(null); + if (span != null) span.error(e).finish(); + throw e; } } diff --git a/instrumentation/grpc/src/test/java/brave/grpc/BaseITTracingClientInterceptor.java b/instrumentation/grpc/src/test/java/brave/grpc/BaseITTracingClientInterceptor.java index cf5e173d56..e522cacbf1 100644 --- a/instrumentation/grpc/src/test/java/brave/grpc/BaseITTracingClientInterceptor.java +++ b/instrumentation/grpc/src/test/java/brave/grpc/BaseITTracingClientInterceptor.java @@ -17,9 +17,12 @@ import brave.CurrentSpanCustomizer; import brave.ScopedSpan; import brave.SpanCustomizer; +import brave.Tag; import brave.propagation.CurrentTraceContext.Scope; import brave.propagation.SamplingFlags; import brave.propagation.TraceContext; +import brave.rpc.RpcRequestParser; +import brave.rpc.RpcResponseParser; import brave.rpc.RpcRuleSampler; import brave.rpc.RpcTracing; import brave.test.ITRemote; @@ -40,6 +43,7 @@ import io.grpc.examples.helloworld.GreeterGrpc; import io.grpc.examples.helloworld.HelloReply; import io.grpc.examples.helloworld.HelloRequest; +import io.grpc.internal.GrpcUtil; import io.grpc.stub.StreamObserver; import java.io.IOException; import java.util.Iterator; @@ -82,7 +86,7 @@ ManagedChannel newClient() { ManagedChannel newClient(ClientInterceptor... clientInterceptors) { return usePlainText(ManagedChannelBuilder.forAddress("localhost", server.port()) - .intercept(clientInterceptors)).build(); + .intercept(clientInterceptors)).build(); } /** Extracted as {@link ManagedChannelBuilder#usePlaintext()} is a version-specific signature */ @@ -188,29 +192,6 @@ ManagedChannel newClient(ClientInterceptor... clientInterceptors) { } } - @Test public void customSampler() { - closeClient(client); - - RpcTracing rpcTracing = RpcTracing.newBuilder(tracing).clientSampler(RpcRuleSampler.newBuilder() - .putRule(methodEquals("SayHelloWithManyReplies"), NEVER_SAMPLE) - .putRule(serviceEquals("helloworld.greeter"), ALWAYS_SAMPLE) - .build()).build(); - - grpcTracing = GrpcTracing.create(rpcTracing); - client = newClient(); - - // unsampled - // NOTE: An iterator request is lazy: invoking the iterator invokes the request - GreeterGrpc.newBlockingStub(client).sayHelloWithManyReplies(HELLO_REQUEST).hasNext(); - - // sampled - GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST); - - assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).name()) - .isEqualTo("helloworld.greeter/sayhello"); - // @After will also check that sayHelloWithManyReplies was not sampled - } - @Test public void reportsClientKindToZipkin() { GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST); @@ -221,14 +202,14 @@ ManagedChannel newClient(ClientInterceptor... clientInterceptors) { GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST); assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).name()) - .isEqualTo("helloworld.greeter/sayhello"); + .isEqualTo("helloworld.greeter/sayhello"); } @Test public void onTransportException_addsErrorTag() { server.stop(); assertThatThrownBy(() -> GraterGrpc.newBlockingStub(client).seyHallo(HELLO_REQUEST)) - .isInstanceOf(StatusRuntimeException.class); + .isInstanceOf(StatusRuntimeException.class); // The error format of the exception message can differ from the span's "error" tag in CI Span span = reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, ".*Connection refused.*"); @@ -237,7 +218,7 @@ ManagedChannel newClient(ClientInterceptor... clientInterceptors) { @Test public void addsErrorTag_onUnimplemented() { assertThatThrownBy(() -> GraterGrpc.newBlockingStub(client).seyHallo(HELLO_REQUEST)) - .isInstanceOf(StatusRuntimeException.class); + .isInstanceOf(StatusRuntimeException.class); Span span = reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, "UNIMPLEMENTED"); assertThat(span.tags().get("grpc.status_code")).isEqualTo("UNIMPLEMENTED"); @@ -262,31 +243,31 @@ ManagedChannel newClient(ClientInterceptor... clientInterceptors) { closeClient(client); client = newClient( - new ClientInterceptor() { - @Override public ClientCall interceptCall( - MethodDescriptor method, CallOptions callOptions, Channel next) { - return new SimpleForwardingClientCall(next.newCall(method, callOptions)) { - @Override - public void start(Listener responseListener, Metadata headers) { - tracing.tracer().currentSpanCustomizer().annotate("start"); - super.start(responseListener, headers); - } - - @Override public void sendMessage(ReqT message) { - tracing.tracer().currentSpanCustomizer().annotate("sendMessage"); - super.sendMessage(message); - } - }; - } - }, - grpcTracing.newClientInterceptor() + new ClientInterceptor() { + @Override public ClientCall interceptCall( + MethodDescriptor method, CallOptions callOptions, Channel next) { + return new SimpleForwardingClientCall(next.newCall(method, callOptions)) { + @Override + public void start(Listener responseListener, Metadata headers) { + tracing.tracer().currentSpanCustomizer().annotate("start"); + super.start(responseListener, headers); + } + + @Override public void sendMessage(ReqT message) { + tracing.tracer().currentSpanCustomizer().annotate("sendMessage"); + super.sendMessage(message); + } + }; + } + }, + grpcTracing.newClientInterceptor() ); GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST); assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).annotations()) - .extracting(Annotation::value) - .containsOnly("start", "sendMessage"); + .extracting(Annotation::value) + .containsOnly("start", "sendMessage"); } @Test public void clientParserTest() { @@ -323,8 +304,8 @@ protected String spanName(MethodDescriptor methodDesc Span span = reporter.takeRemoteSpan(Span.Kind.CLIENT); assertThat(span.name()).isEqualTo("unary"); assertThat(span.tags()).containsKeys( - "grpc.message_received", "grpc.message_sent", - "grpc.message_received.visible", "grpc.message_sent.visible" + "grpc.message_received", "grpc.message_sent", + "grpc.message_received.visible", "grpc.message_sent.visible" ); reporter.takeLocalSpan(); } @@ -341,7 +322,7 @@ protected String spanName(MethodDescriptor methodDesc client = newClient(); Iterator replies = GreeterGrpc.newBlockingStub(client) - .sayHelloWithManyReplies(HelloRequest.newBuilder().setName("this is dog").build()); + .sayHelloWithManyReplies(HelloRequest.newBuilder().setName("this is dog").build()); assertThat(replies).toIterable().hasSize(10); // all response messages are tagged to the same span @@ -354,7 +335,8 @@ protected String spanName(MethodDescriptor methodDesc closeClient(client); client = newClient(new ClientInterceptor() { @Override public ClientCall interceptCall( - MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { + MethodDescriptor methodDescriptor, CallOptions callOptions, + Channel channel) { ClientCall call = channel.newCall(methodDescriptor, callOptions); return new SimpleForwardingClientCall(call) { @Override public void start(Listener responseListener, Metadata headers) { @@ -365,7 +347,7 @@ protected String spanName(MethodDescriptor methodDesc }, grpcTracing.newClientInterceptor()); assertThatThrownBy(() -> GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST)) - .isInstanceOf(IllegalStateException.class); + .isInstanceOf(IllegalStateException.class); reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, "I'm a bad interceptor."); } @@ -373,7 +355,8 @@ protected String spanName(MethodDescriptor methodDesc closeClient(client); client = newClient(new ClientInterceptor() { @Override public ClientCall interceptCall( - MethodDescriptor methodDescriptor, CallOptions callOptions, Channel channel) { + MethodDescriptor methodDescriptor, CallOptions callOptions, + Channel channel) { ClientCall call = channel.newCall(methodDescriptor, callOptions); return new SimpleForwardingClientCall(call) { @Override public void halfClose() { @@ -384,7 +367,7 @@ protected String spanName(MethodDescriptor methodDesc }, grpcTracing.newClientInterceptor()); assertThatThrownBy(() -> GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST)) - .isInstanceOf(IllegalStateException.class); + .isInstanceOf(IllegalStateException.class); reporter.takeRemoteSpanWithError(Span.Kind.CLIENT, "I'm a bad interceptor."); } @@ -403,11 +386,11 @@ protected String spanName(MethodDescriptor methodDesc } assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).tags()) - .containsKey("grpc.message_send.1"); + .containsKey("grpc.message_send.1"); // Response processing happens on the invocation (parent) trace context assertThat(reporter.takeLocalSpan().tags()) - .containsKey("grpc.message_recv.1"); + .containsKey("grpc.message_recv.1"); } @Test public void messageTagging_streaming() { @@ -416,28 +399,28 @@ protected String spanName(MethodDescriptor methodDesc ScopedSpan span = tracing.tracer().startScopedSpan("parent"); try { Iterator replies = GreeterGrpc.newBlockingStub(client) - .sayHelloWithManyReplies(HELLO_REQUEST); + .sayHelloWithManyReplies(HELLO_REQUEST); assertThat(replies).toIterable().hasSize(10); } finally { span.finish(); } assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).tags()) - .containsKey("grpc.message_send.1"); + .containsKey("grpc.message_send.1"); // Response processing happens on the invocation (parent) trace context // Intentionally verbose here to show 10 replies assertThat(reporter.takeLocalSpan().tags()).containsKeys( - "grpc.message_recv.1", - "grpc.message_recv.2", - "grpc.message_recv.3", - "grpc.message_recv.4", - "grpc.message_recv.5", - "grpc.message_recv.6", - "grpc.message_recv.7", - "grpc.message_recv.8", - "grpc.message_recv.9", - "grpc.message_recv.10" + "grpc.message_recv.1", + "grpc.message_recv.2", + "grpc.message_recv.3", + "grpc.message_recv.4", + "grpc.message_recv.5", + "grpc.message_recv.6", + "grpc.message_recv.7", + "grpc.message_recv.8", + "grpc.message_recv.9", + "grpc.message_recv.10" ); } @@ -448,27 +431,27 @@ void initMessageTaggingClient() { closeClient(client); client = newClient( - new ClientInterceptor() { - @Override public ClientCall interceptCall( - MethodDescriptor method, CallOptions callOptions, Channel next) { - return new SimpleForwardingClientCall(next.newCall(method, callOptions)) { - @Override public void start(Listener responseListener, Metadata headers) { - super.start(new SimpleForwardingClientCallListener(responseListener) { - @Override public void onMessage(RespT message) { - customizer.tag("grpc.message_recv." + recvs.getAndIncrement(), - message.toString()); - delegate().onMessage(message); - } - }, headers); - } - - @Override public void sendMessage(ReqT message) { - customizer.tag("grpc.message_send." + sends.getAndIncrement(), message.toString()); - delegate().sendMessage(message); - } - }; - } - }, grpcTracing.newClientInterceptor()); + new ClientInterceptor() { + @Override public ClientCall interceptCall( + MethodDescriptor method, CallOptions callOptions, Channel next) { + return new SimpleForwardingClientCall(next.newCall(method, callOptions)) { + @Override public void start(Listener responseListener, Metadata headers) { + super.start(new SimpleForwardingClientCallListener(responseListener) { + @Override public void onMessage(RespT message) { + customizer.tag("grpc.message_recv." + recvs.getAndIncrement(), + message.toString()); + delegate().onMessage(message); + } + }, headers); + } + + @Override public void sendMessage(ReqT message) { + customizer.tag("grpc.message_send." + sends.getAndIncrement(), message.toString()); + delegate().sendMessage(message); + } + }; + } + }, grpcTracing.newClientInterceptor()); } /** @@ -509,6 +492,65 @@ void initMessageTaggingClient() { assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).parentId()).isNull(); } + /* RpcTracing-specific feature tests */ + + @Test public void customSampler() { + closeClient(client); + + RpcTracing rpcTracing = RpcTracing.newBuilder(tracing).clientSampler(RpcRuleSampler.newBuilder() + .putRule(methodEquals("SayHelloWithManyReplies"), NEVER_SAMPLE) + .putRule(serviceEquals("helloworld.greeter"), ALWAYS_SAMPLE) + .build()).build(); + + grpcTracing = GrpcTracing.create(rpcTracing); + client = newClient(); + + // unsampled + // NOTE: An iterator request is lazy: invoking the iterator invokes the request + GreeterGrpc.newBlockingStub(client).sayHelloWithManyReplies(HELLO_REQUEST).hasNext(); + + // sampled + GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST); + + assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).name()) + .isEqualTo("helloworld.greeter/sayhello"); + // @After will also check that sayHelloWithManyReplies was not sampled + } + + @Test public void customParser() { + closeClient(client); + + Tag methodType = new Tag("grpc.method_type") { + @Override protected String parseValue(GrpcRequest input, TraceContext context) { + return input.methodDescriptor().getType().name(); + } + }; + + Tag responseEncoding = new Tag("grpc.response_encoding") { + @Override protected String parseValue(GrpcResponse input, TraceContext context) { + return input.headers().get(GrpcUtil.MESSAGE_ENCODING_KEY); + } + }; + + grpcTracing = GrpcTracing.create(RpcTracing.newBuilder(tracing) + .clientRequestParser((req, context, span) -> { + RpcRequestParser.DEFAULT.parse(req, context, span); + if (req instanceof GrpcRequest) methodType.tag((GrpcRequest) req, span); + }) + .clientResponseParser((res, context, span) -> { + RpcResponseParser.DEFAULT.parse(res, context, span); + if (res instanceof GrpcResponse) responseEncoding.tag((GrpcResponse) res, span); + }).build()); + + client = newClient(); + + GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST); + + assertThat(reporter.takeRemoteSpan(Span.Kind.CLIENT).tags()) + .containsEntry("grpc.method_type", "UNARY") + .containsEntry("grpc.response_encoding", "identity"); + } + static final class StreamObserverAdapter implements StreamObserver { final AssertableCallback callback; diff --git a/instrumentation/grpc/src/test/java/brave/grpc/BaseITTracingServerInterceptor.java b/instrumentation/grpc/src/test/java/brave/grpc/BaseITTracingServerInterceptor.java index b9255d26f3..1145831932 100644 --- a/instrumentation/grpc/src/test/java/brave/grpc/BaseITTracingServerInterceptor.java +++ b/instrumentation/grpc/src/test/java/brave/grpc/BaseITTracingServerInterceptor.java @@ -15,10 +15,13 @@ import brave.CurrentSpanCustomizer; import brave.SpanCustomizer; +import brave.Tag; import brave.internal.Nullable; import brave.propagation.B3SingleFormat; import brave.propagation.SamplingFlags; import brave.propagation.TraceContext; +import brave.rpc.RpcRequestParser; +import brave.rpc.RpcResponseParser; import brave.rpc.RpcRuleSampler; import brave.rpc.RpcTracing; import brave.test.ITRemote; @@ -46,6 +49,7 @@ import io.grpc.examples.helloworld.GreeterGrpc; import io.grpc.examples.helloworld.HelloReply; import io.grpc.examples.helloworld.HelloRequest; +import io.grpc.internal.GrpcUtil; import java.io.IOException; import java.util.Iterator; import java.util.concurrent.TimeUnit; @@ -84,15 +88,15 @@ void init(@Nullable ServerInterceptor userInterceptor) throws IOException { // tracing interceptor needs to go last ServerInterceptor tracingInterceptor = grpcTracing.newServerInterceptor(); ServerInterceptor[] interceptors = userInterceptor != null - ? new ServerInterceptor[] {userInterceptor, tracingInterceptor} - : new ServerInterceptor[] {tracingInterceptor}; + ? new ServerInterceptor[] {userInterceptor, tracingInterceptor} + : new ServerInterceptor[] {tracingInterceptor}; server = ServerBuilder.forPort(PickUnusedPort.get()) - .addService(ServerInterceptors.intercept(new GreeterImpl(grpcTracing), interceptors)) - .build().start(); + .addService(ServerInterceptors.intercept(new GreeterImpl(grpcTracing), interceptors)) + .build().start(); client = usePlainText(ManagedChannelBuilder.forAddress("localhost", server.getPort())) - .build(); + .build(); } /** Extracted as {@link ManagedChannelBuilder#usePlaintext()} is a version-specific signature */ @@ -154,7 +158,7 @@ void init(@Nullable ServerInterceptor userInterceptor) throws IOException { init(new ServerInterceptor() { @Override public ServerCall.Listener interceptCall(ServerCall call, - Metadata headers, ServerCallHandler next) { + Metadata headers, ServerCallHandler next) { fromUserInterceptor.set(tracing.currentTraceContext().get()); return next.startCall(call, headers); } @@ -163,7 +167,7 @@ public ServerCall.Listener interceptCall(ServerCall ServerCall.Listener interceptCall(ServerCall GreeterGrpc.newBlockingStub(client) - .sayHello(HelloRequest.newBuilder().setName("bad").build())); + .sayHello(HelloRequest.newBuilder().setName("bad").build())); Span span = reporter.takeRemoteSpanWithError(Span.Kind.SERVER, "IllegalArgumentException"); assertThat(span.tags()).containsEntry("grpc.status_code", "UNKNOWN"); @@ -192,8 +196,8 @@ public ServerCall.Listener interceptCall(ServerCall GreeterGrpc.newBlockingStub(client) - .sayHello(HelloRequest.newBuilder().setName("testerror").build())) - .isInstanceOf(StatusRuntimeException.class); + .sayHello(HelloRequest.newBuilder().setName("testerror").build())) + .isInstanceOf(StatusRuntimeException.class); Span span = reporter.takeRemoteSpanWithError(Span.Kind.SERVER, "testerror"); assertThat(span.tags().get("grpc.status_code")).isNull(); @@ -228,8 +232,8 @@ protected String spanName(MethodDescriptor methodDesc Span span = reporter.takeRemoteSpan(Span.Kind.SERVER); assertThat(span.name()).isEqualTo("unary"); assertThat(span.tags().keySet()).containsExactlyInAnyOrder( - "grpc.message_received", "grpc.message_sent", - "grpc.message_received.visible", "grpc.message_sent.visible" + "grpc.message_received", "grpc.message_sent", + "grpc.message_received.visible", "grpc.message_sent.visible" ); } @@ -244,52 +248,31 @@ protected String spanName(MethodDescriptor methodDesc init(); Iterator replies = GreeterGrpc.newBlockingStub(client) - .sayHelloWithManyReplies(HELLO_REQUEST); + .sayHelloWithManyReplies(HELLO_REQUEST); assertThat(replies).toIterable().hasSize(10); // all response messages are tagged to the same span assertThat(reporter.takeRemoteSpan(Span.Kind.SERVER).tags()).hasSize(10); } - @Test public void customSampler() throws IOException { - RpcTracing rpcTracing = RpcTracing.newBuilder(tracing).serverSampler(RpcRuleSampler.newBuilder() - .putRule(methodEquals("SayHelloWithManyReplies"), NEVER_SAMPLE) - .putRule(serviceEquals("helloworld.greeter"), ALWAYS_SAMPLE) - .build()).build(); - grpcTracing = GrpcTracing.create(rpcTracing); - init(); - - // unsampled - // NOTE: An iterator request is lazy: invoking the iterator invokes the request - GreeterGrpc.newBlockingStub(client).sayHelloWithManyReplies(HELLO_REQUEST).hasNext(); - - // sampled - GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST); - - assertThat(reporter.takeRemoteSpan(Span.Kind.SERVER).name()) - .isEqualTo("helloworld.greeter/sayhello"); - - // @After will also check that sayHelloWithManyReplies was not sampled - } - // Make sure we work well with bad user interceptors. - @Test public void userInterceptor_ThrowsOnStartCall() throws IOException { + @Test public void userInterceptor_throwsOnStartCall() throws IOException { init(new ServerInterceptor() { @Override public ServerCall.Listener interceptCall( - ServerCall call, Metadata metadata, ServerCallHandler next) { + ServerCall call, Metadata metadata, ServerCallHandler next) { throw new IllegalStateException("I'm a bad interceptor."); } }); assertThatThrownBy(() -> GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST)) - .isInstanceOf(StatusRuntimeException.class); + .isInstanceOf(StatusRuntimeException.class); reporter.takeRemoteSpanWithError(Span.Kind.SERVER, "I'm a bad interceptor."); } @Test public void userInterceptor_throwsOnSendMessage() throws IOException { init(new ServerInterceptor() { @Override public ServerCall.Listener interceptCall( - ServerCall call, Metadata metadata, ServerCallHandler next) { + ServerCall call, Metadata metadata, ServerCallHandler next) { return next.startCall(new SimpleForwardingServerCall(call) { @Override public void sendMessage(RespT message) { throw new IllegalStateException("I'm a bad interceptor."); @@ -299,14 +282,14 @@ protected String spanName(MethodDescriptor methodDesc }); assertThatThrownBy(() -> GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST)) - .isInstanceOf(StatusRuntimeException.class); + .isInstanceOf(StatusRuntimeException.class); reporter.takeRemoteSpanWithError(Span.Kind.SERVER, "I'm a bad interceptor."); } @Test public void userInterceptor_throwsOnClose() throws IOException { init(new ServerInterceptor() { @Override public ServerCall.Listener interceptCall( - ServerCall call, Metadata metadata, ServerCallHandler next) { + ServerCall call, Metadata metadata, ServerCallHandler next) { return next.startCall(new SimpleForwardingServerCall(call) { @Override public void close(Status status, Metadata trailers) { throw new IllegalStateException("I'm a bad interceptor."); @@ -316,14 +299,14 @@ protected String spanName(MethodDescriptor methodDesc }); assertThatThrownBy(() -> GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST)) - .isInstanceOf(StatusRuntimeException.class); + .isInstanceOf(StatusRuntimeException.class); reporter.takeRemoteSpanWithError(Span.Kind.SERVER, "I'm a bad interceptor."); } @Test public void userInterceptor_throwsOnOnHalfClose() throws IOException { init(new ServerInterceptor() { @Override public ServerCall.Listener interceptCall( - ServerCall call, Metadata metadata, ServerCallHandler next) { + ServerCall call, Metadata metadata, ServerCallHandler next) { return new SimpleForwardingServerCallListener(next.startCall(call, metadata)) { @Override public void onHalfClose() { throw new IllegalStateException("I'm a bad interceptor."); @@ -333,7 +316,7 @@ protected String spanName(MethodDescriptor methodDesc }); assertThatThrownBy(() -> GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST)) - .isInstanceOf(StatusRuntimeException.class); + .isInstanceOf(StatusRuntimeException.class); reporter.takeRemoteSpanWithError(Span.Kind.SERVER, "I'm a bad interceptor."); } @@ -349,7 +332,7 @@ protected String spanName(MethodDescriptor methodDesc init(new ServerInterceptor() { @Override public ServerCall.Listener interceptCall( - ServerCall call, Metadata headers, ServerCallHandler next) { + ServerCall call, Metadata headers, ServerCallHandler next) { call = new SimpleForwardingServerCall(call) { @Override public void sendMessage(RespT message) { delegate().sendMessage(message); @@ -368,37 +351,91 @@ public ServerCall.Listener interceptCall( GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST); assertThat(reporter.takeRemoteSpan(Span.Kind.SERVER).tags()).containsKeys( - "grpc.message_recv.0", "grpc.message_send.0" + "grpc.message_recv.0", "grpc.message_send.0" ); Iterator replies = GreeterGrpc.newBlockingStub(client) - .sayHelloWithManyReplies(HELLO_REQUEST); + .sayHelloWithManyReplies(HELLO_REQUEST); assertThat(replies).toIterable().hasSize(10); // Intentionally verbose here to show that only one recv and 10 replies assertThat(reporter.takeRemoteSpan(Span.Kind.SERVER).tags()).containsKeys( - "grpc.message_recv.1", - "grpc.message_send.1", - "grpc.message_send.2", - "grpc.message_send.3", - "grpc.message_send.4", - "grpc.message_send.5", - "grpc.message_send.6", - "grpc.message_send.7", - "grpc.message_send.8", - "grpc.message_send.9", - "grpc.message_send.10" + "grpc.message_recv.1", + "grpc.message_send.1", + "grpc.message_send.2", + "grpc.message_send.3", + "grpc.message_send.4", + "grpc.message_send.5", + "grpc.message_send.6", + "grpc.message_send.7", + "grpc.message_send.8", + "grpc.message_send.9", + "grpc.message_send.10" ); } + /* RpcTracing-specific feature tests */ + + @Test public void customSampler() throws IOException { + RpcTracing rpcTracing = RpcTracing.newBuilder(tracing).serverSampler(RpcRuleSampler.newBuilder() + .putRule(methodEquals("SayHelloWithManyReplies"), NEVER_SAMPLE) + .putRule(serviceEquals("helloworld.greeter"), ALWAYS_SAMPLE) + .build()).build(); + grpcTracing = GrpcTracing.create(rpcTracing); + init(); + + // unsampled + // NOTE: An iterator request is lazy: invoking the iterator invokes the request + GreeterGrpc.newBlockingStub(client).sayHelloWithManyReplies(HELLO_REQUEST).hasNext(); + + // sampled + GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST); + + assertThat(reporter.takeRemoteSpan(Span.Kind.SERVER).name()) + .isEqualTo("helloworld.greeter/sayhello"); + + // @After will also check that sayHelloWithManyReplies was not sampled + } + + @Test public void customParser() throws IOException { + Tag methodType = new Tag("grpc.method_type") { + @Override protected String parseValue(GrpcRequest input, TraceContext context) { + return input.methodDescriptor().getType().name(); + } + }; + + Tag responseEncoding = new Tag("grpc.response_encoding") { + @Override protected String parseValue(GrpcResponse input, TraceContext context) { + return input.headers().get(GrpcUtil.MESSAGE_ENCODING_KEY); + } + }; + + grpcTracing = GrpcTracing.create(RpcTracing.newBuilder(tracing) + .serverRequestParser((req, context, span) -> { + RpcRequestParser.DEFAULT.parse(req, context, span); + if (req instanceof GrpcRequest) methodType.tag((GrpcRequest) req, span); + }) + .serverResponseParser((res, context, span) -> { + RpcResponseParser.DEFAULT.parse(res, context, span); + if (res instanceof GrpcResponse) responseEncoding.tag((GrpcResponse) res, span); + }).build()); + init(); + + GreeterGrpc.newBlockingStub(client).sayHello(HELLO_REQUEST); + + assertThat(reporter.takeRemoteSpan(Span.Kind.SERVER).tags()) + .containsEntry("grpc.method_type", "UNARY") + .containsEntry("grpc.response_encoding", "identity"); + } + Channel clientWithB3SingleHeader(TraceContext parent) { return ClientInterceptors.intercept(client, new ClientInterceptor() { @Override public ClientCall interceptCall( - MethodDescriptor method, CallOptions callOptions, Channel next) { + MethodDescriptor method, CallOptions callOptions, Channel next) { return new SimpleForwardingClientCall(next.newCall(method, callOptions)) { @Override public void start(Listener responseListener, Metadata headers) { headers.put(Key.of("b3", ASCII_STRING_MARSHALLER), - B3SingleFormat.writeB3SingleFormat(parent)); + B3SingleFormat.writeB3SingleFormat(parent)); super.start(responseListener, headers); } }; diff --git a/instrumentation/grpc/src/test/java/brave/grpc/GrpcClientResponseTest.java b/instrumentation/grpc/src/test/java/brave/grpc/GrpcClientResponseTest.java index 44b3bb6a08..f32e89c125 100644 --- a/instrumentation/grpc/src/test/java/brave/grpc/GrpcClientResponseTest.java +++ b/instrumentation/grpc/src/test/java/brave/grpc/GrpcClientResponseTest.java @@ -30,18 +30,21 @@ public class GrpcClientResponseTest { MethodDescriptor methodDescriptor = TestObjects.METHOD_DESCRIPTOR; CallOptions callOptions = CallOptions.DEFAULT; ClientCall call = mock(ClientCall.class); - Metadata headers = new Metadata(); + Metadata headers = new Metadata(), trailers = new Metadata(); GrpcClientRequest request = - new GrpcClientRequest(singletonMap("b3", b3Key), methodDescriptor, callOptions, call, headers); + new GrpcClientRequest(singletonMap("b3", b3Key), methodDescriptor, callOptions, call, + headers); Status status = Status.CANCELLED; - Metadata trailers = new Metadata(); - Throwable error; - GrpcClientResponse response = new GrpcClientResponse(request, status, trailers, error); + GrpcClientResponse response = new GrpcClientResponse(request, headers, status, trailers); @Test public void request() { assertThat(response.request()).isSameAs(request); } + @Test public void headers() { + assertThat(response.headers()).isSameAs(headers); + } + @Test public void status() { assertThat(response.status()).isSameAs(status); } @@ -58,17 +61,10 @@ public class GrpcClientResponseTest { assertThat(response.error()).isNull(); } - @Test public void error() { - RuntimeException error = new RuntimeException("noodles"); - GrpcClientResponse response = new GrpcClientResponse(request, status, trailers, error); - - assertThat(response.error()).isSameAs(error); - } - @Test public void error_fromStatus() { RuntimeException error = new RuntimeException("noodles"); status = Status.fromThrowable(error); - GrpcClientResponse response = new GrpcClientResponse(request, status, trailers, null); + GrpcClientResponse response = new GrpcClientResponse(request, headers, status, trailers); assertThat(response.error()).isSameAs(error); assertThat(response.errorCode()).isEqualTo("UNKNOWN"); @@ -76,7 +72,7 @@ public class GrpcClientResponseTest { @Test public void errorCode_nullWhenOk() { status = Status.OK; - GrpcClientResponse response = new GrpcClientResponse(request, status, trailers, null); + GrpcClientResponse response = new GrpcClientResponse(request, headers, status, trailers); assertThat(response.errorCode()).isNull(); } diff --git a/instrumentation/grpc/src/test/java/brave/grpc/GrpcServerResponseTest.java b/instrumentation/grpc/src/test/java/brave/grpc/GrpcServerResponseTest.java index 5e92990f0f..2631684ba5 100644 --- a/instrumentation/grpc/src/test/java/brave/grpc/GrpcServerResponseTest.java +++ b/instrumentation/grpc/src/test/java/brave/grpc/GrpcServerResponseTest.java @@ -26,17 +26,20 @@ public class GrpcServerResponseTest { Key b3Key = Key.of("b3", Metadata.ASCII_STRING_MARSHALLER); ServerCall call = mock(ServerCall.class); - Metadata headers = new Metadata(); + Metadata headers = new Metadata(), trailers = new Metadata(); GrpcServerRequest request = - new GrpcServerRequest(singletonMap("b3", b3Key), call, headers); + new GrpcServerRequest(singletonMap("b3", b3Key), call, headers); Status status = Status.CANCELLED; - Metadata trailers = new Metadata(); - GrpcServerResponse response = new GrpcServerResponse(request, status, trailers, null); + GrpcServerResponse response = new GrpcServerResponse(request, headers, status, trailers); @Test public void request() { assertThat(response.request()).isSameAs(request); } + @Test public void headers() { + assertThat(response.headers()).isSameAs(headers); + } + @Test public void status() { assertThat(response.status()).isSameAs(status); } @@ -53,17 +56,10 @@ public class GrpcServerResponseTest { assertThat(response.error()).isNull(); } - @Test public void error() { - RuntimeException error = new RuntimeException("noodles"); - GrpcServerResponse response = new GrpcServerResponse(request, status, trailers, error); - - assertThat(response.error()).isSameAs(error); - } - @Test public void error_fromStatus() { RuntimeException error = new RuntimeException("noodles"); status = Status.fromThrowable(error); - GrpcServerResponse response = new GrpcServerResponse(request, status, trailers, null); + GrpcServerResponse response = new GrpcServerResponse(request, headers, status, trailers); assertThat(response.error()).isSameAs(error); assertThat(response.errorCode()).isEqualTo("UNKNOWN"); @@ -71,7 +67,7 @@ public class GrpcServerResponseTest { @Test public void errorCode_nullWhenOk() { status = Status.OK; - GrpcServerResponse response = new GrpcServerResponse(request, status, trailers, null); + GrpcServerResponse response = new GrpcServerResponse(request, headers, status, trailers); assertThat(response.errorCode()).isNull(); } diff --git a/instrumentation/grpc/src/test/java/brave/grpc/TestServer.java b/instrumentation/grpc/src/test/java/brave/grpc/TestServer.java index 4751fe0a5b..664ddce827 100644 --- a/instrumentation/grpc/src/test/java/brave/grpc/TestServer.java +++ b/instrumentation/grpc/src/test/java/brave/grpc/TestServer.java @@ -16,11 +16,13 @@ import brave.propagation.Propagation; import brave.propagation.TraceContext.Extractor; import brave.propagation.TraceContextOrSamplingFlags; +import io.grpc.ForwardingServerCall.SimpleForwardingServerCall; import io.grpc.Metadata; import io.grpc.Metadata.Key; import io.grpc.Server; import io.grpc.ServerBuilder; import io.grpc.ServerCall; +import io.grpc.ServerCall.Listener; import io.grpc.ServerCallHandler; import io.grpc.ServerInterceptor; import io.grpc.ServerInterceptors; @@ -31,33 +33,40 @@ import java.util.concurrent.TimeUnit; class TestServer { + static final Key CUSTOM_KEY = Key.of("custom", Metadata.ASCII_STRING_MARSHALLER); final BlockingQueue delayQueue = new LinkedBlockingQueue<>(); - final BlockingQueue requestQueue = new LinkedBlockingQueue<>(); + final BlockingQueue requests = new LinkedBlockingQueue<>(); final Extractor extractor; final Server server; TestServer(Map> nameToKey, Propagation propagation) { extractor = propagation.extractor(GrpcServerRequest::propagationField); server = ServerBuilder.forPort(PickUnusedPort.get()) - .addService(ServerInterceptors.intercept(new GreeterImpl(null), new ServerInterceptor() { - - @Override - public ServerCall.Listener interceptCall(ServerCall call, - Metadata headers, ServerCallHandler next) { - Long delay = delayQueue.poll(); - if (delay != null) { - try { - Thread.sleep(delay); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new AssertionError("interrupted sleeping " + delay); - } - } - requestQueue.add(extractor.extract(new GrpcServerRequest(nameToKey, call, headers))); - return next.startCall(call, headers); - } - })) - .build(); + .addService(ServerInterceptors.intercept( + new GreeterImpl(null), + new ServerInterceptor() { + @Override + public Listener interceptCall(ServerCall call, + Metadata headers, ServerCallHandler next) { + Long delay = delayQueue.poll(); + if (delay != null) { + try { + Thread.sleep(delay); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new AssertionError("interrupted sleeping " + delay); + } + } + requests.add(extractor.extract(new GrpcServerRequest(nameToKey, call, headers))); + return next.startCall(new SimpleForwardingServerCall(call) { + @Override public void sendHeaders(Metadata headers) { + headers.put(CUSTOM_KEY, "brave"); + super.sendHeaders(headers); + } + }, headers); + } + })) + .build(); } void start() throws IOException { @@ -80,7 +89,7 @@ int port() { TraceContextOrSamplingFlags takeRequest() { try { - return requestQueue.poll(3, TimeUnit.SECONDS); + return requests.poll(3, TimeUnit.SECONDS); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new AssertionError(e); diff --git a/instrumentation/http/README.md b/instrumentation/http/README.md index 78584b859e..e58e1ff84f 100644 --- a/instrumentation/http/README.md +++ b/instrumentation/http/README.md @@ -168,7 +168,7 @@ You generally need to... 1. Start the span and add trace headers to the request 2. Put the span in scope so things like log integration works 3. Invoke the request -4. Catch any errors +4. If there was a Throwable, add it to the span 5. Complete the span ```java @@ -238,7 +238,7 @@ You generally need to... 1. Extract any trace IDs from headers and start the span 2. Put the span in scope so things like log integration works 3. Process the request -4. Catch any errors +4. If there was a Throwable, add it to the span 5. Complete the span ```java diff --git a/instrumentation/http/src/test/java/brave/http/HttpClientRequestSetterTest.java b/instrumentation/http/src/test/java/brave/http/HttpClientRequestSetterTest.java index 617e60d06b..725bf23b45 100644 --- a/instrumentation/http/src/test/java/brave/http/HttpClientRequestSetterTest.java +++ b/instrumentation/http/src/test/java/brave/http/HttpClientRequestSetterTest.java @@ -13,7 +13,7 @@ */ package brave.http; -import brave.propagation.Propagation; +import brave.propagation.Propagation.Setter; import brave.test.propagation.PropagationSetterTest; import java.util.Collections; import java.util.LinkedHashMap; @@ -52,8 +52,7 @@ public class HttpClientRequestSetterTest extends PropagationSetterTest setter() { + @Override protected Setter setter() { return SETTER; } diff --git a/instrumentation/rpc/RATIONALE.md b/instrumentation/rpc/RATIONALE.md index 6631f6a00e..33fcd298db 100644 --- a/instrumentation/rpc/RATIONALE.md +++ b/instrumentation/rpc/RATIONALE.md @@ -1,5 +1,38 @@ # brave-instrumentation-rpc rationale +## Why doesn't HTTP layer over RPC? +RPC is its own abstraction, that HTTP has RPC-like characteristics is +incidental to our model. Moreover, HTTP is a specified (by httpbis) abstraction +where RPC is largely conventional. To the degree frameworks such as Apache +Dubbo, Thrift and Avro are similar is not due to a specification, in other +words. + +The OpenTelemetry group tried to layer HTTP over the gRPC model, and it didn't +work out very well, as you can see in their [status mapping](https://github.com/open-telemetry/opentelemetry-specification/blob/bfb060b23113ba9af492f8c63dd89ecfc500810b/specification/trace/semantic_conventions/http.md#status). + +For example, one of the most common HTTP status, 400, was not even mappable to +gRPC. OpenTelemetry mapped HTTP redirect and not available codes to +`Deadline exceeded`. It is a good example of confusion which we can learn from, +as confusing users is a non-goal of telemetry. + +Instead, we keep the more defined HTTP in its own abstraction, which allows it +to be congruent with existing practices such as alerting on HTTP status. + +## Why don't we model Messaging and Storage as RPC? +We believe that even if underlying drivers like Redis have RPC like +characteristics, this is not the best abstraction to optimize for when modeling +service abstraction communication. + +In other words, a tool like Redis is more effectively understood as a storage +service, vs modeling it as an RPC framework, like Apache Thrift or Avro. + +Conversely, we believe that RPC services, especially where users define the IDL +have business relevance. The message/method/function names are more likely to +reflect business processes. This relevance helps users navigate impact. If we +also put MySQL driver communication in the same abstraction, it would at best +create more data to filter out and at worst, drown out the ability for users to +find their business functions. + ## `RpcRequest` overview The `RpcRequest` type is a model adapter, used initially for sampling, but in @@ -8,15 +41,14 @@ portably create a span name without special knowledge of the RPC library. This is repeating the work we did in HTTP, which allows allows library agnostic data and sampling policy. -## `RpcRequest` initially with `method()` and `service()` properties +## Why start with `method()` and `service()` properties? -### Why in general? In order to lower initial effort, we decided to scope the initial implementation of RPC to just things needed by samplers, injection and extraction. Among these were the desire to create a sampling rule to match requests to an "auth service" or a "play" method. -## Examples please? +### Examples please? `rpc.method` - The unqualified, case-sensitive method name. Prefer the name defined in IDL to any mapped Java method name. @@ -51,7 +83,7 @@ there are established practice of `method` and `service` in tools such as While there could be confusion around `rpc.method` vs Java method, that exists anyway and is easy to explain: When IDL exists, prefer its service and method -name to any matched Java names. +name to any matched Java names. `rpc.service` unfortunately conflicts with our term `Endpoint.serviceName`, but only if you ignore the `Endpoint` prefix. Given this, we feel it fits in with @@ -87,3 +119,89 @@ parameterized sampling. This happens before a span name is chosen. Moreover, a future change will allow parsing into a span name from the same type. In other words, `RpcRequest` is an intermediate model that must evaluate properties before a span exists. + +## Why does `RpcHandler.handleFinish` have no error parameter? +In practice, "rpc.error_code" sometimes ends up in an exception, and parsers +usually don't look at exception subtypes. For example, Dubbo's `RpcException` +includes the error code. + +Since `Response.error()` exists anyway, it is better to be consistent than have +those writing parsers have to pin to framework specific code in order to know +an error code. + +## The "rpc.error_code" tag +RPC frameworks are not consistent in response status. There's often no success +code, and in minimal cases only an error bit. As one-way RPC implies no success +response will return, representing success is not portable. + +Even in the case of errors, we could choose between a code and a message. For +example, we have people tracing SOAP services "faultcode" and "faultstring" are +both strings. + +An error code is more likely to be fixed cardinality as messages often include +variables. In other words, we choose error code as it is more supported, and it +supports search and aggregation. + +### Why do we tag "rpc.error_code" -> "" as opposed to true? +The edge case of only knowing an RPC error exists (bit) is handled the same as +the "error" tag: set "rpc.error_code" to empty string (""). This allows search +and aggregation on the tag key "rpc.error_code" to operate, and without leading +users towards accidentally tagging "rpc.error_code" = "false". + +See [the core rationale](../brave/RATIONALE.md) for more about the empty string +case. + +### Why do we prefer "rpc.error_code" as a word and not a number? +In the case of errors, code is usually a string not a number. Even when it is a +number, you can often find that number is a string ordinal, as opposed to a +meaningful number like HTTP status codes. + +HTTP status codes are grouped by classification, and have existed so long that +support teams can usually identify meaning by looking at a number like 401. +Being triple digits, it is relatively easy to search for what an HTTP status +means. + +RPC error code numbers are usually like enum ordinals. In other words, the +number has no significance (no grouping), which makes this not ideal for +troubleshooting. + +Let's take the example of Dubbo's error code number 2 vs its name +"TIMEOUT_EXCEPTION". There is no generic documentation on Dubbo errors. If +given only the number 2, a user unfamiliar with how Dubbo works internally will +have a hard time. For example, searching Dubbo's code base for "2" will return +less relevant results than searching for "TIMEOUT_EXCEPTION". More importantly, +if we return "TIMEOUT_EXCEPTION", it is possible a user will not even have to +look to see if there is documentation or not! + +For all these reasons, we prefer code names, not numbers. + +### Why not "rpc.error_message"? +A long form error message is unlikely to be portable due to lack of frameworks +defining more than even an error bit in their RPC message types. + +When they exist, error messages can have nice attributes. For example, they can +include variables and can be localized. However, it is these same traits that +make them less useful in search and aggregation. + +Moreover, long form messages usually materialize as exception messages, which +are handled directly by `MutableSpan.error()`. For example, SOAP has a +"faultcode" and "faultstring". The "faultcode" becomes the "rpc.error_code". +The "faultstring" may still be added to the span if it is exception message. + +Regardless, if there is a framework that has an error message that isn't in the +exception, they can tag it directly with `RpcResponseParser`. + +### Why not re-use OpenTelemetry "status" +OpenTelemetry chose to re-use gRPC status codes as a generic "status" type. +https://github.com/open-telemetry/opentelemetry-java/blob/c24e4a963669cfd044d7478a801e8f170faba4fe/api/src/main/java/io/opentelemetry/trace/Status.java#L38 + +In practice this is not portable for a number of reasons. + +While errors like as "method doesn't exist on the other side" are possible generically. There's no +current art to suggest errors can be identified portably across RPC frameworks. In fact, even +success is not identifiable portably, notably due to "one-way" RPC, used widely including in Apache +Thrift, Avro and Dubbo frameworks. Even errors are usually not defined more than boolean `true`. + +One-way RPC is typically implemented similar to messaging. Whether the response was even accepted or +not is unknowable. Hence, you cannot set "OK" status and expect it to be correct, much less map to +response status which you'll never receive. diff --git a/instrumentation/rpc/README.md b/instrumentation/rpc/README.md index 7d3e11910e..d8a66440f0 100644 --- a/instrumentation/rpc/README.md +++ b/instrumentation/rpc/README.md @@ -1,32 +1,70 @@ # brave-instrumentation-rpc -This is a helper for RPC libraries such as gRPC and Dubbo. Specifically, this -includes samplers for clients and servers, configured with `RpcTracing`. +Most instrumentation are based on RPC communication. For this reason, +we have specialized handlers for RPC clients and servers. All of these +are configured with `RpcTracing`. -The `RpcTracing` class holds a reference to a tracing component and -sampling policy. +The `RpcTracing` class holds a reference to a tracing component, +instructions on what to put into RPC spans, and sampling policy. + +## Span data policy +By default, the following are added to both RPC client and server spans: +* Span.name is the RPC service/method. Ex. "zipkin.proto3.SpanService/Report" + * If the service is absent, the method is the name and visa versa. +* Tags: + * "rpc.method", eg "Report" + * "rpc.service", eg "zipkin.proto3.SpanService" + * "rpc.error_code", eg "CANCELLED" + * "error" the RPC error code if there is no exception +* Remote IP and port information + +Naming and tags are configurable in a library-agnostic way. For example, +the same `RpcTracing` component configures gRPC or Dubbo identically. + +For example, to add a framework-specific tag for RPC clients, you can do this: + +```java +rpcTracing = rpcTracingBuilder + .clientRequestParser((req, context, span) -> { + RpcRequestParser.DEFAULT.parse(req, context, span); + if (req instanceof DubboRequest) { + tagArguments(((DubboRequest) req).invocation().getArguments()); + } + }).build(); + +// gRPC would silently ignore the DubboRequest parsing +grpc = GrpcTracing.create(rpcTracing); +dubbo = DubboTracing.create(rpcTracing); +``` + +*Note*: Data including the span name can be overwritten any time. For example, +if you don't know a good span name until the response, it is fine to replace it +then. ## Sampling Policy The default sampling policy is to use the default (trace ID) sampler for client and server requests. For example, if there's a incoming request that has no trace IDs in its -headers, the sampler indicated by `RpcTracing.Builder.serverSampler` -decides whether or not to start a new trace. Once a trace is in progress, it is -used for any outgoing messages (client requests). +headers, the sampler indicated by `Tracing.Builder.sampler` decides whether +or not to start a new trace. Once a trace is in progress, it is used for +any outgoing RPC client requests. -On the other hand, you may have outgoing requests didn't originate from a -server. For example, bootstrapping your application might call a discovery -service. In this case, the policy defined by `RpcTracing.Builder.clientSampler` -decides if a new trace will be started or not. +On the other hand, you may have RPC client requests that didn't originate +from a server. For example, you may be bootstrapping your application, +and that makes an RPC call to a system service. The default policy will +start a trace for any RPC call, even ones that didn't come from a server +request. + +This allows you to declare rules based on RPC patterns. These decide +which sample rate to apply. You can change the sampling policy by specifying it in the `RpcTracing` -component. The default implementation `RpcRuleSampler` allows you to -declare rules based on declare rules based on RPC properties and apply an -appropriate sampling rate. +component. The default implementation is `RpcRuleSampler`, which allows +you to declare rules based on RPC properties. Ex. Here's a sampler that traces 100 "Report" requests per second. This -doesn't start new traces for requests to the scribe service. Other +doesn't start new traces for requests to the "scribe" service. Other requests will use a global rate provided by the tracing component. ```java @@ -40,7 +78,123 @@ rpcTracingBuilder.serverSampler(RpcRuleSampler.newBuilder() # Developing new instrumentation -Check for [instrumentation written here](../) and [Zipkin's list](https://zipkin.io/pages/tracers_instrumentation.html) +Check for [instrumentation written here](../) and [Zipkin's list](https://zipkin.io/pages/existing_instrumentations.html) before rolling your own Rpc instrumentation! Besides documentation here, you should look at the [core library documentation](../../brave/README.md) as it covers topics including propagation. You may find our [feature tests](src/test/java/brave/rpc/features) helpful, too. + +## Rpc Client + +The first step in developing RPC client instrumentation is implementing +`RpcClientRequest` and `RpcClientResponse` for your native library. +This ensures users can portably control tags using `RpcClientParser`. + +Next, you'll need to indicate how to insert trace IDs into the outgoing +request. Often, this is as simple as `Request::setHeader`. + +With these two items, you now have the most important parts needed to +trace your server library. You'll likely initialize the following in a +constructor like so: +```java +MyTracingFilter(RpcTracing rpcTracing) { + tracer = rpcTracing.tracing().tracer(); + handler = RpcClientHandler.create(rpcTracing); +} +``` + +### Synchronous Interceptors + +Synchronous interception is the most straight forward instrumentation. +You generally need to... +1. Start the span and add trace headers to the request +2. Put the span in scope so things like log integration works +3. Invoke the request +4. If there was a Throwable, add it to the span +5. Complete the span + +```java +RpcClientRequestWrapper requestWrapper = new RpcClientRequestWrapper(request); +Span span = handler.handleSend(requestWrapper); // 1. +ClientResponse response = null; +Throwable error = null; +try (Scope ws = currentTraceContext.newScope(span.context())) { // 2. + return response = invoke(request); // 3. +} catch (Throwable e) { + error = e; // 4. + throw e; +} finally { + RpcClientResponseWrapper responseWrapper = + ? new RpcClientResponseWrapper(requestWrapper, response, error); + handler.handleReceive(responseWrapper, span); // 5. +} +``` + +### Asynchronous callbacks + +Asynchronous callbacks are a bit more complicated as they can happen on +different threads. This means you need to manually carry the trace context from +where the RPC call is scheduled until when the request actually starts. + +You generally need to... +1. Stash the invoking trace context as a property of the request +2. Retrieve that context when the request starts +3. Use that context when creating the client span + +```java +public void onSchedule(RpcContext context) { + TraceContext invocationContext = currentTraceContext().get(); + context.setAttribute(TraceContext.class, invocationContext); // 1. +} + +// use the invocation context in callback associated with starting the request +public void onStart(RpcContext context, RpcClientRequest req) { + TraceContext parent = context.getAttribute(TraceContext.class); // 2. + + RpcClientRequestWrapper request = new RpcClientRequestWrapper(req); + Span span = handler.handleSendWithParent(request, parent); // 3. +``` + +## Rpc Server + +The first step in developing RPC server instrumentation is implementing +`brave.RpcServerRequest` and `brave.RpcServerResponse` for your native +library. This ensures your instrumentation can extract headers, sample and +control tags. + +With these two implemented, you have the most important parts needed to trace +your server library. Initialize the RPC server handler that uses the request +and response types along with the tracer. + +```java +MyTracingInterceptor(RpcTracing rpcTracing) { + tracer = rpcTracing.tracing().tracer(); + handler = RpcServerHandler.create(rpcTracing); +} +``` + +### Synchronous Interceptors + +Synchronous interception is the most straight forward instrumentation. +You generally need to... +1. Extract any trace IDs from headers and start the span +2. Put the span in scope so things like log integration works +3. Process the request +4. If there was a Throwable, add it to the span +5. Complete the span + +```java +RpcServerRequestWrapper requestWrapper = new RpcServerRequestWrapper(request); +Span span = handler.handleReceive(requestWrapper); // 1. +ServerResponse response = null; +Throwable error = null; +try (Scope ws = currentTraceContext.newScope(span.context())) { // 2. + return response = process(request); // 3. +} catch (Throwable e) { + error = e; // 4. + throw e; +} finally { + RpcServerResponseWrapper responseWrapper = + new RpcServerResponseWrapper(requestWrapper, response, error); + handler.handleSend(responseWrapper, span); // 5. +} +``` diff --git a/instrumentation/rpc/pom.xml b/instrumentation/rpc/pom.xml index af4442f65b..3849f2ba85 100644 --- a/instrumentation/rpc/pom.xml +++ b/instrumentation/rpc/pom.xml @@ -14,7 +14,9 @@ the License. --> - + io.zipkin.brave brave-instrumentation-parent diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcClientHandler.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcClientHandler.java new file mode 100644 index 0000000000..2d1fa4c05f --- /dev/null +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcClientHandler.java @@ -0,0 +1,136 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.Span; +import brave.SpanCustomizer; +import brave.Tracer; +import brave.internal.Nullable; +import brave.propagation.TraceContext; +import brave.propagation.TraceContext.Injector; +import brave.sampler.SamplerFunction; + +/** + * This standardizes a way to instrument RPC clients, particularly in a way that encourages use of + * portable customizations via {@link RpcRequestParser} and {@link RpcResponseParser}. + * + *

Synchronous interception is the most straight forward instrumentation. + * + *

You generally need to: + *

    + *
  1. Start the span and add trace headers to the request
    2. + *
  2. Put the span in scope so things like log integration works
    3. + *
  3. Invoke the request
    4. + *
  4. If there was a Throwable, add it to the span
    5. + *
  5. Complete the span
    6. + *
+ * + * 
{@code
+ * RpcClientRequestWrapper requestWrapper = new RpcClientRequestWrapper(request);
+ * Span span = handler.handleSend(requestWrapper); // 1.
+ * ClientResponse response = null;
+ * Throwable error = null;
+ * try (Scope ws = currentTraceContext.newScope(span.context())) { // 2.
+ *   return response = invoke(request); // 3.
+ * } catch (Throwable e) {
+ *   error = e; // 4.
+ *   throw e;
+ * } finally {
+ *   RpcClientResponseWrapper responseWrapper =
+ *     new RpcClientResponseWrapper(requestWrapper, response, error);
+ *   handler.handleReceive(responseWrapper, span); // 5.
+ * }
+ * }
+ * + * @since 5.12 + */ +public final class RpcClientHandler extends RpcHandler { + /** @since 5.12 */ + public static RpcClientHandler create(RpcTracing rpcTracing) { + if (rpcTracing == null) throw new NullPointerException("rpcTracing == null"); + return new RpcClientHandler(rpcTracing); + } + + final Tracer tracer; + final SamplerFunction sampler; + final Injector injector; + + RpcClientHandler(RpcTracing rpcTracing) { + super(rpcTracing.clientRequestParser(), rpcTracing.clientResponseParser()); + this.tracer = rpcTracing.tracing().tracer(); + this.injector = rpcTracing.tracing.propagation().injector(RpcClientRequest.SETTER); + this.sampler = rpcTracing.clientSampler(); + } + + /** + * Starts the client span after assigning it a name and tags. This {@link + * Injector#inject(TraceContext, Object) injects} the trace context onto the request before + * returning. + * + *

Call this before sending the request on the wire. + * + * @see #handleSendWithParent(RpcClientRequest, TraceContext) + * @see RpcTracing#clientSampler() + * @see RpcTracing#clientRequestParser() + * @since 5.12 + */ + public Span handleSend(RpcClientRequest request) { + if (request == null) throw new NullPointerException("request == null"); + return handleSend(request, tracer.nextSpan(sampler, request)); + } + + /** + * Like {@link #handleSend(RpcClientRequest)}, except explicitly controls the parent of the client + * span. + * + * @param parent the parent of the client span representing this request, or null for a new + * trace. + * @see Tracer#nextSpanWithParent(SamplerFunction, Object, TraceContext) + * @since 5.12 + */ + public Span handleSendWithParent(RpcClientRequest request, @Nullable TraceContext parent) { + if (request == null) throw new NullPointerException("request == null"); + return handleSend(request, tracer.nextSpanWithParent(sampler, request, parent)); + } + + /** + * Like {@link #handleSend(RpcClientRequest)}, except explicitly controls the span representing + * the request. + * + * @since 5.12 + */ + public Span handleSend(RpcClientRequest request, Span span) { + if (request == null) throw new NullPointerException("request == null"); + if (span == null) throw new NullPointerException("span == null"); + injector.inject(span.context(), request); + return handleStart(request, span); + } + + /** + * Finishes the client span after assigning it tags according to the response or error. + * + *

This is typically called once the response headers are received, and after the span is + * {@link Tracer.SpanInScope#close() no longer in scope}. + * + *

Note: It is valid to have a {@link RpcClientResponse} that only includes an + * {@linkplain RpcClientResponse#error() error}. However, it is better to also include the + * {@linkplain RpcClientResponse#request() request}. + * + * @see RpcResponseParser#parse(RpcResponse, TraceContext, SpanCustomizer) + * @since 5.12 + */ + public void handleReceive(RpcClientResponse response, Span span) { + handleFinish(response, span); + } +} diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcClientRequest.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcClientRequest.java index 8e838c3270..782c1c329e 100644 --- a/instrumentation/rpc/src/main/java/brave/rpc/RpcClientRequest.java +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcClientRequest.java @@ -14,15 +14,52 @@ package brave.rpc; import brave.Span; +import brave.baggage.BaggagePropagation; +import brave.propagation.Propagation; +import brave.propagation.Propagation.Setter; +import brave.propagation.TraceContext; +import brave.propagation.TraceContext.Injector; /** - * Marks an interface for use in injection and {@link RpcRuleSampler}. This gives a standard type - * to consider when parsing an outgoing context. + * Marks an interface for use in {@link RpcClientHandler#handleSend(RpcClientRequest)}. This gives a + * standard type to consider when parsing an outgoing context. * + * @see RpcClientResponse * @since 5.8 */ public abstract class RpcClientRequest extends RpcRequest { + static final Setter SETTER = new Setter() { + @Override public void put(RpcClientRequest request, String key, String value) { + request.propagationField(key, value); + } + + @Override public String toString() { + return "RpcClientRequest::propagationField"; + } + }; + @Override public final Span.Kind spanKind() { return Span.Kind.CLIENT; } + + /** + * Sets a propagation field with the indicated name. {@code null} values are unsupported. + * + *

Notes

+ *

This is only used when {@link Injector#inject(TraceContext, Object) injecting} a trace + * context as internally implemented by {@link RpcClientHandler}. Calls during sampling or parsing + * are invalid and may be ignored by instrumentation. + * + *

Header based requests will use headers, but this could set RPC + * envelopes or even binary data. + * + * @param keyName key used for {@link Setter#put} + * @param value value used for {@link Setter#put} + * @see #SETTER + * @see Propagation#keys() + * @see BaggagePropagation#allKeyNames(Propagation) + * @since 5.12 + */ + protected void propagationField(String keyName, String value) { + } } diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcClientResponse.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcClientResponse.java new file mode 100644 index 0000000000..a249556786 --- /dev/null +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcClientResponse.java @@ -0,0 +1,44 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.Span; + +/** + * Marks an interface for use in {@link RpcClientHandler#handleReceive(RpcClientResponse, Span)}. + * This gives a standard type to consider when parsing an incoming context. + * + * @see RpcClientRequest + * @since 5.10 + */ +public abstract class RpcClientResponse extends RpcResponse { + @Override public final Span.Kind spanKind() { + return Span.Kind.CLIENT; + } + + /** + * Like {@link RpcRequest#parseRemoteIpAndPort(Span)} for when the client library cannot read + * socket information before a request is made. + * + *

To reduce overhead, only implement this when not implementing {@link + * RpcRequest#parseRemoteIpAndPort(Span)}. + * + * @since 5.10 + */ + // This is on the response object because clients often don't know their final IP until after the + // request started. + public boolean parseRemoteIpAndPort(Span span) { + return false; + } +} diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcHandler.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcHandler.java new file mode 100644 index 0000000000..08b046d8a7 --- /dev/null +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcHandler.java @@ -0,0 +1,82 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.Span; +import brave.internal.Platform; + +import static brave.internal.Throwables.propagateIfFatal; + +abstract class RpcHandler { + final RpcRequestParser requestParser; + final RpcResponseParser responseParser; + + RpcHandler(RpcRequestParser requestParser, RpcResponseParser responseParser) { + this.requestParser = requestParser; + this.responseParser = responseParser; + } + + Span handleStart(Req request, Span span) { + if (span.isNoop()) return span; + + try { + parseRequest(request, span); + } catch (Throwable t) { + propagateIfFatal(t); + Platform.get().log("error parsing request {0}", request, t); + } finally { + // all of the above parsing happened before a timestamp on the span + long timestamp = request.startTimestamp(); + if (timestamp == 0L) { + span.start(); + } else { + span.start(timestamp); + } + } + return span; + } + + void parseRequest(Req request, Span span) { + span.kind(request.spanKind()); + request.parseRemoteIpAndPort(span); + requestParser.parse(request, span.context(), span.customizer()); + } + + void parseResponse(Resp response, Span span) { + responseParser.parse(response, span.context(), span.customizer()); + } + + void handleFinish(Resp response, Span span) { + if (response == null) throw new NullPointerException("response == null"); + if (span.isNoop()) return; + + if (response.error() != null) { + span.error(response.error()); // Ensures MutableSpan.error() for FinishedSpanHandler + } + + try { + parseResponse(response, span); + } catch (Throwable t) { + propagateIfFatal(t); + Platform.get().log("error parsing response {0}", response, t); + } finally { + long finishTimestamp = response.finishTimestamp(); + if (finishTimestamp == 0L) { + span.finish(); + } else { + span.finish(finishTimestamp); + } + } + } +} diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcRequest.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcRequest.java index b0acc3e9d4..2131f3860e 100644 --- a/instrumentation/rpc/src/main/java/brave/rpc/RpcRequest.java +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcRequest.java @@ -13,13 +13,15 @@ */ package brave.rpc; +import brave.Clock; import brave.Request; import brave.Span; import brave.internal.Nullable; +import brave.propagation.TraceContext; import java.lang.reflect.Method; /** - * Abstract request type used for parsing and sampling of rpc clients and servers. + * Abstract request type used for parsing and sampling of RPC clients and servers. * * @see RpcClientRequest * @see RpcServerRequest @@ -41,6 +43,7 @@ public abstract class RpcRequest extends Request { * {@link Method#getName() Java method name}, or in a different case format. * * @return the RPC method name or null if unreadable. + * @since 5.8 */ @Nullable public abstract String method(); @@ -61,9 +64,42 @@ public abstract class RpcRequest extends Request { * name}. * * @return the RPC namespace or null if unreadable. + * @since 5.8 */ @Nullable public abstract String service(); + /** + * The timestamp in epoch microseconds of the beginning of this request or zero to take this + * implicitly from the current clock. Defaults to zero. + * + *

This is helpful in two scenarios: late parsing and avoiding redundant timestamp overhead. + * If a server span, this helps reach the "original" beginning of the request, which is always + * prior to parsing. + * + *

Note: Overriding has the same problems as using {@link brave.Span#start(long)}. For + * example, it can result in negative duration if the clock used is allowed to correct backwards. + * It can also result in misalignments in the trace, unless {@link brave.Tracing.Builder#clock(Clock)} + * uses the same implementation. + * + * @see RpcResponse#finishTimestamp() + * @see brave.Span#start(long) + * @see brave.Tracing#clock(TraceContext) + * @since 5.12 + */ + public long startTimestamp() { + return 0L; + } + + /** + * Override and return true when it is possible to parse the {@link Span#remoteIpAndPort(String, + * int) remote IP and port} from the {@link #unwrap() delegate}. Defaults to false. + * + * @since 5.12 + */ + public boolean parseRemoteIpAndPort(Span span) { + return false; + } + RpcRequest() { // sealed type: only client and server } } diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcRequestParser.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcRequestParser.java new file mode 100644 index 0000000000..554ab5decc --- /dev/null +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcRequestParser.java @@ -0,0 +1,69 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.SpanCustomizer; +import brave.propagation.TraceContext; + +/** + * Use this to control the request data recorded for an {@link TraceContext#sampledLocal() sampled + * RPC client or server span}. + * + *

Here's an example that adds default tags, and if Apache Dubbo, Java arguments: + * 

{@code
+ * rpcTracing = rpcTracingBuilder
+ *   .clientRequestParser((req, context, span) -> {
+ *      RpcRequestParser.DEFAULT.parse(req, context, span);
+ *      if (req instanceof DubboRequest) {
+ *        tagArguments(((DubboRequest) req).invocation().getArguments());
+ *      }
+ *   }).build();
+ * }
+ * + * @see RpcResponseParser + */ +// @FunctionalInterface: do not add methods as it will break api +public interface RpcRequestParser { + RpcRequestParser DEFAULT = new Default(); + + /** + * Implement to choose what data from the RPC request are parsed into the span representing it. + * + * @see Default + */ + void parse(RpcRequest request, TraceContext context, SpanCustomizer span); + + /** + * The default data policy sets the span name to {@code ${rpc.service}/${rpc.method}} or only the + * method or service. This also tags "rpc.service" and "rpc.method" when present. + */ + class Default implements RpcRequestParser { + @Override public void parse(RpcRequest req, TraceContext context, SpanCustomizer span) { + String service = req.service(); + String method = req.method(); + if (service == null && method == null) return; + if (service == null) { + span.tag(RpcTags.METHOD.key(), method); + span.name(method); + } else if (method == null) { + span.tag(RpcTags.SERVICE.key(), service); + span.name(service); + } else { + span.tag(RpcTags.SERVICE.key(), service); + span.tag(RpcTags.METHOD.key(), method); + span.name(service + "/" + method); + } + } + } +} diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcResponse.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcResponse.java new file mode 100644 index 0000000000..4f08b8432d --- /dev/null +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcResponse.java @@ -0,0 +1,81 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.Clock; +import brave.Response; +import brave.Span; +import brave.Tags; +import brave.internal.Nullable; +import brave.propagation.TraceContext; + +/** + * Abstract response type used for parsing and sampling of RPC clients and servers. + * + * @see RpcClientResponse + * @see RpcServerResponse + * @since 5.12 + */ +public abstract class RpcResponse extends Response { + /** + * The request that initiated this RPC response or {@code null} if unknown. + * + * @since 5.12 + */ + @Override @Nullable public RpcRequest request() { + return null; + } + + /** + * Returns the shortest human readable error code name. Ex. {code io.grpc.Status.Code.name()} may + * return "CANCELLED". + * + *

Conventionally, this is used as the {@link Tags#ERROR "error" tag} and optionally tagged + * separately as {@linkplain RpcTags#ERROR_CODE "rpc.error_code"}. + * + *

Notes

+ * This is not a success code. On success, return {@code null}. Do not return "OK" or similar as + * it will interfere with error interpretation. + * + *

When only a boolean value is available, this should return empty string ("") on {@code + * true} and {@code null} on false. + * + *

When an error code has both a numeric and text label, return the text. + * + * @since 5.12 + */ + @Nullable public abstract String errorCode(); + + /** + * The timestamp in epoch microseconds of the end of this request or zero to take this implicitly + * from the current clock. Defaults to zero. + * + *

This is helpful in two scenarios: late parsing and avoiding redundant timestamp overhead. + * For example, you can asynchronously handle span completion without losing precision of the + * actual end. + * + *

Note: Overriding has the same problems as using {@link Span#finish(long)}. For + * example, it can result in negative duration if the clock used is allowed to correct backwards. + * It can also result in misalignments in the trace, unless {@link brave.Tracing.Builder#clock(Clock)} + * uses the same implementation. + * + * @see RpcRequest#startTimestamp() + * @see brave.Span#finish(long) + * @see brave.Tracing#clock(TraceContext) + * @since 5.12 + */ + public long finishTimestamp() { + return 0L; + } +} diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcResponseParser.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcResponseParser.java new file mode 100644 index 0000000000..8dc249e788 --- /dev/null +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcResponseParser.java @@ -0,0 +1,83 @@ +/* + * Copyright 2013-2020 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.Span; +import brave.SpanCustomizer; +import brave.Tags; +import brave.Tracing; +import brave.handler.FinishedSpanHandler; +import brave.propagation.TraceContext; + +/** + * Use this to control the response data recorded for an {@link TraceContext#sampledLocal() sampled + * RPC client or server span}. + * + *

Here's an example that adds default tags, and if gRPC, the response encoding: + * 

{@code
+ * Tag responseEncoding = new Tag("grpc.response_encoding") {
+ *   @Override protected String parseValue(GrpcResponse input, TraceContext context) {
+ *     return input.headers().get(GrpcUtil.MESSAGE_ENCODING_KEY);
+ *   }
+ * };
+ *
+ * RpcResponseParser addResponseEncoding = (res, context, span) -> {
+ *   RpcResponseParser.DEFAULT.parse(res, context, span);
+ *   if (res instanceof GrpcResponse) responseEncoding.tag((GrpcResponse) res, span);
+ * };
+ *
+ * grpcTracing = GrpcTracing.create(RpcTracing.newBuilder(tracing)
+ *     .clientResponseParser(addResponseEncoding);
+ *     .serverResponseParser(addResponseEncoding).build());
+ * }
+ * + * @see RpcRequestParser + * @since 5.12 + */ +// @FunctionalInterface: do not add methods as it will break api +public interface RpcResponseParser { + RpcResponseParser DEFAULT = new Default(); + + /** + * Implement to choose what data from the RPC response are parsed into the span representing it. + * + *

Note: This is called after {@link Span#error(Throwable)}, which means any "error" tag set + * here will overwrite what the {@linkplain Tracing#errorParser() error parser} set. + * + * @see Default + * @since 5.12 + */ + void parse(RpcResponse response, TraceContext context, SpanCustomizer span); + + /** + * The default data policy sets {@link RpcTags#ERROR_CODE} tag, and also "error", if there is no + * {@linkplain RpcResponse#error() exception}. + * + *

Note:The exception will be tagged by default in Zipkin, but if you are using a + * {@link FinishedSpanHandler} to another destination, you should process accordingly. + * + * @since 5.12 + */ + class Default implements RpcResponseParser { + @Override public void parse(RpcResponse response, TraceContext context, SpanCustomizer span) { + String errorCode = response.errorCode(); + if (errorCode != null) { + span.tag(RpcTags.ERROR_CODE.key(), errorCode); + if (response.error() == null) { + span.tag(Tags.ERROR.key(), errorCode); + } + } + } + } +} diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcRuleSampler.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcRuleSampler.java index 36c4f830c2..4850e78e23 100644 --- a/instrumentation/rpc/src/main/java/brave/rpc/RpcRuleSampler.java +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcRuleSampler.java @@ -20,7 +20,7 @@ import brave.sampler.SamplerFunction; /** - * Assigns sample rates to rpc requests. + * Assigns sample rates to RPC requests. * *

Ex. Here's a sampler that traces 100 "Report" requests per second. This doesn't start new * traces for requests to the scribe service. Other requests will use a global rate provided by the diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcServerHandler.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcServerHandler.java new file mode 100644 index 0000000000..39dfba9d07 --- /dev/null +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcServerHandler.java @@ -0,0 +1,118 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.Span; +import brave.SpanCustomizer; +import brave.Tracer; +import brave.propagation.TraceContext; +import brave.propagation.TraceContext.Extractor; +import brave.propagation.TraceContextOrSamplingFlags; +import brave.sampler.SamplerFunction; + +/** + * This standardizes a way to instrument RPC servers, particularly in a way that encourages use of + * portable customizations via {@link RpcRequestParser} and {@link RpcResponseParser}. + * + *

Synchronous interception is the most straight forward instrumentation. + * + *

You generally need to: + *

    + *
  1. Extract any trace IDs from headers and start the span
    2. + *
  2. Put the span in scope so things like log integration works
    3. + *
  3. Process the request
    4. + *
  4. If there was a Throwable, add it to the span
    5. + *
  5. Complete the span
    6. + *
+ * 
{@code
+ * RpcServerRequestWrapper requestWrapper = new RpcServerRequestWrapper(request);
+ * Span span = handler.handleReceive(requestWrapper); // 1.
+ * ServerResponse response = null;
+ * Throwable error = null;
+ * try (Scope ws = currentTraceContext.newScope(span.context())) { // 2.
+ *   return response = process(request); // 3.
+ * } catch (Throwable e) {
+ *   error = e; // 4.
+ *   throw e;
+ * } finally {
+ *   RpcServerResponseWrapper responseWrapper =
+ *     new RpcServerResponseWrapper(requestWrapper, response, error);
+ *   handler.handleSend(responseWrapper, span); // 5.
+ * }
+ * }
+ * + * @since 5.12 + */ +public final class RpcServerHandler extends RpcHandler { + /** @since 5.12 */ + public static RpcServerHandler create(RpcTracing rpcTracing) { + if (rpcTracing == null) throw new NullPointerException("rpcTracing == null"); + return new RpcServerHandler(rpcTracing); + } + + final Tracer tracer; + final Extractor extractor; + final SamplerFunction sampler; + + RpcServerHandler(RpcTracing rpcTracing) { + super(rpcTracing.serverRequestParser(), rpcTracing.serverResponseParser()); + this.tracer = rpcTracing.tracing().tracer(); + this.extractor = rpcTracing.tracing().propagation().extractor(RpcServerRequest.GETTER); + this.sampler = rpcTracing.serverSampler(); + } + + /** + * Conditionally joins a span, or starts a new trace, depending on if a trace context was + * extracted from the request. Tags are added before the span is started. + * + *

This is typically called before the request is processed by the actual library. + * + * @see RpcTracing#serverSampler() + * @see RpcTracing#serverRequestParser() + * @since 5.12 + */ + public Span handleReceive(RpcServerRequest request) { + Span span = nextSpan(extractor.extract(request), request); + return handleStart(request, span); + } + + /** Creates a potentially noop span representing this request */ + Span nextSpan(TraceContextOrSamplingFlags extracted, RpcServerRequest request) { + Boolean sampled = extracted.sampled(); + // only recreate the context if the RPC sampler made a decision + if (sampled == null && (sampled = sampler.trySample(request)) != null) { + extracted = extracted.sampled(sampled.booleanValue()); + } + return extracted.context() != null + ? tracer.joinSpan(extracted.context()) + : tracer.nextSpan(extracted); + } + + /** + * Finishes the server span after assigning it tags according to the response or error. + * + *

This is typically called once the response headers are sent, and after the span is {@link + * brave.Tracer.SpanInScope#close() no longer in scope}. + * + *

Note: It is valid to have a {@link RpcServerResponse} that only includes an + * {@linkplain RpcServerResponse#error() error}. However, it is better to also include the + * {@linkplain RpcServerResponse#request() request}. + * + * @see RpcResponseParser#parse(RpcResponse, TraceContext, SpanCustomizer) + * @since 5.12 + */ + public void handleSend(RpcServerResponse response, Span span) { + handleFinish(response, span); + } +} diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcServerRequest.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcServerRequest.java index de8b77c8f5..7f2e70c3a4 100644 --- a/instrumentation/rpc/src/main/java/brave/rpc/RpcServerRequest.java +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcServerRequest.java @@ -14,15 +14,55 @@ package brave.rpc; import brave.Span; +import brave.baggage.BaggagePropagation; +import brave.internal.Nullable; +import brave.propagation.Propagation; +import brave.propagation.Propagation.Getter; +import brave.propagation.TraceContext; /** - * Marks an interface for use in extraction and {@link RpcRuleSampler}. This gives a standard type - * to consider when parsing an incoming context. + * Marks an interface for use in {@link RpcServerHandler#handleReceive(RpcServerRequest)}. This + * gives a standard type to consider when parsing an incoming context. * + * @see RpcServerResponse * @since 5.8 */ public abstract class RpcServerRequest extends RpcRequest { + static final Getter GETTER = new Getter() { + @Override public String get(RpcServerRequest request, String key) { + return request.propagationField(key); + } + + @Override public String toString() { + return "RpcServerRequest::propagationField"; + } + }; + @Override public final Span.Kind spanKind() { return Span.Kind.SERVER; } + + /** + * Returns one value corresponding to the specified {@link Getter#get propagation field}, or + * null. + * + *

Note: Header based requests will use headers, but this could be read from RPC + * envelopes or even binary data. + *

Notes

+ *

This is only used when {@link TraceContext.Injector#inject(TraceContext, Object) injecting} + * a trace context as internally implemented by {@link RpcClientHandler}. Calls during sampling or + * parsing are invalid and may be ignored by instrumentation. + * + *

Header based requests will use headers, but this could set RPC + * envelopes or even binary data. + * + * @param keyName key used for {@link Getter#get(Object, Object)} + * @see #GETTER + * @see Propagation#keys() + * @see BaggagePropagation#allKeyNames(Propagation) + * @since 5.12 + */ + @Nullable protected String propagationField(String keyName) { + return null; + } } diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcServerResponse.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcServerResponse.java new file mode 100644 index 0000000000..9ea85d138f --- /dev/null +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcServerResponse.java @@ -0,0 +1,29 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.Span; + +/** + * Marks an interface for use in {@link RpcServerHandler#handleSend(RpcServerResponse, Span)}. This + * gives a standard type to consider when parsing an incoming context. + * + * @see RpcClientRequest + * @since 5.10 + */ +public abstract class RpcServerResponse extends RpcResponse { + @Override public final Span.Kind spanKind() { + return Span.Kind.SERVER; + } +} diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcTags.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcTags.java new file mode 100644 index 0000000000..f5932fb65d --- /dev/null +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcTags.java @@ -0,0 +1,70 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.SpanCustomizer; +import brave.Tag; +import brave.propagation.TraceContext; + +/** + * Standard tags used in {@linkplain RpcRequestParser request} and {@linkplain RpcResponseParser + * response parsers}. + * + * @see RpcRequestParser + * @see RpcResponseParser + * @since 5.11 + */ +public final class RpcTags { + /** + * This tags "rpc.method" as the value of {@link RpcRequest#method()}. + * + * @see RpcRequest#method() + * @see RpcRequestParser#parse(RpcRequest, TraceContext, SpanCustomizer) + * @since 5.11 + */ + public static final Tag METHOD = new Tag("rpc.method") { + @Override protected String parseValue(RpcRequest input, TraceContext context) { + return input.method(); + } + }; + + /** + * This tags "rpc.service" as the value of {@link RpcRequest#service()}. + * + * @see RpcRequest#service() + * @see RpcRequestParser#parse(RpcRequest, TraceContext, SpanCustomizer) + * @since 5.11 + */ + public static final Tag SERVICE = new Tag("rpc.service") { + @Override protected String parseValue(RpcRequest input, TraceContext context) { + return input.service(); + } + }; + + /** + * This tags "rpc.error_code" as the value of {@link RpcResponse#errorCode()}. + * + * @see RpcResponse#errorCode() + * @see RpcResponseParser#parse(RpcResponse, TraceContext, SpanCustomizer) + * @since 5.11 + */ + public static final Tag ERROR_CODE = new Tag("rpc.error_code") { + @Override protected String parseValue(RpcResponse input, TraceContext context) { + return input.errorCode(); + } + }; + + RpcTags() { + } +} diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcTracing.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcTracing.java index 74e69da877..b26723dbe8 100644 --- a/instrumentation/rpc/src/main/java/brave/rpc/RpcTracing.java +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcTracing.java @@ -13,6 +13,7 @@ */ package brave.rpc; +import brave.Span; import brave.Tracing; import brave.internal.Nullable; import brave.sampler.SamplerFunction; @@ -22,7 +23,7 @@ /** * Instances built via {@link #create(Tracing)} or {@link #newBuilder(Tracing)} are registered - * automatically such that statically configured instrumentation like HTTP clients can use {@link + * automatically such that statically configured instrumentation like RPC clients can use {@link * #current()}. * * @since 5.8 @@ -45,12 +46,52 @@ public Tracing tracing() { return tracing; } + /** + * Used by {@link RpcClientHandler#handleSend(RpcClientRequest)} to add a span name and tags about + * the request before it is sent to the server. + * + * @since 5.12 + */ + public RpcRequestParser clientRequestParser() { + return clientRequestParser; + } + + /** + * Used by {@link RpcClientHandler#handleReceive(RpcClientResponse, Span)} to add tags about the + * response received from the server. + * + * @since 5.12 + */ + public RpcResponseParser clientResponseParser() { + return clientResponseParser; + } + + /** + * Used by {@link RpcServerHandler#handleReceive(RpcServerRequest)} to add a span name and tags + * about the request before the server processes it. + * + * @since 5.12 + */ + public RpcRequestParser serverRequestParser() { + return serverRequestParser; + } + + /** + * Used by {@link RpcServerHandler#handleSend(RpcServerResponse, Span)} to add tags about the + * response sent to the client. + * + * @since 5.12 + */ + public RpcResponseParser serverResponseParser() { + return serverResponseParser; + } + /** * Returns an overriding sampling decision for a new trace. Defaults to ignore the request and use * the {@link SamplerFunctions#deferDecision() trace ID instead}. * *

This decision happens when a trace was not yet started in process. For example, you may be - * making an rpc request as a part of booting your application. You may want to opt-out of tracing + * making an RPC request as a part of booting your application. You may want to opt-out of tracing * client requests that did not originate from a server request. * * @see SamplerFunctions @@ -82,11 +123,16 @@ public Builder toBuilder() { } final Tracing tracing; - final SamplerFunction clientSampler; - final SamplerFunction serverSampler; + final RpcRequestParser clientRequestParser, serverRequestParser; + final RpcResponseParser clientResponseParser, serverResponseParser; + final SamplerFunction clientSampler, serverSampler; RpcTracing(Builder builder) { this.tracing = builder.tracing; + this.clientRequestParser = builder.clientRequestParser; + this.serverRequestParser = builder.serverRequestParser; + this.clientResponseParser = builder.clientResponseParser; + this.serverResponseParser = builder.serverResponseParser; this.clientSampler = builder.clientSampler; this.serverSampler = builder.serverSampler; // assign current IFF there's no instance already current @@ -95,18 +141,25 @@ public Builder toBuilder() { public static final class Builder { Tracing tracing; - SamplerFunction clientSampler; - SamplerFunction serverSampler; + RpcRequestParser clientRequestParser, serverRequestParser; + RpcResponseParser clientResponseParser, serverResponseParser; + SamplerFunction clientSampler, serverSampler; Builder(Tracing tracing) { if (tracing == null) throw new NullPointerException("tracing == null"); this.tracing = tracing; + this.clientRequestParser = this.serverRequestParser = RpcRequestParser.DEFAULT; + this.clientResponseParser = this.serverResponseParser = RpcResponseParser.DEFAULT; this.clientSampler = SamplerFunctions.deferDecision(); this.serverSampler = SamplerFunctions.deferDecision(); } Builder(RpcTracing source) { this.tracing = source.tracing; + this.clientRequestParser = source.clientRequestParser; + this.serverRequestParser = source.serverRequestParser; + this.clientResponseParser = source.clientResponseParser; + this.serverResponseParser = source.serverResponseParser; this.clientSampler = source.clientSampler; this.serverSampler = source.serverSampler; } @@ -118,6 +171,62 @@ public Builder tracing(Tracing tracing) { return this; } + /** + * Overrides the tagging policy for RPC client requests. + * + * @see RpcTracing#clientRequestParser() + * @since 5.12 + */ + public Builder clientRequestParser(RpcRequestParser clientRequestParser) { + if (clientRequestParser == null) { + throw new NullPointerException("clientRequestParser == null"); + } + this.clientRequestParser = clientRequestParser; + return this; + } + + /** + * Overrides the tagging policy for RPC client responses. + * + * @see RpcTracing#clientResponseParser() + * @since 5.12 + */ + public Builder clientResponseParser(RpcResponseParser clientResponseParser) { + if (clientResponseParser == null) { + throw new NullPointerException("clientResponseParser == null"); + } + this.clientResponseParser = clientResponseParser; + return this; + } + + /** + * Overrides the tagging policy for RPC server requests. + * + * @see RpcTracing#serverRequestParser() + * @since 5.12 + */ + public Builder serverRequestParser(RpcRequestParser serverRequestParser) { + if (serverRequestParser == null) { + throw new NullPointerException("serverRequestParser == null"); + } + this.serverRequestParser = serverRequestParser; + return this; + } + + /** + * Overrides the tagging policy for RPC server responses. + * + * @see RpcTracing#serverResponseParser() + * @since 5.12 + */ + public Builder serverResponseParser(RpcResponseParser serverResponseParser) { + if (serverResponseParser == null) { + throw new NullPointerException("serverResponseParser == null"); + } + this.serverResponseParser = serverResponseParser; + return this; + } + /** @see RpcTracing#clientSampler() */ public Builder clientSampler(SamplerFunction clientSampler) { if (clientSampler == null) throw new NullPointerException("clientSampler == null"); diff --git a/instrumentation/rpc/src/main/java/brave/rpc/RpcTracingCustomizer.java b/instrumentation/rpc/src/main/java/brave/rpc/RpcTracingCustomizer.java index ad1fe78bac..09f75ceaf0 100644 --- a/instrumentation/rpc/src/main/java/brave/rpc/RpcTracingCustomizer.java +++ b/instrumentation/rpc/src/main/java/brave/rpc/RpcTracingCustomizer.java @@ -25,7 +25,7 @@ * component}. * *

This also allows one object to customize both {@link Tracing}, via {@link TracingCustomizer}, - * and the rpc layer {@link RpcTracing}, by implementing both customizer interfaces. + * and the RPC layer {@link RpcTracing}, by implementing both customizer interfaces. * *

Integration examples

* diff --git a/instrumentation/rpc/src/test/java/brave/rpc/CurrentRpcTracingTest.java b/instrumentation/rpc/src/test/java/brave/rpc/CurrentRpcTracingTest.java index a801b59d04..d309ce9f20 100644 --- a/instrumentation/rpc/src/test/java/brave/rpc/CurrentRpcTracingTest.java +++ b/instrumentation/rpc/src/test/java/brave/rpc/CurrentRpcTracingTest.java @@ -48,7 +48,7 @@ public class CurrentRpcTracingTest { RpcTracing current = RpcTracing.create(tracing); assertThat(RpcTracing.current()) - .isSameAs(current); + .isSameAs(current); } @Test public void setsNotCurrentOnClose() { @@ -91,6 +91,6 @@ public class CurrentRpcTracingTest { rpcTracings.remove(null); // depending on race, we should have either one instance or none assertThat(rpcTracings.isEmpty() || rpcTracings.size() == 1) - .isTrue(); + .isTrue(); } } diff --git a/instrumentation/rpc/src/test/java/brave/rpc/RpcTracingClassLoaderTest.java b/instrumentation/rpc/src/test/java/brave/rpc/ITRpcTracingClassLoader.java similarity index 81% rename from instrumentation/rpc/src/test/java/brave/rpc/RpcTracingClassLoaderTest.java rename to instrumentation/rpc/src/test/java/brave/rpc/ITRpcTracingClassLoader.java index dcfb1e7d16..f6cd55a0d0 100644 --- a/instrumentation/rpc/src/test/java/brave/rpc/RpcTracingClassLoaderTest.java +++ b/instrumentation/rpc/src/test/java/brave/rpc/ITRpcTracingClassLoader.java @@ -20,7 +20,14 @@ import static brave.test.util.ClassLoaders.assertRunIsUnloadable; import static org.assertj.core.api.Assertions.assertThat; -public class RpcTracingClassLoaderTest { +// Mockito tests eagerly log which triggers the log4j log manager, which then makes this run fail if +// run in the same JVM. The easy workaround is to move this to IT, which forces another JVM. +// +// Other workarounds: +// * Stop using log4j2 as we don't need it anyway +// * Stop using the log4j2 log manager, at least in this project +// * Do some engineering like this: https://stackoverflow.com/a/28657203/2232476 +public class ITRpcTracingClassLoader { @Test public void unloadable_afterClose() { assertRunIsUnloadable(ClosesRpcTracing.class, getClass().getClassLoader()); } diff --git a/instrumentation/rpc/src/test/java/brave/rpc/RpcClientHandlerTest.java b/instrumentation/rpc/src/test/java/brave/rpc/RpcClientHandlerTest.java new file mode 100644 index 0000000000..67efe32528 --- /dev/null +++ b/instrumentation/rpc/src/test/java/brave/rpc/RpcClientHandlerTest.java @@ -0,0 +1,174 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.Tracing; +import brave.propagation.CurrentTraceContext.Scope; +import brave.propagation.TraceContext; +import brave.sampler.Sampler; +import brave.sampler.SamplerFunction; +import brave.sampler.SamplerFunctions; +import java.util.ArrayList; +import java.util.List; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import zipkin2.Span; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Answers.CALLS_REAL_METHODS; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class RpcClientHandlerTest { + TraceContext context = TraceContext.newBuilder().traceId(1L).spanId(1L).sampled(true).build(); + List spans = new ArrayList<>(); + + RpcTracing httpTracing; + RpcClientHandler handler; + + @Mock(answer = CALLS_REAL_METHODS) RpcClientRequest request; + @Mock(answer = CALLS_REAL_METHODS) RpcClientResponse response; + + @Before public void init() { + init(httpTracingBuilder(tracingBuilder())); + when(request.method()).thenReturn("Report"); + } + + void init(RpcTracing.Builder builder) { + close(); + httpTracing = builder.build(); + handler = RpcClientHandler.create(httpTracing); + } + + RpcTracing.Builder httpTracingBuilder(Tracing.Builder tracingBuilder) { + return RpcTracing.newBuilder(tracingBuilder.build()); + } + + Tracing.Builder tracingBuilder() { + return Tracing.newBuilder().spanReporter(spans::add); + } + + @After public void close() { + Tracing current = Tracing.current(); + if (current != null) current.close(); + } + + @Test public void externalTimestamps() { + when(request.startTimestamp()).thenReturn(123000L); + when(response.finishTimestamp()).thenReturn(124000L); + + brave.Span span = handler.handleSend(request); + handler.handleReceive(response, span); + + assertThat(spans.get(0).durationAsLong()).isEqualTo(1000L); + } + + @Test public void handleSend_traceIdSamplerSpecialCased() { + Sampler sampler = mock(Sampler.class); + + init(httpTracingBuilder(tracingBuilder().sampler(sampler)) + .clientSampler(SamplerFunctions.deferDecision())); + + assertThat(handler.handleSend(request).isNoop()).isTrue(); + + verify(sampler).isSampled(anyLong()); + } + + @Test public void handleSend_neverSamplerSpecialCased() { + Sampler sampler = mock(Sampler.class); + + init(httpTracingBuilder(tracingBuilder().sampler(sampler)) + .clientSampler(SamplerFunctions.neverSample())); + + assertThat(handler.handleSend(request).isNoop()).isTrue(); + + verifyNoMoreInteractions(sampler); + } + + @Test public void handleSend_samplerSeesRpcClientRequest() { + SamplerFunction clientSampler = mock(SamplerFunction.class); + init(httpTracingBuilder(tracingBuilder()).clientSampler(clientSampler)); + + handler.handleSend(request); + + verify(clientSampler).trySample(request); + } + + @Test public void handleSendWithParent_overrideContext() { + try (Scope ws = httpTracing.tracing.currentTraceContext().newScope(context)) { + brave.Span span = handler.handleSendWithParent(request, null); + + // If the overwrite was successful, we have a root span. + assertThat(span.context().parentIdAsLong()).isZero(); + } + } + + @Test public void handleSendWithParent_overrideNull() { + try (Scope ws = httpTracing.tracing.currentTraceContext().newScope(null)) { + brave.Span span = handler.handleSendWithParent(request, context); + + // If the overwrite was successful, we have a child span. + assertThat(span.context().parentIdAsLong()).isEqualTo(context.spanId()); + } + } + + @Test public void handleReceive_finishesSpanEvenIfUnwrappedNull() { + brave.Span span = mock(brave.Span.class); + when(span.context()).thenReturn(context); + when(span.customizer()).thenReturn(span); + + handler.handleReceive(mock(RpcClientResponse.class), span); + + verify(span).isNoop(); + verify(span).context(); + verify(span).customizer(); + verify(span).finish(); + verifyNoMoreInteractions(span); + } + + @Test public void handleReceive_finishesSpanEvenIfUnwrappedNull_withError() { + brave.Span span = mock(brave.Span.class); + when(span.context()).thenReturn(context); + when(span.customizer()).thenReturn(span); + + Exception error = new RuntimeException("peanuts"); + when(response.error()).thenReturn(error); + + handler.handleReceive(response, span); + + verify(span).isNoop(); + verify(span).context(); + verify(span).customizer(); + verify(span).error(error); + verify(span).finish(); + verifyNoMoreInteractions(span); + } + + @Test public void handleReceive_responseRequired() { + brave.Span span = mock(brave.Span.class); + + assertThatThrownBy(() -> handler.handleReceive(null, span)) + .isInstanceOf(NullPointerException.class) + .hasMessage("response == null"); + } +} diff --git a/instrumentation/rpc/src/test/java/brave/rpc/RpcClientRequestSetterTest.java b/instrumentation/rpc/src/test/java/brave/rpc/RpcClientRequestSetterTest.java new file mode 100644 index 0000000000..6306d9eaec --- /dev/null +++ b/instrumentation/rpc/src/test/java/brave/rpc/RpcClientRequestSetterTest.java @@ -0,0 +1,55 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.propagation.Propagation.Setter; +import brave.test.propagation.PropagationSetterTest; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; + +import static brave.rpc.RpcClientRequest.SETTER; + +public class RpcClientRequestSetterTest extends PropagationSetterTest { + Map propagationFields = new LinkedHashMap<>(); + + @Override protected RpcClientRequest request() { + return new RpcClientRequest() { + @Override public Object unwrap() { + return null; + } + + @Override public String method() { + return null; + } + + @Override public String service() { + return null; + } + + @Override protected void propagationField(String keyName, String value) { + propagationFields.put(keyName, value); + } + }; + } + + @Override protected Setter setter() { + return SETTER; + } + + @Override protected Iterable read(RpcClientRequest request, String key) { + String result = propagationFields.get(key); + return result != null ? Collections.singletonList(result) : Collections.emptyList(); + } +} diff --git a/instrumentation/rpc/src/test/java/brave/rpc/RpcHandlerTest.java b/instrumentation/rpc/src/test/java/brave/rpc/RpcHandlerTest.java new file mode 100644 index 0000000000..008d430ad5 --- /dev/null +++ b/instrumentation/rpc/src/test/java/brave/rpc/RpcHandlerTest.java @@ -0,0 +1,121 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.Span; +import brave.SpanCustomizer; +import brave.handler.FinishedSpanHandler; +import brave.propagation.TraceContext; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class RpcHandlerTest { + TraceContext context = TraceContext.newBuilder().traceId(1L).spanId(10L).build(); + @Mock Span span; + @Mock SpanCustomizer spanCustomizer; + @Mock RpcRequest request; + @Mock RpcResponse response; + @Mock RpcRequestParser requestParser; + @Mock RpcResponseParser responseParser; + RpcHandler handler; + + @Before public void init() { + handler = new RpcHandler(requestParser, responseParser) { + }; + when(span.context()).thenReturn(context); + when(span.customizer()).thenReturn(spanCustomizer); + } + + @Test public void handleStart_nothingOnNoop_success() { + when(span.isNoop()).thenReturn(true); + + handler.handleStart(request, span); + + verify(span, never()).start(); + } + + @Test public void handleStart_parsesTagsWithCustomizer() { + when(span.isNoop()).thenReturn(false); + when(request.spanKind()).thenReturn(Span.Kind.SERVER); + + handler.handleStart(request, span); + + verify(requestParser).parse(request, context, spanCustomizer); + } + + @Test public void handleStart_addsRemoteEndpointWhenParsed() { + handler = new RpcHandler(RpcRequestParser.DEFAULT, RpcResponseParser.DEFAULT) { + @Override void parseRequest(RpcRequest request, Span span) { + span.remoteIpAndPort("1.2.3.4", 0); + } + }; + + handler.handleStart(request, span); + + verify(span).remoteIpAndPort("1.2.3.4", 0); + } + + @Test public void handleStart_startedEvenIfParsingThrows() { + when(span.isNoop()).thenReturn(false); + doThrow(new RuntimeException()).when(requestParser).parse(request, context, spanCustomizer); + + handler.handleStart(request, span); + + verify(span).start(); + } + + @Test public void handleFinish_nothingOnNoop() { + when(span.isNoop()).thenReturn(true); + + handler.handleFinish(response, span); + + verify(span, never()).finish(); + } + + @Test public void handleFinish_parsesTagsWithCustomizer() { + when(span.customizer()).thenReturn(spanCustomizer); + + handler.handleFinish(response, span); + + verify(responseParser).parse(response, context, spanCustomizer); + } + + /** Allows {@link FinishedSpanHandler} to see the error regardless of parsing. */ + @Test public void handleFinish_errorRecordedInSpan() { + RuntimeException error = new RuntimeException("foo"); + when(response.error()).thenReturn(error); + + handler.handleFinish(response, span); + + verify(span).error(error); + } + + @Test public void handleFinish_finishedEvenIfParsingThrows() { + when(span.isNoop()).thenReturn(false); + doThrow(new RuntimeException()).when(responseParser).parse(response, context, spanCustomizer); + + handler.handleFinish(response, span); + + verify(span).finish(); + } +} diff --git a/instrumentation/rpc/src/test/java/brave/rpc/RpcParserTest.java b/instrumentation/rpc/src/test/java/brave/rpc/RpcParserTest.java new file mode 100644 index 0000000000..08dc920704 --- /dev/null +++ b/instrumentation/rpc/src/test/java/brave/rpc/RpcParserTest.java @@ -0,0 +1,55 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.SpanCustomizer; +import brave.propagation.TraceContext; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class RpcParserTest { + TraceContext context = TraceContext.newBuilder().traceId(1L).spanId(10L).build(); + @Mock SpanCustomizer spanCustomizer; + @Mock RpcRequest request; + @Mock RpcResponse response; + + @Test public void request_addsNameServiceAndMethod() { + when(request.service()).thenReturn("zipkin.proto3.SpanService"); + when(request.method()).thenReturn("Report"); + + RpcRequestParser.DEFAULT.parse(request, context, spanCustomizer); + + verify(spanCustomizer).name("zipkin.proto3.SpanService/Report"); + verify(spanCustomizer).tag("rpc.service", "zipkin.proto3.SpanService"); + verify(spanCustomizer).tag("rpc.method", "Report"); + verifyNoMoreInteractions(spanCustomizer); + } + + @Test public void response_setsErrorTagToErrorCode() { + when(response.errorCode()).thenReturn("CANCELLED"); + + RpcResponseParser.DEFAULT.parse(response, context, spanCustomizer); + + verify(spanCustomizer).tag("rpc.error_code", "CANCELLED"); + verify(spanCustomizer).tag("error", "CANCELLED"); + verifyNoMoreInteractions(spanCustomizer); + } +} diff --git a/instrumentation/rpc/src/test/java/brave/rpc/RpcRequestParserTest.java b/instrumentation/rpc/src/test/java/brave/rpc/RpcRequestParserTest.java new file mode 100644 index 0000000000..9d11f2301d --- /dev/null +++ b/instrumentation/rpc/src/test/java/brave/rpc/RpcRequestParserTest.java @@ -0,0 +1,80 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.SpanCustomizer; +import brave.propagation.TraceContext; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class RpcRequestParserTest { + TraceContext context = TraceContext.newBuilder().traceId(1L).spanId(10L).build(); + @Mock RpcRequest request; + @Mock SpanCustomizer span; + + RpcRequestParser requestParser = RpcRequestParser.DEFAULT; + + @Test public void requestParser_noData() { + requestParser.parse(request, context, span); + + verify(request).service(); + verify(request).method(); + verifyNoMoreInteractions(request, span); + } + + @Test public void requestParser_onlyMethod() { + when(request.method()).thenReturn("Report"); + + requestParser.parse(request, context, span); + + verify(request).service(); + verify(request).method(); + verify(span).tag("rpc.method", "Report"); + verify(span).name("Report"); + verifyNoMoreInteractions(request, span); + } + + @Test public void requestParser_onlyService() { + when(request.service()).thenReturn("zipkin.proto3.SpanService"); + + requestParser.parse(request, context, span); + + verify(request).service(); + verify(request).method(); + verify(span).tag("rpc.service", "zipkin.proto3.SpanService"); + verify(span).name("zipkin.proto3.SpanService"); + verifyNoMoreInteractions(request, span); + } + + @Test public void requestParser_methodAndService() { + when(request.service()).thenReturn("zipkin.proto3.SpanService"); + when(request.method()).thenReturn("Report"); + + requestParser.parse(request, context, span); + + verify(request).service(); + verify(request).method(); + verify(span).tag("rpc.method", "Report"); + verify(span).tag("rpc.service", "zipkin.proto3.SpanService"); + verify(span).name("zipkin.proto3.SpanService/Report"); + verifyNoMoreInteractions(request, span); + } +} diff --git a/instrumentation/rpc/src/test/java/brave/rpc/RpcResponseParserTest.java b/instrumentation/rpc/src/test/java/brave/rpc/RpcResponseParserTest.java new file mode 100644 index 0000000000..21f96a73fc --- /dev/null +++ b/instrumentation/rpc/src/test/java/brave/rpc/RpcResponseParserTest.java @@ -0,0 +1,66 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.SpanCustomizer; +import brave.propagation.TraceContext; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class RpcResponseParserTest { + TraceContext context = TraceContext.newBuilder().traceId(1L).spanId(10L).build(); + @Mock RpcResponse response; + @Mock SpanCustomizer span; + + RpcResponseParser responseParser = RpcResponseParser.DEFAULT; + + @Test public void responseParser_noData() { + responseParser.parse(response, context, span); + + verify(response).errorCode(); + verifyNoMoreInteractions(response, span); + } + + @Test public void responseParser_errorCode_whenErrorNull() { + when(response.errorCode()).thenReturn("CANCELLED"); + + responseParser.parse(response, context, span); + + verify(response).errorCode(); + verify(response).error(); + verify(span).tag("rpc.error_code", "CANCELLED"); + verify(span).tag("error", "CANCELLED"); + verifyNoMoreInteractions(response, span); + } + + /** Ensure we don't obviate a better "error" tag later when an exception is present. */ + @Test public void responseParser_errorCode_whenError() { + when(response.error()).thenReturn(new RuntimeException()); + when(response.errorCode()).thenReturn("CANCELLED"); + + responseParser.parse(response, context, span); + + verify(response).errorCode(); + verify(response).error(); + verify(span).tag("rpc.error_code", "CANCELLED"); + verifyNoMoreInteractions(response, span); + } +} diff --git a/instrumentation/rpc/src/test/java/brave/rpc/RpcRuleSamplerTest.java b/instrumentation/rpc/src/test/java/brave/rpc/RpcRuleSamplerTest.java index 48642cca4c..9e8e1da3b5 100644 --- a/instrumentation/rpc/src/test/java/brave/rpc/RpcRuleSamplerTest.java +++ b/instrumentation/rpc/src/test/java/brave/rpc/RpcRuleSamplerTest.java @@ -32,8 +32,8 @@ public class RpcRuleSamplerTest { @Mock RpcServerRequest request; RpcRuleSampler sampler = RpcRuleSampler.newBuilder() - .putRule(methodEquals("health"), Sampler.ALWAYS_SAMPLE) - .build(); + .putRule(methodEquals("health"), Sampler.ALWAYS_SAMPLE) + .build(); @Test public void matches() { Map samplerToAnswer = new LinkedHashMap<>(); @@ -42,71 +42,71 @@ public class RpcRuleSamplerTest { samplerToAnswer.forEach((sampler, answer) -> { this.sampler = RpcRuleSampler.newBuilder() - .putRule(methodEquals("health"), sampler) - .build(); + .putRule(methodEquals("health"), sampler) + .build(); when(request.method()).thenReturn("health"); assertThat(this.sampler.trySample(request)) - .isEqualTo(answer); + .isEqualTo(answer); // consistent answer assertThat(this.sampler.trySample(request)) - .isEqualTo(answer); + .isEqualTo(answer); }); } @Test public void nullOnNull() { assertThat(sampler.trySample(null)) - .isNull(); + .isNull(); } @Test public void unmatched() { sampler = RpcRuleSampler.newBuilder() - .putRule(methodEquals("log"), Sampler.ALWAYS_SAMPLE) - .build(); + .putRule(methodEquals("log"), Sampler.ALWAYS_SAMPLE) + .build(); assertThat(sampler.trySample(request)) - .isNull(); + .isNull(); when(request.method()).thenReturn("health"); // consistent answer assertThat(sampler.trySample(request)) - .isNull(); + .isNull(); } @Test public void exampleCustomMatcher() { Matcher playInTheUSA = request -> (!"health".equals(request.method())); sampler = RpcRuleSampler.newBuilder() - .putRule(playInTheUSA, RateLimitingSampler.create(100)) - .build(); + .putRule(playInTheUSA, RateLimitingSampler.create(100)) + .build(); when(request.method()).thenReturn("log"); assertThat(sampler.trySample(request)) - .isTrue(); + .isTrue(); when(request.method()).thenReturn("health"); assertThat(sampler.trySample(request)) - .isNull(); // unmatched because country is health + .isNull(); // unmatched because country is health } @Test public void putAllRules() { RpcRuleSampler base = RpcRuleSampler.newBuilder() - .putRule(methodEquals("health"), Sampler.NEVER_SAMPLE) - .build(); + .putRule(methodEquals("health"), Sampler.NEVER_SAMPLE) + .build(); sampler = RpcRuleSampler.newBuilder() - .putAllRules(base) - .build(); + .putAllRules(base) + .build(); when(request.method()).thenReturn("log"); assertThat(sampler.trySample(request)) - .isNull(); + .isNull(); } // empty may sound unintuitive, but it allows use of the same type when always deferring diff --git a/instrumentation/rpc/src/test/java/brave/rpc/RpcServerHandlerTest.java b/instrumentation/rpc/src/test/java/brave/rpc/RpcServerHandlerTest.java new file mode 100644 index 0000000000..3269fb4b1a --- /dev/null +++ b/instrumentation/rpc/src/test/java/brave/rpc/RpcServerHandlerTest.java @@ -0,0 +1,155 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.Span; +import brave.Tracing; +import brave.propagation.TraceContext; +import brave.sampler.Sampler; +import brave.sampler.SamplerFunction; +import brave.sampler.SamplerFunctions; +import java.util.ArrayList; +import java.util.List; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Answers.CALLS_REAL_METHODS; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class RpcServerHandlerTest { + List spans = new ArrayList<>(); + TraceContext context = TraceContext.newBuilder().traceId(1L).spanId(1L).sampled(true).build(); + + RpcTracing httpTracing; + RpcServerHandler handler; + + @Mock(answer = CALLS_REAL_METHODS) RpcServerRequest request; + @Mock(answer = CALLS_REAL_METHODS) RpcServerResponse response; + + @Before public void init() { + init(httpTracingBuilder(tracingBuilder())); + } + + void init(RpcTracing.Builder builder) { + close(); + httpTracing = builder.build(); + handler = RpcServerHandler.create(httpTracing); + when(request.method()).thenReturn("Report"); + } + + RpcTracing.Builder httpTracingBuilder(Tracing.Builder tracingBuilder) { + return RpcTracing.newBuilder(tracingBuilder.build()); + } + + Tracing.Builder tracingBuilder() { + return Tracing.newBuilder().spanReporter(spans::add); + } + + @After public void close() { + Tracing current = Tracing.current(); + if (current != null) current.close(); + } + + @Test public void handleReceive_traceIdSamplerSpecialCased() { + Sampler sampler = mock(Sampler.class); + + init(httpTracingBuilder(tracingBuilder().sampler(sampler)) + .serverSampler(SamplerFunctions.deferDecision())); + + assertThat(handler.handleReceive(request).isNoop()).isTrue(); + + verify(sampler).isSampled(anyLong()); + } + + @Test public void handleReceive_neverSamplerSpecialCased() { + Sampler sampler = mock(Sampler.class); + + init(httpTracingBuilder(tracingBuilder().sampler(sampler)) + .serverSampler(SamplerFunctions.neverSample())); + + assertThat(handler.handleReceive(request).isNoop()).isTrue(); + + verifyNoMoreInteractions(sampler); + } + + @Test public void handleReceive_samplerSeesRpcServerRequest() { + SamplerFunction serverSampler = mock(SamplerFunction.class); + init(httpTracingBuilder(tracingBuilder()).serverSampler(serverSampler)); + + handler.handleReceive(request); + + verify(serverSampler).trySample(request); + } + + @Test public void externalTimestamps() { + when(request.startTimestamp()).thenReturn(123000L); + when(response.finishTimestamp()).thenReturn(124000L); + + Span span = handler.handleReceive(request); + handler.handleSend(response, span); + + assertThat(spans.get(0).durationAsLong()).isEqualTo(1000L); + } + + @Test public void handleSend_finishesSpanEvenIfUnwrappedNull() { + brave.Span span = mock(brave.Span.class); + when(span.context()).thenReturn(context); + when(span.customizer()).thenReturn(span); + + handler.handleSend(response, span); + + verify(span).isNoop(); + verify(span).context(); + verify(span).customizer(); + verify(span).finish(); + verifyNoMoreInteractions(span); + } + + @Test public void handleSend_finishesSpanEvenIfUnwrappedNull_withError() { + brave.Span span = mock(brave.Span.class); + when(span.context()).thenReturn(context); + when(span.customizer()).thenReturn(span); + + Exception error = new RuntimeException("peanuts"); + when(response.error()).thenReturn(error); + + handler.handleSend(response, span); + + verify(span).isNoop(); + verify(span).context(); + verify(span).customizer(); + verify(span).error(error); + verify(span).finish(); + verifyNoMoreInteractions(span); + } + + @Test public void handleSend_oneOfResponseError() { + brave.Span span = mock(brave.Span.class); + + assertThatThrownBy(() -> handler.handleSend(null, span)) + .isInstanceOf(NullPointerException.class) + .hasMessage("response == null"); + } +} diff --git a/instrumentation/rpc/src/test/java/brave/rpc/RpcTagsTest.java b/instrumentation/rpc/src/test/java/brave/rpc/RpcTagsTest.java new file mode 100644 index 0000000000..5fcf2be747 --- /dev/null +++ b/instrumentation/rpc/src/test/java/brave/rpc/RpcTagsTest.java @@ -0,0 +1,71 @@ +/* + * Copyright 2013-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.rpc; + +import brave.SpanCustomizer; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +/** This only tests things not already covered in {@code brave.TagTest} */ +@RunWith(MockitoJUnitRunner.class) +public class RpcTagsTest { + @Mock SpanCustomizer span; + @Mock RpcRequest request; + @Mock RpcResponse response; + + @Test public void method() { + when(request.method()).thenReturn("Report"); + RpcTags.METHOD.tag(request, span); + + verify(span).tag("rpc.method", "Report"); + } + + @Test public void method_null() { + RpcTags.METHOD.tag(request, span); + + verifyNoMoreInteractions(span); + } + + @Test public void service() { + when(request.service()).thenReturn("zipkin.proto3.SpanService"); + RpcTags.SERVICE.tag(request, span); + + verify(span).tag("rpc.service", "zipkin.proto3.SpanService"); + } + + @Test public void service_null() { + RpcTags.SERVICE.tag(request, span); + + verifyNoMoreInteractions(span); + } + + @Test public void error_code() { + when(response.errorCode()).thenReturn("CANCELLED"); + RpcTags.ERROR_CODE.tag(response, span); + + verify(span).tag("rpc.error_code", "CANCELLED"); + } + + @Test public void error_code_null() { + RpcTags.ERROR_CODE.tag(response, span); + + verifyNoMoreInteractions(span); + } +} diff --git a/instrumentation/rpc/src/test/java/brave/rpc/RpcTracingTest.java b/instrumentation/rpc/src/test/java/brave/rpc/RpcTracingTest.java index e9d17bc8ce..6502f9d446 100644 --- a/instrumentation/rpc/src/test/java/brave/rpc/RpcTracingTest.java +++ b/instrumentation/rpc/src/test/java/brave/rpc/RpcTracingTest.java @@ -28,20 +28,20 @@ public class RpcTracingTest { RpcTracing rpcTracing = RpcTracing.newBuilder(tracing).build(); assertThat(rpcTracing.clientSampler()) - .isSameAs(deferDecision()); + .isSameAs(deferDecision()); assertThat(rpcTracing.serverSampler()) - .isSameAs(deferDecision()); + .isSameAs(deferDecision()); } @Test public void toBuilder() { RpcTracing rpcTracing = RpcTracing.newBuilder(tracing).build(); assertThat(rpcTracing.toBuilder().build()) - .usingRecursiveComparison() - .isEqualTo(rpcTracing); + .usingRecursiveComparison() + .isEqualTo(rpcTracing); assertThat(rpcTracing.toBuilder().clientSampler(neverSample()).build()) - .usingRecursiveComparison() - .isEqualTo(RpcTracing.newBuilder(tracing).clientSampler(neverSample()).build()); + .usingRecursiveComparison() + .isEqualTo(RpcTracing.newBuilder(tracing).clientSampler(neverSample()).build()); } } diff --git a/instrumentation/rpc/src/test/java/brave/rpc/features/ExampleTest.java b/instrumentation/rpc/src/test/java/brave/rpc/features/ExampleTest.java index a12e7dc2f2..d3c50d59c7 100644 --- a/instrumentation/rpc/src/test/java/brave/rpc/features/ExampleTest.java +++ b/instrumentation/rpc/src/test/java/brave/rpc/features/ExampleTest.java @@ -32,11 +32,11 @@ public class ExampleTest { // This mainly shows that we don't accidentally rely on package-private access @Test public void showConstruction() { rpcTracing = RpcTracing.newBuilder(tracing) - .serverSampler(RpcRuleSampler.newBuilder() - .putRule(serviceEquals("scribe"), Sampler.NEVER_SAMPLE) - .putRule(methodEquals("Report"), RateLimitingSampler.create(100)) - .build()) - .clientSampler(SamplerFunctions.neverSample()) - .build(); + .serverSampler(RpcRuleSampler.newBuilder() + .putRule(serviceEquals("scribe"), Sampler.NEVER_SAMPLE) + .putRule(methodEquals("Report"), RateLimitingSampler.create(100)) + .build()) + .clientSampler(SamplerFunctions.neverSample()) + .build(); } }