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

fix!: standardize supported versions and set upper bound limit #2196

Merged
merged 62 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from 54 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
9d76835
feat(amqplib): cap supported versions to 1
blumamir May 10, 2024
af9d138
docs: update guideline with supportedVersions instructions
blumamir May 10, 2024
1602d53
refactor: align all supported version to common format
blumamir May 11, 2024
0c7a463
chore: markdown lint
blumamir May 11, 2024
bb8c2bb
chore: markdown lint
blumamir May 11, 2024
7b0e95d
docs: add supported versions for aws-sdk
blumamir May 11, 2024
7186912
fix(bunyan): README text
blumamir May 11, 2024
9d269ae
docs: update README
blumamir May 11, 2024
79f4fe8
chore: use caret for single major version
blumamir May 11, 2024
4e31da4
docs: fixes to README
blumamir May 11, 2024
0d238c3
docs: fix example in guidelines
blumamir May 11, 2024
0c07a5c
docs: task list in guidelines
blumamir May 11, 2024
548b8c4
docs: fix markdown
blumamir May 11, 2024
622a97c
fix: README nits
blumamir May 11, 2024
eecf73f
docs: add supported versions for node internal modules
blumamir May 11, 2024
47cdc01
docs: remove comment from redis packages
blumamir May 11, 2024
fa20292
chore: lint markdown
blumamir May 11, 2024
857fea3
docs: create a script to update node dist README automatically
blumamir May 11, 2024
8405555
docs: make supportedVersions md list
blumamir May 11, 2024
8455d96
ci: verify node dist readme
blumamir May 11, 2024
b84dbf3
ci: update script name
blumamir May 11, 2024
f2cb90d
chore: align all READMEs to use dash for list style
blumamir May 11, 2024
47713dd
chore: markdown lint
blumamir May 11, 2024
1ac55f1
fix(restify): forgotten supportedVersions
blumamir May 11, 2024
6a367bf
revert: remove script for auto instrumentation README generation
blumamir May 11, 2024
8582727
Update GUIDELINES.md
blumamir May 14, 2024
5e66cff
Update GUIDELINES.md
blumamir May 14, 2024
c94ea20
Update GUIDELINES.md
blumamir May 14, 2024
f471dff
Update plugins/node/instrumentation-amqplib/src/amqplib.ts
blumamir May 14, 2024
c188978
fix: typo
blumamir May 14, 2024
b354455
chore: use Node.js in texts
blumamir May 14, 2024
88ab4fa
Merge remote-tracking branch 'upstream/main' into supported-versions
blumamir May 14, 2024
e5ca81b
Update GUIDELINES.md
blumamir May 15, 2024
5b5375a
Update GUIDELINES.md
blumamir May 15, 2024
eba1e0d
Update GUIDELINES.md
blumamir May 15, 2024
096f5c5
Update GUIDELINES.md
blumamir May 15, 2024
831c32d
Update GUIDELINES.md
blumamir May 15, 2024
b2bafbe
Update GUIDELINES.md
blumamir May 15, 2024
87f84dd
Update GUIDELINES.md
blumamir May 15, 2024
ea5718a
Update GUIDELINES.md
blumamir May 15, 2024
aaca52d
Merge remote-tracking branch 'upstream/main' into supported-versions
blumamir May 16, 2024
7f71c9a
revert: add back guideline on versioning of redis instrumentations
blumamir May 16, 2024
3dff4b5
Update GUIDELINES.md
blumamir May 16, 2024
1d02804
Update GUIDELINES.md
blumamir May 16, 2024
314f302
Update GUIDELINES.md
blumamir May 16, 2024
e02a7c3
Update GUIDELINES.md
blumamir May 16, 2024
fc98fcd
Update plugins/node/instrumentation-lru-memoizer/README.md
blumamir May 16, 2024
7eacab4
Update plugins/node/opentelemetry-instrumentation-connect/README.md
blumamir May 16, 2024
4168fea
fix: from code review
blumamir May 17, 2024
cb72c24
Merge remote-tracking branch 'origin/supported-versions' into support…
blumamir May 17, 2024
35ab033
Merge remote-tracking branch 'upstream/main' into supported-versions
blumamir May 18, 2024
cf866e1
docs: make GUIDELINES shorter
blumamir May 18, 2024
d5a4290
docs: align runtime metrics
blumamir May 18, 2024
93e2961
fix: some fixes from code review
blumamir May 18, 2024
3abf5af
Merge remote-tracking branch 'upstream/main' into supported-versions
blumamir Jun 19, 2024
fb4815e
docs: remove quote from README
blumamir Jun 19, 2024
684f719
chore: replace caret range to >= <
blumamir Jun 19, 2024
fdd6a19
Update GUIDELINES.md
blumamir Jun 19, 2024
89eb937
Update GUIDELINES.md
blumamir Jun 19, 2024
3402f50
docs: nit lower bound version
blumamir Jun 19, 2024
22a8139
Merge remote-tracking branch 'origin/supported-versions' into support…
blumamir Jun 19, 2024
96638c2
Merge branch 'main' into supported-versions
blumamir Jun 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions GUIDELINES.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,84 @@ Instrumentation may add additional patch/unpatch messages for specific functions

The cases above are not covered by the base class and offer additional context to the user troubleshooting an issue with the instrumentation.

## Supported Versions

Supported versions can refer to 2 entities in the context of OpenTelemetry instrumentations:

- `Instrumented Package` - This is the user-facing package/s that the end user has installed in his application and is familiar with.
- `Patched Package` - These are the packages that are being patched in practice to achieve the instrumentation goals. It may be `Instrumented Package` itself or transitive internal dependencies of the `Instrumented Package`.

### `Instrumented Package` Documentation

Instrumentation should have a "## Supported Versions" section in the README file that lists the supported versions range of the instrumented package. This range should hide and consolidate any internal implementation details like the use of internal modules, different patch logic for different versions, etc. It should focus on the relevance to the human consumer.

### `Patched Package`s Supported Versions

The packages to patch are specified in the `InstrumentationNodeModuleDefinition` and `InstrumentationNodeModuleFile` classes. Instrumentation can specify arrays with different package names and version ranges to use to implement the instrumentation logic. example use:

```js
const supportedVersions = ['>=1.2.3 <3'];

protected init() {

const someModuleFile = new InstrumentationNodeModuleFile(
'foo/lib/some-file.js',
supportedVersions,
myFilePatch,
myFileUnpatch,
);

const module = new InstrumentationNodeModuleDefinition(
'foo',
supportedVersions,
myModulePatch,
myModuleUnpatch,
[someModuleFile]
);
return module
}
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following feels like a lot of prose for the topic. I'm worried that if GUIDELINES get too long then they don't get read. Would you consider a (possibly much) shorter proposed alternative for this section?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I am open to making it shorter.

Wondering if the guideline file is meant to be read fully by contributors, or more as a reference for a specific topic that documents our decisions and reasoning so it can be used for review, changes etc.

Feel free to suggest new shorter text 🙏

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am torn on this - on one hand, all of this detail is really useful context. On the other, it indeed is a lot of text. What do you think about leading with the common example start to finish up front, then a lower section for "Variations" which can be linked from the common example for folks who know they have something different. A "tldr" does tend to be useful when there is a lot of content.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made changes to the GUIDELINES. making it shorter, moving stuff around. might not be perfect yet, but please have a second look when you get a chance

### Variations

There can be few variations between the instrumented package and the patched package:

- Single Module - instrumentation patches the same module that is instrumented.
- Different Modules - instrumentation patches internal modules with different names and version ranges as of the instrumented package.
- Node.js Core Modules - instrumentation patches a Node.js internal module.
- Multiple Modules - instrumentation may instrument a set of (potentially large number of) user-facing instrumented packages.
- Patch Logic - instrumentation may use the `moduleExports` to patch, or hooks up to other mechanisms for recording signals. examples are: Node.js diagnostics channel, patching globals (like `window` being patched in browser instrumentations, or patches arbitrary lambda function handlers, etc.

### Range Specification

For versions that are a closed range, instrumentations should prefer to specify the supported versions of the instrumented package as `>=x.y.z <w` to promote consistency and readability across the code-base.

If an instrumentation supports just one major version of the instrumented package, it can specify the version range as `^x.y.z` or `^x`, which are equivalent but more readable.

Instrumentation should use an upper and lower bounds for the version ranges it uses for patches. This is to ensure that any new major versions of the instrumented package are not automatically patched by the instrumentation, which could lead to unexpected behavior.

New major versions should be reviewed and tested before being added to the supported versions list.

Specific guidelines for different cases:

- For `Different Modules`, instrumentations can use an upper limit on patched packages but it is unknown which future versions of the instrumented package will continue to use it. Thus it is ok to use an open upper limit `>=1.2.3` for the instrumented package.
blumamir marked this conversation as resolved.
Show resolved Hide resolved
- For `Node.js Core Modules`, the supported versions range is set to `['*']` to advertise that the instrumentation is compatible with all versions of Node.js that OpenTelemetry supports.
- For `Multiple Modules`, the supported versions range should be specified for each module in the README file with the supported versions.
- For `Different Patch Logic`, the use of supported versions can sometimes be more flexible, and the README should specify useful versioning information.

### Add New Supported Versions

When a new major version of the instrumented package is released, renovate bot will open a PR in contrib which helps maintainers to become aware of it. The instrumentation maintainer should review the new version and check compatibility with existing code. It can then be added to the supported versions list to be released in the next version of the instrumentation.

Checklist for adding a new version to the supported versions list:

- [ ] Review which functions are patched by the instrumentation and if they were changed in the new version that need support in code.
- [ ] Check for breaking changes in the new version that could affect the instrumentation.
- [ ] Test the instrumentation with the new version to ensure it works as expected.
- [ ] Update the supported versions list in the instrumentation code, perhaps with different patches and additional `InstrumentationNodeModuleDefinition`s that target the new version.
- [ ] Update the README file to reflect the support for new versions.
- [ ] For instrumentations that use test-all-versions `.tav.yaml`, add the new version to the list of versions to test.
blumamir marked this conversation as resolved.
Show resolved Hide resolved

## package.json

### Description
Expand Down
2 changes: 1 addition & 1 deletion packages/winston-transport/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const logger = winston.createLogger({

### Supported versions

Winston `3.x`
- [`winston`](https://www.npmjs.com/package/winston) versions `>=3.0.0 <4`

## Useful links

Expand Down
2 changes: 1 addition & 1 deletion plugins/node/instrumentation-amqplib/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ npm install --save @opentelemetry/instrumentation-amqplib

## Supported Versions

- `>=0.5.5`
- [`amqplib`](https://www.npmjs.com/package/amqplib) versions `>=0.5.5 <1`

## Usage

Expand Down
10 changes: 6 additions & 4 deletions plugins/node/instrumentation-amqplib/src/amqplib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ import {
} from './utils';
import { VERSION } from './version';

const supportedVersions = ['>=0.5.5 <1'];
blumamir marked this conversation as resolved.
Show resolved Hide resolved
blumamir marked this conversation as resolved.
Show resolved Hide resolved

export class AmqplibInstrumentation extends InstrumentationBase {
protected override _config!: AmqplibInstrumentationConfig;

Expand All @@ -92,28 +94,28 @@ export class AmqplibInstrumentation extends InstrumentationBase {
protected init() {
const channelModelModuleFile = new InstrumentationNodeModuleFile(
'amqplib/lib/channel_model.js',
['>=0.5.5'],
supportedVersions,
this.patchChannelModel.bind(this),
this.unpatchChannelModel.bind(this)
);

const callbackModelModuleFile = new InstrumentationNodeModuleFile(
'amqplib/lib/callback_model.js',
['>=0.5.5'],
supportedVersions,
this.patchChannelModel.bind(this),
this.unpatchChannelModel.bind(this)
);

const connectModuleFile = new InstrumentationNodeModuleFile(
'amqplib/lib/connect.js',
['>=0.5.5'],
supportedVersions,
this.patchConnect.bind(this),
this.unpatchConnect.bind(this)
);

const module = new InstrumentationNodeModuleDefinition(
'amqplib',
['>=0.5.5'],
supportedVersions,
undefined,
undefined,
[channelModelModuleFile, connectModuleFile, callbackModelModuleFile]
Expand Down
4 changes: 4 additions & 0 deletions plugins/node/instrumentation-cucumber/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ Compatible with OpenTelemetry JS API and SDK `1.0+`.
npm install --save @opentelemetry/instrumentation-cucumber
```

## Supported Versions

- [`@cucumber/cucumber`](https://www.npmjs.com/package/@cucumber/cucumber) versions `>=8.0.0 <11`

## Usage

```js
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ type Cucumber = typeof cucumber;
type Hook = (typeof hooks)[number];
type Step = (typeof steps)[number];

const supportedVersions = ['>=8.0.0 <11'];

export class CucumberInstrumentation extends InstrumentationBase {
private module: Cucumber | undefined;

Expand All @@ -57,7 +59,7 @@ export class CucumberInstrumentation extends InstrumentationBase {
return [
new InstrumentationNodeModuleDefinition(
'@cucumber/cucumber',
['^8.0.0', '^9.0.0', '^10.0.0'],
supportedVersions,
(moduleExports: Cucumber) => {
this.module = moduleExports;
steps.forEach(step => {
Expand All @@ -83,7 +85,7 @@ export class CucumberInstrumentation extends InstrumentationBase {
[
new InstrumentationNodeModuleFile(
'@cucumber/cucumber/lib/runtime/test_case_runner.js',
['^8.0.0', '^9.0.0', '^10.0.0'],
supportedVersions,
moduleExports => {
if (isWrapped(moduleExports.default.prototype.run)) {
this._unwrap(moduleExports.default.prototype, 'run');
Expand Down
4 changes: 2 additions & 2 deletions plugins/node/instrumentation-dataloader/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ Compatible with OpenTelemetry JS API and SDK `1.0+`.
npm install --save @opentelemetry/instrumentation-dataloader
```

### Supported Versions
## Supported Versions

- `^2.0.0`
- [`dataloader`](https://www.npmjs.com/package/dataloader) versions `^2.0.0`

## Usage

Expand Down
4 changes: 4 additions & 0 deletions plugins/node/instrumentation-fs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ See the full list of instrumented functions in [constants.ts](src/constants.ts);
npm install --save @opentelemetry/instrumentation-fs
```

## Supported Versions

- Node.js `>=14`

## Usage

```js
Expand Down
2 changes: 1 addition & 1 deletion plugins/node/instrumentation-kafkajs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ npm install --save @opentelemetry/instrumentation-kafkajs

### Supported versions

- `<3.0.0`
- [`kafkajs`](https://www.npmjs.com/package/kafkajs) versions `>=0.1.0 <3`

## Usage

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export class KafkaJsInstrumentation extends InstrumentationBase {

const module = new InstrumentationNodeModuleDefinition(
'kafkajs',
['< 3'],
['>=0.1.0 <3'],
(moduleExports: typeof kafkaJs) => {
unpatch(moduleExports);
this._wrap(
Expand Down
2 changes: 1 addition & 1 deletion plugins/node/instrumentation-lru-memoizer/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ npm install --save @opentelemetry/instrumentation-lru-memoizer

## Supported Versions

- `>=1.3 <3`
- [`lru-memorizer`](https://www.npmjs.com/package/lru-memoizer) versions `>=1.3 <3`

## Usage

Expand Down
2 changes: 1 addition & 1 deletion plugins/node/instrumentation-mongoose/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ npm install --save @opentelemetry/instrumentation-mongoose

## Supported Versions

- `>=5.9.7 <7`
- [`mongoose`](https://www.npmjs.com/package/mongoose) versions `>=5.9.7 <7`

## Usage

Expand Down
12 changes: 6 additions & 6 deletions plugins/node/instrumentation-runtime-node/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@
This module provides automatic metric instrumentation that exposes measurements from the [Performance measurement APIs](https://nodejs.org/api/perf_hooks.html) (i.e. `perf_hooks`).
While currently it is limited to metrics, it may be modified to produce other signals in the future.

## Supported Versions

- Node.js `>=14.10`

<!-- - 14.6.0 - this package uses _private properties_ -->

## Example

```bash
Expand Down Expand Up @@ -58,12 +64,6 @@ nodejs_performance_event_loop_utilization 0.010140079547955264
|---|---|---|---|---|
| [`eventLoopUtilizationMeasurementInterval`](./src/types.ts#L25) | `int` | millisecond | `5000` | The approximate number of milliseconds for which to calculate event loop utilization averages. A larger value will result in more accurate averages at the expense of less granular data. Should be set to below the scrape interval of your metrics collector to avoid duplicated data points. |

## Supported Node.js versions

v14.10.0+

<!-- - 14.6.0 - this package uses _private properties_ -->

## Useful links

- For more information on OpenTelemetry, visit: <https://opentelemetry.io/>
Expand Down
2 changes: 1 addition & 1 deletion plugins/node/instrumentation-socket.io/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ npm install --save @opentelemetry/instrumentation-socket.io

## Supported Versions

- `>=2 <5`
- [socket.io](https://www.npmjs.com/package/socket.io) versions `>=2.0.0 <5`

## Usage

Expand Down
2 changes: 1 addition & 1 deletion plugins/node/instrumentation-tedious/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ npm install --save @opentelemetry/instrumentation-tedious

## Supported Versions

- `>=1.11.0 <=15`
- [tedious](https://www.npmjs.com/package/tedious) `>=1.11.0 <16`

## Usage

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export class TediousInstrumentation extends InstrumentationBase {
return [
new InstrumentationNodeModuleDefinition(
TediousInstrumentation.COMPONENT,
['>=1.11.0 <=15'],
['>=1.11.0 <16'],
(moduleExports: typeof tedious) => {
const ConnectionPrototype: any = moduleExports.Connection.prototype;
for (const method of PATCHED_METHODS) {
Expand Down
2 changes: 1 addition & 1 deletion plugins/node/instrumentation-undici/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ npm install --save @opentelemetry/instrumentation-undici

## Supported Versions

- `undici@>=5.12.0`
- [`undici`](https://www.npmjs.com/package/undici) version `>=5.12.0`

## Usage

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ Compatible with OpenTelemetry JS API and SDK `1.0+`.
npm install --save @opentelemetry/instrumentation-aws-lambda
```

## Supported Versions

- This package will instrument the lambda execution regardless of versions.

## Usage

Create a file to initialize the instrumentation, such as `lambda-wrapper.js`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ If total installation size is not constrained, it is recommended to use the [`@o
npm install --save @opentelemetry/instrumentation-aws-sdk
```

## Supported Versions

- [`aws-sdk`](https://www.npmjs.com/package/aws-sdk) versions `>=2.308.0 <3`
- `@aws-sdk/client-*` versions `>=3.0.0 <4`

## Usage

For further automatic instrumentation instruction see the [@opentelemetry/instrumentation](https://www.npmjs.com/package/@opentelemetry/instrumentation) package.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ npm install --save @opentelemetry/instrumentation-bunyan

## Supported Versions

- `bunyan@^1.0.0`
- [`bunyan`](https://www.npmjs.com/package/bunyan) versions `^1.0.0`

## Usage

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class BunyanInstrumentation extends InstrumentationBase {
return [
new InstrumentationNodeModuleDefinition(
'bunyan',
['<2.0'],
['^1.0.0'],
(module: any) => {
const instrumentation = this;
const Logger =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ Compatible with OpenTelemetry JS API and SDK `1.0+`.
npm install --save @opentelemetry/instrumentation-cassandra-driver
```

### Supported Versions

- [`cassandra-driver`](https://www.npmjs.com/package/cassandra-driver) versions `>=4.4 <5.0`

## Usage

```js
Expand Down Expand Up @@ -45,10 +49,6 @@ await client.execute('select * from foo');
| `responseHook` | `CassandraDriverResponseCustomAttributeFunction` | `undefined` | Hook for adding custom attributes before response is handled |
| `maxQueryLength` | `number` | `65536` | If `enhancedDatabaseReporting` is enabled, limits the attached query strings to this length. |

### Supported versions

`>=4.4 <5.0`

## Semantic Conventions

This package uses `@opentelemetry/semantic-conventions` version `1.22+`, which implements Semantic Convention [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ npm install --save @opentelemetry/instrumentation-http @opentelemetry/instrument

### Supported Versions

- `^3.0.0`
- [`connect`](https://www.npmjs.com/package/connect) versions `^3.0.0`

## Usage

Expand Down
4 changes: 4 additions & 0 deletions plugins/node/opentelemetry-instrumentation-dns/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ If total installation size is not constrained, it is recommended to use the [`@o
npm install --save @opentelemetry/instrumentation-dns
```

## Supported Versions

- Node.js `>=14`

## Usage

```js
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ npm install --save @opentelemetry/instrumentation-http @opentelemetry/instrument

### Supported Versions

- `^4.0.0`
- [`express`](https://www.npmjs.com/package/express) version `^4.0.0`

## Usage

Expand Down
Loading
Loading