RFC: Trace Identifiers

[tedsuo]

Proposal

https://github.com/opentracing/specification/blob/tedsuo/trace-identifiers/rfc/trace_identifiers.md

The Opentracing model of computation specifies two primary object types, Spans and Traces, but does not specify identifiers for these objects. The lack of identifiers makes it very difficult to correlate tracing data with data in other systems. This complicates a number of important tasks, and prevents the creation of reusable trace observers.

To address these difficulties, the OpenTracing SpanContext interface is extended to include SpanID and TraceID accessors.

Prior Proposals

#107 Trace-Parent Accessors

[objectiser]

include any client-side trace identifiers

[yurishkuro]

nit: make these bullet points, for easier reading

[erabug]

Alternatively, consider a table:

field	format	example	description
trace-id	16-byte array	4bf92f3577b34da6a3ce929d0e0e4736	The ID of the whole trace forest. If all bytes are 0, the Trace-Parent may be ignored.
span-id	8-byte array	00f067aa0ba902b7	The ID of the caller span (parent). If all bytes are 0, the Trace-Parent may be ignored.

(here's the raw markdown)

| field | format | example | description |
| :---  | :---   | :---    | :---        |
| `trace-id` | 16-byte array | 4bf92f3577b34da6a3ce929d0e0e4736 | The ID of the whole trace forest. If all bytes are 0, the `Trace-Parent` may be ignored. |
| `span-id`  | 8-byte array  | 00f067aa0ba902b7                 | The ID of the caller span (parent). If all bytes are 0, the `Trace-Parent` may be ignored. |

[yurishkuro]

bullet points

[erabug]

Same as above, consider a table:

| field | format | description |
| :---  | :---   | :---        |
| `TraceId` | 64 or 128-bit | The ID of the trace. Every span in a trace shares this ID. |
| `SpanId`  | 64-bit | Indicates the position of the current operation in the trace tree. The value may or may not be derived from the value of the `traceId`. |

[tedsuo]

I like the table approach!

[yurishkuro]

The comments about memory allocations might be better moved to the Risks section, as a performance consideration. It could be relevant even in cases when we don't introduce additional exposition formats, e.g. when the internal representation that the tracer uses is not a string. Many existing tracers represent span id as uint64, so to support the new API change efficiently they might want to cache the string representation.

[tedsuo]

Yes, makes sense.

[erabug]

Alternatively, consider a table:

field	format	example	description
trace-id	16-byte array	4bf92f3577b34da6a3ce929d0e0e4736	The ID of the whole trace forest. If all bytes are 0, the Trace-Parent may be ignored.
span-id	8-byte array	00f067aa0ba902b7	The ID of the caller span (parent). If all bytes are 0, the Trace-Parent may be ignored.

(here's the raw markdown)

| field | format | example | description |
| :---  | :---   | :---    | :---        |
| `trace-id` | 16-byte array | 4bf92f3577b34da6a3ce929d0e0e4736 | The ID of the whole trace forest. If all bytes are 0, the `Trace-Parent` may be ignored. |
| `span-id`  | 8-byte array  | 00f067aa0ba902b7                 | The ID of the caller span (parent). If all bytes are 0, the `Trace-Parent` may be ignored. |

[erabug]

Same as above, consider a table:

| field | format | description |
| :---  | :---   | :---        |
| `TraceId` | 64 or 128-bit | The ID of the trace. Every span in a trace shares this ID. |
| `SpanId`  | 64-bit | Indicates the position of the current operation in the trace tree. The value may or may not be derived from the value of the `traceId`. |

[erabug]

If the Trace-Context spec changes, these format specifications will become stale. Perhaps we could include the spec version described here as a signal to make sure you check the Trace-Context spec as the source of truth. (Same with the B3 protocol below.) Alternatively, just add a warning that these formats are subject to change.

[tedsuo]

Good point!

[erabug]

This sentence nicely orients you to the context of trace-id and span-id within the header. A similar sentence under the B3 Headers section would also be nice.

[erabug]

s/Opentracing/OpenTracing. In fact, I might reword the opening with a more positive bent and explain the change first, then the why, and link to the use cases you explain further down.

The OpenTracing SpanContext interface extends to include SpanID and TraceID accessors. Identifiers for the two primary object types make it easier to correlate tracing data with data in other systems, simplify important tasks, and allow the creation of reusable trace observers. Some use cases are detailed below.

[erabug]

+1 to use cases! At the top you mention that the lack of identifiers "complicates a number of important tasks" in addition to preventing use in systems like logging and reusable tracer observers (which are each called out in separate sections). What are these other important tasks that IDs facilitate?

[erabug]

I feel like this is missing the "therefore" at the end, i.e.

• Logging systems are primary consumers (sources?) of trace data but they can't directly access the data
• Including request identifiers is a common logging practice
• Tracing systems know these identifiers, therefore it makes sense for them to provide accessors

[erabug]

We should probably be more specific about these risks and implications. On the last call, Chris raised the issue that the ID returned might not be the one you're expecting (for example, the implementation's concept of a trace ID rather than the one propagated in the header).

Is there also concern about being too closely tied to protocol specifics? Should we be explicit the decision that trace and span IDs are "special cases" and worth the risk?

[tedsuo]

Yes, there's general agreement that this is useful for secondary systems, and since most tracers support these identifiers (and many are looking to standardize on Trace-Context) this is a good time to add them.

Part of the spec is that you should not assume anything about the value returned, other than it's uniqueness. Traces are globally unique, spans are unique within a trace. These are the only two properties a caller should depend on if they want to write observers which are vendor-neutral.

Happy to add more language making this clearer.

[erabug]

Nitpick that bullet points typically do not include periods. Also, it might be nice to add a "therefore" to the introduction, like

Trace and span identifiers would allow these tracer observers to create trace metadata without having to pay attention to the request headers.

[tedsuo]

Good point! Get it? Point? Like bullet point?

Sorry.

[erabug]

Perhaps call out here that an empty string is a valid return type.

[tedsuo]

@erabug @yurishkuro thanks for the feedback! I've made changes based on your suggestions.

One pedantic note: I like the way the tables look. But, technically, tables are not part of basic markdown, only GitHub. We've discussed avoiding whatever-flavored markdown in our docs, since it complicated which tool you use to render them. Is this a red flag for RFCs as well?

[cwe1ss]

LGTM! 👍

Just curious: Has it been discussed if using Object instead of String would be an option to prevent allocations? I guess it doesn' make much sense as most users of these IDs need a human-readable format, so having e.g. a byte array wouldn't be of much help anyway, right?

[cwe1ss]

nit: "OpenTracing" instead of "Opentracing"

[isaachier]

formating should be formatting

[isaachier]

What I picture for a C API would not incur allocation in the library. Just ask the user for a buffer.

#define TRACE_ID_STR_LEN 16
int span_context_trace_id(const custom_span_context* ctx, void* buf, int len)
{
    return snprintf(buf, len, "%" PRIx64 "%016" PRIx64, span_context->trace_id.high, span_context->trace_id.low);
}

[tedsuo]

Good approach! In many languages, this option will probably not be available, as the APIs you are trying to put the data into often will not support buffer types (or the language may not have them at all), but for C this is good.

[isaachier]

@cwe1ss if the only issue were how to represent the array, this at least could be done statically as a fixed-size array. I think the bigger issue here is why expose only a string interface and not also a raw byte interface?

[tedsuo]

@isaachier @cwe1ss regarding the use of String as the initial type. These accessors don't presume to know what the underlying format is; instead they provide formats which the caller may find useful. String is the most common and widely supported format, so at the cross-language level, it represents the lowest common denominator. However, on a language by language level, additional accessors which return different formats may be useful. Please see the "Alternate Formats" section for details:
https://github.com/opentracing/specification/blob/tedsuo/trace-identifiers/rfc/trace_identifiers.md#alternate-formats

[isaachier]

@tedsuo can we set a cap on the maximum size the string can be. This would simplify the C API considerably.

[tedsuo]

@isaachier a cap is a good idea. There is surely a reasonable upper bound, possibly 16-bytes.

[tedsuo]

@isaachier I added a section on length and formatting under Risks:
https://github.com/opentracing/specification/blob/tedsuo/trace-identifiers/rfc/trace_identifiers.md#restrictions-on-length-and-formatting

Would you be willing to move forwards with testing a string interface? We can continue discussing restrictions in the context of a release candidate.

[cwe1ss]

nit: "implemnetation"

[isaachier]

Ya no worries I'm fine with this approach.

[felixbarny]

Be consistent in describing the length in bits vs bytes (in TraceContext, you wrote the length is 16 bytes) . Bits is more commonly used I feel.

[tedsuo]

Ok, I normalized on bits, and included a note about character space (HEXDIG vs opaque).