Skip to content

Commit 1992818

Browse files
authored
Merge branch 'main' into ci/switch_e2e_env
2 parents 86c6d92 + 4c1bcc3 commit 1992818

File tree

4 files changed

+139
-74
lines changed

4 files changed

+139
-74
lines changed

packages/logger/src/Logger.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
getXRayTraceIdFromEnv,
1414
isDevMode,
1515
} from '@aws-lambda-powertools/commons/utils/env';
16-
import type { Context, Handler } from 'aws-lambda';
16+
import type { Callback, Context, Handler } from 'aws-lambda';
1717
import merge from 'lodash.merge';
1818
import {
1919
LogJsonIndent,
@@ -493,19 +493,17 @@ class Logger extends Utility implements LoggerInterface {
493493
// access `myClass` as `this` in a decorated `myClass.myMethod()`.
494494
descriptor.value = async function (
495495
this: Handler,
496-
event,
497-
context,
498-
callback
496+
...args: [unknown, Context, Callback]
499497
) {
500498
loggerRef.refreshSampleRateCalculation();
501-
loggerRef.addContext(context);
502-
loggerRef.logEventIfEnabled(event, options?.logEvent);
499+
loggerRef.addContext(args[1]);
500+
loggerRef.logEventIfEnabled(args[0], options?.logEvent);
503501
if (options?.correlationIdPath) {
504-
loggerRef.setCorrelationId(event, options?.correlationIdPath);
502+
loggerRef.setCorrelationId(args[0], options?.correlationIdPath);
505503
}
506504

507505
try {
508-
return await originalMethod.apply(this, [event, context, callback]);
506+
return await originalMethod.apply(this, args);
509507
} catch (error) {
510508
if (options?.flushBufferOnUncaughtError) {
511509
loggerRef.flushBuffer();

packages/metrics/src/Metrics.ts

Lines changed: 64 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,10 @@ class Metrics extends Utility implements MetricsInterface {
240240
super();
241241

242242
this.dimensions = {};
243-
this.setOptions(options);
243+
this.setEnvConfig();
244+
this.setConsole();
244245
this.#logger = options.logger || this.console;
246+
this.setOptions(options);
245247
}
246248

247249
/**
@@ -293,38 +295,16 @@ class Metrics extends Utility implements MetricsInterface {
293295
* @param dimensions - An object with key-value pairs of dimensions
294296
*/
295297
public addDimensions(dimensions: Dimensions): void {
296-
const newDimensionSet: Dimensions = {};
297-
for (const [key, value] of Object.entries(dimensions)) {
298-
if (
299-
isStringUndefinedNullEmpty(key) ||
300-
isStringUndefinedNullEmpty(value)
301-
) {
302-
this.#logger.warn(
303-
`The dimension ${key} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
304-
);
305-
continue;
306-
}
307-
if (
308-
Object.hasOwn(this.dimensions, key) ||
309-
Object.hasOwn(this.defaultDimensions, key) ||
310-
Object.hasOwn(newDimensionSet, key)
311-
) {
312-
this.#logger.warn(
313-
`Dimension "${key}" has already been added. The previous value will be overwritten.`
314-
);
315-
}
316-
newDimensionSet[key] = value;
317-
}
318-
298+
const newDimensions = this.#sanitizeDimensions(dimensions);
319299
const currentCount = this.getCurrentDimensionsCount();
320-
const newSetCount = Object.keys(newDimensionSet).length;
300+
const newSetCount = Object.keys(newDimensions).length;
321301
if (currentCount + newSetCount >= MAX_DIMENSION_COUNT) {
322302
throw new RangeError(
323303
`The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
324304
);
325305
}
326306

327-
this.dimensionSets.push(newDimensionSet);
307+
this.dimensionSets.push(newDimensions);
328308
}
329309

330310
/**
@@ -432,12 +412,6 @@ class Metrics extends Utility implements MetricsInterface {
432412
public captureColdStartMetric(functionName?: string): void {
433413
if (!this.getColdStart()) return;
434414
const singleMetric = this.singleMetric();
435-
436-
if (this.defaultDimensions.service) {
437-
singleMetric.setDefaultDimensions({
438-
service: this.defaultDimensions.service,
439-
});
440-
}
441415
const value = this.functionName?.trim() ?? functionName?.trim();
442416
if (value && value.length > 0) {
443417
singleMetric.addDimension('function_name', value);
@@ -598,17 +572,15 @@ class Metrics extends Utility implements MetricsInterface {
598572
// access `myClass` as `this` in a decorated `myClass.myMethod()`.
599573
descriptor.value = async function (
600574
this: Handler,
601-
event: unknown,
602-
context: Context,
603-
callback: Callback
575+
...args: [unknown, Context, Callback]
604576
): Promise<unknown> {
605577
if (captureColdStartMetric) {
606-
metricsRef.captureColdStartMetric(context.functionName);
578+
metricsRef.captureColdStartMetric(args[1].functionName);
607579
}
608580

609581
let result: unknown;
610582
try {
611-
result = await originalMethod.apply(this, [event, context, callback]);
583+
result = await originalMethod.apply(this, args);
612584
} finally {
613585
metricsRef.publishStoredMetrics();
614586
}
@@ -823,15 +795,20 @@ class Metrics extends Utility implements MetricsInterface {
823795
*
824796
* @param dimensions - The dimensions to be added to the default dimensions object
825797
*/
826-
public setDefaultDimensions(dimensions: Dimensions | undefined): void {
827-
const targetDimensions = {
798+
public setDefaultDimensions(dimensions: Dimensions): void {
799+
const newDimensions = this.#sanitizeDimensions(dimensions);
800+
const currentCount = Object.keys(this.defaultDimensions).length;
801+
const newSetCount = Object.keys(newDimensions).length;
802+
if (currentCount + newSetCount >= MAX_DIMENSION_COUNT) {
803+
throw new RangeError(
804+
`The number of metric dimensions must be lower than ${MAX_DIMENSION_COUNT}`
805+
);
806+
}
807+
808+
this.defaultDimensions = {
828809
...this.defaultDimensions,
829-
...dimensions,
810+
...newDimensions,
830811
};
831-
if (MAX_DIMENSION_COUNT <= Object.keys(targetDimensions).length) {
832-
throw new Error('Max dimension count hit');
833-
}
834-
this.defaultDimensions = targetDimensions;
835812
}
836813

837814
/**
@@ -888,7 +865,6 @@ class Metrics extends Utility implements MetricsInterface {
888865
public singleMetric(): Metrics {
889866
return new Metrics({
890867
namespace: this.namespace,
891-
serviceName: this.dimensions.service,
892868
defaultDimensions: this.defaultDimensions,
893869
singleMetric: true,
894870
logger: this.#logger,
@@ -1058,33 +1034,33 @@ class Metrics extends Utility implements MetricsInterface {
10581034
functionName,
10591035
} = options;
10601036

1061-
this.setEnvConfig();
1062-
this.setConsole();
10631037
this.setCustomConfigService(customConfigService);
10641038
this.setDisabled();
10651039
this.setNamespace(namespace);
1066-
this.setService(serviceName);
1067-
this.setDefaultDimensions(defaultDimensions);
1040+
const resolvedServiceName = this.#resolveServiceName(serviceName);
1041+
this.setDefaultDimensions(
1042+
defaultDimensions
1043+
? { service: resolvedServiceName, ...defaultDimensions }
1044+
: { service: resolvedServiceName }
1045+
);
10681046
this.setFunctionNameForColdStartMetric(functionName);
10691047
this.isSingleMetric = singleMetric || false;
10701048

10711049
return this;
10721050
}
10731051

10741052
/**
1075-
* Set the service to be used.
1053+
* Set the service dimension that will be included in the metrics.
10761054
*
10771055
* @param service - The service to be used
10781056
*/
1079-
private setService(service: string | undefined): void {
1080-
const targetService =
1057+
#resolveServiceName(service?: string): string {
1058+
return (
10811059
service ||
10821060
this.getCustomConfigService()?.getServiceName() ||
10831061
this.#envConfig.serviceName ||
1084-
this.defaultServiceName;
1085-
if (targetService.length > 0) {
1086-
this.setDefaultDimensions({ service: targetService });
1087-
}
1062+
this.defaultServiceName
1063+
);
10881064
}
10891065

10901066
/**
@@ -1191,6 +1167,38 @@ class Metrics extends Utility implements MetricsInterface {
11911167
**/
11921168
return 0;
11931169
}
1170+
1171+
/**
1172+
* Sanitizes the dimensions by removing invalid entries and skipping duplicates.
1173+
*
1174+
* @param dimensions - The dimensions to sanitize.
1175+
*/
1176+
#sanitizeDimensions(dimensions: Dimensions): Dimensions {
1177+
const newDimensions: Dimensions = {};
1178+
for (const [key, value] of Object.entries(dimensions)) {
1179+
if (
1180+
isStringUndefinedNullEmpty(key) ||
1181+
isStringUndefinedNullEmpty(value)
1182+
) {
1183+
this.#logger.warn(
1184+
`The dimension ${key} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
1185+
);
1186+
continue;
1187+
}
1188+
if (
1189+
Object.hasOwn(this.dimensions, key) ||
1190+
Object.hasOwn(this.defaultDimensions, key) ||
1191+
Object.hasOwn(newDimensions, key)
1192+
) {
1193+
this.#logger.warn(
1194+
`Dimension "${key}" has already been added. The previous value will be overwritten.`
1195+
);
1196+
}
1197+
newDimensions[key] = value;
1198+
}
1199+
1200+
return newDimensions;
1201+
}
11941202
}
11951203

11961204
export { Metrics };

packages/metrics/tests/unit/dimensions.test.ts

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ describe('Working with dimensions', () => {
358358

359359
// Assess
360360
expect(() => metrics.setDefaultDimensions({ extra: 'test' })).toThrowError(
361-
'Max dimension count hit'
361+
'The number of metric dimensions must be lower than 29'
362362
);
363363
});
364364

@@ -552,4 +552,62 @@ describe('Working with dimensions', () => {
552552
})
553553
);
554554
});
555+
556+
it('warns when setDefaultDimensions overwrites existing dimensions', () => {
557+
// Prepare
558+
const metrics = new Metrics({
559+
namespace: DEFAULT_NAMESPACE,
560+
defaultDimensions: { environment: 'prod' },
561+
});
562+
563+
// Act
564+
metrics.setDefaultDimensions({ region: 'us-east-1' });
565+
metrics.setDefaultDimensions({
566+
environment: 'staging', // overwrites default dimension
567+
});
568+
569+
// Assess
570+
expect(console.warn).toHaveBeenCalledOnce();
571+
expect(console.warn).toHaveBeenCalledWith(
572+
'Dimension "environment" has already been added. The previous value will be overwritten.'
573+
);
574+
});
575+
576+
it.each([
577+
{ value: undefined, name: 'valid-name' },
578+
{ value: null, name: 'valid-name' },
579+
{ value: '', name: 'valid-name' },
580+
{ value: 'valid-value', name: '' },
581+
])(
582+
'skips invalid default dimension values in setDefaultDimensions ($name)',
583+
({ value, name }) => {
584+
// Arrange
585+
const metrics = new Metrics({
586+
singleMetric: true,
587+
namespace: DEFAULT_NAMESPACE,
588+
});
589+
590+
// Act
591+
metrics.setDefaultDimensions({
592+
validDimension: 'valid',
593+
[name as string]: value as string,
594+
});
595+
596+
metrics.addMetric('test', MetricUnit.Count, 1);
597+
metrics.publishStoredMetrics();
598+
599+
// Assess
600+
expect(console.warn).toHaveBeenCalledWith(
601+
`The dimension ${name} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
602+
);
603+
604+
expect(console.log).toHaveEmittedEMFWith(
605+
expect.objectContaining({ validDimension: 'valid' })
606+
);
607+
608+
expect(console.log).toHaveEmittedEMFWith(
609+
expect.not.objectContaining({ [name]: value })
610+
);
611+
}
612+
);
555613
});

packages/parser/src/parserDecorator.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { HandlerMethodDecorator } from '@aws-lambda-powertools/commons/types';
22
import type { StandardSchemaV1 } from '@standard-schema/spec';
3-
import type { Context, Handler } from 'aws-lambda';
3+
import type { Callback, Context, Handler } from 'aws-lambda';
44
import { parse } from './parser.js';
55
import type { Envelope, ParserOptions } from './types/index.js';
66
import type { ParserOutput } from './types/parser.js';
@@ -75,21 +75,22 @@ export const parser = <
7575
>(
7676
options: ParserOptions<TSchema, TEnvelope, TSafeParse>
7777
): HandlerMethodDecorator => {
78-
return (_target, _propertyKey, descriptor) => {
79-
// biome-ignore lint/style/noNonNullAssertion: The descriptor.value is the method this decorator decorates, it cannot be undefined.
80-
const original = descriptor.value!;
78+
return (
79+
_target: unknown,
80+
_propertyKey: string | symbol,
81+
descriptor: PropertyDescriptor
82+
) => {
83+
const original = descriptor.value;
8184

8285
const { schema, envelope, safeParse } = options;
8386

8487
descriptor.value = async function (
8588
this: Handler,
86-
event: ParserOutput<TSchema, TEnvelope, TSafeParse>,
87-
context: Context,
88-
callback
89+
...args: [ParserOutput<TSchema, TEnvelope, TSafeParse>, Context, Callback]
8990
) {
90-
const parsedEvent = parse(event, envelope, schema, safeParse);
91+
const parsedEvent = parse(args[0], envelope, schema, safeParse);
9192

92-
return original.call(this, parsedEvent, context, callback);
93+
return original.apply(this, [parsedEvent, ...args.slice(1)]);
9394
};
9495

9596
return descriptor;

0 commit comments

Comments
 (0)