opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

[Discussion] Handling forward and backward compatibility dependency ranges

Open MSNev opened this issue 1 year ago • 3 comments

As part of the Nov 8th, 2023 meeting we had a discussion about the @opentelemetry/api version ranges and whether it should be a peerDependency or a standard dependency. This issue is a continuation of that discussion with a proposal about how to handle this moving forward with SDK v2.x.

Simplistically, the issue "seems" to be more about what are we guaranteeing with this version range definition

      "peerDependencies": {
        "@opentelemetry/api": ">=1.0.0 <1.5.0"
      }

As this is effectively defining both forward compatibility vs backward compatibility, while we can't necessarily easily change this for the existing usages of v1.x of the SDK (implementation) we should consider how we want to handle this moving forward with the v2 SDK and all other components.

Definitions

To start let me define what I mean by forward and backward compatibility.

Forward compatibility

Provides the ability of a "newer" component (someone / thing, wants to use a newer version of a component (like the API)) but it will continue to work with an "older" implementation that was created for an older version (so someone / thing wanting to use a v1.9 "feature" but will continue to work happily with an implementation of only v1.0 version).

Backward compatibility

Provides for the ability of an "older" component (someone / thing, wants to use a older version of a component (like the API)) but it will continue to work with any "newer" implementation that was created for newer version (so someone / thing designed for v1.0 of the API will continue to work with an implementation of the newer version of the API).

Because we are declaring the range "@opentelemetry/api": ">=1.0.0 <1.5.0" we are effectively declaring that we are providing "both" forward and backward compatibility from v1.0 but only up to v1.4.x, so you can have any implementation of any of these versions and it will work with any combination of the range. And as a peerDependency this further enforces all of the combinations.

Proposal

While we MUST explicitly support Backward Compatibility for all versions of the same major number (ie. v1.x.x) but we SHOULD NOT support Forward Compatibility beyond the "base" minimum. This WILL require that for each new release of the components we MUST lift the "base" version to the current released version, so we are declaring that we are supporting forward compatibility with implementations from this released version.

So to summarize

  • Change the range to be more encompassing to define that we will provide full "backward compatibility" support (implicit as part of semconv) up until v2.x and document that all users SHOULD also use the same range eg. "@opentelemetry/api": ">=1.x.x <2.0.0" (or just ^1.x.x)
  • Whenever we release "newer" versions of the components we lift ALL of the minimum "base" versions (the 1.x.x) to be the new released version. So we are again guaranteeing backward compatibility up to the maximum but NOT forward compatibility (implementation) with any version prior to this new base version. So it would become "@opentelemetry/api": ">=1.x.5 <2.0.0" (or just ^1.x.5) (or any minor version bump)
  • Change the usage of peerDependency to dependency (Complications exist)
    • Only really required IF the component is using a concrete implementation of the dependency, if it is only using interfaces / types then it can remain as a peerDependency (but this is not ideal)

So to guarantee backward compatibility we MUST

  • Only make changes to interfaces / base classes in a compatible manner
    • new properties / functions MUST be optional
  • All usage of these new properties / functions (within the implementations) MUST check for their existence and handle when they are not present (the backward compatibility checks)
  • change existing exported function signatures (in a compatible manner)
    • eg. arguments can take multiple "types" or have new optional/default arguments, but the code has to check for the type / presence of properties / arguments.
      • Note: This can still cause typing issues (when an existing argument takes new multiple or extension definitions), but as long as all components have their "base" version lifted this can be resolved.
      • And we can declare that any generated TypeScript warnings can be safely ignored (eg. cast as any etc -- not ideal for all projects)

But it means we can safely use the following from the new base version onwards.

  • export new interfaces
  • export new functions / helpers
  • export new implementations of the interfaces / classes

And then the components using this new "base" can safely just use the new interfaces / functions (as long as they are defined as dependencies -- or peerDependencies in the limited case above)

Complications

There are of course some complications with this proposal, which are not necessarily "small" or easily fixed, based on the guarantees' already defined.

By declaring the usages as dependencies it is very likely that some projects (because of the historical ranges) may or WILL end up with multiple implementation versions embedded within their packages (not ideal). And each package / bundle will reference their own directly included implementations (so anything using the "newer" version will be calling the "newer version" (of that component) for an "included" features / functions.) This is part of the packaging / bundling process.

When multiple versions of a component are used the implementation ("objects") passing between the versions (should) be compatible (based on the backward compatible declaration (passing from old -> new implementation)), but of course they WILL break in a forward compatible (attempt to use new features/signatures -> with old implementation) scenario, we SHOULD explicitly NOT support this later case. And when (not if) any cases arising from this combination, we say it is explicitly not supported and the user of these incompatible combinations SHOULD be able to move (update their package) to the latest released (implementation) version which will provide for this backward compatibility and also resolving any of the forward compatibility issues.

  • Yes this is a HARD call.

What this doesn't resolve

  • Vender/Component A creates their own implementation of some component / interface (API for example) version X.
    • This is effectively fixing the implementation to this version X
  • Application defines or wants to use a component/interface Y (X+1 etc) and this uses features / functionality that are only available with version Y implementations, so when the Application "initializes" with Vender A's component (version X capability) the application with either
    • fail to resolve packages or compile because of package dependencies (best case scenario)
    • receive a No-Op version at runtime and silently do nothing (because there is now multiple package versions) -- not ideal
    • Throw exception(s) when they start using / passing unexpected values to the "older" implementation (really not ideal at all)

In all cases there are simplistically 2 ways to "handle" this

  • The application MUST stay on the base version to the Vender A implementation and cannot upgrade to anything that uses newer feature Y capability
  • The Vender MUST release an updated version to support the newer version (implementation) or Y.

This will require that the Vender MUST release a new version of their component (with the new base version) to allow the application to updateor use newer instrumentations with this new version (and any other dependencies that are required to support this new version).

The No-Op implementation scenario

To resolve this we will need to update the register / fetch global compatibility layer to be able to better handle this scenario and enforce the backward compatibility checks with an option to not necessarily return a No-Op version but to throw an exception (or other mechanism) to indicate that the implementation(s) are not compatible with the version of the API (or other component) that is being used. This of course goes against the existing specification, so it may need to be done via an optional "please fail if not compatible" flag that can be used during development / testing to ensure that the application is not using incompatible versions.

There are other possible scenarios / solutions that we can discuss as part of this issue

MSNev avatar Nov 08 '23 20:11 MSNev

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[bot] avatar Jan 22 '24 06:01 github-actions[bot]

To resolve this we will need to update the register / fetch global compatibility layer to be able to better handle this scenario and enforce the backward compatibility checks with an option to not necessarily return a No-Op version but to throw an exception (or other mechanism) to indicate that the implementation(s) are not compatible with the version of the API (or other component) that is being used.

I'm wondering if there's be interest in adding a diag.debug to https://github.com/open-telemetry/opentelemetry-js/blob/6bb2f16d8589ffaf1b991752e6c086b2c36bf656/api/src/internal/global-utils.ts#L75 when it fails a isCompatible check?

I was debugging a silent failure due to the Node.js AWS lambda layer having a minor version smaller than the later imported API version - which lead to the proxy tracer resolving to the no op tracer/creating no op spans in a very confusing manner. Since I already had the debug log level enabled to debug if it was a sampler issue, an extra debug log warning about incompatible API versions when the API is trying to grab the global trace provider would be extremely helpful.

Imo a higher log level such as warn or error can even be warranted since failing to return a global due to incompatible API versions can lead to a variety of silent failures.

I can open a PR if that'd be helpful to others.

MikeShi42 avatar Jun 19 '24 10:06 MikeShi42

Related:

  • #4832

JamieDanielson avatar Aug 07 '24 14:08 JamieDanielson