From 3bd038712ae7dbbb9d758cbdb2b22b3a0bcec36d Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Thu, 10 Nov 2022 11:19:59 +0000 Subject: [PATCH 01/18] chore(docs): update environment variable table * Correct the default value for the POWERTOOLS_METRICS_NAMESPACE env var * Refer reader to utility page for further info on env vars --- docs/index.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/index.md b/docs/index.md index 88b35688a6..74410a2dcb 100644 --- a/docs/index.md +++ b/docs/index.md @@ -286,7 +286,7 @@ Core utilities such as Tracing, Logging, and Metrics will be available across al | Environment variable | Description | Utility | Default | |----------------------------------------------|----------------------------------------------------------------------------------------|---------------------------|-----------------------| | **POWERTOOLS_SERVICE_NAME** | Sets service name used for tracing namespace, metrics dimension and structured logging | All | `"service_undefined"` | -| **POWERTOOLS_METRICS_NAMESPACE** | Sets namespace used for metrics | [Metrics](./core/metrics) | `None` | +| **POWERTOOLS_METRICS_NAMESPACE** | Sets namespace used for metrics | [Metrics](./core/metrics) | `"default_namespace"` | | **POWERTOOLS_TRACE_ENABLED** | Explicitly disables tracing | [Tracer](./core/tracer) | `true` | | **POWERTOOLS_TRACER_CAPTURE_RESPONSE** | Captures Lambda or method return as metadata. | [Tracer](./core/tracer) | `true` | | **POWERTOOLS_TRACER_CAPTURE_ERROR** | Captures Lambda or method exception as metadata. | [Tracer](./core/tracer) | `true` | @@ -295,6 +295,8 @@ Core utilities such as Tracing, Logging, and Metrics will be available across al | **POWERTOOLS_LOGGER_SAMPLE_RATE** | Debug log sampling | [Logger](./core/logger) | `0` | | **LOG_LEVEL** | Sets logging level | [Logger](./core/logger) | `INFO` | +Each Utility page provides information on example values and allowed values + ## Tenets These are our core principles to guide our decision making. From 72c8e35ca83e88113d88820f26be8a8c7e6f67cb Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Thu, 10 Nov 2022 11:55:31 +0000 Subject: [PATCH 02/18] chore(docs): add default, allowed and example values to utility docs * Extract default and allowed values from description to their own cols * Add allowed values, example and default columns * Correct default value for metrics namespace --- docs/core/logger.md | 12 ++++++------ docs/core/metrics.md | 8 ++++---- docs/core/tracer.md | 14 +++++++------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/docs/core/logger.md b/docs/core/logger.md index ab0bd44986..bcafa63de8 100644 --- a/docs/core/logger.md +++ b/docs/core/logger.md @@ -52,12 +52,12 @@ The library requires two settings. You can set them as environment variables, or These settings will be used across all logs emitted: -| Setting | Description | Environment variable | Constructor parameter | -|-------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------|---------------------------------|-----------------------| -| **Service name** | Sets the name of service of which the Lambda function is part of, that will be present across all log statements | `POWERTOOLS_SERVICE_NAME` | `serviceName` | -| **Logging level** | Sets how verbose Logger should be (INFO, by default). Supported values are: `DEBUG`, `INFO`, `WARN`, `ERROR` | `LOG_LEVEL` | `logLevel` | -| **Log incoming event** | Whether to log or not the incoming event when using the decorator or middleware. Supported values are: `true`, or `false`, disabled by default | `POWERTOOLS_LOGGER_LOG_EVENT` | `logEvent` | -| **Debug log sampling** | Probability that a Lambda invocation will print all the log items regardless of the log level setting. Supported values range from `0.0` to `1` | `POWERTOOLS_LOGGER_SAMPLE_RATE` | `sampleRateValue` | +| Setting | Description | Environment variable | Default Value | Allowed Values | Example Value | Constructor parameter | +|-------------------------|------------------------------------------------------------------------------------------------------------------|---------------------------------|---------------------|--------------------------------|--------------------|-----------------------| +| **Service name** | Sets the name of service of which the Lambda function is part of, that will be present across all log statements | `POWERTOOLS_SERVICE_NAME` | `service_undefined` | Any string | `serverlessAirline`| `serviceName` | +| **Logging level** | Sets how verbose Logger should be | `LOG_LEVEL` | `info` |`DEBUG`, `INFO`, `WARN`, `ERROR`| `ERROR` | `logLevel` | +| **Log incoming event** | Whether to log or not the incoming event when using the decorator or middleware. | `POWERTOOLS_LOGGER_LOG_EVENT` | `false` | `true`, `false` | `false` | `logEvent` | +| **Debug log sampling** | Probability that a Lambda invocation will print all the log items regardless of the log level setting. | `POWERTOOLS_LOGGER_SAMPLE_RATE` | `0` | `0.0` to `1` | `0.5` | `sampleRateValue` | #### Example using AWS Serverless Application Model (SAM) diff --git a/docs/core/metrics.md b/docs/core/metrics.md index 3b2d4baa3f..4e26954a46 100644 --- a/docs/core/metrics.md +++ b/docs/core/metrics.md @@ -66,10 +66,10 @@ The library requires two settings. You can set them as environment variables, or These settings will be used across all metrics emitted: -| Setting | Description | Environment variable | Constructor parameter | -|----------------------|---------------------------------------------------------------------------------|--------------------------------|-----------------------| -| **Service** | Optionally, sets **service** metric dimension across all metrics e.g. `payment` | `POWERTOOLS_SERVICE_NAME` | `serviceName` | -| **Metric namespace** | Logical container where all metrics will be placed e.g. `serverlessAirline` | `POWERTOOLS_METRICS_NAMESPACE` | `namespace` | +| Setting | Description | Environment variable | Default | Allowed Values | Example | Constructor parameter | +|----------------------|-----------------------------------------------------------------|-------------------------------|--------------------|----------------|--------------------|-----------------------| +| **Service** | Optionally, sets **service** metric dimension across all metrics| `POWERTOOLS_SERVICE_NAME` | `service_undefined`| Any string | `serverlessAirline`| `serviceName` | +| **Metric namespace** | Logical container where all metrics will be placed | `POWERTOOLS_METRICS_NAMESPACE`| `default_namespace`| Any string | `serverlessAirline`| `namespace` | !!! tip Use your application name or main service as the metric namespace to easily group all metrics diff --git a/docs/core/tracer.md b/docs/core/tracer.md index ec90e4977a..c51e76a21f 100644 --- a/docs/core/tracer.md +++ b/docs/core/tracer.md @@ -50,13 +50,13 @@ The `Tracer` utility must always be instantiated outside of the Lambda handler. The library has three optional settings. You can set them as environment variables, or pass them in the constructor: -| Setting | Description | Environment variable | Constructor parameter | -|----------------------------|-----------------------------------------------------------------------------------------------------------------| -------------------------------------------|------------------------| -| **Service name** | Sets an annotation with the **name of the service** across all traces e.g. `serverlessAirline` | `POWERTOOLS_SERVICE_NAME` | `serviceName` | -| **Tracing enabled** | Enables or disables tracing. By default tracing is enabled when running in AWS Lambda | `POWERTOOLS_TRACE_ENABLED` | `enabled` | -| **Capture HTTPs Requests** | Defines whether HTTPs requests will be traced or not, enabled by default when tracing is also enabled | `POWERTOOLS_TRACER_CAPTURE_HTTPS_REQUESTS` | `captureHTTPsRequests` | -| **Capture Response** | Defines whether functions responses are serialized as metadata, enabled by default when tracing is also enabled | `POWERTOOLS_TRACER_CAPTURE_RESPONSE` | `captureResult` | -| **Capture Errors** | Defines whether functions errors are serialized as metadata, enabled by default when tracing is also enabled | `POWERTOOLS_TRACER_CAPTURE_ERROR` | N/A | +| Setting | Description | Environment variable | Default | Allowed Values | Example | Constructor parameter | +|----------------------------|-----------------------------------------------------------------------| -------------------------------------------|--------------------|------------------|--------------------|------------------------| +| **Service name** | Sets an annotation with the **name of the service** across all traces | `POWERTOOLS_SERVICE_NAME` | `service_undefined`| Any string | `serverlessAirline`| `serviceName` | +| **Tracing enabled** | Enables or disables tracing. | `POWERTOOLS_TRACE_ENABLED` | `true `| `true` or `false`| `false` | `enabled` | +| **Capture HTTPs Requests** | Defines whether HTTPs requests will be traced or not | `POWERTOOLS_TRACER_CAPTURE_HTTPS_REQUESTS` | `true` | `true` or `false`| `false` | `captureHTTPsRequests` | +| **Capture Response** | Defines whether functions responses are serialized as metadata | `POWERTOOLS_TRACER_CAPTURE_RESPONSE` | `true` | `true` or `false`| `false` | `captureResult` | +| **Capture Errors** | Defines whether functions errors are serialized as metadata | `POWERTOOLS_TRACER_CAPTURE_ERROR` | `true` | `true` or `false`| `false` | N/A | !!! note Before your use this utility, your AWS Lambda function must have [Active Tracing enabled](https://docs.aws.amazon.com/lambda/latest/dg/services-xray.html) as well as [have permissions](https://docs.aws.amazon.com/lambda/latest/dg/services-xray.html#services-xray-permissions) to send traces to AWS X-Ray From 99cce63b8a98ba0fe748a84d886a016842c8f624 Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Fri, 11 Nov 2022 12:25:02 +0000 Subject: [PATCH 03/18] refactor: set default service name in utility class Previously the Metric and Tracer classes did not set a service name if one was not provided. There was no default service name. This commit sets a default service name in the Utility class, and updates the Tracer, Metric and Logger classes to use it. --- packages/commons/src/Utility.ts | 6 +++++- packages/logger/src/Logger.ts | 4 +--- packages/metrics/src/Metrics.ts | 2 +- packages/tracer/src/Tracer.ts | 5 +++-- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/commons/src/Utility.ts b/packages/commons/src/Utility.ts index 9991f18cef..9f5e4ae4bc 100644 --- a/packages/commons/src/Utility.ts +++ b/packages/commons/src/Utility.ts @@ -56,8 +56,8 @@ * ``` */ export class Utility { - private coldStart: boolean = true; + private readonly defaultServiceName: string = 'service_undefined'; public getColdStart(): boolean { if (this.coldStart) { @@ -69,6 +69,10 @@ export class Utility { return false; } + public getDefaultServiceName(): string { + return this.defaultServiceName; + } + public isColdStart(): boolean { return this.getColdStart(); } diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index 92f0d9ee07..61bcb25ea2 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -118,8 +118,6 @@ class Logger extends Utility implements ClassThatLogs { private static readonly defaultLogLevel: LogLevel = 'INFO'; - private static readonly defaultServiceName: string = 'service_undefined'; - // envVarsService is always initialized in the constructor in setOptions() private envVarsService!: EnvironmentVariablesService; @@ -786,7 +784,7 @@ class Logger extends Utility implements ClassThatLogs { this.getEnvVarsService().getCurrentEnvironment(), sampleRateValue: this.getSampleRateValue(), serviceName: - serviceName || this.getCustomConfigService()?.getServiceName() || this.getEnvVarsService().getServiceName() || Logger.defaultServiceName, + serviceName || this.getCustomConfigService()?.getServiceName() || this.getEnvVarsService().getServiceName() || this.getDefaultServiceName(), }, persistentLogAttributes, ); diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts index 10e500b5f7..dbde8e0438 100644 --- a/packages/metrics/src/Metrics.ts +++ b/packages/metrics/src/Metrics.ts @@ -473,7 +473,7 @@ class Metrics extends Utility implements MetricsInterface { private setService(service: string | undefined): void { const targetService = (service || this.getCustomConfigService()?.getServiceName() || - this.getEnvVarsService().getServiceName()) as string; + this.getEnvVarsService().getServiceName()) as string || this.getDefaultServiceName(); if (targetService.length > 0) { this.setDefaultDimensions({ service: targetService }); } diff --git a/packages/tracer/src/Tracer.ts b/packages/tracer/src/Tracer.ts index 072e237e4c..237ea3e906 100644 --- a/packages/tracer/src/Tracer.ts +++ b/packages/tracer/src/Tracer.ts @@ -128,7 +128,7 @@ class Tracer extends Utility implements TracerInterface { // envVarsService is always initialized in the constructor in setOptions() private envVarsService!: EnvironmentVariablesService; - private serviceName?: string; + private serviceName!: string; private tracingEnabled: boolean = true; @@ -189,7 +189,7 @@ class Tracer extends Utility implements TracerInterface { * */ public addServiceNameAnnotation(): void { - if (!this.isTracingEnabled() || this.serviceName === undefined) { + if (!this.isTracingEnabled()) { return; } this.putAnnotation('Service', this.serviceName); @@ -836,6 +836,7 @@ class Tracer extends Utility implements TracerInterface { return; } + this.serviceName = this.getDefaultServiceName(); } /** From 126eb4b2e39c419f73ff8c27779432cf82abc86b Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Fri, 11 Nov 2022 12:56:34 +0000 Subject: [PATCH 04/18] test: fix Logger tests Add missing `defaultServiceName` property to expected Logger object Correct typo in test names --- packages/logger/tests/unit/Logger.test.ts | 5 +++++ packages/logger/tests/unit/helpers.test.ts | 22 +++++++++++++--------- packages/tracer/src/Tracer.ts | 2 +- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/packages/logger/tests/unit/Logger.test.ts b/packages/logger/tests/unit/Logger.test.ts index 17b7969230..f7369c58a4 100644 --- a/packages/logger/tests/unit/Logger.test.ts +++ b/packages/logger/tests/unit/Logger.test.ts @@ -571,6 +571,7 @@ describe('Class: Logger', () => { console: expect.any(Console), coldStart: false, // This is now false because the `coldStart` attribute has been already accessed once by the `addContext` method customConfigService: undefined, + defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), logEvent: false, logFormatter: expect.any(PowertoolLogFormatter), @@ -1269,6 +1270,7 @@ describe('Class: Logger', () => { console: expect.any(Console), coldStart: true, customConfigService: undefined, + defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), logEvent: false, logFormatter: expect.any(PowertoolLogFormatter), @@ -1293,6 +1295,7 @@ describe('Class: Logger', () => { console: expect.any(Console), coldStart: true, customConfigService: undefined, + defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), logEvent: false, logFormatter: expect.any(PowertoolLogFormatter), @@ -1319,6 +1322,7 @@ describe('Class: Logger', () => { console: expect.any(Console), coldStart: true, customConfigService: undefined, + defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), logEvent: false, logFormatter: expect.any(PowertoolLogFormatter), @@ -1343,6 +1347,7 @@ describe('Class: Logger', () => { console: expect.any(Console), coldStart: true, customConfigService: undefined, + defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), logEvent: false, logFormatter: expect.any(PowertoolLogFormatter), diff --git a/packages/logger/tests/unit/helpers.test.ts b/packages/logger/tests/unit/helpers.test.ts index 9309bea207..fc1c941c07 100644 --- a/packages/logger/tests/unit/helpers.test.ts +++ b/packages/logger/tests/unit/helpers.test.ts @@ -46,13 +46,14 @@ describe('Helper: createLogger function', () => { }, envVarsService: expect.any(EnvironmentVariablesService), customConfigService: undefined, + defaultServiceName: 'service_undefined', logLevel: 'DEBUG', logFormatter: expect.any(PowertoolLogFormatter), })); }); - test('when no parameters are set, returns a Logger instance with the correct proprieties', () => { + test('when no parameters are set, returns a Logger instance with the correct properties', () => { // Prepare const loggerOptions = { @@ -74,6 +75,7 @@ describe('Helper: createLogger function', () => { expect(logger).toBeInstanceOf(Logger); expect(logger).toEqual({ coldStart: true, + defaultServiceName: 'service_undefined', customConfigService: expect.any(EnvironmentVariablesService), envVarsService: expect.any(EnvironmentVariablesService), logEvent: false, @@ -100,7 +102,7 @@ describe('Helper: createLogger function', () => { }); - test('when no constructor parameters and no environment variables are set, returns a Logger instance with the default proprieties', () => { + test('when no constructor parameters and no environment variables are set, returns a Logger instance with the default properties', () => { // Prepare const loggerOptions = undefined; @@ -115,6 +117,7 @@ describe('Helper: createLogger function', () => { expect(logger).toEqual({ coldStart: true, customConfigService: undefined, + defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), logEvent: false, logFormatter: expect.any(PowertoolLogFormatter), @@ -138,7 +141,7 @@ describe('Helper: createLogger function', () => { }); - test('when a custom logFormatter is passed, returns a Logger instance with the correct proprieties', () => { + test('when a custom logFormatter is passed, returns a Logger instance with the correct properties', () => { // Prepare const loggerOptions:ConstructorOptions = { @@ -166,7 +169,7 @@ describe('Helper: createLogger function', () => { })); }); - test('when a custom serviceName is passed, returns a Logger instance with the correct proprieties', () => { + test('when a custom serviceName is passed, returns a Logger instance with the correct properties', () => { // Prepare const loggerOptions:ConstructorOptions = { @@ -194,7 +197,7 @@ describe('Helper: createLogger function', () => { })); }); - test('when a custom logLevel is passed, returns a Logger instance with the correct proprieties', () => { + test('when a custom logLevel is passed, returns a Logger instance with the correct properties', () => { // Prepare const loggerOptions:ConstructorOptions = { @@ -236,6 +239,7 @@ describe('Helper: createLogger function', () => { expect(logger).toEqual({ coldStart: true, customConfigService: undefined, + defaultServiceName: 'service_undefined', envVarsService: expect.any(EnvironmentVariablesService), logEvent: false, logFormatter: expect.any(PowertoolLogFormatter), @@ -258,7 +262,7 @@ describe('Helper: createLogger function', () => { }); }); - test('when a custom sampleRateValue is passed, returns a Logger instance with the correct proprieties', () => { + test('when a custom sampleRateValue is passed, returns a Logger instance with the correct properties', () => { // Prepare const loggerOptions:ConstructorOptions = { @@ -286,7 +290,7 @@ describe('Helper: createLogger function', () => { })); }); - test('when a custom customConfigService is passed, returns a Logger instance with the correct proprieties', () => { + test('when a custom customConfigService is passed, returns a Logger instance with the correct properties', () => { const configService: ConfigServiceInterface = { get(name: string): string { @@ -335,7 +339,7 @@ describe('Helper: createLogger function', () => { })); }); - test('when custom persistentLogAttributes is passed, returns a Logger instance with the correct proprieties', () => { + test('when custom persistentLogAttributes is passed, returns a Logger instance with the correct properties', () => { // Prepare const loggerOptions:ConstructorOptions = { @@ -377,7 +381,7 @@ describe('Helper: createLogger function', () => { })); }); - test('when A custom environment is passed, returns a Logger instance with the correct proprieties', () => { + test('when a custom environment is passed, returns a Logger instance with the correct properties', () => { // Prepare const loggerOptions:ConstructorOptions = { diff --git a/packages/tracer/src/Tracer.ts b/packages/tracer/src/Tracer.ts index 237ea3e906..65ee29ae6c 100644 --- a/packages/tracer/src/Tracer.ts +++ b/packages/tracer/src/Tracer.ts @@ -189,7 +189,7 @@ class Tracer extends Utility implements TracerInterface { * */ public addServiceNameAnnotation(): void { - if (!this.isTracingEnabled()) { + if (!this.isTracingEnabled() || this.serviceName === undefined) { return; } this.putAnnotation('Service', this.serviceName); From b1c11bf308b8f33d876aa8b11eec2312992b6078 Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Fri, 11 Nov 2022 13:10:41 +0000 Subject: [PATCH 05/18] test(tracer): remove invalid test Remove a test that is no longer valid. As there is a default service name, the tracer should not be enabled even if there is a service name Update the `addServiceAnnotation` function to remove redundant check --- packages/tracer/src/Tracer.ts | 2 +- packages/tracer/tests/unit/Tracer.test.ts | 15 --------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/packages/tracer/src/Tracer.ts b/packages/tracer/src/Tracer.ts index 65ee29ae6c..237ea3e906 100644 --- a/packages/tracer/src/Tracer.ts +++ b/packages/tracer/src/Tracer.ts @@ -189,7 +189,7 @@ class Tracer extends Utility implements TracerInterface { * */ public addServiceNameAnnotation(): void { - if (!this.isTracingEnabled() || this.serviceName === undefined) { + if (!this.isTracingEnabled()) { return; } this.putAnnotation('Service', this.serviceName); diff --git a/packages/tracer/tests/unit/Tracer.test.ts b/packages/tracer/tests/unit/Tracer.test.ts index f541f2e4db..51f389e90a 100644 --- a/packages/tracer/tests/unit/Tracer.test.ts +++ b/packages/tracer/tests/unit/Tracer.test.ts @@ -134,21 +134,6 @@ describe('Class: Tracer', () => { }); - test('when called while a serviceName has not been set, it does nothing', () => { - - // Prepare - delete process.env.POWERTOOLS_SERVICE_NAME; - const tracer: Tracer = new Tracer(); - const putAnnotation = jest.spyOn(tracer, 'putAnnotation').mockImplementation(() => null); - - // Act - tracer.addServiceNameAnnotation(); - - // Assess - expect(putAnnotation).toBeCalledTimes(0); - - }); - }); describe('Method: addResponseAsMetadata', () => { From 517f4010565b2abb4af130cf7ba4ca83c245a817 Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Fri, 11 Nov 2022 13:51:27 +0000 Subject: [PATCH 06/18] docs(metrics): correct default value --- docs/core/metrics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/core/metrics.md b/docs/core/metrics.md index 4e26954a46..661cc78de6 100644 --- a/docs/core/metrics.md +++ b/docs/core/metrics.md @@ -69,7 +69,7 @@ These settings will be used across all metrics emitted: | Setting | Description | Environment variable | Default | Allowed Values | Example | Constructor parameter | |----------------------|-----------------------------------------------------------------|-------------------------------|--------------------|----------------|--------------------|-----------------------| | **Service** | Optionally, sets **service** metric dimension across all metrics| `POWERTOOLS_SERVICE_NAME` | `service_undefined`| Any string | `serverlessAirline`| `serviceName` | -| **Metric namespace** | Logical container where all metrics will be placed | `POWERTOOLS_METRICS_NAMESPACE`| `default_namespace`| Any string | `serverlessAirline`| `namespace` | +| **Metric namespace** | Logical container where all metrics will be placed | `POWERTOOLS_METRICS_NAMESPACE`| `default_namespace`| Any string | `serverlessAirline`| `default_namespace` | !!! tip Use your application name or main service as the metric namespace to easily group all metrics From 6ca800c6574e8a256d184b5b2c4afdef55717ac4 Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Mon, 14 Nov 2022 14:42:24 +0000 Subject: [PATCH 07/18] test(metrics): update metric tests to account for default service dim The `service_name` is now set by default. This means it is always present in metric dimensions. This commit updates the tests to account for this change --- packages/metrics/tests/unit/Metrics.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/metrics/tests/unit/Metrics.test.ts b/packages/metrics/tests/unit/Metrics.test.ts index b3f0086d70..613f265dfc 100644 --- a/packages/metrics/tests/unit/Metrics.test.ts +++ b/packages/metrics/tests/unit/Metrics.test.ts @@ -76,7 +76,7 @@ describe('Class: Metrics', () => { metrics.addDimension(additionalDimension.name, additionalDimension.value); const loggedData = metrics.serializeMetrics(); - expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(1); + expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(2); expect(loggedData[additionalDimension.name]).toEqual(additionalDimension.value); }); @@ -104,15 +104,15 @@ describe('Class: Metrics', () => { expect(console.log).toBeCalledTimes(2); expect(loggedData[0][dimensionItem.name]).toEqual(dimensionItem.value); - expect(loggedData[0]._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(1); + expect(loggedData[0]._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(2); expect(loggedData[1][dimensionItem.name]).toBeUndefined(); - expect(loggedData[1]._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(0); + expect(loggedData[1]._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(1); }); test('Adding more than max dimensions should throw error', () => { expect.assertions(1); const metrics = new Metrics(); - for (let x = 0; x < MAX_DIMENSION_COUNT; x++) { + for (let x = 1; x < MAX_DIMENSION_COUNT; x++) { metrics.addDimension(`Dimension-${x}`, `value-${x}`); } try { @@ -130,7 +130,7 @@ describe('Class: Metrics', () => { metrics.addDimensions(additionalDimensions); const loggedData = metrics.serializeMetrics(); - expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(2); + expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(3); Object.keys(additionalDimensions).forEach((key) => { expect(loggedData[key]).toEqual(additionalDimensions[key]); }); @@ -249,7 +249,7 @@ describe('Class: Metrics', () => { metrics.addMetric('test_name', MetricUnits.Seconds, 10); metrics.addDimension(additionalDimension.name, additionalDimension.value); const loggedData = metrics.serializeMetrics(); - expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(2); + expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(3); expect(loggedData[additionalDimension.name]).toEqual(additionalDimension.value); metrics.clearDimensions(); } @@ -259,7 +259,7 @@ describe('Class: Metrics', () => { const loggedData = JSON.parse(consoleSpy.mock.calls[0][0]); expect(console.log).toBeCalledTimes(1); - expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(1); + expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(2); expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0]).toContain('default'); expect(loggedData.default).toContain('defaultValue'); }); @@ -661,7 +661,7 @@ describe('Class: Metrics', () => { metrics.addMetric('test_name', MetricUnits.Seconds, 10); metrics.addDimension(additionalDimension.name, additionalDimension.value); const loggedData = metrics.serializeMetrics(); - expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(2); + expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(3); expect(loggedData[additionalDimension.name]).toEqual(additionalDimension.value); metrics.clearDimensions(); @@ -673,7 +673,7 @@ describe('Class: Metrics', () => { const loggedData = JSON.parse(consoleSpy.mock.calls[0][0]); expect(console.log).toBeCalledTimes(1); - expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(1); + expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(2); expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0]).toContain('default'); expect(loggedData.default).toContain('defaultValue'); }); From 04b24342fc2f8d9495ffe928daddaf32aef4526b Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Mon, 14 Nov 2022 15:39:01 +0000 Subject: [PATCH 08/18] test(tracer): add test for default service name --- packages/tracer/tests/unit/Tracer.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/tracer/tests/unit/Tracer.test.ts b/packages/tracer/tests/unit/Tracer.test.ts index 51f389e90a..563876b94e 100644 --- a/packages/tracer/tests/unit/Tracer.test.ts +++ b/packages/tracer/tests/unit/Tracer.test.ts @@ -133,6 +133,21 @@ describe('Class: Tracer', () => { expect(putAnnotation).toBeCalledWith('Service', 'foo'); }); + test('when called when a serviceName has not been set in the constructor or environment variables, it adds the default service name as an annotation', () => { + + process.env = { ...ENVIRONMENT_VARIABLES, POWERTOOLS_SERVICE_NAME : undefined }; + // Prepare + const tracer: Tracer = new Tracer(); + const putAnnotation = jest.spyOn(tracer, 'putAnnotation').mockImplementation(() => null); + + // Act + tracer.addServiceNameAnnotation(); + + // Assess + expect(putAnnotation).toBeCalledTimes(1); + expect(putAnnotation).toBeCalledWith('Service', 'service_undefined'); + + }); }); From 8d109c444b3d2ff298108c8c911372e1b16e2881 Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Mon, 14 Nov 2022 15:39:56 +0000 Subject: [PATCH 09/18] test(utility): add test for new method Add test for getDefaultServiceName method --- packages/commons/tests/unit/Utility.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/commons/tests/unit/Utility.test.ts b/packages/commons/tests/unit/Utility.test.ts index 42db79d7af..c3eb2fa8bb 100644 --- a/packages/commons/tests/unit/Utility.test.ts +++ b/packages/commons/tests/unit/Utility.test.ts @@ -12,6 +12,14 @@ describe('Class: Utility', () => { jest.resetModules(); }); + describe('Method: getDefaultServiceName', ()=> { + test('it returns the default service name', ()=> { + const utility = new Utility(); + + expect(utility.getDefaultServiceName).toBe('service_undefined'); + }); + }); + describe('Method: getColdStart', () => { test('when called multiple times on the parent class, it returns true the first time, then false afterwards', () => { From 4837b226e171ffa49c8f9bc958721a7fbff2e11f Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Mon, 14 Nov 2022 19:33:12 +0000 Subject: [PATCH 10/18] test(utility): fix getdefaultservicename test --- packages/commons/tests/unit/Utility.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/commons/tests/unit/Utility.test.ts b/packages/commons/tests/unit/Utility.test.ts index c3eb2fa8bb..9da2083231 100644 --- a/packages/commons/tests/unit/Utility.test.ts +++ b/packages/commons/tests/unit/Utility.test.ts @@ -16,7 +16,7 @@ describe('Class: Utility', () => { test('it returns the default service name', ()=> { const utility = new Utility(); - expect(utility.getDefaultServiceName).toBe('service_undefined'); + expect(utility.getDefaultServiceName()).toBe('service_undefined'); }); }); From 9b0d01f77384361808ff7960a762e7e2af55a166 Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Tue, 15 Nov 2022 08:23:31 +0000 Subject: [PATCH 11/18] refactor: extract isvalidservicename method to utility class This commit extracts the isValidServiceName method from the Tracer class to the Utility class. --- packages/commons/src/Utility.ts | 10 ++++++++++ packages/commons/tests/unit/Utility.test.ts | 12 ++++++++++++ packages/tracer/src/Tracer.ts | 10 ---------- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/packages/commons/src/Utility.ts b/packages/commons/src/Utility.ts index 9f5e4ae4bc..63ebdd0ffb 100644 --- a/packages/commons/src/Utility.ts +++ b/packages/commons/src/Utility.ts @@ -77,4 +77,14 @@ export class Utility { return this.getColdStart(); } + /** + * Validate that the service name provided is valid. + * Used internally during initialization. + * + * @param serviceName - Service name to validate + */ + public static isValidServiceName(serviceName?: string): boolean { + return typeof serviceName === 'string' && serviceName.trim().length > 0; + } + } \ No newline at end of file diff --git a/packages/commons/tests/unit/Utility.test.ts b/packages/commons/tests/unit/Utility.test.ts index 9da2083231..103a3333a0 100644 --- a/packages/commons/tests/unit/Utility.test.ts +++ b/packages/commons/tests/unit/Utility.test.ts @@ -150,4 +150,16 @@ describe('Class: Utility', () => { }); + describe('Method: isValidServiceName', () => { + test('it should allow valid strings', ()=> { + const goodName = 'serverlessAirline'; + expect(Utility.isValidServiceName(goodName)).toBeTruthy(); + }); + + test('it should not allow empty strings', ()=> { + const tooShort = ''; + + expect(Utility.isValidServiceName(tooShort)).toBeFalsy(); + }); + }); }); \ No newline at end of file diff --git a/packages/tracer/src/Tracer.ts b/packages/tracer/src/Tracer.ts index 237ea3e906..f090eff1ba 100644 --- a/packages/tracer/src/Tracer.ts +++ b/packages/tracer/src/Tracer.ts @@ -684,16 +684,6 @@ class Tracer extends Utility implements TracerInterface { return this.getEnvVarsService().getSamLocal() !== ''; } - /** - * Validate that the service name provided is valid. - * Used internally during initialization. - * - * @param serviceName - Service name to validate - */ - private static isValidServiceName(serviceName?: string): boolean { - return typeof serviceName === 'string' && serviceName.trim().length > 0; - } - /** * Setter for `captureError` based on configuration passed and environment variables. * Used internally during initialization. From 4430e1a24c618bbc37880dd7ed616fb8d06e1166 Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Tue, 15 Nov 2022 08:25:20 +0000 Subject: [PATCH 12/18] test(utility): fix test description --- packages/commons/tests/unit/Utility.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/commons/tests/unit/Utility.test.ts b/packages/commons/tests/unit/Utility.test.ts index 103a3333a0..c19e7296fc 100644 --- a/packages/commons/tests/unit/Utility.test.ts +++ b/packages/commons/tests/unit/Utility.test.ts @@ -13,7 +13,7 @@ describe('Class: Utility', () => { }); describe('Method: getDefaultServiceName', ()=> { - test('it returns the default service name', ()=> { + test('it should return the default service name', ()=> { const utility = new Utility(); expect(utility.getDefaultServiceName()).toBe('service_undefined'); From 999262989eea541b84be1bca16c06fbe1ecec534 Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Tue, 15 Nov 2022 09:21:12 +0000 Subject: [PATCH 13/18] refactor: change utility class methods from public to protected --- packages/commons/src/Utility.ts | 11 +++++------ packages/tracer/src/Tracer.ts | 6 +++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/commons/src/Utility.ts b/packages/commons/src/Utility.ts index 63ebdd0ffb..c1c2a0fc05 100644 --- a/packages/commons/src/Utility.ts +++ b/packages/commons/src/Utility.ts @@ -69,22 +69,21 @@ export class Utility { return false; } - public getDefaultServiceName(): string { - return this.defaultServiceName; - } - public isColdStart(): boolean { return this.getColdStart(); } + protected getDefaultServiceName(): string { + return this.defaultServiceName; + } + /** * Validate that the service name provided is valid. * Used internally during initialization. * * @param serviceName - Service name to validate */ - public static isValidServiceName(serviceName?: string): boolean { + protected isValidServiceName(serviceName?: string): boolean { return typeof serviceName === 'string' && serviceName.trim().length > 0; } - } \ No newline at end of file diff --git a/packages/tracer/src/Tracer.ts b/packages/tracer/src/Tracer.ts index f090eff1ba..d6c695f493 100644 --- a/packages/tracer/src/Tracer.ts +++ b/packages/tracer/src/Tracer.ts @@ -807,21 +807,21 @@ class Tracer extends Utility implements TracerInterface { * @param serviceName - Name of the service to use */ private setServiceName(serviceName?: string): void { - if (serviceName !== undefined && Tracer.isValidServiceName(serviceName)) { + if (serviceName !== undefined && this.isValidServiceName(serviceName)) { this.serviceName = serviceName; return; } const customConfigValue = this.getCustomConfigService()?.getServiceName(); - if (customConfigValue !== undefined && Tracer.isValidServiceName(customConfigValue)) { + if (customConfigValue !== undefined && this.isValidServiceName(customConfigValue)) { this.serviceName = customConfigValue; return; } const envVarsValue = this.getEnvVarsService().getServiceName(); - if (envVarsValue !== undefined && Tracer.isValidServiceName(envVarsValue)) { + if (envVarsValue !== undefined && this.isValidServiceName(envVarsValue)) { this.serviceName = envVarsValue; return; From 9082a195d0435166106c9c57dd1bd6d61b8120d4 Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Tue, 15 Nov 2022 11:31:33 +0000 Subject: [PATCH 14/18] test(utility): refactor tests to allow for protected methods --- packages/commons/tests/unit/Utility.test.ts | 33 ++++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/packages/commons/tests/unit/Utility.test.ts b/packages/commons/tests/unit/Utility.test.ts index c19e7296fc..b8a89f4af7 100644 --- a/packages/commons/tests/unit/Utility.test.ts +++ b/packages/commons/tests/unit/Utility.test.ts @@ -14,9 +14,19 @@ describe('Class: Utility', () => { describe('Method: getDefaultServiceName', ()=> { test('it should return the default service name', ()=> { - const utility = new Utility(); + class PowerTool extends Utility { + public constructor() { + super(); + } + + public dummyMethod(): string { + return this.getDefaultServiceName(); + } + } - expect(utility.getDefaultServiceName()).toBe('service_undefined'); + const powertool = new PowerTool(); + const result = powertool.dummyMethod(); + expect(result).toBe('service_undefined'); }); }); @@ -151,15 +161,30 @@ describe('Class: Utility', () => { }); describe('Method: isValidServiceName', () => { + class PowerTool extends Utility { + public constructor() { + super(); + } + + public dummyMethod(name:string): boolean { + return this.isValidServiceName(name); + } + } test('it should allow valid strings', ()=> { + const powertool = new PowerTool(); const goodName = 'serverlessAirline'; - expect(Utility.isValidServiceName(goodName)).toBeTruthy(); + + const result = powertool.dummyMethod(goodName); + + expect(result).toBe(true); }); test('it should not allow empty strings', ()=> { const tooShort = ''; + const powertool = new PowerTool(); + const result = powertool.dummyMethod(tooShort); - expect(Utility.isValidServiceName(tooShort)).toBeFalsy(); + expect(result).toBe(false); }); }); }); \ No newline at end of file From 8307cd920826e155bb3346052c66e6885df26614 Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Tue, 15 Nov 2022 11:39:11 +0000 Subject: [PATCH 15/18] docs(index): remove quotation marks from default values --- docs/index.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/index.md b/docs/index.md index 74410a2dcb..570a3f19ad 100644 --- a/docs/index.md +++ b/docs/index.md @@ -285,8 +285,8 @@ Core utilities such as Tracing, Logging, and Metrics will be available across al | Environment variable | Description | Utility | Default | |----------------------------------------------|----------------------------------------------------------------------------------------|---------------------------|-----------------------| -| **POWERTOOLS_SERVICE_NAME** | Sets service name used for tracing namespace, metrics dimension and structured logging | All | `"service_undefined"` | -| **POWERTOOLS_METRICS_NAMESPACE** | Sets namespace used for metrics | [Metrics](./core/metrics) | `"default_namespace"` | +| **POWERTOOLS_SERVICE_NAME** | Sets service name used for tracing namespace, metrics dimension and structured logging | All | `service_undefined` | +| **POWERTOOLS_METRICS_NAMESPACE** | Sets namespace used for metrics | [Metrics](./core/metrics) | `default_namespace` | | **POWERTOOLS_TRACE_ENABLED** | Explicitly disables tracing | [Tracer](./core/tracer) | `true` | | **POWERTOOLS_TRACER_CAPTURE_RESPONSE** | Captures Lambda or method return as metadata. | [Tracer](./core/tracer) | `true` | | **POWERTOOLS_TRACER_CAPTURE_ERROR** | Captures Lambda or method exception as metadata. | [Tracer](./core/tracer) | `true` | From 2bf7c481cfee8e072c6436c8e33bcea5c48de9a1 Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Tue, 15 Nov 2022 11:52:00 +0000 Subject: [PATCH 16/18] test(metrics): add comments to explain magic numbers --- packages/metrics/tests/unit/Metrics.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/metrics/tests/unit/Metrics.test.ts b/packages/metrics/tests/unit/Metrics.test.ts index 613f265dfc..af6a85bf69 100644 --- a/packages/metrics/tests/unit/Metrics.test.ts +++ b/packages/metrics/tests/unit/Metrics.test.ts @@ -76,6 +76,7 @@ describe('Class: Metrics', () => { metrics.addDimension(additionalDimension.name, additionalDimension.value); const loggedData = metrics.serializeMetrics(); + // Expect the additional dimension, and the service dimension expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(2); expect(loggedData[additionalDimension.name]).toEqual(additionalDimension.value); }); @@ -104,14 +105,17 @@ describe('Class: Metrics', () => { expect(console.log).toBeCalledTimes(2); expect(loggedData[0][dimensionItem.name]).toEqual(dimensionItem.value); + // Expect the additional dimension, and the service dimension expect(loggedData[0]._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(2); expect(loggedData[1][dimensionItem.name]).toBeUndefined(); + // Expect just the service dimension expect(loggedData[1]._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(1); }); test('Adding more than max dimensions should throw error', () => { expect.assertions(1); const metrics = new Metrics(); + // The service dimension is already set, so start from 1 for (let x = 1; x < MAX_DIMENSION_COUNT; x++) { metrics.addDimension(`Dimension-${x}`, `value-${x}`); } @@ -130,6 +134,7 @@ describe('Class: Metrics', () => { metrics.addDimensions(additionalDimensions); const loggedData = metrics.serializeMetrics(); + // Expect the additional dimensions, and the service dimension expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(3); Object.keys(additionalDimensions).forEach((key) => { expect(loggedData[key]).toEqual(additionalDimensions[key]); @@ -249,6 +254,7 @@ describe('Class: Metrics', () => { metrics.addMetric('test_name', MetricUnits.Seconds, 10); metrics.addDimension(additionalDimension.name, additionalDimension.value); const loggedData = metrics.serializeMetrics(); + // Expect the additional dimensions, and the service dimension expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(3); expect(loggedData[additionalDimension.name]).toEqual(additionalDimension.value); metrics.clearDimensions(); @@ -259,6 +265,7 @@ describe('Class: Metrics', () => { const loggedData = JSON.parse(consoleSpy.mock.calls[0][0]); expect(console.log).toBeCalledTimes(1); + // Expect the additional dimension, and the service dimension expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(2); expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0]).toContain('default'); expect(loggedData.default).toContain('defaultValue'); @@ -661,6 +668,7 @@ describe('Class: Metrics', () => { metrics.addMetric('test_name', MetricUnits.Seconds, 10); metrics.addDimension(additionalDimension.name, additionalDimension.value); const loggedData = metrics.serializeMetrics(); + // Expect the additional dimensions, and the service dimension expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(3); expect(loggedData[additionalDimension.name]).toEqual(additionalDimension.value); metrics.clearDimensions(); @@ -673,6 +681,7 @@ describe('Class: Metrics', () => { const loggedData = JSON.parse(consoleSpy.mock.calls[0][0]); expect(console.log).toBeCalledTimes(1); + // Expect the additional dimension, and the service dimension expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0].length).toEqual(2); expect(loggedData._aws.CloudWatchMetrics[0].Dimensions[0]).toContain('default'); expect(loggedData.default).toContain('defaultValue'); From e404a29e4bab2fe372e0da7fb49b1bc4a33c03b4 Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Tue, 15 Nov 2022 11:54:07 +0000 Subject: [PATCH 17/18] doc(tracer): add explanation for definite assignment assertion operator --- packages/tracer/src/Tracer.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/tracer/src/Tracer.ts b/packages/tracer/src/Tracer.ts index d6c695f493..6d846b371f 100644 --- a/packages/tracer/src/Tracer.ts +++ b/packages/tracer/src/Tracer.ts @@ -128,6 +128,7 @@ class Tracer extends Utility implements TracerInterface { // envVarsService is always initialized in the constructor in setOptions() private envVarsService!: EnvironmentVariablesService; + // serviceName is always initialized in the constructor in setOptions() private serviceName!: string; private tracingEnabled: boolean = true; From 8a207836ac3dec2f0c9e717297dcc82614aca383 Mon Sep 17 00:00:00 2001 From: Connor Kirkpatrick Date: Tue, 15 Nov 2022 12:17:28 +0000 Subject: [PATCH 18/18] test(tracer): refactor test to match project norms --- packages/tracer/tests/unit/Tracer.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/tracer/tests/unit/Tracer.test.ts b/packages/tracer/tests/unit/Tracer.test.ts index 563876b94e..129ecb694b 100644 --- a/packages/tracer/tests/unit/Tracer.test.ts +++ b/packages/tracer/tests/unit/Tracer.test.ts @@ -133,10 +133,11 @@ describe('Class: Tracer', () => { expect(putAnnotation).toBeCalledWith('Service', 'foo'); }); + test('when called when a serviceName has not been set in the constructor or environment variables, it adds the default service name as an annotation', () => { - process.env = { ...ENVIRONMENT_VARIABLES, POWERTOOLS_SERVICE_NAME : undefined }; // Prepare + delete process.env.POWERTOOLS_SERVICE_NAME; const tracer: Tracer = new Tracer(); const putAnnotation = jest.spyOn(tracer, 'putAnnotation').mockImplementation(() => null);