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

Add transition plan for upcoming breaking changes to the unstable HTTP semantic conventions #3381

Closed
wants to merge 3 commits into from

Conversation

trask
Copy link
Member

@trask trask commented Apr 11, 2023

Part of #3362

I'd like to get consensus in this PR on the high-level transition plan.

Changes

Adds transition plan for upcoming breaking changes to the unstable HTTP semantic conventions.

@trask trask force-pushed the transition-plan branch 2 times, most recently from c3eef69 to 36c100f Compare April 11, 2023 19:01
@trask trask marked this pull request as ready for review April 11, 2023 19:12
@trask trask requested review from a team April 11, 2023 19:12
>
> After October 1, 2023, HTTP instrumentation published from
> [OpenTelemetry repositories](https://github.com/orgs/open-telemetry/repositories)
> which adopt the latest HTTP semantic conventions SHOULD provide 3-months of overlap where
Copy link
Contributor

@lmolkova lmolkova Apr 11, 2023

Choose a reason for hiding this comment

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

is my understanding correct that the overlap time starts on Oct 1 and completes on Jan 1 2024 for everyone? I.e. if I add new instrumentation on Dec 1, I only need one month of overlap and if I ship it on Feb 1 2024, I don't need to worry about 1.20.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

the idea here is to create an overlap period when any instrumentation upgrades from the old to new HTTP semconv, so that when users upgrade to a new version and are surprised by the change, they can opt-out of the new semconv and continue emitting the old semconv for some time

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed some clarifications

Copy link
Contributor

Choose a reason for hiding this comment

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

Just clarifying: It seems that the answer is whenever I ship instrumentation (even if it's the first version and even if it's 2030), I should still offer 3 month grace period?

If so, I hope we can remove this warning at some point once 1.20.0 support becomes irrelevant

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh ya, I need to clarify some more, the intention is that this only applies to instrumentations that previously supported the old semconv and are now updating to the new semconv.

@arminru arminru added semconv:HTTP area:semantic-conventions Related to semantic conventions labels Apr 12, 2023
@@ -26,6 +26,20 @@ operations. By adding HTTP attributes to metric events it allows for finely tune

<!-- tocstop -->

> **Warning**
> HTTP instrumentation SHOULD NOT emit HTTP semantic conventions later than
> [v1.20.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/metrics/semantic_conventions/http-metrics.md)
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit unclear. What does "later than v1.20.0" mean? How are instrumentation developers supposed to know if a particular convention they look at is "later than v1.20.0" or not? Do we plan to label conventions with version number they were introduced from in this document?

Copy link
Member Author

Choose a reason for hiding this comment

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

How are instrumentation developers supposed to know if a particular convention they look at is "later than v1.20.0" or not?

I was thinking by looking at the link I provided here that points to v1.20.0 version of the HTTP semantic conventions page.

I can definitely spell that out clearer in this warning.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, please make it clearer.

Copy link
Member

@joaopgrassi joaopgrassi Apr 17, 2023

Choose a reason for hiding this comment

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

I'm also a bit confused by the sentence, had to read it multiple times but still not sure so here it goes 😅

HTTP instrumentation SHOULD NOT emit HTTP semantic conventions later than 1.20

If my understanding is correct, this means until August 1 2023, instrumentations should NOT emit conventions newer than 1.20? (or conventions that were added/modified after 1.20)?

Not sure why but the "later" word there just messed with my brain.

> **Warning**
> HTTP instrumentation SHOULD NOT emit HTTP semantic conventions later than
> [v1.20.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/metrics/semantic_conventions/http-metrics.md)
> until August 1, 2023 (an opt-in flag to emit the latest HTTP semantic conventions
Copy link
Member

Choose a reason for hiding this comment

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

I would highly prefer to extend this period to 6 months.

@trask
Copy link
Member Author

trask commented Apr 17, 2023

Closing in favor of #3404

@trask trask closed this Apr 17, 2023
@trask trask deleted the transition-plan branch April 17, 2023 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions semconv:HTTP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants