From 69b2163be1729697ebc69549aa8fb6e61be1b94d Mon Sep 17 00:00:00 2001 From: Michael Beemer Date: Wed, 21 Dec 2022 16:38:25 -0500 Subject: [PATCH] feat!: update the otel hook to be spec compliant (#179) --- libs/hooks/open-telemetry/README.md | 44 +++++- libs/hooks/open-telemetry/jest.config.ts | 13 +- libs/hooks/open-telemetry/project.json | 4 +- .../src/lib/open-telemetry-hook.spec.ts | 130 ++++++------------ .../src/lib/open-telemetry-hook.ts | 77 ++++------- tsconfig.base.json | 17 ++- 6 files changed, 127 insertions(+), 158 deletions(-) diff --git a/libs/hooks/open-telemetry/README.md b/libs/hooks/open-telemetry/README.md index 221b066bf..b0fd1d21b 100644 --- a/libs/hooks/open-telemetry/README.md +++ b/libs/hooks/open-telemetry/README.md @@ -1,21 +1,59 @@ # OpenTelemetry Hook +The OpenTelemetry hook for OpenFeature provides a [spec compliant][otel-spec] way to automatically add a feature flag evaluation to a span as a span event. Since feature flags are dynamic and affect runtime behavior, it’s important to collect relevant feature flag telemetry signals. This can be used to determine the impact a feature has on a request, enabling enhanced observability use cases, such as A/B testing or progressive feature releases. + ## Installation ``` $ npm install @openfeature/open-telemetry-hook ``` -Required peer dependencies +### Peer dependencies + +Confirm that the following peer dependencies are installed. ``` $ npm install @openfeature/js-sdk @opentelemetry/api ``` -## Building +## Usage + +OpenFeature provides various ways to register hooks. The location that a hook is registered affects when the hook is run. It's recommended to register the `OpenTelemetryHook` globally in most situations but it's possible to only enable the hook on specific clients. You should **never** register the `OpenTelemetryHook` globally and on a client. + +More information on hooks can be found in the [OpenFeature documentation][hook-concept]. + +### Register Globally + +The `OpenTelemetryHook` can be set on the OpenFeature singleton. This will ensure that every flag evaluation will always create a span event, if am active span is available. + +```typescript +import { OpenFeature } from '@openfeature/js-sdk'; +import { OpenTelemetryHook } from '@openfeature/open-telemetry-hook'; + +OpenFeature.addHooks(new OpenTelemetryHook()); +``` + +### Register Per Client + +The `OpenTelemetryHook` can be set on an individual client. This should only be done if it wasn't set globally and other clients shouldn't use this hook. Setting the hook on the client will ensure that every flag evaluation performed by this client will always create a span event, if am active span is available. + +```typescript +import { OpenFeature } from '@openfeature/js-sdk'; +import { OpenTelemetryHook } from '@openfeature/open-telemetry-hook'; + +const client = OpenFeature.getClient('my-app'); +client.addHooks(new OpenTelemetryHook()); +``` + +## Development + +### Building Run `nx package hooks-open-telemetry` to build the library. -## Running unit tests +### Running unit tests Run `nx test hooks-open-telemetry` to execute the unit tests via [Jest](https://jestjs.io). + +[otel-spec]: https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/feature-flags/ +[hook-concept]: https://docs.openfeature.dev/docs/reference/concepts/hooks diff --git a/libs/hooks/open-telemetry/jest.config.ts b/libs/hooks/open-telemetry/jest.config.ts index 536f9db79..f765f9f34 100644 --- a/libs/hooks/open-telemetry/jest.config.ts +++ b/libs/hooks/open-telemetry/jest.config.ts @@ -2,14 +2,13 @@ export default { displayName: 'hooks-open-telemetry', preset: '../../../jest.preset.js', - globals: { - 'ts-jest': { - tsconfig: '/tsconfig.spec.json', - }, - }, transform: { - '^.+\\.[tj]s$': 'ts-jest', + '^.+\\.[tj]s$': [ + 'ts-jest', + { + tsconfig: '/tsconfig.spec.json', + }, + ], }, moduleFileExtensions: ['ts', 'js', 'html'], - coverageDirectory: '../../../coverage/libs/hooks/open-telemetry', }; diff --git a/libs/hooks/open-telemetry/project.json b/libs/hooks/open-telemetry/project.json index d5f89a704..cfb64a91e 100644 --- a/libs/hooks/open-telemetry/project.json +++ b/libs/hooks/open-telemetry/project.json @@ -81,7 +81,9 @@ ], "options": { "jestConfig": "libs/hooks/open-telemetry/jest.config.ts", - "passWithNoTests": true + "passWithNoTests": true, + "codeCoverage": true, + "coverageDirectory": "coverage/libs/hooks/open-telemetry" } } }, diff --git a/libs/hooks/open-telemetry/src/lib/open-telemetry-hook.spec.ts b/libs/hooks/open-telemetry/src/lib/open-telemetry-hook.spec.ts index c359e8681..b37bbfda2 100644 --- a/libs/hooks/open-telemetry/src/lib/open-telemetry-hook.spec.ts +++ b/libs/hooks/open-telemetry/src/lib/open-telemetry-hook.spec.ts @@ -1,20 +1,13 @@ import { EvaluationDetails, HookContext } from '@openfeature/js-sdk'; -const setAttributes = jest.fn(); -const setAttribute = jest.fn(); +const addEvent = jest.fn(); const recordException = jest.fn(); -const end = jest.fn(); -const startSpan = jest.fn(() => ({ - setAttributes, - setAttribute, - recordException, - end, -})); -const getTracer = jest.fn(() => ({ startSpan })); + +const getActiveSpan = jest.fn(() => ({ addEvent, recordException })); jest.mock('@opentelemetry/api', () => ({ trace: { - getTracer, + getActiveSpan, }, })); @@ -33,7 +26,7 @@ describe('OpenTelemetry Hooks', () => { context: {}, defaultValue: true, flagValueType: 'boolean', - logger: console + logger: console, }; let otelHook: OpenTelemetryHook; @@ -46,73 +39,36 @@ describe('OpenTelemetry Hooks', () => { jest.clearAllMocks(); }); - it('should use the same span with all the hooks', () => { - const evaluationDetails: EvaluationDetails = { - flagKey: hookContext.flagKey, - - value: true, - }; - - const setSpanMapSpy = jest.spyOn(otelHook['spanMap'], 'set'); - const testError = new Error(); - - otelHook.before(hookContext); - expect(setSpanMapSpy).toBeCalled(); - - otelHook.after(hookContext, evaluationDetails); - expect(setAttribute).toBeCalledWith('feature_flag.evaluated_value', 'true'); - - otelHook.error(hookContext, testError); - expect(recordException).toBeCalledWith(testError); - - otelHook.finally(hookContext); - expect(end).toBeCalled(); - }); - - describe('before hook', () => { - it('should start a new span', () => { - expect(otelHook.before(hookContext)).toBeUndefined(); - expect(getTracer).toBeCalled(); - expect(startSpan).toBeCalledWith('testProvider testFlagKey', { - attributes: { - 'feature_flag.flag_key': 'testFlagKey', - 'feature_flag.provider_name': 'testProvider', - }, - }); - expect(otelHook['spanMap'].has(hookContext)).toBeTruthy; - }); - }); - describe('after hook', () => { - it('should set the variant as a span attribute', () => { + it('should use the variant value on the span event', () => { const evaluationDetails: EvaluationDetails = { flagKey: hookContext.flagKey, value: true, variant: 'enabled', - }; // The before hook should + }; - otelHook.before(hookContext); otelHook.after(hookContext, evaluationDetails); - expect(setAttribute).toBeCalledWith( - 'feature_flag.evaluated_variant', - 'enabled' - ); + expect(addEvent).toBeCalledWith('feature_flag', { + 'feature_flag.key': 'testFlagKey', + 'feature_flag.provider_name': 'testProvider', + 'feature_flag.variant': 'enabled', + }); }); - it('should set the value as a span attribute', () => { + it('should use a stringified value as the variant value on the span event', () => { const evaluationDetails: EvaluationDetails = { flagKey: hookContext.flagKey, value: true, - }; // The before hook should + }; - otelHook.before(hookContext); otelHook.after(hookContext, evaluationDetails); - expect(setAttribute).toBeCalledWith( - 'feature_flag.evaluated_value', - 'true' - ); + expect(addEvent).toBeCalledWith('feature_flag', { + 'feature_flag.key': 'testFlagKey', + 'feature_flag.provider_name': 'testProvider', + 'feature_flag.variant': 'true', + }); }); it('should set the value without extra quotes if value is already a string', () => { @@ -120,46 +76,40 @@ describe('OpenTelemetry Hooks', () => { flagKey: hookContext.flagKey, value: 'already-string', }; - - otelHook.before(hookContext); otelHook.after(hookContext, evaluationDetails); - expect(setAttribute).toBeCalledWith( - 'feature_flag.evaluated_value', - 'already-string' - ); + expect(addEvent).toBeCalledWith('feature_flag', { + 'feature_flag.key': 'testFlagKey', + 'feature_flag.provider_name': 'testProvider', + 'feature_flag.variant': 'already-string', + }); + }); + + it('should not call addEvent because there is no active span', () => { + getActiveSpan.mockReturnValueOnce(undefined); + const evaluationDetails: EvaluationDetails = { + flagKey: hookContext.flagKey, + value: true, + variant: 'enabled', + }; + + otelHook.after(hookContext, evaluationDetails); + expect(addEvent).not.toBeCalled(); }); }); describe('error hook', () => { const testError = new Error(); - it('should not call recordException because the span is undefined', () => { - otelHook.error(hookContext, testError); - expect(otelHook['spanMap'].has(hookContext)).toBeFalsy; - expect(recordException).not.toBeCalledWith(testError); - }); - it('should call recordException with a test error', () => { - otelHook.before(hookContext); otelHook.error(hookContext, testError); - expect(otelHook['spanMap'].has(hookContext)).toBeTruthy; expect(recordException).toBeCalledWith(testError); }); - }); - - describe('finally hook', () => { - it('should not call end because the span is undefined', () => { - otelHook.finally(hookContext); - expect(otelHook['spanMap'].has(hookContext)).toBeFalsy; - expect(end).not.toBeCalled(); - }); - it('should call end to finish the span', () => { - otelHook.before(hookContext); - otelHook.finally(hookContext); - expect(otelHook['spanMap'].has(hookContext)).toBeTruthy; - expect(end).toBeCalled(); + it('should not call recordException because there is no active span', () => { + getActiveSpan.mockReturnValueOnce(undefined); + otelHook.error(hookContext, testError); + expect(recordException).not.toBeCalled(); }); }); }); diff --git a/libs/hooks/open-telemetry/src/lib/open-telemetry-hook.ts b/libs/hooks/open-telemetry/src/lib/open-telemetry-hook.ts index 0a52b16cf..833db462c 100644 --- a/libs/hooks/open-telemetry/src/lib/open-telemetry-hook.ts +++ b/libs/hooks/open-telemetry/src/lib/open-telemetry-hook.ts @@ -1,65 +1,36 @@ -import { - Hook, - HookContext, - EvaluationDetails, - FlagValue, -} from '@openfeature/js-sdk'; -import { Span, Tracer, trace } from '@opentelemetry/api'; +import { Hook, HookContext, EvaluationDetails, FlagValue } from '@openfeature/js-sdk'; +import { trace } from '@opentelemetry/api'; -const SpanProperties = Object.freeze({ - FLAG_KEY: 'feature_flag.flag_key', +const eventName = 'feature_flag'; +const spanEventProperties = Object.freeze({ + FLAG_KEY: 'feature_flag.key', PROVIDER_NAME: 'feature_flag.provider_name', - VARIANT: 'feature_flag.evaluated_variant', - VALUE: 'feature_flag.evaluated_value', + VARIANT: 'feature_flag.variant', }); export class OpenTelemetryHook implements Hook { - private spanMap = new WeakMap(); - private tracer: Tracer; - - constructor() { - this.tracer = trace.getTracer( - '@openfeature/open-telemetry-hook', - '5.1.1' // x-release-please-version - ); - } - - before(hookContext: HookContext) { - const span = this.tracer.startSpan( - `${hookContext.providerMetadata.name} ${hookContext.flagKey}`, - { - attributes: { - [SpanProperties.FLAG_KEY]: hookContext.flagKey, - [SpanProperties.PROVIDER_NAME]: hookContext.providerMetadata.name, - }, + after(hookContext: HookContext, evaluationDetails: EvaluationDetails) { + const currentTrace = trace.getActiveSpan(); + if (currentTrace) { + let variant = evaluationDetails.variant; + + if (!variant) { + if (typeof evaluationDetails.value === 'string') { + variant = evaluationDetails.value; + } else { + variant = JSON.stringify(evaluationDetails.value); + } } - ); - this.spanMap.set(hookContext, span); - } - - after( - hookContext: HookContext, - evaluationDetails: EvaluationDetails - ) { - if (evaluationDetails.variant) { - this.spanMap - .get(hookContext) - ?.setAttribute(SpanProperties.VARIANT, evaluationDetails.variant); - } else { - const value = - typeof evaluationDetails.value === 'string' - ? evaluationDetails.value - : JSON.stringify(evaluationDetails.value); - this.spanMap.get(hookContext)?.setAttribute(SpanProperties.VALUE, value); + currentTrace.addEvent(eventName, { + [spanEventProperties.FLAG_KEY]: hookContext.flagKey, + [spanEventProperties.PROVIDER_NAME]: hookContext.providerMetadata.name, + [spanEventProperties.VARIANT]: variant, + }); } } - error(hookContext: HookContext, err: Error) { - this.spanMap.get(hookContext)?.recordException(err); - } - - finally(hookContext: HookContext) { - this.spanMap.get(hookContext)?.end(); + error(_: HookContext, err: Error) { + trace.getActiveSpan()?.recordException(err); } } diff --git a/tsconfig.base.json b/tsconfig.base.json index 4eb659355..5fd99f134 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -9,14 +9,20 @@ "esModuleInterop": true, "experimentalDecorators": true, "importHelpers": true, + "strictNullChecks": true, "target": "es2015", "module": "esnext", - "lib": ["esnext", "dom"], + "lib": [ + "esnext", + "dom" + ], "skipLibCheck": true, "skipDefaultLibCheck": true, "baseUrl": ".", "paths": { - "@openfeature/flagd-provider": ["libs/providers/flagd/src/index.ts"], + "@openfeature/flagd-provider": [ + "libs/providers/flagd/src/index.ts" + ], "@openfeature/flagd-web-provider": [ "libs/providers/flagd-web/src/index.ts" ], @@ -28,5 +34,8 @@ ] } }, - "exclude": ["node_modules", "tmp"] -} + "exclude": [ + "node_modules", + "tmp" + ] +} \ No newline at end of file