-
Notifications
You must be signed in to change notification settings - Fork 897
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
Add OpenTelemetry protocol design goals and requirements #193
Add OpenTelemetry protocol design goals and requirements #193
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, assuming not much change from open-telemetry/opentelemetry-proto#22 and open-telemetry/opentelemetry-proto#21.
|
||
#### Compression | ||
|
||
The protocol must achieve high compression ratios for telemetry data. The protocol design must consider batching of telemetry data and grouping of similar data (both can help to achieve better compression using common compression algorithms). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without sacrificing the goal of being "Load Balancer Friendly".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we have a number of requirements that may actually be at odds, and there may be tradeoff's involved. I think it is implicitly understood (and not just for Compression vs Load Balancing, etc).
|
||
#### Compression | ||
|
||
The protocol must achieve high compression ratios for telemetry data. The protocol design must consider batching of telemetry data and grouping of similar data (both can help to achieve better compression using common compression algorithms). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For batching, I wonder if the protocol should support streaming - the receiver should be able to start processing the 1st item upon arrival without having to wait for the entire batch to finish transmission.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be somewhat difficult to achieve. I think this requirement should be in the realms of the special exporters and special protocols which aim to minimize the overall latency. Streaming SDK implementation were discussed a few times and I think they are somewhat niche, so I'd leave that out from general requirements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with these requirements, and they're well stated. Thanks!
Do we expect the exchange protocol to be the same across all transports? HTTP/1.1, gRPC, and UDP all have different qualities, and can support different features, such as backpressure. Alternatively, these protocol design goals part of how we pick which transports we want to support? In our data formats discussions, we initially proposed to have "exchange protocol" and "transport" be split into separate layers, but I'm starting to wonder if that is realistic. Should we instead be defining an exchange protocol and a transport together as a single choice, and only add support for a new transport when we decide we want to offer a different protocol with different trade offs? |
Yes, I am aiming to choose one that shows best overral performance based on prototyping / measurements. I do not see the need to support multiple transports.
The separation is to aid understanding, not because we think they must be exposed as separate components to the user. You are right that we should define it as a single choice. There is plenty of alternate choice for telemetry protocols and transports if the users wants something different (e.g. OpenCensus, Jaeger, etc). These are well supported by their vendors and also by OpenTelemetry Service. It is not difficult for vendors to implement OpenTelemetry library exporters for their protocol and the user will not have to user OpenTelemetry protocol if they don't want to. |
My 2c:
|
I agree, that implementation should clearly delineate these 2. I am not sure if you are also suggesting that the end users should be able to swap out for a different transport or that you advocate that it is a possibility for OpenTelemetry maintainers to change the transport in the future or provide multiple transports for the end user to choose from. Can you clarify this? |
I agree. Definitely not something I would want to do either. It has to be a well-known, stable, battle tested encoding mechanism with ubiquitous availability of implementations in wide selection of languages that are supported by OpenTelemetry. |
This is what I have in the doc, do you think it is not enough?
|
Both, I think. The main thing is for the SDK to treat encoding and transport as independently pluggable |
maybe this should be called out in the requirements |
That's probably enough for now. |
@yurishkuro The SDK will treat the protocol as pluggable. The protocol has to implement the Exporter interface. The encoding and transport are internal implementation details of the Exporter which the SDK is not concerned with. Whether we want to clearly expose the OpenTelemtry encoding and transport as separate components to the end-user can be decided later. I think it can be a detail of particular implementation of OpenTelemetry protocol specification. I will aim to write the specification itself in a way that makes it clear what is encoding and what is the transport. |
e4a8c80
to
2a77ccf
Compare
@yurishkuro I added this as the last requirement to the doc. |
Reviewers, I need more comments or one more approval to merge this. |
Moving the design goals and requirements docs from opentelemetry-proto to this repository. The design goals and requirements will help us design the right wire protocol. The design goals part has been already approved and merged in opentelemetry-proto, the requirements part were under review here: open-telemetry/opentelemetry-proto#22
2a77ccf
to
fd5730d
Compare
Since this is going directly into Specification, I still find it problematic that we're conflating encoding and transport under the single "wire protocol" term. When I first saw the PR title I thought this would be about data model and encoding, not the transport concerns like reliable delivery. I think it's reasonable to converge on the data model/encoding, but the transport is very dependent on specific user requirements |
@tigrannajaryan can you respond to @yurishkuro 's concerns. Thanks. |
@yurishkuro sorry for late response. I eliminated the term "wire protocol" in favour of simply "protocol", I agree it was not adding any value. I think it is important to define how the data will be sent over a particular transport and make that part of the specification. It does not preclude from specifying other transports that carry the same telemetry data in the future as extensions to the protocol. However, I feel that not having any transport defined at all in the protocol RFC would reduce the value of such RFC significantly because it will be essentially non-implementable in code. I am aiming to clearly define at least one transport in the RFC and specifically mention how it can be extended to other transports in the future. |
@yurishkuro if you are OK with current wording let's merge this PR. |
Thanks. |
…try#193) * Add OpenTelemetry protocol design goals and requirements Moving the design goals and requirements docs from opentelemetry-proto to this repository. The design goals and requirements will help us design the right wire protocol. The design goals part has been already approved and merged in opentelemetry-proto, the requirements part were under review here: open-telemetry/opentelemetry-proto#22 * Address PR comments
…try#193) * Add OpenTelemetry protocol design goals and requirements Moving the design goals and requirements docs from opentelemetry-proto to this repository. The design goals and requirements will help us design the right wire protocol. The design goals part has been already approved and merged in opentelemetry-proto, the requirements part were under review here: open-telemetry/opentelemetry-proto#22 * Address PR comments
Moving the design goals and requirements docs from opentelemetry-proto
to this repository.
The design goals and requirements will help us design the right wire protocol.
The design goals part has been already approved and merged in
opentelemetry-proto, the requirements part were under review here:
open-telemetry/opentelemetry-proto#22