Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

Record version 1.0 of the OpenTracing specification #20

Merged
merged 3 commits into from
Dec 2, 2016
Merged

Conversation

bhs
Copy link
Contributor

@bhs bhs commented Nov 28, 2016

This is a slightly more formal and less language-specific "spelling" of the content at opentracing.io/spec.

Once this is merged, the opentracing.io/spec content will still exist but will mostly be a link to this document and some short explanatory / contextual text with links to other areas of the docs site.

I also want to create .yaml versions of the "official" standard tags and (hopefully) standard structured log keys, include them in this repo, and link to them from this spec.

cc @yurishkuro @wu-sheng (who both commented on the branch pre-PR), as well as @adriancole @jmacd @cwe1ss @dawallin @CodingFabian @basvanbeek @beberlei @dkuebric @lookfwd @michaelsembwever who might want to weigh in.

This is a slightly more formal and less language-specific spelling of
the spec on opentracing.io/spec

There should be no parameters.

**Returns** the `SpanContext` for the given `Span`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add: The returned value can be used as span reference even after the original span is Finished.

Required parameters
- The new **operation name**, which supersedes whatever was passed in when the `Span` was started

#### Set `Span` tags
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/tags/tag/


#### Set a **baggage** item

Baggage items are key:value string pairs that apply to the given `Span`, its `SpanContext`, and **all `Spans` which directly or transitively _reference_ the local `Span`.** That is, baggage items propagate in-band along with the trace itself.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this clarify that it only applies to childOf/followsFrom references? Granted, there are no other standard reference types today, but future ones (or custom ones) may not imply baggage propagation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should make that clarification until other reference types exist.


Each **Span** encapsulates the following state:

- A start timestamp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • An operation name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

method name, a function name, or the name of a subtask or stage within a
larger computation). The operation name should be **the most general
string that identifies a (statistically) interesting class of `Span`
instances**. That is, `"get_user"` is better than `"get_user/314159"`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw the current spec had a table with 3 examples, maybe we can keep it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had been planning to make this doc more formal, but upon reflection it's probably better to err on the side of over-explaining subtle things like this. Re-added.

@yurishkuro
Copy link
Member

yurishkuro commented Nov 28, 2016

@bensigelman do you think this should completely replace http://opentracing.io/documentation/pages/spec.html page? There's a very large overlap, but some explanations from the existing page are not included here, for example there was a page-long description of childOf/followsFrom references, which I think should be included in the Spec.

- **HTTP Headers**: a string-to-string map with keys and values that are suitable for use in HTTP headers. That is, keys must not require strict casing and values must be URL-encoded
- **Binary**: a (single) arbitrary binary blob representing a `SpanContext`

### `Span`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing Finish

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch++

Copy link

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it is commonly misunderstood. should this doc say anywhere that some impls may do noop (ex not record timestamp etc) and that baggage and other wire details are not required to be compatible with other implementations?


- A start timestamp
- A finish timestamp
- A set of zero or more key:value **Span Tags**. The keys must be strings. The

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there no such constraints on "span logs"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, added.

Optional parameters
- An explicit timestamp. If specified, it must fall between the local start and finish time for the span.

Note that the OpenTracing project documents certain **"standard log keys"** which have prescribed semantic meanings. TODO: add these log keys to a central .yaml file and link here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah especially as logging just a string is no longer in the api, this should be linked prior to merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I just did it and added to this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO can be fixed now

The data_conventions.yaml is a more formal way of specifying what has so
far been hosted at
http://opentracing.io/documentation/pages/api/data-conventions
Copy link
Contributor Author

@bhs bhs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've responded to review comments... part of this is to add a .yaml file of conventional span tag and log keys.

cc #10


- A start timestamp
- A finish timestamp
- A set of zero or more key:value **Span Tags**. The keys must be strings. The
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, added.


Each **Span** encapsulates the following state:

- A start timestamp
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

method name, a function name, or the name of a subtask or stage within a
larger computation). The operation name should be **the most general
string that identifies a (statistically) interesting class of `Span`
instances**. That is, `"get_user"` is better than `"get_user/314159"`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had been planning to make this doc more formal, but upon reflection it's probably better to err on the side of over-explaining subtle things like this. Re-added.

- **HTTP Headers**: a string-to-string map with keys and values that are suitable for use in HTTP headers. That is, keys must not require strict casing and values must be URL-encoded
- **Binary**: a (single) arbitrary binary blob representing a `SpanContext`

### `Span`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch++

Optional parameters
- An explicit timestamp. If specified, it must fall between the local start and finish time for the span.

Note that the OpenTracing project documents certain **"standard log keys"** which have prescribed semantic meanings. TODO: add these log keys to a central .yaml file and link here.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I just did it and added to this PR.


#### Set a **baggage** item

Baggage items are key:value string pairs that apply to the given `Span`, its `SpanContext`, and **all `Spans` which directly or transitively _reference_ the local `Span`.** That is, baggage items propagate in-band along with the trace itself.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should make that clarification until other reference types exist.

- The tag key, which must be a string
- The tag value, which must be either a string, a boolean value, or a numeric type

Note that the OpenTracing project documents certain **"standard tags"** which have prescribed semantic meanings. TODO: add these tags to a central .yaml file and link here.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO can be fixed now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, done.


For example, here are potential **operation names** for a `Span` that gets hypothetical account information:

| Operation Name | Guidance |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice


Both injection and extraction rely on an extensible **format** parameter that dictates the type of the associated "carrier" as well as how a `SpanContext` is encoded in that carrier. All of the following **format**s must be supported by all Tracer implementations.
- **Text Map**: an arbitrary string-to-string map with an unrestricted character set for both keys and values
- **HTTP Headers**: a string-to-string map with keys and values that are suitable for use in HTTP headers. That is, keys must not require strict casing and values must be URL-encoded
Copy link

@codefromthecrypt codefromthecrypt Nov 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http header values are documented as ISO-8859-1/ASCII octets or mime encoded values. Since we aren't saying that, the "URL-encoded" part should be a link explaining why senders and receivers be looking at that vs what's normal.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise, it would be simple, as we'd just put a link to https://tools.ietf.org/html/rfc7230#section-3.2.4 referencing the standard encoding rules

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adriancole thanks for the IETF pointer. The trouble with HTTP headers is that they're so inconsistently implemented... i.e., being "standards-compliant" doesn't really count for much. I will add a link to the RFC 7230 and also a little more text to clarify.


**Returns** the `SpanContext` for the given `Span`. The returned value may be used even after the `Span` is finished.

#### Overwrite the operation name

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, this is something that comes up on the server side of various web frameworks where the Span is created before app-level data is necessarily available.

@wu-sheng
Copy link
Member

@bensigelman , what is the relation between specification.md and data_conventions.yaml. It seems two separated files.

Maybe, should add a link in specification, guide reader to data_conventions ?

@bhs
Copy link
Contributor Author

bhs commented Nov 29, 2016

@wu-sheng my intention with this repository was to make it more about data and specifications and less about "documentation" per se (which is the job of opentracing.io's repo). That said, the commit I'm about to make does include a cross-ref to the .yaml file from the spec file.

I do think the .yaml should be its own thing since various scripts could be built to process it and auto-generate standard tags in each language repository.

@wu-sheng
Copy link
Member

@bensigelman , oh, I see.

@bhs
Copy link
Contributor Author

bhs commented Nov 30, 2016

I'll merge this in 24-48h if there aren't further comments. Thanks!

@wu-sheng
Copy link
Member

wu-sheng commented Dec 1, 2016

@bensigelman , this will be a seperate page in spec doc? And should I add up a translation on this?

@bhs
Copy link
Contributor Author

bhs commented Dec 1, 2016

@wu-sheng I was going to keep the existing opentracing.io/spec URL valid but make it much shorter... probably just a description of OpenTracing's master versioning scheme as described at #2, a link to the formal specification in this PR, and maybe some text about what the spec actually means (i.e., that it describes "pure" OpenTracing APIs and not their implementations from various vendors). Maybe there will be a diagram or something.

Regarding translation: I mean, it's great if you want to translate it, sure. It's a lot of ongoing work to maintain that but I'm certainly not complaining!

@wu-sheng
Copy link
Member

wu-sheng commented Dec 1, 2016

Yes, translation is always an on-going job, I assume. :)
I just want to do it, on a checkpoint, or a frozen version.

@bhs bhs merged commit 0a8279a into master Dec 2, 2016
@bhs bhs deleted the bhs/spec branch December 2, 2016 16:16
bhs pushed a commit that referenced this pull request Dec 2, 2016
Include that last commit I didn't push on #20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants