Add data format section and a description of timestamps #111
Conversation
- Added explanation of representation of times and durations
danielkhan
requested review from
AloisReitbauer,
bogdandrutu,
c24t,
carlosalberto,
iredelmeier,
reyang,
SergeyKanzhelev,
songy23,
tedsuo and
yurishkuro
as code owners
specification/tracing-api.md
Outdated
| OpenTelemetry stores time values using nanosecond (ns) precision. | ||
| To overcome platform limitations, this data is represented as a tuple | ||
| [<long> seconds, <int> nanoseconds], where nanoseconds is the remainder that can | ||
| not be represented in seconds. |
There was a problem hiding this comment.
I am not clear which part of OpenTelemetry this paragraph refers to. The API does not "store" anything, and when it accepts timestamps from the user, e.g. as part of SpanData, the representation ideally should be idiomatic to the language. E.g. in Python the time is a float as seconds since epoch, sub-seconds are in the decimals.
There was a problem hiding this comment.
@yurishkuro, I looked how the current Java Implementation does it: https://developers.google.com/protocol-buffers/docs/reference/java/com/google/protobuf/util/Timestamps
Some platforms use the tuple representation and it's easy to convert back and forth between decimals and tuples. There has to be an API surface to set a timestamp and there we have to decide on how this can be done.
There was a problem hiding this comment.
My point is that "how java does it" should not necessarily inform the cross-language spec, and storage representation does not belong in the API spec. Even in Java, I would expect people to use Instant type, not the proto definition you pointed to, which makes it irrelevant to the end user how instant is actually implemented. I would limit this PR to the language about precision only.
There was a problem hiding this comment.
I removed the wording on tuples and I am now just mentioning that the representation is language specific. Does this work for you?
specification/tracing-api.md
Outdated
| ##### Examples: | ||
| * Given a Unix timestamp of `1560345793106ms`, the resulting tuple would be | ||
| `[1560345793, 106000000]`. | ||
| * Given a duration of `500,000376ms`, the resulting tuple would be `[0, 500000376]`. |
There was a problem hiding this comment.
these examples either not needed, or need to be prefixed with an explanation of tuple representation. I am not sure they are especially useful here.
There was a problem hiding this comment.
You are right. Given the previous change, they are obsolete now. I removed them.
There was a problem hiding this comment.
LGTM. Like the idea of making it language idiomatic as mentioned by @yurishkuro.
…ry#111) * Add section on data formats. Fixes open-telemetry#85 - Added explanation of representation of times and durations * Removed wording on tuples * Removed examples
* Added auto resource detection proposal * Removed resource provider concept as per review comments * Changed the proposal back to separating resource detection from the tracer/meter providers, clarified default resource detection in more detail, and added more points to the trade-offs & mitigations section * Wrap lines Co-authored-by: Sergey Kanzhelev <[email protected]>
* Added auto resource detection proposal * Removed resource provider concept as per review comments * Changed the proposal back to separating resource detection from the tracer/meter providers, clarified default resource detection in more detail, and added more points to the trade-offs & mitigations section * Wrap lines Co-authored-by: Sergey Kanzhelev <[email protected]>
…ry#111) * Add section on data formats. Fixes open-telemetry#85 - Added explanation of representation of times and durations * Removed wording on tuples * Removed examples
Closes API: explain what timestamp precision should be #85