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

Mark initial set of HTTP semantic conventions as frozen #105

Merged
merged 4 commits into from
Jun 23, 2023

Conversation

trask
Copy link
Member

@trask trask commented Jun 13, 2023

@reyang reyang marked this pull request as ready for review June 22, 2023 06:45
@reyang reyang requested review from a team June 22, 2023 06:45
@reyang
Copy link
Member

reyang commented Jun 22, 2023

Hopefully this is the last PR for https://github.com/orgs/open-telemetry/projects/41/views/4.

image

@reyang reyang requested review from arminru, jsuereth and lmolkova June 22, 2023 06:49
@reyang reyang merged commit 7add90d into open-telemetry:main Jun 23, 2023
@pellared
Copy link
Member

pellared commented Jun 23, 2023

I kindly suggest taking a look at #128 before going Stable.

When I read feature-freeze definition I feel that it may be too late. Hopefully, I am wrong.

Personally, I do not feel comfortable that so many attributes are marked as Required and their values may contain PII. Take notice that it may not be easy to use a Collector to get rid of the attributes e.g. "mobile" instrumentation (Android, iOS).

@reyang
Copy link
Member

reyang commented Jun 23, 2023

When I read feature-freeze definition I feel that it may be too late. Hopefully, I am wrong.

https://github.com/open-telemetry/opentelemetry-specification/blob/v1.21.0/specification/document-status.md#feature-freeze

Changes that address production issues with existing features are still accepted.

@pellared
Copy link
Member

Changes that address production issues with existing features are still accepted.

I just want to share that the example of "mobile instrumentation" and PII is a real problem for one of our customers 😉

I do not have capacity to propose any PR in the following days (maybe even weeks). But I proposed some ideas in #128

@Oberon00
Copy link
Member

Changes that address production issues with existing features are still accepted.

I don't think this is meant for new production issues and critical things. Otherwise a missing feature could also cause production issues.

@Oberon00
Copy link
Member

If we want to address PII things, we need to lift the feature freeze (I think this will happen after stabilization is complete & released?)

@@ -55,6 +55,8 @@ operations. By adding HTTP attributes to metric events it allows for finely tune

### Metric: `http.server.duration`

**Status**: [Experimental, Feature-freeze][DocumentStatus]
Copy link
Member

Choose a reason for hiding this comment

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

@trask what does it mean for a metric to have both experimental and Feature freeze simultaneously?

Copy link
Member

@reyang reyang Aug 8, 2023

Choose a reason for hiding this comment

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

Feature-freeze is orthogonal to Experimental/Stable https://github.com/open-telemetry/opentelemetry-specification/blob/ad1537a46eb856eeb915786fd7cf81e07bdeb426/specification/document-status.md?plain=1#L19.

(personally I find it counter intuitive - since only "Experimental", "Experimental + Feature-freeze" and "Stable" would make sense)

@trask trask deleted the http-frozen branch August 14, 2023 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mark HTTP semantic conventions as Feature-freeze
9 participants