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

Add backward compatibility check mode #271

Merged
merged 11 commits into from
Feb 24, 2024

Conversation

lmolkova
Copy link
Contributor

Implements back-compat checks against released version
See open-telemetry/semantic-conventions#740 for the context

Usage:

-f path/to/semantic-conventions/model compatibility --previous-version 1.24.0

If I run it against current semconv main, I get

Found 5 compatibility issues:
                 attribute 'system.processes.status': was removed
                 metric 'system.processes.count': was removed
                 metric 'system.processes.created': was removed
                 attribute 'container.labels': was removed
                 attribute 'k8s.pod.labels': was removed

@lmolkova lmolkova requested review from a team February 20, 2024 22:33
@lmolkova lmolkova changed the title Add compatibility check mode Add backward compatibility check mode Feb 20, 2024
@lmolkova lmolkova force-pushed the add-compat-check-mode branch 10 times, most recently from 0744a57 to fd3be67 Compare February 20, 2024 23:29
Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

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

This is a great additiion! Thanks for putting this together.

  • I echo Alexandra's concerns around download success. However, I don't feel the need to reapprove on fix.
  • I had one other comment on additioinal stability checking we can do, but consider it non-blocking.

Copy link
Member

@joaopgrassi joaopgrassi left a comment

Choose a reason for hiding this comment

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

This is great! Thanks for working on it.

Just a question I had while reviewing/reading the issue: Who is the target group of this new task in the tool? Like, when is it going to be executed? During a release of semconv? By SDK authors? Ran automatically as part of a CI/CD in semconv? It's not clear and maybe this should be stated somewhere?

@lmolkova
Copy link
Contributor Author

@joaopgrassi my intention is to add it to CI checks - here's draft PR that demonstrates it open-telemetry/semantic-conventions#764

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.

4 participants