Add OpenTelemetry Protocol (OTLP) RFC #35
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tigrannajaryan
requested review from
AloisReitbauer,
bogdandrutu,
c24t,
carlosalberto,
iredelmeier,
reyang,
SergeyKanzhelev,
songy23,
tedsuo and
yurishkuro
as code owners
This was referenced Aug 29, 2019
Closed
yurishkuro
reviewed
Aug 30, 2019
rnburn
reviewed
Aug 31, 2019
tigrannajaryan
force-pushed
the
feature/tigran/addunary
branch
from
e44be7f
to
45b18f6
Compare
|
Admins, please assign a number to this RFC. |
akehlenbeck
reviewed
Sep 9, 2019
tigrannajaryan
force-pushed
the
feature/tigran/addunary
branch
from
45b18f6
to
b5dd050
Compare
|
Admins, can you please assign an RFC number to this? Also, based on the RFC process shouldn't this be merged in |
tigrannajaryan
force-pushed
the
feature/tigran/addunary
branch
2 times, most recently
from
bc936e0
to
824f9a4
Compare
|
Reviewers, please have another look. I changed wording around Unary to remove the restriction that only Unary request may be in flight. |
tigrannajaryan
force-pushed
the
feature/tigran/addunary
branch
2 times, most recently
from
cf36971
to
9fe913b
Compare
|
UPDATE: Further experiments have shown that Streaming implementation In addition, one of the potential benefits that we could As a result Streaming mode was removed from the RFC and Please have another look. |
tigrannajaryan
pushed a commit
to tigrannajaryan/rfcs
that referenced
this pull request
Oct 11, 2019
This is a continuation of OTLP RFC proposal open-telemetry#35 This change defines the data format used by Span and Resource messages. The data format is a result of research of prior art (primarily OpenCensus and Jaeger), as well as experimentation and benchmarking done as part of OTLP RFC proposal. Go benchmark source code is available at https://github.com/tigrannajaryan/exp-otelproto (use `make benchmark-encoding` target). Benchmarking shows that depending on the payload composition this data format is about 4x-5x faster in encoding and 2x-3x faster in decoding equivalent data compared to OpenCensus data format (all benchmarks in Go). Notable differences from OpenCensus: - Attribute key/value pairs are represented as a list rather than as a map. This results in significant performance gains and at the same time changes the semantic of attributes because now it is possible to have multiple attributes with the same key. This is also in-line with Jaeger's tags representation. - Removed unnecessary wrappers such as google.protobuf.Timestamp which resulted in significant performance improvements for certain payload compositions (e.g. lots of TimedEvents). - Resource labels use the same data type as Span attributes which now allows to have labels with other data types (OpenCensus only allowed strings).
tigrannajaryan
pushed a commit
to tigrannajaryan/rfcs
that referenced
this pull request
Oct 11, 2019
This is a continuation of OTLP RFC proposal open-telemetry#35 This change defines the data format used by Span and Resource messages. The data format is a result of research of prior art (primarily OpenCensus and Jaeger), as well as experimentation and benchmarking done as part of OTLP RFC proposal. Go benchmark source code is available at https://github.com/tigrannajaryan/exp-otelproto (use `make benchmark-encoding` target). Benchmarking shows that depending on the payload composition this data format is about 4x-5x faster in encoding and 2x-3x faster in decoding equivalent data compared to OpenCensus data format (all benchmarks in Go). Notable differences from OpenCensus: - Attribute key/value pairs are represented as a list rather than as a map. This results in significant performance gains and at the same time changes the semantic of attributes because now it is possible to have multiple attributes with the same key. This is also in-line with Jaeger's tags representation. - Removed unnecessary wrappers such as google.protobuf.Timestamp which resulted in significant performance improvements for certain payload compositions (e.g. lots of TimedEvents). - Resource labels use the same data type as Span attributes which now allows to have labels with other data types (OpenCensus only allowed strings).
tigrannajaryan
pushed a commit
to tigrannajaryan/rfcs
that referenced
this pull request
Oct 11, 2019
This is a continuation of OTLP RFC proposal open-telemetry#35 This change defines the data format used by Span and Resource messages. The data format is a result of research of prior art (primarily OpenCensus and Jaeger), as well as experimentation and benchmarking done as part of OTLP RFC proposal. Go benchmark source code is available at https://github.com/tigrannajaryan/exp-otelproto (use `make benchmark-encoding` target). Benchmarking shows that depending on the payload composition this data format is about 4x-5x faster in encoding and 2x-3x faster in decoding equivalent data compared to OpenCensus data format (all benchmarks in Go). Notable differences from OpenCensus: - Attribute key/value pairs are represented as a list rather than as a map. This results in significant performance gains and at the same time changes the semantic of attributes because now it is possible to have multiple attributes with the same key. This is also in-line with Jaeger's tags representation. - Removed unnecessary wrappers such as google.protobuf.Timestamp which resulted in significant performance improvements for certain payload compositions (e.g. lots of TimedEvents). - Resource labels use the same data type as Span attributes which now allows to have labels with other data types (OpenCensus only allowed strings).
|
Reviewers, all questions and comments on this PR have been answered/address. Please have another look. We need to move forward with this, the protocol planned to be part of Alpha 0.3 milestone and the earlier we have an approved spec the better chances to have an implementation ready by 0.3 as well. |
|
I'm not an official reviewer, but I'm glad to see that the concurrent-unary-model was sufficient and that the streaming model has been removed. LGTM. Thanks for the prototyping you did around that issue Tigran. |
yurishkuro
approved these changes
Oct 16, 2019
SergeyKanzhelev
approved these changes
Oct 16, 2019
OpenTelemetry Protocol (OTLP) specification describes the encoding, transport and delivery mechanism of telemetry data between telemetry sources, intermediate nodes such as collectors and telemetry backends.
Experiments have shown that Streaming implementation results in significantly more complex code. The performance benefits of Streaming mode were only visible in very small batches and are not applicable to real life scenarios when under high load we aim to have large batches. In addition, one of the potential benefits that we could get with Streaming - the ability to optimize and avoid repetition of Resource does not seem to have much real life usage. As a result Streaming mode was removed from the RFC and Unary mode is the only mode that is now part of the spec.
tigrannajaryan
force-pushed
the
feature/tigran/addunary
branch
from
b82c74e
to
785bd02
Compare
|
Please have one more look. I believe the only remaining open question is whether to include Author and Acknowledgements sections. I am happy to go either way (my preference would be to keep Acknowledgements section since this document is a result of several people's work, not only my own, which won't be visible if we only look at the PR or commit history). |
|
I changed the status to approved, so that this gets merged in approved status (when it gets merged). If I understand correctly this is the right flow for RFCs. |
|
Admins, I think we have exhausted questions on this PR and have a principal agreement about the primary content - the protocol itself. The only outstanding issue is a meta regarding including or not including an Author/Acknowledgements section that lists people who contributed to the RFC. Can you please make a call on this? I believe such a decision will be applicable to other RFCs (and I am happy with either approach). |
yurishkuro
approved these changes
Oct 24, 2019
|
Admins, I need one more approval or more comments on this. |
jmacd
approved these changes
Oct 29, 2019
|
@yurishkuro thank you. |
tigrannajaryan
pushed a commit
to tigrannajaryan/rfcs
that referenced
this pull request
Oct 29, 2019
This is a continuation of OTLP RFC proposal open-telemetry#35 This change defines the data format used by Span and Resource messages. The data format is a result of research of prior art (primarily OpenCensus and Jaeger), as well as experimentation and benchmarking done as part of OTLP RFC proposal. Go benchmark source code is available at https://github.com/tigrannajaryan/exp-otelproto (use `make benchmark-encoding` target). Benchmarking shows that depending on the payload composition this data format is about 4x-5x faster in encoding and 2x-3x faster in decoding equivalent data compared to OpenCensus data format (all benchmarks in Go). Notable differences from OpenCensus: - Attribute key/value pairs are represented as a list rather than as a map. This results in significant performance gains and at the same time changes the semantic of attributes because now it is possible to have multiple attributes with the same key. This is also in-line with Jaeger's tags representation. - Removed unnecessary wrappers such as google.protobuf.Timestamp which resulted in significant performance improvements for certain payload compositions (e.g. lots of TimedEvents). - Resource labels use the same data type as Span attributes which now allows to have labels with other data types (OpenCensus only allowed strings).
bogdandrutu
pushed a commit
that referenced
this pull request
Nov 7, 2019
* Add OTLP Trace Data Format specification This is a continuation of OTLP RFC proposal #35 This change defines the data format used by Span and Resource messages. The data format is a result of research of prior art (primarily OpenCensus and Jaeger), as well as experimentation and benchmarking done as part of OTLP RFC proposal. Go benchmark source code is available at https://github.com/tigrannajaryan/exp-otelproto (use `make benchmark-encoding` target). Benchmarking shows that depending on the payload composition this data format is about 4x-5x faster in encoding and 2x-3x faster in decoding equivalent data compared to OpenCensus data format (all benchmarks in Go). Notable differences from OpenCensus: - Attribute key/value pairs are represented as a list rather than as a map. This results in significant performance gains and at the same time changes the semantic of attributes because now it is possible to have multiple attributes with the same key. This is also in-line with Jaeger's tags representation. - Removed unnecessary wrappers such as google.protobuf.Timestamp which resulted in significant performance improvements for certain payload compositions (e.g. lots of TimedEvents). - Resource labels use the same data type as Span attributes which now allows to have labels with other data types (OpenCensus only allowed strings). * Address review comments * Add protobuf type qualifier to pre-formatted blocks * Add a note about future goals for the protocol * Address review comments * Make sure all field comments start with field name * Change status to approved * Clarify start and end time expectations. * Address Sergey's comments * Remove string length requirement * Add StatusCode enum
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
OpenTelemetry Protocol (OTLP) specification describes the encoding, transport and delivery mechanism of telemetry data between telemetry sources, intermediate nodes such as collectors and telemetry backends.