Spring-cloud-sleuth: Ability to pass in remoteServiceName in WebClient

[davidmelia]

Hi,

I'm currently using Sleuth and ZipKin together to show the data flow in my microservice stack which works great. Also third party dependencies such as Redis and Kafka are also tagged (using remoteServiceName) and appear in the ZipKin 'Dependencies Page' graph by default.

One thing that is missing is to allow the tagging of a remote service name (3rd party dependency) for webflux webclient endpoints which communicate outside my stack so that ZipKin can display them as a separate entity similar to Redis and Kafka.

Would this be possible as a feature request? The only slightly tricky bit (and the difference with the Redis and Kafka Sleuth integration) is this would have to be on a per WebClient basis. Alternatively maybe you could offer this in the TraceExchangeFilterFunction?

Thanks

[davidmelia]

Hi, just having a play with TraceExchangeFilterFunction and the below is the kind of behaviour I want (I appreciate the below is a poor solution from a Spring point of view. Also you would have to make TraceExchangeFilterFunction public so I can call create)

public final class TraceExchangeFilterFunction implements ExchangeFilterFunction {
......
  public static ExchangeFilterFunction create(ConfigurableApplicationContext springContext, String remoteServiceName) {
    return new CustomTraceExchangeFilterFunction(springContext, remoteServiceName);
  }
.....
 private static final class MonoWebClientTrace extends Mono<ClientResponse> {
    ...
    @Override
    public void subscribe(CoreSubscriber<? super ClientResponse> subscriber) {
     ....
      Span span = handler.handleSendWithParent(wrapper, parent);
      if (remoteServiceName != null) {
        span.remoteServiceName(remoteServiceName);
      }

[codefromthecrypt]

how are you deriving the remote service name?

[davidmelia]

@adriancole I want to new up TraceExchangeFilterFunction passing in the remote service name.

e.g.
webClientBuilder.filter(TraceExchangeFilterFunction.create(springContext, "MyRemoteService")).build();

This gives me the ability to tag any third party dependency I like. The remote service name is on a case by case (repository by repository) basis.

N.B. Only tricky bit is sleuth already creates TraceExchangeFilterFunction for you automatically so in this case I might have to remove the existing TraceExchangeFilterFunction and add my new one.

[codefromthecrypt]

@davidmelia please use a FinishedSpanHandler for now. We'll at some point have a remote service name function. I wouldn't advise trying to rejig sleuth's internals like TraceExchangeFilterFunction

[davidmelia]

@adriancole I think FinishedSpanHandler is too global. For example if I have two third party dependencies in my application then I have a problem.

One solution is I could use tagging

@SpanTag(key = "remoteServiceName", value = "myThirdParty")

and something like

  @Bean
  public FinishedSpanHandler finishedSpanHandler() {
    return new FinishedSpanHandler() {

      @Override
      public boolean handle(TraceContext context, MutableSpan span) {
        if (span.tag("remoteServiceName") != null) {
          span.remoteServiceName(span.tag("remoteServiceName"));
        }

        return true;
      }
    };
  }

but it is way nicer to set on the WebClient directly so it affects all calls if you know what I mean.

[codefromthecrypt]

FinishedSpanHandler is a workaround. we'll definitely have a remote service name function, just there's literally only me working on most things and it won't get into the release going out next week.

[codefromthecrypt]

this change will make the pending release less burdensome https://github.com/openzipkin/brave/pull/1210

[marcingrzejszczak]

Hey @adriancole any update on the Brave side on how you want to tackle this?

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[asarkar]

Along with the remote service name, the host and port would also be good to have. Currently, only the request path is available.

[marcingrzejszczak]

Hey wouldn't the HttpRequestParser bean annotated with @HttpClientRequestParser do it's job? Check this out https://docs.spring.io/spring-cloud-sleuth/docs/3.0.1/reference/htmlsingle/#how-to-cutomize-http-client-spans

[spring-cloud-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[davidmelia]

@marcingrzejszczak I've not tried it but I think what you suggested will work. Ideally I was looking for a way to set on a per webclient basis but I appreciate how the framework works so this would not be possible. In my case I will try and use HttpRequestParser and using the request set the remote service as appropiate.

[marcingrzejszczak]

Ok so let's close the issue!

[davidmelia]

@marcingrzejszczak the way I could get this to work was using:

  @Bean
  public SpanHandler finishedSpanHandler() {
    return new SpanHandler() {
      public boolean end(TraceContext context, MutableSpan span, Cause cause) {
        span.remoteServiceName("Dave");
        return true;
      }
    };
  }

but I have no access to the request. Could we open this up issue up again and could I request the ability to set remoteServiceName from org.springframework.cloud.sleuth.http.HttpRequestParser or brave.http.HttpRequestParser

Thanks

[davidmelia]

Another option is to do something like

  @Bean
  public SpanHandler finishedSpanHandler(MyConfig myConfig) {
    return new SpanHandler() {
      public boolean end(TraceContext context, MutableSpan span, Cause cause) {
        String path = span.tag("http.path");
        if(StringUtils.hasLength(path) && myConfig.getSomeUrl().contains(path)){
          span.remoteServiceName("Dave");
        }
        return true;
      }
    };
  }

but this seems more of a hack than a solution.

[marcingrzejszczak]

Come to think of it you can use the HttpRequestParser and then call BraveSpan.toBrave(span). That will give you back the Brave span on which you can call remoteServiceName

[davidmelia]

Hi @marcingrzejszczak , I tried BraveSpan but there are two problems:

1) HttpRequestParser span is actually of type SpanCustomizer so cannot be passed into BraveSpan.toBrave(span)

2) BraveSpan is not public in 3.0.2 (although looks like it is now public in master)

[marcingrzejszczak]

So with this commit https://github.com/spring-cloud/spring-cloud-sleuth/commit/8d86e19807521c50b333bca98bd938f43af9fcab Span will have the remoteServiceName method on it. You can cast from SpanCustomizer to Span cause Span is the extension of the SpanCustomizer and then you will be able to call the remoteServiceName method directly.

[davidmelia]

@marcingrzejszczak I was just having a play with Sleuth 3.0.3-SNAPSHOT.

I cannot use BraveSpan.toBrave((Span) span); because HttpRequestParser returns a BraveSpanCustomizer which is not a span.

inside BraveSpanCustomizer is a brave.SpanCustomizerShield which also doesn't seem to be a handle on Span.

I don't think it's possible to set remoteServiceName using HttpRequestParser

 @Bean(name = HttpClientRequestParser.NAME)
  public HttpRequestParser remoteServiceHttpRequestParser() {
    return (request, context, span) -> {

      BraveSpanCustomizer braveSpanCustomizer = (BraveSpanCustomizer) span;

      // the below results in a class cast
      BraveSpan.toBrave((Span) braveSpanCustomizer);

    };
  }

[marcingrzejszczak]

 @Bean(name = HttpClientRequestParser.NAME)
  public HttpRequestParser remoteServiceHttpRequestParser() {
    return (request, context, span) -> {

      BraveSpanCustomizer braveSpanCustomizer = (BraveSpanCustomizer) span;

      // this would work if you had access to the method
     brave.SpanCustomizer sc = BraveSpanCustomizer.toBrave(braveSpanCustomizer);

      // the below results in a class cast
      ((brave.Span) sc).setRemoteName(...)

    };
  }

the above would work but you don't have access to the method... Could you please file an issue so that I open all the classes and the toBrave and fromBrave methods?

[marcingrzejszczak]

Ah I understand what you mentioned with SpanCustomizerShield... so that wouldn't work either... That's problematic cause we won't be able to delegate the remote name setting to Brave. Maybe you should actually open an issue in brave and until then you have to use reflection :grimacing: