Skip to content

fix(metrics): Support multiple addMetric() call with the same metric name #390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 90 additions & 42 deletions docs/core/metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,54 @@ You can create metrics using `addMetric`, and you can create dimensions for all
!!! warning "Do not create metrics or dimensions outside the handler"
Metrics or dimensions added in the global scope will only be added during cold start. Disregard if that's the intended behaviour.

### Adding multi-value metrics
You can call `addMetric()` with the same name multiple times. The values will be grouped together in an array.

=== "addMetric() with the same name"

```typescript hl_lines="8 10"
import { Metrics, MetricUnits } from '@aws-lambda-powertools/metrics';
import { Context } from 'aws-lambda';


const metrics = new Metrics({namespace:"serverlessAirline", service:"orders"});

export const handler = async (event: any, context: Context) => {
metrics.addMetric('performedActionA', MetricUnits.Count, 2);
// do something else...
metrics.addMetric('performedActionA', MetricUnits.Count, 1);
}
```
=== "Example CloudWatch Logs excerpt"

```json hl_lines="2-5 18-19"
{
"performedActionA": [
2,
1
],
"_aws": {
"Timestamp": 1592234975665,
"CloudWatchMetrics": [
{
"Namespace": "serverlessAirline",
"Dimensions": [
[
"service"
]
],
"Metrics": [
{
"Name": "performedActionA",
"Unit": "Count"
}
]
}
]
},
"service": "orders"
}
```
### Adding default dimensions

You can use add default dimensions to your metrics by passing them as parameters in 4 ways:
Expand Down Expand Up @@ -264,23 +312,23 @@ See below an example of how to automatically flush metrics with the Middy-compat
{
"bookingConfirmation": 1.0,
"_aws": {
"Timestamp": 1592234975665,
"CloudWatchMetrics": [
{
"Namespace": "exampleApplication",
"Dimensions": [
[
"service"
]
],
"Metrics": [
"Timestamp": 1592234975665,
"CloudWatchMetrics": [
{
"Name": "bookingConfirmation",
"Unit": "Count"
"Namespace": "exampleApplication",
"Dimensions": [
[
"service"
]
],
"Metrics": [
{
"Name": "bookingConfirmation",
"Unit": "Count"
}
]
Comment on lines +315 to +329
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just shifting indentation. The current version misses one more indentation under "_aws"

}
]
}
]
},
"service": "exampleService"
}
Expand Down Expand Up @@ -316,23 +364,23 @@ export class MyFunction {
{
"bookingConfirmation": 1.0,
"_aws": {
"Timestamp": 1592234975665,
"CloudWatchMetrics": [
{
"Namespace": "exampleApplication",
"Dimensions": [
[
"service"
]
],
"Metrics": [
"Timestamp": 1592234975665,
"CloudWatchMetrics": [
{
"Name": "bookingConfirmation",
"Unit": "Count"
"Namespace": "exampleApplication",
"Dimensions": [
[
"service"
]
],
"Metrics": [
{
"Name": "bookingConfirmation",
"Unit": "Count"
}
]
}
]
}
]
},
"service": "exampleService"
}
Expand Down Expand Up @@ -456,23 +504,23 @@ You can add high-cardinality data as part of your Metrics log with `addMetadata`
{
"successfulBooking": 1.0,
"_aws": {
"Timestamp": 1592234975665,
"CloudWatchMetrics": [
{
"Namespace": "exampleApplication",
"Dimensions": [
[
"service"
]
],
"Metrics": [
"Timestamp": 1592234975665,
"CloudWatchMetrics": [
{
"Name": "successfulBooking",
"Unit": "Count"
"Namespace": "exampleApplication",
"Dimensions": [
[
"service"
]
],
"Metrics": [
{
"Name": "successfulBooking",
"Unit": "Count"
}
]
}
]
}
]
},
"service": "booking",
"bookingId": "7051cd10-6283-11ec-90d6-0242ac120003"
Expand Down
2 changes: 1 addition & 1 deletion packages/metrics/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/metrics/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"commit": "commit",
"test": "jest --group=unit --detectOpenHandles --coverage --verbose",
"test:e2e": "jest --group=e2e",
"watch": "jest --watch",
"watch": "jest --group=unit --watch ",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not practical to run e2e test in the watch mode so I filtered this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point !

"build": "tsc",
"lint": "eslint --ext .ts --fix --no-error-on-unmatched-pattern src tests",
"format": "eslint --fix --ext .ts --fix --no-error-on-unmatched-pattern src tests",
Expand Down
36 changes: 30 additions & 6 deletions packages/metrics/src/Metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ class Metrics implements MetricsInterface {
if (!this.namespace) console.warn('Namespace should be defined, default used');

const metricValues = Object.values(this.storedMetrics).reduce(
(result: { [key: string]: number }, { name, value }: { name: string; value: number }) => {
(result: { [key: string]: number | number[] }, { name, value }: { name: string; value: number | number[] }) => {
result[name] = value;

return result;
Expand Down Expand Up @@ -390,6 +390,20 @@ class Metrics implements MetricsInterface {
return <EnvironmentVariablesService> this.envVarsService;
}

private isNewMetric(name: string, unit: MetricUnit): boolean {
if (this.storedMetrics[name]){
// Inconsistent units indicates a bug or typos and we want to flag this to users early
if (this.storedMetrics[name].unit !== unit) {
const currentUnit = this.storedMetrics[name].unit;
throw new Error(`Metric "${name}" has already been added with unit "${currentUnit}", but we received unit "${unit}". Did you mean to use metric unit "${currentUnit}"?`);
}

return false;
} else {
return true;
}
}

private setCustomConfigService(customConfigService?: ConfigServiceInterface): void {
this.customConfigService = customConfigService ? customConfigService : undefined;
}
Expand Down Expand Up @@ -430,12 +444,22 @@ class Metrics implements MetricsInterface {
if (Object.keys(this.storedMetrics).length >= MAX_METRICS_SIZE) {
this.purgeStoredMetrics();
}
this.storedMetrics[name] = {
unit,
value,
name,
};

if (this.isNewMetric(name, unit)) {
this.storedMetrics[name] = {
unit,
value,
name,
};
} else {
const storedMetric = this.storedMetrics[name];
if (!Array.isArray(storedMetric.value)) {
storedMetric.value = [storedMetric.value];
}
storedMetric.value.push(value);
}
}

}

export { Metrics, MetricUnits };
2 changes: 1 addition & 1 deletion packages/metrics/src/types/Metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type ExtraOptions = {
type StoredMetric = {
name: string
unit: MetricUnit
value: number
value: number | number[]
};

type StoredMetrics = {
Expand Down
49 changes: 49 additions & 0 deletions packages/metrics/tests/unit/Metrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,55 @@ describe('Class: Metrics', () => {
expect(serializedMetrics._aws.CloudWatchMetrics[0].Namespace).toBe(DEFAULT_NAMESPACE);
expect(console.warn).toHaveBeenNthCalledWith(1, 'Namespace should be defined, default used');
});

test('Should contain a metric value if added once', ()=> {
const metrics = new Metrics();

metrics.addMetric('test_name', MetricUnits.Count, 1);
const serializedMetrics = metrics.serializeMetrics();

expect(serializedMetrics._aws.CloudWatchMetrics[0].Metrics.length).toBe(1);

expect(serializedMetrics['test_name']).toBe(1);
});

test('Should convert multiple metrics with the same name and unit into an array', ()=> {
const metrics = new Metrics();

metrics.addMetric('test_name', MetricUnits.Count, 2);
metrics.addMetric('test_name', MetricUnits.Count, 1);
const serializedMetrics = metrics.serializeMetrics();

expect(serializedMetrics._aws.CloudWatchMetrics[0].Metrics.length).toBe(1);
expect(serializedMetrics['test_name']).toStrictEqual([ 2, 1 ]);
});

test('Should throw an error if the same metric name is added again with a different unit', ()=> {
const metrics = new Metrics();

metrics.addMetric('test_name', MetricUnits.Count, 2);
try {
metrics.addMetric('test_name', MetricUnits.Seconds, 10);
} catch (e) {
expect((<Error>e).message).toBe('Metric "test_name" has already been added with unit "Count", but we received unit "Seconds". Did you mean to use metric unit "Count"?');
}
});

test('Should contain multiple metric values if added with multiple names', ()=> {
const metrics = new Metrics();

metrics.addMetric('test_name', MetricUnits.Count, 1);
metrics.addMetric('test_name2', MetricUnits.Count, 2);
const serializedMetrics = metrics.serializeMetrics();

expect(serializedMetrics._aws.CloudWatchMetrics[0].Metrics).toStrictEqual([
{ Name: 'test_name', Unit: 'Count' },
{ Name: 'test_name2', Unit: 'Count' },
]);

expect(serializedMetrics['test_name']).toBe(1);
expect(serializedMetrics['test_name2']).toBe(2);
});
});

describe('Feature: Clearing Metrics ', () => {
Expand Down
37 changes: 34 additions & 3 deletions packages/metrics/tests/unit/middleware/middy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,47 @@ describe('Middy middleware', () => {
succeed: () => console.log('Succeeded!'),
};

test('when a metrics instance receive multiple metrics with the same name, it prints multiple values in an array format', async () => {
// Prepare
const metrics = new Metrics({ namespace:'serverlessAirline', service:'orders' });

const lambdaHandler = (): void => {
metrics.addMetric('successfulBooking', MetricUnits.Count, 2);
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
};

const handler = middy(lambdaHandler).use(logMetrics(metrics));

// Act
await handler(event, context, () => console.log('Lambda invoked!'));

// Assess
expect(console.log).toHaveBeenNthCalledWith(1, JSON.stringify({
'_aws': {
'Timestamp': 1466424490000,
'CloudWatchMetrics': [{
'Namespace': 'serverlessAirline',
'Dimensions': [
['service']
],
'Metrics': [{ 'Name': 'successfulBooking', 'Unit': 'Count' }],
}],
},
'service': 'orders',
'successfulBooking': [
2,
1,
],
}));
});

test('when a metrics instance is passed WITH custom options, it prints the metrics in the stdout', async () => {

// Prepare
const metrics = new Metrics({ namespace:'serverlessAirline', service:'orders' });

const lambdaHandler = (): void => {
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
};
const metricsOptions: ExtraOptions = {
raiseOnEmptyMetrics: true,
Expand Down Expand Up @@ -107,7 +140,6 @@ describe('Middy middleware', () => {

const lambdaHandler = (): void => {
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
};

const handler = middy(lambdaHandler).use(logMetrics(metrics));
Expand Down Expand Up @@ -140,7 +172,6 @@ describe('Middy middleware', () => {

const lambdaHandler = (): void => {
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
};
const metricsOptions: ExtraOptions = {
raiseOnEmptyMetrics: true
Expand Down