Add remoteEndpoint in RPC spans

[gdong42]

This is an enhancement request to support zipkin remoteEndpoint in auto-instrumented RPC spans, something like examples from the docs at https://cloud.spring.io/spring-cloud-sleuth/multi/multi__features.html#_rpc_tracing:

// before you send a request, add metadata that describes the operation
span = tracer.newTrace().name("get").type(CLIENT);
span.tag("clnt/finagle.version", "6.36.0");
span.tag(TraceKeys.HTTP_PATH, "/api");
span.remoteEndpoint(Endpoint.builder()        // <=======   remoteEndpoint 
    .serviceName("backend")
    .ipv4(127 << 24 | 1)
    .port(8080).build());

// when the request is scheduled, start the span
span.start();

// if you have callbacks for when data is on the wire, note those events
span.annotate(Constants.WIRE_SEND);
span.annotate(Constants.WIRE_RECV);

// when the response is complete, finish the span
span.finish();

This would be a nice feature and would open up opportunities for users to stream through the spans, and build up service performance analytics without cross-joining with other spans.

For clients, this would require all RPC clients, e.g. FeignClient, to build spans with additional target service names. For server side spans, not sure it requires according changes.

Let me know if this makes sense.

[marcingrzejszczak]

Can you elaborate exactly on what in your opinion is missing in the current instrumentation? I don't think I follow.

[gdong42]

For example, from zipkin spans we collected,

  "_source": {
    "traceId": "cd4c1ce935007431",
    "duration": 6000,
    "localEndpoint": {             <==  can we have remoteEndpoint in addition to localEndpoint?
      "serviceName": "user-api",
      "ipv4": "10.40.189.141",
      "port": 9900
    },
    "timestamp_millis": 1532573979010,
    "kind": "CLIENT",
    "name": "http:/creditsystem/getusertotalcredit",
    "id": "aed0510b80f4a6f7",
    "parentId": "cd4c1ce935007431",
    "timestamp": 1532573979010000,
    "tags": {
      "http.host": "10.40.185.135",
      "http.method": "GET",
      ...
    }
  }

Above span has client-side service name as user-api in its localEndpoint, but lacks target service name in remoteEndpoint.

Similarly in SERVER spans, we only have localEndpoint as the server-side service name, but not knowing which service is calling it. Maybe we can set remoteEndpoint as the calling service name in this case.

[marcingrzejszczak]

Isn't it Zipkin that has the data on both sides (client and server) and can come to such conclusions properly?

[gdong42]

True, but we would like to conclude the whole RPC in source -> destination form in a single span. The advantage is that we can subscribe to the sleuth/zipkin MQ broker and streaming through spans to compute analysis data without saving and joining with other spans..

The background is that we want to build performance analysis functions based on zipkin data. Zipkin can query traces from the whole trace aspect vertically. This is nice, but it would be even better if we can look at service endpoint aggregations to give us horizontal statistics.

Hope this explains what we want to achieve.

[marcingrzejszczak]

WDYT @adriancole ?

[codefromthecrypt]

in HttpTracing component, you can scope spans to a service with httpTracing.clientOf("your-serviceName").. this is helpful when the remote service name is static for the client. of course you can override this explicitly with Span.remoteEndpoint. However, it might not be ideal as setting this could remove better data already set. Will be interesting to see the api or code impact of what you are interested in.

[marcingrzejszczak]

If we could read Span.remoteEndpoint we could set it only if it wasn't set already.

[codefromthecrypt]

two things here.. in the next patch of sleuth, Span.remoteServiceName(your_name) will be exposed for writes. In the next minor (hopefully) brave itself will have a handler for aggregation openzipkin/brave#557

either way I think there's no action to take in sleuth at the moment