From bfa3ba3a91cae0bd388e7c0e767b19c3ebe18640 Mon Sep 17 00:00:00 2001 From: Rowan Manning <138944+rowanmanning@users.noreply.github.com> Date: Wed, 12 Jun 2024 17:59:18 +0100 Subject: [PATCH] chore!: stop using default exports This is going to cause us lots of problems when we want to export new methods, e.g. methods to send custom metrics which I'll be doing in a later commit. Nobody is using the JavaScript API at the moment and so this is a safe major version bump for most. It's also worth addressing this while use is still low. See #945. --- packages/opentelemetry/README.md | 14 +++++--- packages/opentelemetry/docs/migration.md | 32 +++++++++++++++++++ packages/opentelemetry/lib/index.js | 5 +-- packages/opentelemetry/setup.js | 8 ++--- .../opentelemetry/test/unit/lib/index.spec.js | 20 +++++------- .../opentelemetry/test/unit/setup.spec.js | 30 ++++++++--------- 6 files changed, 70 insertions(+), 39 deletions(-) create mode 100644 packages/opentelemetry/docs/migration.md diff --git a/packages/opentelemetry/README.md b/packages/opentelemetry/README.md index aa5f384b..db06e659 100644 --- a/packages/opentelemetry/README.md +++ b/packages/opentelemetry/README.md @@ -30,6 +30,7 @@ An [OpenTelemetry](https://opentelemetry.io/docs/what-is-opentelemetry/) client * [`options.tracing.authorizationHeader`](#optionstracingauthorizationheader) * [`options.tracing.samplePercentage`](#optionstracingsamplepercentage) * [`OTEL_` environment variables](#otel_-environment-variables) +* [Migrating](#migrating) * [Contributing](#contributing) * [License](#license) @@ -117,9 +118,9 @@ OpenTelemetry will be [configured](#configuration-options) with environment vari If you'd like to customise the OpenTelemetry config more and have control over what runs, you can include in your code: ```js -import setupOpenTelemetry from '@dotcom-reliability-kit/opentelemetry'; +import opentelemetry from '@dotcom-reliability-kit/opentelemetry'; // or -const setupOpenTelemetry = require('@dotcom-reliability-kit/opentelemetry'); +const opentelemetry = require('@dotcom-reliability-kit/opentelemetry'); ``` Call the function, passing in [configuration options](#configuration-options): @@ -128,7 +129,7 @@ Call the function, passing in [configuration options](#configuration-options): > This **must** be the first function called in your application for OpenTelemetry to be set up correctly (including before other `import`/`require` statements). ```js -setupOpenTelemetry({ /* ... */ }); +opentelemetry.setup({ /* ... */ }); ``` @@ -225,7 +226,7 @@ EXAMPLE=true npm start For the [manual setup](#manual-setup), you'll need to use an options object, e.g. ```js -setupOpenTelemetry({ +opentelemetry.setup({ example: true }); ``` @@ -285,6 +286,11 @@ OpenTelemetry itself can be configured through `OTEL_`-prefixed environment vari > We strongly advise against using these. The power of this module is consistency and any application-specific changes should be considered. If you use these environment variables we won't offer support if things break. +## Migrating + +Consult the [Migration Guide](./docs/migration.md) if you're trying to migrate to a later major version of this package. + + ## Contributing See the [central contributing guide for Reliability Kit](https://github.com/Financial-Times/dotcom-reliability-kit/blob/main/docs/contributing.md). diff --git a/packages/opentelemetry/docs/migration.md b/packages/opentelemetry/docs/migration.md new file mode 100644 index 00000000..cbffc57a --- /dev/null +++ b/packages/opentelemetry/docs/migration.md @@ -0,0 +1,32 @@ + +# Migration guide for @dotcom-reliability-kit/opentelemetry + +This document outlines how to migrate to the latest version of the Reliability Kit opentelemetry package. Throughout this guide we use the following emoji and labels to indicate the level of change required: + +Emoji | Label | Meaning +----------------|:------------------|:------- +:red_circle: | Breaking | A breaking change which will likely require code or config changes to resolve +:orange_circle: | Possibly Breaking | A breaking change that is unlikely to require code changes but things outside of the code (e.g. logs) may have changed + +* [Migrating from v1 to v2](#migrating-from-v1-to-v2) + * [JavaScript API changes](#javascript-api-changes) + + +## Migrating from v1 to v2 + +### JavaScript API changes + +**:red_circle: Breaking:** If you're using the manual setup of OpenTelemetry via the JavaScript API, the setup method is no longer a default export. Instead, use a named `setup` export: + +```diff +- import setupOpenTelemetry from '@dotcom-reliability-kit/opentelemetry'; ++ import opentelemetry from '@dotcom-reliability-kit/opentelemetry'; +// or +- const setupOpenTelemetry = require('@dotcom-reliability-kit/opentelemetry'); ++ const opentelemetry = require('@dotcom-reliability-kit/opentelemetry'); + +- setupOpenTelemetry({ /* ... */ }); ++ opentelemetry.setup({ /* ... */ }); +``` + +If you're using the `--require` method or importing `@dotcom-reliability-kit/opentelemetry/setup` then this is not a breaking change. diff --git a/packages/opentelemetry/lib/index.js b/packages/opentelemetry/lib/index.js index fb9e32e0..c897bd1c 100644 --- a/packages/opentelemetry/lib/index.js +++ b/packages/opentelemetry/lib/index.js @@ -79,7 +79,4 @@ function setupOpenTelemetry({ sdk.start(); } -module.exports = setupOpenTelemetry; - -// @ts-ignore -module.exports.default = module.exports; +exports.setup = setupOpenTelemetry; diff --git a/packages/opentelemetry/setup.js b/packages/opentelemetry/setup.js index a30cd35a..07bf290a 100644 --- a/packages/opentelemetry/setup.js +++ b/packages/opentelemetry/setup.js @@ -1,6 +1,6 @@ -const setupOpenTelemetry = require('./lib/index.js'); +const opentelemetry = require('.'); -/** @type {setupOpenTelemetry.TracingOptions | undefined} */ +/** @type {opentelemetry.TracingOptions | undefined} */ let tracing = undefined; if (process.env.OPENTELEMETRY_TRACING_ENDPOINT) { tracing = { @@ -12,7 +12,7 @@ if (process.env.OPENTELEMETRY_TRACING_ENDPOINT) { }; } -/** @type {setupOpenTelemetry.MetricsOptions | undefined} */ +/** @type {opentelemetry.MetricsOptions | undefined} */ let metrics = undefined; if (process.env.OPENTELEMETRY_METRICS_ENDPOINT) { metrics = { @@ -21,7 +21,7 @@ if (process.env.OPENTELEMETRY_METRICS_ENDPOINT) { }; } -setupOpenTelemetry({ +opentelemetry.setup({ metrics, tracing }); diff --git a/packages/opentelemetry/test/unit/lib/index.spec.js b/packages/opentelemetry/test/unit/lib/index.spec.js index 08bcd90f..b86616e6 100644 --- a/packages/opentelemetry/test/unit/lib/index.spec.js +++ b/packages/opentelemetry/test/unit/lib/index.spec.js @@ -30,16 +30,12 @@ logger.createChildLogger.mockReturnValue('mock child logger'); DiagLogLevel.INFO = 'mock info log level'; // Import the OTel function for testing -const setupOpenTelemetry = require('../../../lib/index'); +const opentelemetry = require('../../../lib/index'); describe('@dotcom-reliability-kit/opentelemetry', () => { - it('exports a function', () => { - expect(typeof setupOpenTelemetry).toStrictEqual('function'); - }); - - describe('setupOpenTelemetry(options)', () => { + describe('.setup(options)', () => { beforeAll(() => { - setupOpenTelemetry({ + opentelemetry.setup({ tracing: { endpoint: 'mock-tracing-endpoint', samplePercentage: 137 @@ -101,7 +97,7 @@ describe('@dotcom-reliability-kit/opentelemetry', () => { describe('when no options are set', () => { beforeAll(() => { NodeSDK.mockClear(); - setupOpenTelemetry(); + opentelemetry.setup(); }); it('still instantiates and starts the OpenTelemetry Node SDK with the created config', () => { @@ -118,7 +114,7 @@ describe('@dotcom-reliability-kit/opentelemetry', () => { describe('when an authorization header is passed into the root options (deprecated)', () => { beforeAll(() => { createTracingConfig.mockReset(); - setupOpenTelemetry({ + opentelemetry.setup({ authorizationHeader: 'mock-authorization-header-root', tracing: { endpoint: 'mock-tracing-endpoint' @@ -138,7 +134,7 @@ describe('@dotcom-reliability-kit/opentelemetry', () => { describe('when an authorization header is passed into the tracing options', () => { beforeAll(() => { createTracingConfig.mockReset(); - setupOpenTelemetry({ + opentelemetry.setup({ tracing: { authorizationHeader: 'mock-authorization-header-tracing', endpoint: 'mock-tracing-endpoint' @@ -158,7 +154,7 @@ describe('@dotcom-reliability-kit/opentelemetry', () => { describe('when an authorization header is passed into both the root options (deprecated) and tracing options', () => { beforeAll(() => { createTracingConfig.mockReset(); - setupOpenTelemetry({ + opentelemetry.setup({ authorizationHeader: 'mock-authorization-header-root', tracing: { authorizationHeader: 'mock-authorization-header-tracing', @@ -179,7 +175,7 @@ describe('@dotcom-reliability-kit/opentelemetry', () => { describe('when OTEL_ environment variables are defined', () => { beforeAll(() => { process.env.OTEL_MOCK = 'mock'; - setupOpenTelemetry(); + opentelemetry.setup(); }); it('logs a warning that these environment variables are not supported', () => { diff --git a/packages/opentelemetry/test/unit/setup.spec.js b/packages/opentelemetry/test/unit/setup.spec.js index 89c9f668..25ef2382 100644 --- a/packages/opentelemetry/test/unit/setup.spec.js +++ b/packages/opentelemetry/test/unit/setup.spec.js @@ -1,21 +1,21 @@ -describe('setupOpenTelemetry', () => { - let setupOpenTelemetry; +describe('setup', () => { + let opentelemetry; beforeEach(() => { jest.resetModules(); - jest.mock('../../lib/index.js', () => jest.fn()); - setupOpenTelemetry = require('../../lib/index.js'); + jest.mock('../..', () => ({ setup: jest.fn() })); + opentelemetry = require('../..'); }); - it('should call setupOpenTelemetry with the correct parameters', () => { + it('should call opentelemetry.setup with the correct parameters', () => { process.env.OPENTELEMETRY_TRACING_ENDPOINT = 'MOCK_TRACING_ENDPOINT'; process.env.OPENTELEMETRY_AUTHORIZATION_HEADER = 'MOCK_AUTH_HEADER'; process.env.OPENTELEMETRY_METRICS_ENDPOINT = 'MOCK_METRICS_ENDPOINT'; process.env.OPENTELEMETRY_API_GATEWAY_KEY = 'MOCK_API_GATEWAY_KEY'; require('../../setup.js'); - expect(setupOpenTelemetry).toHaveBeenCalledTimes(1); - expect(setupOpenTelemetry).toHaveBeenCalledWith({ + expect(opentelemetry.setup).toHaveBeenCalledTimes(1); + expect(opentelemetry.setup).toHaveBeenCalledWith({ tracing: { authorizationHeader: 'MOCK_AUTH_HEADER', endpoint: 'MOCK_TRACING_ENDPOINT' @@ -33,8 +33,8 @@ describe('setupOpenTelemetry', () => { process.env.OPENTELEMETRY_METRICS_ENDPOINT = 'MOCK_METRICS_ENDPOINT'; require('../../setup.js'); - expect(setupOpenTelemetry).toHaveBeenCalledTimes(1); - expect(setupOpenTelemetry).toHaveBeenCalledWith({ + expect(opentelemetry.setup).toHaveBeenCalledTimes(1); + expect(opentelemetry.setup).toHaveBeenCalledWith({ metrics: { apiGatewayKey: 'MOCK_API_GATEWAY_KEY', endpoint: 'MOCK_METRICS_ENDPOINT' @@ -49,8 +49,8 @@ describe('setupOpenTelemetry', () => { process.env.OPENTELEMETRY_TRACING_ENDPOINT = 'MOCK_TRACING_ENDPOINT'; require('../../setup.js'); - expect(setupOpenTelemetry).toHaveBeenCalledTimes(1); - expect(setupOpenTelemetry).toHaveBeenCalledWith({ + expect(opentelemetry.setup).toHaveBeenCalledTimes(1); + expect(opentelemetry.setup).toHaveBeenCalledWith({ tracing: { authorizationHeader: 'MOCK_AUTH_HEADER', endpoint: 'MOCK_TRACING_ENDPOINT' @@ -65,8 +65,8 @@ describe('setupOpenTelemetry', () => { process.env.OPENTELEMETRY_TRACING_SAMPLE_PERCENTAGE = '50'; require('../../setup.js'); - expect(setupOpenTelemetry).toHaveBeenCalledTimes(1); - expect(setupOpenTelemetry).toHaveBeenCalledWith({ + expect(opentelemetry.setup).toHaveBeenCalledTimes(1); + expect(opentelemetry.setup).toHaveBeenCalledWith({ tracing: { authorizationHeader: 'MOCK_AUTH_HEADER', endpoint: 'MOCK_TRACING_ENDPOINT', @@ -82,8 +82,8 @@ describe('setupOpenTelemetry', () => { process.env.OPENTELEMETRY_TRACING_SAMPLE_PERCENTAGE = 'nope'; require('../../setup.js'); - expect(setupOpenTelemetry).toHaveBeenCalledTimes(1); - expect(setupOpenTelemetry).toHaveBeenCalledWith({ + expect(opentelemetry.setup).toHaveBeenCalledTimes(1); + expect(opentelemetry.setup).toHaveBeenCalledWith({ tracing: { authorizationHeader: 'MOCK_AUTH_HEADER', endpoint: 'MOCK_TRACING_ENDPOINT',