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.

Sign up for GitHub

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

Jump to bottom

Versioning and Stability for OpenTelemetry #1291

Merged
merged 48 commits into from Jan 20, 2021
Merged

Versioning and Stability for OpenTelemetry #1291

merged 48 commits into from Jan 20, 2021

Conversation

tedsuo
Copy link
Contributor

@tedsuo tedsuo commented Dec 14, 2020 •
edited

Changes

  • Adds new versioning procedures and stability guarantees for the OpenTelemetry Clients.
  • Expands the overview to include a high level description of the OpenTelemetry client architecture.
  • Expands the glossary and adds clarifications.

Related issues

Related OTEPS

@tedsuo tedsuo requested review from a team as code owners December 14, 2020 21:39
anuraaga
anuraaga reviewed Dec 15, 2020
View reviewed changes
specification/glossary.md Outdated Show resolved Hide resolved
specification/versioning-and-stability.md Outdated Show resolved Hide resolved
specification/versioning-and-stability.md Outdated Show resolved Hide resolved
specification/versioning-and-stability.md Outdated Show resolved Hide resolved
specification/versioning-and-stability.md Outdated Show resolved Hide resolved
jsuereth
jsuereth approved these changes Dec 15, 2020
View reviewed changes
Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

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

Thanks for the heroic effort to define this @tedsuo! My comments are only "issues", but in general this a huge step for the community and I'm really glad to see it happen.

My comments are mostly things I think we should open issues for and resolve via separate discussions, but just wanted to check to see if others agree.

specification/versioning-and-stability.md Outdated Show resolved Hide resolved
specification/versioning-and-stability.md Outdated Show resolved Hide resolved
specification/versioning-and-stability.md Outdated Show resolved Hide resolved
@carlosalberto carlosalberto mentioned this pull request Dec 16, 2020
Merged
jkwatson
jkwatson reviewed Jan 4, 2021
View reviewed changes
specification/glossary.md Outdated Show resolved Hide resolved
jkwatson
jkwatson approved these changes Jan 4, 2021
View reviewed changes
Copy link
Contributor

@jkwatson jkwatson left a comment

Choose a reason for hiding this comment

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

👍 Thanks! A couple of small clarifications, but this generally is excellent.

jkwatson
jkwatson reviewed Jan 4, 2021
View reviewed changes
specification/overview.md Outdated Show resolved Hide resolved
tigrannajaryan
tigrannajaryan reviewed Jan 7, 2021
View reviewed changes
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

Ted, thanks a lot for leading this!

specification/error-handling.md
@@ -62,7 +62,7 @@ The mechanism by which end users set or register a custom error handler should f
### Examples

These are examples of how end users might register custom error handlers.
Examples are for illustration purposes only. Language library authors
Examples are for illustration purposes only. OpenTelemetry client authors
Copy link
Member

Choose a reason for hiding this comment

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

Did we discuss this renaming and make a decision on this? I am not against the change but it seems to me it is not a clear improvement over previous wording ("client" can be a confusing term).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, no group decision here. I began using the term "OpenTelemetry client" because "language library" wasn't clear - we have many libraries in each language, and none of these libraries contain a language. "Client" is a more common term for the set of components we are referring too. Open to other options.

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 haven't been able to elicit much comment on this one. There's some preference for "OpenTelemetry client" over "language library" as it is a more standard term 🤷‍♀️

Copy link
Member

Choose a reason for hiding this comment

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

How is "OpenTelemetry client" different from "OpenTelemetry SDK", which is what we generally use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yurishkuro OpenTelemetry client replaces "language library." It contains: API, SDK, SemConv, and Contrib.

specification/glossary.md Show resolved Hide resolved
specification/glossary.md Show resolved Hide resolved
specification/versioning-and-stability.md Outdated Show resolved Hide resolved
@andrewhsu andrewhsu mentioned this pull request Jan 12, 2021
Open
7 tasks
MrAlias
MrAlias approved these changes Jan 12, 2021
View reviewed changes
specification/library-guidelines.md Outdated Show resolved Hide resolved
specification/overview.md Outdated Show resolved Hide resolved
specification/overview.md Outdated Show resolved Hide resolved
specification/overview.md Outdated Show resolved Hide resolved
specification/versioning-and-stability.md Outdated Show resolved Hide resolved
specification/versioning-and-stability.md Outdated Show resolved Hide resolved
specification/versioning-and-stability.md Outdated Show resolved Hide resolved
andrewhsu
andrewhsu approved these changes Jan 13, 2021
View reviewed changes
Copy link
Member

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

LGTM

i put in minor suggestions for changes to fix grammar and make things more clear, but they are not blockers in my mind

tigrannajaryan
tigrannajaryan approved these changes Jan 18, 2021
View reviewed changes
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

LGTM

@tedsuo
Copy link
Contributor Author

tedsuo commented Jan 20, 2021

Thank you everyone for all of the input. @open-telemetry/technical-committee can I get a merge? I believe this is complete for now.

Future work:

  • Data stability: we need to advance the work in metrics farther before we can define this.
  • Opentelemetry client vs language library: terminology bikeshed anyone may take up if they care.

@carlosalberto
Copy link
Contributor

@tedsuo Thanks! Will merge now as all comments are resolved and we have had enough approvals for some days now.

Would you mind the issues for anything that is still needed? If there's still no issue for that, that is (as we have at least one issue regarding telemetry data already, etc).

cc @arminru @yurishkuro

@carlosalberto carlosalberto merged commit 1d54507 into open-telemetry:master Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chulkilee chulkilee chulkilee left review comments

@GregMefford GregMefford GregMefford left review comments

@anuraaga anuraaga anuraaga approved these changes

@jkwatson jkwatson jkwatson approved these changes

@iNikem iNikem iNikem approved these changes

@yurishkuro yurishkuro yurishkuro left review comments

@arminru arminru arminru left review comments

@jsuereth jsuereth jsuereth approved these changes

@andrewhsu andrewhsu andrewhsu approved these changes

@carlosalberto carlosalberto carlosalberto approved these changes

@jmacd jmacd jmacd approved these changes

@tigrannajaryan tigrannajaryan tigrannajaryan approved these changes

@cijothomas cijothomas cijothomas approved these changes

@MrAlias MrAlias MrAlias approved these changes

Assignees

@bogdandrutu bogdandrutu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add signal/concern to the Glossary