Skip to content

Commit

Permalink
chore!: stop using default exports
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rowanmanning committed Jun 19, 2024
1 parent 91abf60 commit 3112b44
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 39 deletions.
14 changes: 10 additions & 4 deletions packages/opentelemetry/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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 * as 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):
Expand All @@ -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({ /* ... */ });
```

<table>
Expand Down Expand Up @@ -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
});
```
Expand Down Expand Up @@ -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).
Expand Down
33 changes: 33 additions & 0 deletions packages/opentelemetry/docs/migration.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@

# 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
:yellow_circle: | Deprecation | A deprecated feature which will require code changes in the future

* [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 * as 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.
5 changes: 1 addition & 4 deletions packages/opentelemetry/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,4 @@ function setupOpenTelemetry({
sdk.start();
}

module.exports = setupOpenTelemetry;

// @ts-ignore
module.exports.default = module.exports;
exports.setup = setupOpenTelemetry;
8 changes: 4 additions & 4 deletions packages/opentelemetry/setup.js
Original file line number Diff line number Diff line change
@@ -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 = {
Expand All @@ -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 = {
Expand All @@ -21,7 +21,7 @@ if (process.env.OPENTELEMETRY_METRICS_ENDPOINT) {
};
}

setupOpenTelemetry({
opentelemetry.setup({
metrics,
tracing
});
20 changes: 8 additions & 12 deletions packages/opentelemetry/test/unit/lib/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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'
Expand All @@ -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'
Expand All @@ -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',
Expand All @@ -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', () => {
Expand Down
30 changes: 15 additions & 15 deletions packages/opentelemetry/test/unit/setup.spec.js
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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'
Expand All @@ -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'
Expand All @@ -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',
Expand All @@ -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',
Expand Down

0 comments on commit 3112b44

Please sign in to comment.