Skip to content
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

Declare Trace part of the protocol as Stable #154

Closed
tigrannajaryan opened this issue Jun 4, 2020 · 7 comments · Fixed by #160
Closed

Declare Trace part of the protocol as Stable #154

tigrannajaryan opened this issue Jun 4, 2020 · 7 comments · Fixed by #160
Assignees

Comments

@tigrannajaryan
Copy link
Member

Problem

Trace part of the protocol is currently declared Beta. According to our maturity definitions this means we can make breaking changes at most once every 3 months.

Breaking changes in the APIs once every 3 months are annoying but can be reasonably dealt with. You can choose your time and update each application that uses the API to the newer version individually when you want.

Unfortunately the impact of a breaking change in the protocol is much higher. An incompatible protocol change in a deployed fleet of apps and Collectors is a headache and requires coordinated simultaneous updating of all participants of the network which in a large deployment can be very difficult or practically impossible to do.

This impacts the adoption of OTLP. I have been told by end users that they want to use OTLP because of its benefits but "Beta" guarantees are a showstopper.

Proposal

I would like us to declare Traces portion of the protocol as "Stable". The last breaking change we made to Traces portion of the protocol was 3 months ago, so there is not much churn happening in the Traces protocol.

There is currently one outstanging breaking change: #150. We need to make a decision on this (either accept or reject). After that I suggest that the Trace part of the protocol is declared "Stable".

We will still be able to improve OTLP, however only in backwards compatible manner (which is still pretty good given ProtoBuf's nice handling of newly added fields). Any truly incompatible additions will have to be implemented using versioning and graceful fallbacks. This slightly complicates the work but is not impossible to handle. Given that there is no anticipation of frequent breaking changes to the protocol I think this is an acceptable cost to pay to encourage OTLP adoption.

Note: this only applies to Traces part of the protocol. Metrics are still under active development and will stay in "Alpha" for now.

I would like to understand what people think about this proposal.

@tigrannajaryan
Copy link
Member Author

@open-telemetry/specs-approvers please comment.

@fbogsany
Copy link

fbogsany commented Jun 4, 2020

We (Shopify) want to start using OTLP throughout our trace collection pipeline. The Beta status is a blocker for us to roll it out in production. We’d very much like to see the Traces part of the protocol declared Stable.

@bogdandrutu
Copy link
Member

There is also the attributes part which is not fully finalized (no support for arrays as defined in specs), maybe that is the only blocker to declare stable.

@tigrannajaryan
Copy link
Member Author

@bogdandrutu yes, you are right. And we need to do that properly to make sure it is consistent in the future with the support for array values that Logs proto also need.
I am going to work on this and will aim to submit proposals for all breaking and anticipated changes as soon as I can.

@arminru
Copy link
Member

arminru commented Jun 9, 2020

Currently there are two outstanding breaking changes:
#150 (agreement required, @bogdandrutu I think they're waiting for your opinion on this)
#157 (uncontroversial, will hopefully merge soon, we should certainly include this in the first stable version)

@tigrannajaryan
Copy link
Member Author

#157 is now merged and #150 is discarded.

I went over the entire spec and do not see any other areas of the spec that require changes in the Trace protocol/schema now or in the near future.

I suggest that we therefore declare the Trace part of the protocol Stable. I will prepare a PR for that and we can also discuss in the next Spec SIG meeting.

tigrannajaryan pushed a commit to tigrannajaryan/opentelemetry-proto that referenced this issue Jun 22, 2020
@tigrannajaryan tigrannajaryan self-assigned this Jun 22, 2020
@carlosalberto
Copy link
Contributor

@tigrannajaryan open-telemetry/opentelemetry-specification#376 was re-opened (which is related to one of the mentioned PRs - the nested maps one).

I suggest we hold on a little bit till some discussion happens there (and I suggest attending the SIG call tomorrow, so we could discuss a little bit a few last-time doubts).

carlosalberto pushed a commit that referenced this issue Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants