Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support local spans via a LOCAL_COMPONENT("lc") binary annotation #808

Closed
codefromthecrypt opened this issue Nov 2, 2015 · 0 comments
Closed
Labels
enhancement model Modeling of traces

Comments

@codefromthecrypt
Copy link
Member

Currently it is tribal knowledge that local (in-process) spans exist. We need a way to designate span as local, and offer insight into how to use them. This can lead us to answer what status quo could be on this, and how they might be tested.

Firstly, local spans need to be annotated with a namespace that can be used like service. A discussion on github suggests using a LOCAL_COMPONENT("lc") binary annotation for this.

Here's a json view:

{
  "key" : "lc",
  "value" : "encryption-service",
  "endpoint" : {
    "serviceName" : "proxy",
    "ipv4" : "1.2.3.4",
    "port" : 345
  }
}

You'll notice that the binary annotation holds almost all information needed to identify the context of the span. It contains a searchable annotation which contains its namespace, in this case encryption-service. Via the existing contract of binary annotations, it also includes the service that contains it, in this case proxy.

What's left is the skeleton of the span. How do we capture the duration of the span?

I'm aware of three options:

TL;DR; is that I think the latter is best.

If we re-use "cs/cr" annotations, we avoid logic around dependency graphs where server annotations are special-cased. In other words, we avoid accidentally cluttering it with local spans. We can also hang off an existing explanation where we use "cs/cr" to address uninstrumented systems. It might be the least code changes.

{
  "traceId" : "0000000000000005",
  "name" : "encrypt",
  "id" : "000000000000029a",
  "parentId" : "0000000000000002",
  "annotations" : [
    {
      "timestamp" : 60,
      "value" : "cs",
      "endpoint" : {
        "serviceName" : "proxy",
        "ipv4" : "1.2.3.4",
        "port" : 345
      }
    },
    {
      "timestamp" : 100,
      "value" : "cr",
      "endpoint" : {
        "serviceName" : "proxy",
        "ipv4" : "1.2.3.4",
        "port" : 345
      }
    }
  ],
  "binaryAnnotations" : [
    {
      "key" : "lc",
      "value" : "encryption-service",
      "endpoint" : {
        "serviceName" : "proxy",
        "ipv4" : "1.2.3.4",
        "port" : 345
      }
    }
  ]
}

If we make up new annotations for the bookends, we avoid adding more cognitive weight to "cs/cr", which are already confusing. Instead, all two-character annotation keys prefixed 'l' explain local spans, and those not interested simply don't read their docs. We do have to add more constants, and affect the instrumentation more. While this is nice for documentation containment, it could look overwhelming.

{
  "traceId" : "0000000000000005",
  "name" : "encrypt",
  "id" : "000000000000029a",
  "parentId" : "0000000000000002",
  "annotations" : [
    {
      "timestamp" : 60,
      "value" : "ls",
      "endpoint" : {
        "serviceName" : "proxy",
        "ipv4" : "1.2.3.4",
        "port" : 345
      }
    },
    {
      "timestamp" : 100,
      "value" : "lr",
      "endpoint" : {
        "serviceName" : "proxy",
        "ipv4" : "1.2.3.4",
        "port" : 345
      }
    }
  ],
  "binaryAnnotations" : [
    {
      "key" : "lc",
      "value" : "encryption-service",
      "endpoint" : {
        "serviceName" : "proxy",
        "ipv4" : "1.2.3.4",
        "port" : 345
      }
    }
  ]
}

If we piggy-back off Span.startTs/duration (#807), local spans really only need span metadata and the single binary annotation. This would result in less storage and cognitive burden, as we no longer need to think about annotations in order to understand the duration of a span. This is the most dramatic departure from existing "core annotations" so needs to be specifically supported in UI and presentation.

{
  "traceId" : "0000000000000005",
  "name" : "encrypt",
  "id" : "000000000000029a",
  "parentId" : "0000000000000002",
  "startTs" : 60,
  "duration" : 40,
  "binaryAnnotations" : [
    {
      "key" : "lc",
      "value" : "encryption-service",
      "endpoint" : {
        "serviceName" : "proxy",
        "ipv4" : "1.2.3.4",
        "port" : 345
      }
    }
  ]
}
codefromthecrypt pushed a commit that referenced this issue Nov 4, 2015
This moves existing code around the notion of Span.timestamp,
Span.duration discussed in #807.

The impact from a user POV is minimal. `endTs`, used by the query and UI
formerly looked for the last timestamp in a trace. What this meant is
that if someone clicked search, waited, then clicked search again with
the same `endTs`, an in-flight trace may "disappear" if it has new
activity. Since `endTs` is now based on a stable point (the start), a
trace wouldn't disappear anymore. This impact is so subtle that it is
barely worth discussing.

The primary motivation for this change is to simplify the commodity task
of timestamping and duration stamping spans. This is discussed #807, and
directly supports a new minimal design of local spans (#808).
codefromthecrypt pushed a commit that referenced this issue Nov 7, 2015
Local spans have been discussed at length, but have never been
formalized. This formalizes how to tag a span as local, including
rationale documentation. To help demonstrate the point, zipkin-web and
zipkin-query use this for bootstrap tracing.

Fixes #808
@codefromthecrypt codefromthecrypt added enhancement model Modeling of traces labels Oct 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement model Modeling of traces
Projects
None yet
Development

No branches or pull requests

1 participant