API Breaking Change Postmortem

[dyladan]

Refs:

• recordException should allow additional attributes #4070
• feat(api): add attributes argument to recordException API #4071
• chore: prepare release API 1.5.0, SDK 1.16.0/0.42.0 #4122
• Changes on 9-11-23 break dd-trace #4136
• Revert "feat(api): add attributes argument to recordException API (#4071)" #4137
• chore: release API 1.6.0/SDK 1.17.0/Experimental 0.43.0 #4138

In version 1.5.0 of @opentelemetry/api a change was released which was breaking for users of dd-trace. The change was reverted and 1.6.0 was released with the fix. 1.5.0 was deprecated in NPM with the note "Version 1.5.0 of @opentelemetry/api mistakenly contained a breaking change for some users. Please update to 1.6.0 or later."

Span#recordException was updated from recordException(exception, time) to also allow recordException(exception, attributes, time). dd-trace, which implemented the original Span interface, expected the second argument to be time but after the change it was possible for it to be attributes or time. The new third argument is ignored by an SDK which implements the old interface.

This break does not affect users of the OpenTelemetry JS SDK because the SDK components which implement API interfaces specify a maximum API version. For example, the package @opentelemetry/[email protected] depends on @opentelemetry/api@>=1.0.0 <1.5.0. This means that the release of @opentelemetry/[email protected] is NOT automatically picked up by @opentelemetry/[email protected]. dd-trace depended on @opentelemetry/api@^1.0.0 which is satisfied by any 1.x version.

The OTel JS authors were previously, mistakenly, under the assumption that API implementers would be expected to update when any minor change was made to the API. The versioning and stability guidelines of OpenTelemetry state (emphasis added):

Backward-incompatible changes to API packages MUST NOT be made unless the major version number is incremented. All existing API calls MUST continue to compile and function against all future minor versions of the same major version.

Proposed Changes

1. Update all SDK implementation packages to remove the API maximum version. This means that the OpenTelemetry SDK will continue to work and be tested with any new API release.
2. Updates to existing API interfaces MUST NOT require changes to the SDK in order for the API and SDK to continue to function.

---

/cc @ksstoneware

Maintainers assigned for visibility

[dyladan]

I would like to point out that the versioning guidelines only specify that callers should not be broken and does not specifically state anything about third party SDK developers. The guidelines should be updated to state whether or not third party SDKs should be treated with the same stability guarantees as end-users. I consider the above proposed changes to be temporary until they are either codified in the official spec stability guidelines.

[Flarna]

If I remember correct we had already a discussion in this regard.

SemVer minor in API tends to be breaking towards SDK because usually a new/enhanced API requires an implementation in SDK. Sure, there are cases where API just adds some wrapper/something new which works standalone (e.g. getActiveBaggage, getActiveSpan,...) so that's fine.
But there are others like adding an optional parameter to an existing API (e.g. open-telemetry/opentelemetry-js-api#129) which requires SDK adaption to really work.
Even if new APIs have a default implementation in API it would be quite strange for an API user to end up with having an "partial SDK" installed for a signal.

The OTel SDK hosted here uses a version range for exactly this reason see here.

[Flarna]

refs: #2769

[dyladan]

The spec 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. Our existing practice of defining a maximum API level for a given SDK should continue and we should make it clear to third party SDKs that this is the case.

[dyladan]

Made an issue on dd-trace DataDog/dd-trace-js#3654

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[github-actions]

This issue was closed because it has been stale for 14 days with no activity.