Skip to content

Commit

Permalink
feat(appsync): allow user to configure log retention time (aws#21418)
Browse files Browse the repository at this point in the history
This pull request implements a feature that allows users to configure the log retention period for App Sync logs.

AWS AppSync doesn't allow users to use user-defined log groups, leaving the option to create the log group for the user and set the retention time accordingly. Fortunately, the service always creates its log groups according to a pre-defined naming convention.

> The log group is named following the /aws/appsync/apis/{graphql_api_id} format

ref. https://docs.aws.amazon.com/appsync/latest/devguide/monitoring.html

At the same time, AWS CDK provides a stable [construct](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_logs.LogRetention.html) to set log retention times for any log group. 

Thus, it is completely unnecessary to force every user to learn this detail when it can be abstracted to the construct creating the resource for them – that is what this pull request aims to do.

This is the continuation of work done in aws#20536 – this time with documentation and integration test 😄 

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
nikovirtala authored and josephedward committed Aug 30, 2022
1 parent a19bf04 commit 4004590
Show file tree
Hide file tree
Showing 15 changed files with 2,326 additions and 2 deletions.
25 changes: 24 additions & 1 deletion packages/@aws-cdk/aws-appsync/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,29 @@ new route53.CnameRecord(this, `CnameApiRecord`, {
});
```

## Log Group

AppSync automatically create a log group with the name `/aws/appsync/apis/<graphql_api_id>` upon deployment with
log data set to never expire. If you want to set a different expiration period, use the `logConfig.retention` property.

To obtain the GraphQL API's log group as a `logs.ILogGroup` use the `logGroup` property of the
`GraphqlApi` construct.

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

const logConfig: appsync.LogConfig = {
retention: logs.RetentionDays.ONE_WEEK,
};

new appsync.GraphqlApi(this, 'api', {
authorizationConfig: {},
name: 'myApi',
schema: appsync.Schema.fromAsset(path.join(__dirname, 'myApi.graphql')),
logConfig,
});
```

## Schema

Every GraphQL Api needs a schema to define the Api. CDK offers `appsync.Schema`
Expand Down Expand Up @@ -427,7 +450,7 @@ new appsync.GraphqlApi(this, 'api', {
defaultAuthorization: {
authorizationType: appsync.AuthorizationType.LAMBDA,
lambdaAuthorizerConfig: {
handler: authFunction,
handler: authFunction,
// can also specify `resultsCacheTtl` and `validationRegex`.
},
},
Expand Down
29 changes: 28 additions & 1 deletion packages/@aws-cdk/aws-appsync/lib/graphqlapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { ICertificate } from '@aws-cdk/aws-certificatemanager';
import { IUserPool } from '@aws-cdk/aws-cognito';
import { ManagedPolicy, Role, IRole, ServicePrincipal, Grant, IGrantable } from '@aws-cdk/aws-iam';
import { IFunction } from '@aws-cdk/aws-lambda';
import { ILogGroup, LogGroup, LogRetention, RetentionDays } from '@aws-cdk/aws-logs';
import { ArnFormat, CfnResource, Duration, Expiration, IResolvable, Stack } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { CfnApiKey, CfnGraphQLApi, CfnGraphQLSchema, CfnDomainName, CfnDomainNameApiAssociation } from './appsync.generated';
Expand Down Expand Up @@ -248,6 +249,16 @@ export interface LogConfig {
* @default - None
*/
readonly role?: IRole;

/**
* The number of days log events are kept in CloudWatch Logs.
* By default AppSync keeps the logs infinitely. When updating this property,
* unsetting it doesn't remove the log retention policy.
* To remove the retention policy, set the value to `INFINITE`
*
* @default RetentionDays.INFINITE
*/
readonly retention?: RetentionDays
}

/**
Expand Down Expand Up @@ -459,6 +470,11 @@ export class GraphqlApi extends GraphqlApiBase {
*/
public readonly apiKey?: string;

/**
* the CloudWatch Log Group for this API
*/
public readonly logGroup: ILogGroup;

private schemaResource: CfnGraphQLSchema;
private api: CfnGraphQLApi;
private apiKeyResource?: CfnApiKey;
Expand Down Expand Up @@ -527,6 +543,16 @@ export class GraphqlApi extends GraphqlApiBase {
});
}

const logGroupName = `/aws/appsync/apis/${this.apiId}`;

this.logGroup = LogGroup.fromLogGroupName(this, 'LogGroup', logGroupName);

if (props.logConfig?.retention) {
new LogRetention(this, 'LogRetention', {
logGroupName: this.logGroup.logGroupName,
retention: props.logConfig.retention,
});
};
}

/**
Expand Down Expand Up @@ -620,10 +646,11 @@ export class GraphqlApi extends GraphqlApiBase {
ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSAppSyncPushToCloudWatchLogs'),
],
}).roleArn;
const fieldLogLevel: FieldLogLevel = config.fieldLogLevel ?? FieldLogLevel.NONE;
return {
cloudWatchLogsRoleArn: logsRoleArn,
excludeVerboseContent: config.excludeVerboseContent,
fieldLogLevel: config.fieldLogLevel,
fieldLogLevel: fieldLogLevel,
};
}

Expand Down
3 changes: 3 additions & 0 deletions packages/@aws-cdk/aws-appsync/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
"@aws-cdk/aws-stepfunctions": "0.0.0",
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/integ-runner": "0.0.0",
"@aws-cdk/integ-tests": "0.0.0",
"@aws-cdk/cfn2ts": "0.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@types/jest": "^27.5.2",
Expand All @@ -97,6 +98,7 @@
"@aws-cdk/aws-elasticsearch": "0.0.0",
"@aws-cdk/aws-iam": "0.0.0",
"@aws-cdk/aws-lambda": "0.0.0",
"@aws-cdk/aws-logs": "0.0.0",
"@aws-cdk/aws-opensearchservice": "0.0.0",
"@aws-cdk/aws-rds": "0.0.0",
"@aws-cdk/aws-s3-assets": "0.0.0",
Expand All @@ -113,6 +115,7 @@
"@aws-cdk/aws-elasticsearch": "0.0.0",
"@aws-cdk/aws-iam": "0.0.0",
"@aws-cdk/aws-lambda": "0.0.0",
"@aws-cdk/aws-logs": "0.0.0",
"@aws-cdk/aws-opensearchservice": "0.0.0",
"@aws-cdk/aws-rds": "0.0.0",
"@aws-cdk/aws-s3-assets": "0.0.0",
Expand Down
47 changes: 47 additions & 0 deletions packages/@aws-cdk/aws-appsync/test/appsync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as path from 'path';
import { Template } from '@aws-cdk/assertions';
import { Certificate } from '@aws-cdk/aws-certificatemanager';
import * as iam from '@aws-cdk/aws-iam';
import * as logs from '@aws-cdk/aws-logs';
import * as cdk from '@aws-cdk/core';
import * as appsync from '../lib';

Expand Down Expand Up @@ -191,3 +192,49 @@ test('appsync GraphqlApi should be configured with custom domain when specified'
DomainName: domainName,
});
});

test('log retention should be configured with given retention time when specified', () => {
// GIVEN
const retentionTime = logs.RetentionDays.ONE_WEEK;

// WHEN
new appsync.GraphqlApi(stack, 'log-retention', {
authorizationConfig: {},
name: 'log-retention',
schema: appsync.Schema.fromAsset(path.join(__dirname, 'appsync.test.graphql')),
logConfig: {
retention: retentionTime,
},
});

// THEN
Template.fromStack(stack).hasResourceProperties('Custom::LogRetention', {
LogGroupName: {
'Fn::Join': [
'',
[
'/aws/appsync/apis/',
{
'Fn::GetAtt': [
'logretentionB69DFB48',
'ApiId',
],
},
],
],
},
RetentionInDays: 7,
});
});

test('log retention should not appear when no retention time is specified', () => {
// WHEN
new appsync.GraphqlApi(stack, 'no-log-retention', {
authorizationConfig: {},
name: 'no-log-retention',
schema: appsync.Schema.fromAsset(path.join(__dirname, 'appsync.test.graphql')),
});

// THEN
Template.fromStack(stack).resourceCountIs('Custom::LogRetention', 0);
});
40 changes: 40 additions & 0 deletions packages/@aws-cdk/aws-appsync/test/integ.log-retention.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { join } from 'path';
import { RetentionDays } from '@aws-cdk/aws-logs';
import { App, Stack } from '@aws-cdk/core';
import { ExpectedResult, IntegTest } from '@aws-cdk/integ-tests';
import { GraphqlApi, LogConfig, Schema } from '../lib';

const app = new App();
const stack = new Stack(app, 'AppSyncIntegLogRetention');


const retentionTime = RetentionDays.ONE_WEEK;
const logConfig: LogConfig = {
retention: retentionTime,
};

const api = new GraphqlApi(stack, 'GraphqlApi', {
authorizationConfig: {},
name: 'IntegLogRetention',
schema: Schema.fromAsset(join(__dirname, 'appsync.test.graphql')),
logConfig,
});

const integ = new IntegTest(app, 'Integ', { testCases: [stack] });

const describe = integ.assertions.awsApiCall('CloudWatchLogs',
'describeLogGroups',
{
logGroupNamePrefix: api.logGroup.logGroupName,
});

describe.expect(ExpectedResult.objectLike({
logGroups: [
{
logGroupName: api.logGroup.logGroupName,
retentionInDays: retentionTime,
},
],
}));

app.synth();
Loading

0 comments on commit 4004590

Please sign in to comment.