open-telemetry / opentelemetry-go-contrib

Collection of extensions for OpenTelemetry-Go.
https://opentelemetry.io/
Apache License 2.0
1.13k stars 539 forks source link

HTTP Semconv Migration Plan #5132

Open MadVikingGod opened 6 months ago

MadVikingGod commented 6 months ago

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

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 commented 5 months ago

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 commented 3 months ago

Alternative Plan:

Unknowns

dashpole commented 3 months ago

Is that roughly the equivalent of using OTEL_HTTP_CLIENT_COMPATIBILITY_MODE, with

Current:

Current + 1:

Current + 2:

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 commented 3 months ago

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

also check out https://github.com/open-telemetry/semantic-conventions/issues/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 commented 1 month ago

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 commented 1 month ago
  • 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 commented 1 month ago

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

MadVikingGod commented 1 month ago

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 commented 4 weeks ago

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