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

Provide major/minor/patch version macros #2012

Closed
wjones127 opened this issue Mar 2, 2023 · 7 comments · Fixed by #2014
Closed

Provide major/minor/patch version macros #2012

wjones127 opened this issue Mar 2, 2023 · 7 comments · Fixed by #2014
Assignees

Comments

@wjones127
Copy link

wjones127 commented Mar 2, 2023

Is your feature request related to a problem?

In Apache Arrow Flight C++, we'd like to support a wide range of versions of opentelemetry-cpp. Sometimes there are breaking changes in the API, such as in #1761. It's cool that there are breaking changes (I appreciate the pace at which the project is evolving), except that in our project we don't have a way to check the version and easily adapt our code to support versions both before and after the change.

Describe the solution you'd like

It's common in many C++ projects to provide macros for:

  • OPENTELEMETRY_VERSION_MAJOR
  • OPENTELEMETRY_VERSION_MINOR
  • OPENTELEMETRY_VERSION_PATCH

Then, downstream libraries can write code like:

#if !(OPENTELEMETRY_VERSION_MAJOR <= 2 && OPENTELEMETRY_VERSION_MINOR < 5)
// Handle old API
#else
// Handle new API
#endif

Would you be open to providing these?

Describe alternatives you've considered

We could use CMake try_compile feature to detect what APIs are available at compile time, but that's quite burdensome to maintain.

@lalitb
Copy link
Member

lalitb commented Mar 2, 2023

@wjones127 - Is OPENTELEMETRY_SDK_VERSION macro something you can use -

https://github.com/open-telemetry/opentelemetry-cpp/blob/main/sdk/include/opentelemetry/sdk/version/version.h#LL8C9-L8C35

This get's updated for every release.

@wjones127
Copy link
Author

I don't think OPENTELEMETRY_SDK_VERSION is something we can compare against in the #if directives, which is why the MAJOR/MINOR/PATCH macros typically exist.

Though if we do some work in CMake, we might be able to extract these ourselves.

@wjones127
Copy link
Author

Though if we do some work in CMake, we might be able to extract these ourselves.

Nevermind, we don't have access to the version at configure time.

@lalitb
Copy link
Member

lalitb commented Mar 2, 2023

Yes it would be difficult to tokenize OPENTELEMETRY_SDK_VERSION macro.
I think we should add support for

  • OPENTELEMETRY_SDK_VERSION_* macros
  • make OPENTELEMETRY_SDK_VERSION available as cmake config.

@marcalff
Copy link
Member

marcalff commented Mar 3, 2023

To clarify, opentelemetry-cpp should provide versions for API and SDK, like this:

  • API

    • OPENTELEMETRY_VERSION_MAJOR
    • OPENTELEMETRY_VERSION_MINOR
    • OPENTELEMETRY_VERSION_PATCH
  • SDK

    • OPENTELEMETRY_SDK_VERSION_MAJOR
    • OPENTELEMETRY_SDK_VERSION_MINOR
    • OPENTELEMETRY_SDK_VERSION_PATCH

so that client code can adapt when incompatible changes are done in the API (for example, semantic conventions, links in spans currently under discussion) or in the SDK (for example, options for exporters)

@lalitb
Copy link
Member

lalitb commented Mar 3, 2023

Having separate versions for API and SDK would introduce more confusion, as we have single release version containing both of them. We can instead have one of above set of macros(say)

OPENTELEMETRY_VERSION_MAJOR
OPENTELEMETRY_VERSION_MINOR
OPENTELEMETRY_VERSION_PATCH

And the the guidelines for changing the version (as done now):

  • Patch version increment if there is no API/SDK breaking change in stable components. And also for any breaking changes in experimental components for API/SDK. Experimental components include - components in preview release (e.g., Logs API/SDK), or those marked as experimental as per the specs (e.g., semantic convention).
  • Minor version increment for any breaking changes in API/SDK. And in case of ABI breaking changes in API, also increment the OPENTELEMETRY_ABI_VERSION.
  • Major version will be consistent with the major version of specification release (as done in other SIGs)

@marcalff marcalff self-assigned this Mar 3, 2023
marcalff added a commit to marcalff/opentelemetry-cpp that referenced this issue Mar 3, 2023
@marcalff
Copy link
Member

marcalff commented Mar 3, 2023

Having separate versions for API and SDK would introduce more confusion, as we have single release version containing both of them. We can instead have one of above set of macros(say)

OPENTELEMETRY_VERSION_MAJOR
OPENTELEMETRY_VERSION_MINOR
OPENTELEMETRY_VERSION_PATCH

Agreed, this is sufficient because API and SDK are shipped together.

Implemented, please see PR.

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 a pull request may close this issue.

3 participants