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

Prohibit usage of retired names in semantic conventions #2191

Merged

Conversation

tigrannajaryan
Copy link
Member

This change adds a prohibition clause that requires that no old
metric or attribute name is used for a new attribute.

This is important to ensure reversibility of schema transformation
(converting from a new version to an old version of schema).

Without this restriction the following is possible:

Schema version 1. Attribute A exists.
Schema version 2. Attribute A is renamed to B. Appropriate schema file is created.
Schema version 3. Attribute A is introduced (a completely different new attribute).

Now attempting to go from Version 3 to version 1 is impossible since it requires
renaming B to A (for the change in version 2), but a different attribute A already exists.

@tigrannajaryan tigrannajaryan requested review from a team as code owners December 3, 2021 14:31
@tigrannajaryan tigrannajaryan force-pushed the add-limit-tosechamarenames branch 2 times, most recently from f638843 to 63c9e8f Compare December 3, 2021 14:40
@Oberon00
Copy link
Member

Oberon00 commented Dec 3, 2021

Is it expected that lossless conversion from new to old versions works? That does not seem like a feasible thing to guarantee to me.

@yurishkuro
Copy link
Member

Is it expected that lossless conversion from new to old versions works? That does not seem like a feasible thing to guarantee to me.

I agree, at least there needs to be some grace period after which earlier versions of the schema should be considered unreachable and the old name could be used again.

@tigrannajaryan
Copy link
Member Author

I agree that we probably want to have a retirement policy longer term. But until we have it this is a relatively cheap way to ensure backward conversions to old versions work.

Backward conversions are very important for backends. The backend often will understand a specific telemetry version. When Otel releases a new version the backend cannot be expected to immediately be updated and start understanding that new version. This means backwards conversions are going to be fairly typical and I think if we we can ensure they are guaranteed to work we should do it. I think this particular limitation is worth it.

Besides, it is generally a good practice to follow to avoid confusion. Again, I agree that over long periods of time likely you want to have a different retirement policy, e.g. something that is unused for years can be probably reused. Once we have that policy we can rethink the reuse prohibition rule.

Copy link
Member

@Oberon00 Oberon00 left a comment

Choose a reason for hiding this comment

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

@arminru @dyladan, others at Dynatrace and me discussed this PR shortly. We do not really agree with the motivation, or rather, we do not have that use case yet. But we think that adding this restriction does no harm and is beneficial also for humans to avoid confusing scenarios.

@arminru arminru added area:semantic-conventions Related to semantic conventions area:versioning Related to specification versioning labels Dec 13, 2021
This change adds a prohibition clause that requires that no old
metric or attribute name is used for a new attribute.

This is important to ensure reversibility of schema transformation
(converting from a new version to an old version of schema).

Without this restriction the following is possible:

Schema version 1. Attribute A exists.
Schema version 2. Attribute A is renamed to B. Appropriate schema file is created.
Schema version 3. Attribute A is introduced (a completely different new attribute).

Now attempting to go from Version 3 to version 1 is impossible since it requires
renaming B to A (for the change in version 2), but a different attribute A already exists.
@SergeyKanzhelev SergeyKanzhelev merged commit dfd7915 into open-telemetry:main Dec 13, 2021
@tigrannajaryan tigrannajaryan deleted the add-limit-tosechamarenames branch December 13, 2021 18:06
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 area:versioning Related to specification versioning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants