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

Add more info about OTEL_SEMCONV_STABILITY_OPT_IN #4662

Merged
merged 5 commits into from
Jul 18, 2023
Merged

Add more info about OTEL_SEMCONV_STABILITY_OPT_IN #4662

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3cf3bbf
Add more info about OTEL_SEMCONV_STABILITY_OPT_IN
alanwest Jul 17, 2023
45a15c2
Update src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md
alanwest Jul 17, 2023
95eb622
Update src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md
alanwest Jul 17, 2023
472666e
Update CHANGELOG.md
alanwest Jul 17, 2023
6f69133
Update CHANGELOG.md
alanwest Jul 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,31 @@

## Unreleased

* Updated Semantic Conventions to [v1.21.0](https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md).
This library can emit either old, new, or both attributes.
Users can control which attributes are emitted by setting the environment
variable `OTEL_SEMCONV_STABILITY_OPT_IN`.
([#4537](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4537))
([#4606](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4606))
([#4660](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4660))
* The new HTTP and network semantic conventions can be opted in to by setting
the `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable. This environment
variable supports the following values:
* `http` - emit the new, stable HTTP and networking attributes, and stop
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* `http` - emit the new, stable HTTP and networking attributes, and stop
* `http` - emit the new, frozen (proposed for stable) HTTP and networking attributes, and stop

Copy link
Member Author

Choose a reason for hiding this comment

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

Consider updating the Java release notes 😄 https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases/tag/v1.27.0

emitting the old experimental HTTP and networking attributes that the
instrumentation emitted previously.
* `http/dup` - emit both the old and the stable HTTP and networking
alanwest marked this conversation as resolved.
Show resolved Hide resolved
attributes, allowing for a more seamless transition.
* The default behavior (in the absence of one of these values) is to continue
emitting the same HTTP and network semantic conventions that were emitted in
`1.5.0-beta.1`.
* Note: this option will eventually be removed after the new HTTP and
network semantic conventions are marked stable. At which time this
instrumentation can receive a stable release, and the old HTTP and
network semantic conventions will no longer be supported. A stable release of
this instrumentation will come no sooner than six months from now. This is to
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this condition? The spec only says this:

(stable next major version SHOULD NOT be released prior to October 1, 2023).

Copy link
Member Author

Choose a reason for hiding this comment

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

It says more, but we need to figure out how the following applies to us give we have never released a stable major version:

SHOULD maintain (security patching at a minimum) the existing major version for at least six months after it starts emitting both sets of attributes.

I'm open to removing this condition and saying we can release stable as soon as October, but I think it's important that we understand the intention behind this statement by the semantic conventions WG.

@trask do you have a moment to chime in here? My understanding is that .NET and Java are in slightly different situations.

Java apparently has been releasing 1.x versions of HTTP instrumentation and starting in version 1.27.0 of Java's instrumentation you support the OTEL_SEMCONV_STABILITY_OPT_IN option. Later this year Java plans to release 2.x. The above statement seems clear in Java's case in that you will continue to maintain 1.x for at least 6 months - perhaps with security patches.

.NET is not currently planning to do a 2.x release. This is because we have been consistent in using prerelease identifiers per semver - i.e., 1.5.0-beta, 1.6.0-beta, 1.7.0-rc, 1.8.0-rc, etc. Our plan is that at some version, we will drop the prerelease identifier - for example, this might be version 1.9.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.

Just making sure we plan for the worst case scenario here. @utpilla let's say we release stable with 1.9.0 and then some security patch is required. I think we should plan then to release a 1.8.0-rc.2 hotfix with the security fix. Sound good?

Copy link
Member

Choose a reason for hiding this comment

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

Not clear on this I think we should plan then to release a 1.8.0-rc.2 hotfix with the security fix.

I think we should remove the 6 month constraint and should not put the date here. For .NET as we never released 1.0 so that 6 month condition is not applicable in our case. We are adding this option for smoother transition, users who want a longer transition time period can continue to use the pre-release version.

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 think we should remove the 6 month constraint

Yes, agreed this is fine and I can update the wording...

Not clear on this I think we should plan then to release a 1.8.0-rc.2 hotfix with the security fix.

but our mutual understanding on this point is important. The smooth transition period should allow users who are still dependent on the old conventions to use a version of the instrumentation that is free from security issues. I find this scenario unlikely, but we should at least have a plan for what to do.

Copy link
Member

Choose a reason for hiding this comment

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

Got it, I Agree.

Copy link
Member

Choose a reason for hiding this comment

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

Just making sure we plan for the worst case scenario here. @utpilla let's say we release stable with 1.9.0 and then some security patch is required. I think we should plan then to release a 1.8.0-rc.2 hotfix with the security fix. Sound good?

I think this approach most aligns with the intent.

e.g., the OTEL_SEMCONV_STABILITY_OPT_IN language was designed to apply to also languages which were still on 0.x, and the desire was for those languages to continue security patching 0.x for 6 months after they release 1.0

Copy link
Contributor

Choose a reason for hiding this comment

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

Here is my understanding for how .NET team will follow the spec guidelines:

SHOULD maintain (security patching at a minimum) the existing major version for at least six months after it starts emitting both sets of attributes.

We do a 1.5.1-beta.2 release of the instrumentation libraries this week which would be the first release that supports the OTEL_SEMCONV_STABILITY_OPT_IN configuration. We would support this version (security patches or more) for the next six months even if we end up doing the stable release of instrumentation libraries before this six month duration.

SHOULD drop the environment variable in the next major version (stable next major version SHOULD NOT be released prior to October 1, 2023).

We are not bound by the six months duration requirement of supporting 1.5.1-beta.2 to do a stable release. That is, we could do a stable release earlier than six months from 1.5.1-beta.2 release (while still continuing to support 1.5.1-beta.2 for its intended six months duration).

allow for a transition period for users to experiment with the new semantic
conventions and adapt as necessary. Refer to the specification for more
information regarding the new HTTP and network semantic conventions for both
[spans](https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-spans.md)
and
[metrics](https://github.com/open-telemetry/semantic-conventions/blob/v1.21.0/docs/http/http-metrics.md).
([#4537](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4537),
[#4606](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4606)),
[#4660](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4660))

* Fixed an issue affecting NET 7.0+. If custom propagation is being used
and tags are added to an Activity during sampling then that Activity would be dropped.
Expand Down