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

(draft) [api] document best-practices and limitations #4832

Open
pichlermarc opened this issue Jun 28, 2024 · 0 comments
Open

(draft) [api] document best-practices and limitations #4832

pichlermarc opened this issue Jun 28, 2024 · 0 comments

Comments

@pichlermarc
Copy link
Member

pichlermarc commented Jun 28, 2024

NOTE: draft state - information may be incomplete or incorrect

Description:

We currently do not document limitations and best-practices for API use. There are two (three?) groups of users that are subject to different limitations that we should document. This is a living document to collect things we should document. I think we should add this to the README.md of the @opentelemetry/api package.

(instrumentation/library authors): Caret depending on @opentelemetry/api is safe and encouraged

For instrumentation and library authors, depending on @opentelemetry/api is safe and encouraged. To maximize compability, we recommend to choose the lowest possible version for your caret range. (Author note: #4661 will help with determining which version is the lowest safe one). Using a version that's later than what the user chooses in their app will cause your library to receive a no-op implementation of the SDK, and that means that your library will not emit any telemetry. This is a safe-guard to avoid breaking users that use an older API to register their SDK.

Using a caret range will also allow for your package manager to de-dupe dependencies as typically more than one package uses @opentelemetry/api.

(instrumentation/library authors) API types of different API versions may not be compatible:

NPM allows the presence of two or more versions of the same package in the same app. As this is the case, and as API types get extended with new properties, it is not possible to pass instances generated by older API versions to new API versions, as new functions are not yet present on the old API's type.

Best practice:

Do not pass instances returned by @opentelemetry/api across package boundaries. If you have to (examples for this would be providing a utility package for instrumentation), ensure to depend on the lowest possible @opentelemetry/api version with a caret range to maximize compatibility.

Do:

// '@example/my-awesome-package'

import { metrics, Meter } from '@opentelemetry/api';

class Foo {
  private meter: Meter;
  constructor() {
    // if the version of the API is newer than the version the SDK was registered at, 
    // meter will be a safe no-op from the new API instance.
    meter = metrics.getMeter('my-meter');
  }
}

// my app
import { Foo } from '@example/my-awesome-package';
const foo = new Foo(); 

Do not:

// Do not:
import { metrics } from '@opentelemetry/api';
import { Foo } from '@example/my-awesome-package';

const myMeter = metrics.getMeter('my-meter');

// @example/my-awesome-package may depend on a newer version of the API
// the instance returned by the old API may not implement functionality expected by the new version of the API.
const foo = new Foo(myMeter); 

(Implementors): Caret depending on API is not safe

For implementors @opentelemetry/api does not follow SemVer for the purpose of including new features to existing interfaces. That means the addition of non-optional properties/methods on interfaces.

Best Practice:

Always specify a range of supported API versions. Since new non-optional properties may be included, ensure to at least specify a maximum (the current available version). This will prevent new releases of @opentelemetry/api from breaking your implementation, as the peer-dependency range will not be satisfied. Follow up with a new release that implements this feature as soon as possible.

The user should be able to choose their API version, therefore prefer peerDependency over dependency. Using dependency will pull in the old API version for your package and will cause transpilation errors for TypeScript and runtime errors in JavaScript. Using a peerDependency will cause install errors and let users know about the incompatibility.

(Author note: just came to my mind, we should probably come up with a release schedule for the API to ensure third-party implementors can plan implementing new features accordingly)

Do:

"peerDependency": {
  // upper limit 1.9.0, if the user installs 1.10.0 npm will error to let them know this is not compatible
  "@opentelemetry/api": ">=1.3.0 < 1.10.0"
}

Do not:

"dependency": {
  // this will pull in 1.3.0 for your SDK implementation. A user may depend on 1.4.0 and try to register your SDK. 
  // In TypeScript this will cause transpile errors. 
  // In JavaScript registration will pass, but methods added in 1.4.0 will be `undefined`, most likely causing the user's app to crash.
  "@opentelemetry/api": "1.3.0"
}
"peerDependency": {
  // will allow 1.4.0 to be installed by the user, but your implementation does not yet include methods added in 1.4.0
  // In TypeScript this will cause transpile errors.
  // In JavaScript registration will pass, but methods added in 1.4.0 will be `undefined`, most likely causing the user's app to crash.
  "@opentelemetry/api": "^1.3.0"
}
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

No branches or pull requests

1 participant