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

HTTP Semconv Migration Plan #5132

Closed
MadVikingGod opened this issue Feb 19, 2024 · 10 comments
Closed

HTTP Semconv Migration Plan #5132

MadVikingGod opened this issue Feb 19, 2024 · 10 comments

Comments

@MadVikingGod
Copy link
Contributor

MadVikingGod commented Feb 19, 2024

Reasoning

The HTTP Semantic Conventions made breaking changes from v1.20.0 to v1.21.0; this means that if we did a hard cutover in our instrumentation, any users with dashboards or alarms that used these attributes would break. To ease that, we propose the following plan to provide a path for users to migrate any existing dashboards.

Migration Plan

  • Current release - v0.49.0
    • Support only for v1.20.0 semantic conventions
  • v0.50.0
    • Add support for v1.20.0, v1.24.0, and both semantic conventions
    • Default to v1.20.0
    • Warn users when using v1.20.0
  • v0.51.0
    • Default to v1.24.0
    • Warn users when using v1.20.0 or both semantic conventions
  • v0.52.0
    • Remove support for v1.20.0
    • Remove support for both semantic conventions
    • All users will be using v1.24.0

User Migration Story

The goal is to allow users to upgrade versions of the otelhttp package and enable incremental migrations of dashboards and alerts.

When the user first upgrades to v0.49, they will receive a warning that they are using v1.20.0 semantic conventions. This indicates that action needs to be taken, or their dashboards and alerts will be affected.

They can then set the environment variable OTEL_HTTP_CLIENT_COMPATIBILITY_MODE to http/dup and receive both the original v1.20.0 attributes and the new v1.24.0 attributes. This will allow them to update their dashboards and alerts to use the new attributes and validate they are not missing any data.

@MrAlias
Copy link
Contributor

MrAlias commented Mar 29, 2024

This doesn't include any of the details discussed during yesterdays SIG meeting regarding rolling out the "opt-in" phase over multiple releases for all the instrumentation libraries.

@MadVikingGod
Copy link
Contributor Author

MadVikingGod commented May 29, 2024

Alternative Plan:

  • Current Version:
    • Always export the current version of the semantic convention.
    • Use an environment variable to export the current Version and an incomplete new version. This allows us to release a portion at a time. In this mode, it is acceptable to have two sets of allocations, one for the old code path and one for the new.
    • This is complete when:
      • All supported HTTP instrumentation have this environment variable
      • Both Server instrumentation and Client instrumentation produce duplicates
      • Metrics have support for both
  • Current Version + 1:
    • Always export the newest version of the semantic convention.
    • Use an environment variable to export the newest version and the old version. In this mode, it is acceptable to have two sets of allocations, one for the old code path and one for the new.
  • Current Version + 2:
    • Only export the newest version. Remove all ability to produce the old semantic convention.

Unknowns

  • Should we use the Same Environment Variable from the spec or a different one?

@dashpole
Copy link
Contributor

Is that roughly the equivalent of using OTEL_HTTP_CLIENT_COMPATIBILITY_MODE, with

Current:

  • Default = old
  • Supported = old, dup
  • Diff from current plan: no new

Current + 1:

  • Default = new
  • Supported = new, dup
  • Diff from current plan: no support for old

Current + 2:

  • Always new

That seems like it is still roughly in the spirit of the expected migration plan, and I wouldn't expect any pushback from the spec SIG. If that is accurate, I don't expect any pushback from anyone, but it is good to check. It strikes me as a little bit less work, although nothing crazy, since you still have to implement old, dup, and new at different points in time.

@trask
Copy link
Member

trask commented Jun 4, 2024

the one part that this doesn't quite address from the original plan is the 6 month "grace period" for users once the instrumentation starts emitting new semconv by default

the original plan addressed this by security patching the prior version for 6 months

I think your plan could also address this if

  • "Current Version + 1" supports option to emit only "old"
  • "Current Version + 2" is "some version that is released >= 6 months after Current Version + 1"

also check out open-telemetry/semantic-conventions#791 for other ideas that could help limit the implementation effort

(btw, at the end of the day, I support whatever plan the Go SIG community agrees to)

@VinozzZ
Copy link
Contributor

VinozzZ commented Aug 2, 2024

I'm curious on why we chose to use a different environment variable OTEL_HTTP_CLIENT_COMPATIBILITY_MODE instead of OTEL_SEMCONV_STABILITY_OPT_IN?

@MikeGoldsmith
Copy link
Member

MikeGoldsmith commented Aug 6, 2024

  • Should we use the Same Environment Variable from the spec or a different one?

@MadVikingGod Is there a reason why we would not want to use the spec'd environment variable?

@MrAlias MrAlias added this to the v1.29.0 milestone Aug 8, 2024
@MrAlias
Copy link
Contributor

MrAlias commented Aug 8, 2024

Going to hold the next release until we can resolve the environment variable issue.

@MadVikingGod
Copy link
Contributor Author

When this was first proposed there was discussion back and forward about this change, and that was the last choice made. It should be a single line change to switch the name, and I'm fine with it.

@dashpole
Copy link
Contributor

Added the PR to the 1.29 milestone, and removed this issue

@dashpole dashpole removed this from the v1.29.0 milestone Aug 15, 2024
dashpole pushed a commit that referenced this issue Aug 16, 2024
This change renames the environment variable used for compatibility mode
to `OTEL_SEMCONV_STABILITY_OPT_IN` as per the recommendation in the
spec.

Follow up from [this
comment](#5132 (comment)).

Co-authored-by: Aaron Clawson <MadVikingGod@users.noreply.github.com>
@MrAlias
Copy link
Contributor

MrAlias commented Oct 17, 2024

Closing. We have adopted this plan and are moving forward with it.

@MrAlias MrAlias closed this as completed Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

6 participants