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

OTel Author Note - API minor versions MAY contain additions to existing interfaces #3654

Open
dyladan opened this issue Sep 26, 2023 · 3 comments

Comments

@dyladan
Copy link

dyladan commented Sep 26, 2023

Refs #3624

In version 1.5.0 of @opentelemetry/api a change was released which extended the recordException API in a way that was backwards-compatible for end users but not for SDK implementations. Because dd-trace depends on a permissive version range (^1.0.0) of the API, users of dd-trace automatically received the latest API which failed to compile with the dd-trace SDK.

The OpenTelemetry specification is being updated with more clarity around this situation open-telemetry/opentelemetry-specification#3695

Short version: an SDK implementation should expect that minor versions of the API may include new methods. Minor version stability is guaranteed for users but not implementers. The OpenTelemetry authors recommend the following: dd-trace should define not only a minimum API level but also a maximum. This will ensure that users do not update to versions of the API which are not yet supported by the dd-trace. See an example of this in the upstream SDK here: https://github.com/open-telemetry/opentelemetry-js/blob/24997089daf6d422494d112cf5531fe115eaa201/packages/opentelemetry-sdk-trace-base/package.json#L94

@dyladan
Copy link
Author

dyladan commented Sep 26, 2023

Note that the stability guarantees made by the spec apply to all languages, not just JS. dd-trace should consider how to handle this in all languages, not just JS.

@pichlermarc
Copy link

FYI: the Specification PR to clarify the situation has merged a few months ago (see upgrading guidelines for SDK implementors). Since this repo implements a Trace SDK these guidelines also apply to this repo.

Over the past few months we've not made these changes to the API that would break dd-trace's SDK implementation. Unfortunately, this is not a permanent situation and we will have to keep moving forward with adding features to the API in order to deliver the latest features as defined by the OpenTelemetry Specification. In accordance with the specification these new features may be breaking for SDK implementors.

Please consider making this change and set a maximum API version in addition to the minimum.

@dyladan
Copy link
Author

dyladan commented May 15, 2024

We're going to move forward with this change next week. It looks to me like dd-trace hasn't done anything yet to address the issue. Please be aware this change is coming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants