Skip to content

Commit

Permalink
feat: support max-buffer-size for AWSLogs driver (#26396)
Browse files Browse the repository at this point in the history
When mode is set to `non-blocking`, logs are stored in a buffer. The setting `max-buffer-size` controls the size of this buffer. Being able to set this buffer is very important, the larger the buffer, the less likely there is to be log loss.

Recently I performed benchmarking of `non-blocking` mode, and found that the default buffer size is not sufficient to prevent log loss: moby/moby#45999

We're planning to run an education campaign to ensure ECS customers understand the risk with the default `blocking` mode, and the risk with `non-blocking` mode with the default buffer size. Therefore, we need folks to be able to control this setting in their CDK.



----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
PettitWesley committed Jul 28, 2023
1 parent 9753039 commit a74536b
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,9 @@
"awslogs-stream-prefix": "firelens",
"awslogs-region": {
"Ref": "AWS::Region"
}
},
"mode": "non-blocking",
"max-buffer-size": "26214400b"
}
},
"MemoryReservation": 50,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1526,7 +1526,9 @@
"awslogs-stream-prefix": "firelens",
"awslogs-region": {
"Ref": "AWS::Region"
}
},
"mode": "non-blocking",
"max-buffer-size": "26214400b"
}
},
"firelensConfiguration": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ taskDefinition.addFirelensLogRouter('log_router', {
configFileType: ecs.FirelensConfigFileType.S3,
},
},
logging: new ecs.AwsLogDriver({ streamPrefix: 'firelens' }),
logging: new ecs.AwsLogDriver({
streamPrefix: 'firelens',
mode: ecs.AwsLogDriverMode.NON_BLOCKING,
maxBufferSize: cdk.Size.mebibytes(25),
}),
memoryReservationMiB: 50,
});

Expand Down
6 changes: 5 additions & 1 deletion packages/aws-cdk-lib/aws-ecs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,11 @@ const taskDefinition = new ecs.Ec2TaskDefinition(this, 'TaskDef');
taskDefinition.addContainer('TheContainer', {
image: ecs.ContainerImage.fromRegistry('example-image'),
memoryLimitMiB: 256,
logging: ecs.LogDrivers.awsLogs({ streamPrefix: 'EventDemo' }),
logging: ecs.LogDrivers.awsLogs({
streamPrefix: 'EventDemo',
mode: ecs.AwsLogDriverMode.NON_BLOCKING,
maxBufferSize: Size.mebibytes(25),
}),
});
```

Expand Down
20 changes: 20 additions & 0 deletions packages/aws-cdk-lib/aws-ecs/lib/log-drivers/aws-log-driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Construct } from 'constructs';
import { LogDriver, LogDriverConfig } from './log-driver';
import { removeEmpty } from './utils';
import * as logs from '../../../aws-logs';
import { Size, SizeRoundingBehavior } from '../../../core';
import { ContainerDefinition } from '../container-definition';

/**
Expand Down Expand Up @@ -82,6 +83,16 @@ export interface AwsLogDriverProps {
* @default - AwsLogDriverMode.BLOCKING
*/
readonly mode?: AwsLogDriverMode;

/**
* When AwsLogDriverMode.NON_BLOCKING is configured, this parameter
* controls the size of the non-blocking buffer used to temporarily
* store messages. This parameter is not valid with
* AwsLogDriverMode.BLOCKING.
*
* @default - 1 megabyte if driver mode is non-blocking, otherwise this property is not set
*/
readonly maxBufferSize?: Size;
}

/**
Expand All @@ -106,6 +117,10 @@ export class AwsLogDriver extends LogDriver {
if (props.logGroup && props.logRetention) {
throw new Error('Cannot specify both `logGroup` and `logRetentionDays`.');
}

if (props.maxBufferSize && props.mode !== AwsLogDriverMode.NON_BLOCKING) {
throw new Error('Cannot specify `maxBufferSize` when the driver mode is blocking');
}
}

/**
Expand All @@ -116,6 +131,10 @@ export class AwsLogDriver extends LogDriver {
retention: this.props.logRetention || Infinity,
});

const maxBufferSize = this.props.maxBufferSize
? `${this.props.maxBufferSize.toBytes({ rounding: SizeRoundingBehavior.FLOOR })}b`
: undefined;

this.logGroup.grantWrite(containerDefinition.taskDefinition.obtainExecutionRole());

return {
Expand All @@ -127,6 +146,7 @@ export class AwsLogDriver extends LogDriver {
'awslogs-datetime-format': this.props.datetimeFormat,
'awslogs-multiline-pattern': this.props.multilinePattern,
'mode': this.props.mode,
'max-buffer-size': maxBufferSize,
}),
};
}
Expand Down
29 changes: 29 additions & 0 deletions packages/aws-cdk-lib/aws-ecs/test/aws-log-driver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ describe('aws log driver', () => {
multilinePattern: 'pattern',
streamPrefix: 'hello',
mode: ecs.AwsLogDriverMode.NON_BLOCKING,
maxBufferSize: cdk.Size.mebibytes(25),
}),
});

Expand All @@ -44,6 +45,7 @@ describe('aws log driver', () => {
'awslogs-datetime-format': 'format',
'awslogs-multiline-pattern': 'pattern',
'mode': 'non-blocking',
'max-buffer-size': '26214400b',
},
},
}),
Expand Down Expand Up @@ -146,6 +148,33 @@ describe('aws log driver', () => {

});

test('throws error when specifying maxBufferSize and blocking mode', () => {
// GIVEN
const logGroup = new logs.LogGroup(stack, 'LogGroup');

// THEN
expect(() => new ecs.AwsLogDriver({
logGroup,
streamPrefix: 'hello',
mode: ecs.AwsLogDriverMode.BLOCKING,
maxBufferSize: cdk.Size.mebibytes(25),
})).toThrow(/.*maxBufferSize.*/);

});

test('throws error when specifying maxBufferSize and default settings', () => {
// GIVEN
const logGroup = new logs.LogGroup(stack, 'LogGroup');

// THEN
expect(() => new ecs.AwsLogDriver({
logGroup,
streamPrefix: 'hello',
maxBufferSize: cdk.Size.mebibytes(25),
})).toThrow(/.*maxBufferSize.*/);

});

test('allows cross-region log group', () => {
// GIVEN
const logGroupRegion = 'asghard';
Expand Down

0 comments on commit a74536b

Please sign in to comment.