how do we idiomatically send cs, cr annotations

[codefromthecrypt]

hey there. In making this, I noticed the resulting span for an outgoing request has only server annotations ("sr", "ss"). openzipkin/pyramid_zipkin-example#1

do you have ideas on making client instrumentation as an alternative to this pattern?

    headers = {}
    headers.update(create_http_headers_for_new_span())
    backend_response = requests.get(
        url='http://localhost:9000/api',
        headers=headers,
    )

Ex some interceptor for requests that starts a span, logs "cs" in that span, makes the outgoing request, logs "cr" in the same span once received, potentially adding an error annotation on transit error.

[codefromthecrypt]

one idea is to make a generic filter which adds annotations within a span for a scoped operation..

before: Annotation <- added at the beginning of a task
after: Annotation <- added unconditionally after a task
after_failure: Annotation + ": $error.class: $error.message" <- additionally added when the task failed

ex. I could add do something like (forgive the quasi code)

start_new_span
  filter("cs", "cr", "Client Send Error: $error.class: $error.message")
    httpclient.get
        filter("ws", "wr", "Wire Send Error: $error.class: $error.message")
          http_transport.send

In this case, the span above would include in normal case: ["cs", "ws", "wr", "cr"]

This is the design finagle uses, and elaborated here
openzipkin/openzipkin.github.io#52

[mjbryant]

I'm not totally sure what you're asking/suggesting here. I think there might be a few separate things: how (and whether) pyramid-zipkin logs (1) client spans, (2) error annotations, and (3) wire events.

Re (1): this library actually has no support for logging client spans directly. The zipkin_span decorator/context-manager, when used inside an existing active span context, will log both server and client annotations. This is probably bad - we use this internally to give more structure to our traces (e.g. with this pattern a function call can easily be its own span). But zipkin_span, when used not in an existing active trace, will log only server annotations. This is because we extracted this code from pyramid-zipkin, which was only ever meant to add server-side instrumentation.

In the example using requests, we could have theoretically used an instrumented version of this library, but that'd require some additional code.

Re (2): we don't currently add error annotations to the span, but I don't think this would be hard to do at all. The __exit__ method of the context manager gets passed exception information; we can probably just create the annotations there. If that's a standard, we should start doing that, especially if there will be UI changes to color spans differently based on exceptions. I think that should live in its own issue.

Re (3): I didn't even know this was a standard annotation.

[bplotnick]

I forgot about this ticket... I think with #14 this is done? You can now do with zipkin_client_span. Let me know if there's something we're still missing (ehem... documentation).

We still don't have error annotations, but I think that can be added. I can make a separate ticket for that.

[codefromthecrypt]

separate ticket for error annotation probably makes sense. "error" is gaining traction in various libraries, mostly because of the color red I bet (but also because you can search for "error")