search

mdsol / Medidata.ZipkinTracerModule

[Deprecated] Zipkin Request Tracing for .Net Apps
MIT License
53 stars 20 forks source link

[MCC-204458] Add server address and http.status attributes #70

Closed bvillanueva-mdsol closed 8 years ago

bvillanueva-mdsol commented 8 years ago

@kenyamat @lschreck-mdsol @cabbott @BPONTES @jcarres-mdsol

This PR is about adding attribute "sa" in client tracing. It will help rendering the zipkin UI dependecy graph. "https.status" is also added to record client tracing result. Please review and merge.

Thanks, Brent

kenyamat commented 8 years ago

LGTM

lschreck-mdsol commented 8 years ago

Seems to be :+1: to me, merging.

codefromthecrypt commented 7 years ago

as long as it goes out as bool in json (or 0x01 in thrift) all good! maybe worth a comment. thanks for responding!

On Mon, Apr 24, 2017 at 3:42 AM, Laszlo Schreck notifications@github.com wrote:

@lschreck-mdsol commented on this pull request.

In src/Medidata.ZipkinTracer.Core/SpanTracer.cs https://github.com/mdsol/Medidata.ZipkinTracerModule/pull/70#discussion_r112853663 :

             Timestamp = GetTimeStamp(),

Value = zipkinCoreConstants.CLIENT_SEND };

         newSpan.Annotations.Add(annotation);
  • AddBinaryAnnotation("http.uri", path, newSpan);
  • AddBinaryAnnotation("http.uri", remoteUri.AbsolutePath, newSpan, serviceEndpoint);
  • AddBinaryAnnotation("sa", "1", newSpan, zipkinEndpoint.GetRemoteEndpoint(remoteUri, clientServiceName));

@adriancole https://github.com/adriancole There is a chance that we went after the Ruby implementation initially, and missed the fix https://github.com/openzipkin/zipkin-ruby/commit/a49ac660cd1ecaa96236291bce1c48f5c43e6c84 from @jcarres-mdsol https://github.com/jcarres-mdsol on the .NET side.

So in .NET we can't really express one bit as a discrete data type, (we can write 0b1 as literal in the code but it will get interpreted as a signed 32-bit integer if we don't provide any explicit type for it), but I see in this example http://zipkin.io/pages/data_model.html that "sa" is sent to the server as a Json bool value, therefore we can assign the .NET equivalent true/false here (which will be serialized as a Json bool true/false).

Do you think it is OK?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mdsol/Medidata.ZipkinTracerModule/pull/70#discussion_r112853663, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD612JhNVt1X4T9MfN866PwwysUzQwEks5ry_3_gaJpZM4G7B9p .