Skip to content
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

chore(kinesisfirehose-destinations): refactor logging to combine logGroup and logging properties into loggingConfig #31488

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-kinesisfirehose-alpha/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,16 +183,16 @@ Kinesis Data Firehose will send logs to CloudWatch when data transformation or d
delivery fails. The CDK will enable logging by default and create a CloudWatch LogGroup
and LogStream for your Delivery Stream.

When you create a destination, you can specify a log group. In this log group, The CDK
will create log streams where log events will be sent:
When creating a destination, you can provide a `LoggingConfig`, which can either be an `EnableLogging` or `DisableLogging` instance.
If you use `EnableLogging`, you can specify a log group where the CDK will create log streams to capture and store log events. For example:

```ts
import * as logs from 'aws-cdk-lib/aws-logs';

const logGroup = new logs.LogGroup(this, 'Log Group');
declare const bucket: s3.Bucket;
const destination = new destinations.S3Bucket(bucket, {
logGroup: logGroup,
loggingConfig: new destinations.EnableLogging(logGroup),
});

new firehose.DeliveryStream(this, 'Delivery Stream', {
Expand All @@ -205,7 +205,7 @@ Logging can also be disabled:
```ts
declare const bucket: s3.Bucket;
const destination = new destinations.S3Bucket(bucket, {
logging: false,
loggingConfig: new destinations.DisableLogging(),
});
new firehose.DeliveryStream(this, 'Delivery Stream', {
destinations: [destination],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as kms from 'aws-cdk-lib/aws-kms';
import * as logs from 'aws-cdk-lib/aws-logs';
import * as s3 from 'aws-cdk-lib/aws-s3';
import * as cdk from 'aws-cdk-lib/core';
import { LoggingConfig } from './logging-config';

/**
* Possible compression options Kinesis Data Firehose can use to compress data on delivery.
Expand Down Expand Up @@ -62,20 +63,12 @@ export enum BackupMode {
*/
interface DestinationLoggingProps {
/**
* If true, log errors when data transformation or data delivery fails.
* Configuration that determines whether to log errors during data transformation or delivery failures,
* and specifies the CloudWatch log group for storing error logs.
*
* If `logGroup` is provided, this will be implicitly set to `true`.
*
* @default true - errors are logged.
*/
readonly logging?: boolean;

/**
* The CloudWatch log group where log streams will be created to hold error logs.
*
* @default - if `logging` is set to `true`, a log group will be created for you.
* @default - errors will be logged and a log group will be created for you.
*/
readonly logGroup?: logs.ILogGroup;
readonly loggingConfig?: LoggingConfig;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * from './common';
export * from './s3-bucket';
export * from './logging-config';
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import * as logs from 'aws-cdk-lib/aws-logs';

/**
* Represents the logging configuration for error logs.
*
* This class defines whether logging is enabled or disabled and holds
* the CloudWatch log group where error logs are stored if logging is enabled.
*/
export abstract class LoggingConfig {
/**
* If true, log errors when data transformation or data delivery fails.
*
* `true` when using `EnableLogging`, `false` when using `DisableLogging`.
*/
public abstract logging: boolean;

/**
* The CloudWatch log group where log streams will be created to hold error logs.
*
* @default - if `logging` is set to `true`, a log group will be created for you.
*/
public logGroup?: logs.ILogGroup;
}

/**
* Enables logging for error logs with an optional custom CloudWatch log group.
*
* When this class is used, logging is enabled (`logging: true`) and
* you can optionally provide a CloudWatch log group for storing the error logs.
*
* If no log group is provided, a default one will be created automatically.
*/
export class EnableLogging extends LoggingConfig {
public logging: boolean = true;

constructor(logGroup?: logs.ILogGroup) {
super();
this.logGroup = logGroup;
}
}

/**
* Disables logging for error logs.
*
* When this class is used, logging is disabled (`logging: false`)
* and no CloudWatch log group can be specified.
*/
export class DisableLogging extends LoggingConfig {
public logging: boolean = false;

constructor() {
super();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,16 @@ import * as s3 from 'aws-cdk-lib/aws-s3';
import * as cdk from 'aws-cdk-lib/core';
import { Construct, IDependable, Node } from 'constructs';
import { DestinationS3BackupProps } from '../common';
import { LoggingConfig } from '../logging-config';

export interface DestinationLoggingProps {
/**
* If true, log errors when data transformation or data delivery fails.
* Configuration that determines whether to log errors during data transformation or delivery failures,
* and specifies the CloudWatch log group for storing error logs.
*
* If `logGroup` is provided, this will be implicitly set to `true`.
*
* @default true - errors are logged.
*/
readonly logging?: boolean;

/**
* The CloudWatch log group where log streams will be created to hold error logs.
*
* @default - if `logging` is set to `true`, a log group will be created for you.
* @default - errors will be logged and a log group will be created for you.
*/
readonly logGroup?: logs.ILogGroup;
readonly loggingConfig?: LoggingConfig;

/**
* The IAM role associated with this destination.
Expand Down Expand Up @@ -60,11 +53,8 @@ export interface DestinationBackupConfig extends ConfigWithDependables {
}

export function createLoggingOptions(scope: Construct, props: DestinationLoggingProps): DestinationLoggingConfig | undefined {
if (props.logging === false && props.logGroup) {
throw new Error('logging cannot be set to false when logGroup is provided');
}
if (props.logging !== false || props.logGroup) {
const logGroup = props.logGroup ?? Node.of(scope).tryFindChild('LogGroup') as logs.ILogGroup ?? new logs.LogGroup(scope, 'LogGroup');
if (props.loggingConfig?.logging !== false || props.loggingConfig?.logGroup) {
const logGroup = props.loggingConfig?.logGroup ?? Node.of(scope).tryFindChild('LogGroup') as logs.ILogGroup ?? new logs.LogGroup(scope, 'LogGroup');
const logGroupGrant = logGroup.grantWrite(props.role);
return {
loggingOptions: {
Expand Down Expand Up @@ -152,8 +142,7 @@ export function createBackupConfig(scope: Construct, role: iam.IRole, props?: De
const bucketGrant = bucket.grantReadWrite(role);

const { loggingOptions, dependables: loggingDependables } = createLoggingOptions(scope, {
logging: props.logging,
logGroup: props.logGroup,
loggingConfig: props.loggingConfig,
role,
streamId: 'S3Backup',
}) ?? {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ export class S3Bucket implements firehose.IDestination {
const bucketGrant = this.bucket.grantReadWrite(role);

const { loggingOptions, dependables: loggingDependables } = createLoggingOptions(scope, {
logging: this.props.logging,
logGroup: this.props.logGroup,
loggingConfig: this.props.loggingConfig,
role,
streamId: 'S3Destination',
}) ?? {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ const backupKey = new kms.Key(stack, 'BackupKey', {

new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [new destinations.S3Bucket(bucket, {
logging: true,
logGroup: logGroup,
loggingConfig: new destinations.EnableLogging(logGroup),
processor: processor,
compression: destinations.Compression.GZIP,
dataOutputPrefix: 'regularPrefix',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ describe('S3 destination', () => {

it('bucket and log group grants are depended on by delivery stream', () => {
const logGroup = logs.LogGroup.fromLogGroupName(stack, 'Log Group', 'evergreen');
const destination = new firehosedestinations.S3Bucket(bucket, { role: destinationRole, logGroup });
const destination = new firehosedestinations.S3Bucket(bucket, { role: destinationRole, loggingConfig: new firehosedestinations.EnableLogging(logGroup) });
new firehose.DeliveryStream(stack, 'DeliveryStream', {
destinations: [destination],
});
Expand Down Expand Up @@ -171,7 +171,7 @@ describe('S3 destination', () => {

it('does not create resources or configuration if disabled', () => {
new firehose.DeliveryStream(stack, 'DeliveryStream', {
destinations: [new firehosedestinations.S3Bucket(bucket, { logging: false })],
destinations: [new firehosedestinations.S3Bucket(bucket, { loggingConfig: new firehosedestinations.DisableLogging() })],
});

Template.fromStack(stack).resourceCountIs('AWS::Logs::LogGroup', 0);
Expand All @@ -186,7 +186,7 @@ describe('S3 destination', () => {
const logGroup = new logs.LogGroup(stack, 'Log Group');

new firehose.DeliveryStream(stack, 'DeliveryStream', {
destinations: [new firehosedestinations.S3Bucket(bucket, { logGroup })],
destinations: [new firehosedestinations.S3Bucket(bucket, { loggingConfig: new firehosedestinations.EnableLogging(logGroup) })],
});

Template.fromStack(stack).resourceCountIs('AWS::Logs::LogGroup', 1);
Expand All @@ -200,19 +200,11 @@ describe('S3 destination', () => {
});
});

it('throws error if logging disabled but log group provided', () => {
const destination = new firehosedestinations.S3Bucket(bucket, { logging: false, logGroup: new logs.LogGroup(stack, 'Log Group') });

expect(() => new firehose.DeliveryStream(stack, 'DeliveryStream', {
destinations: [destination],
})).toThrowError('logging cannot be set to false when logGroup is provided');
});

it('grants log group write permissions to destination role', () => {
const logGroup = new logs.LogGroup(stack, 'Log Group');

new firehose.DeliveryStream(stack, 'DeliveryStream', {
destinations: [new firehosedestinations.S3Bucket(bucket, { logGroup, role: destinationRole })],
destinations: [new firehosedestinations.S3Bucket(bucket, { loggingConfig: new firehosedestinations.EnableLogging(logGroup), role: destinationRole })],
});

Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
Expand Down Expand Up @@ -572,8 +564,7 @@ describe('S3 destination', () => {
bufferingInterval: cdk.Duration.minutes(1),
compression: firehosedestinations.Compression.ZIP,
encryptionKey: key,
logging: true,
logGroup: logGroup,
loggingConfig: new firehosedestinations.EnableLogging(logGroup),
},
});
new firehose.DeliveryStream(stack, 'DeliveryStream', {
Expand Down
Loading