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

Add support for API Gateway REST API Logs #6057

Merged
merged 1 commit into from May 6, 2019
Merged

Conversation

pmuens
Copy link
Contributor

@pmuens pmuens commented Apr 26, 2019

What did you implement:

Adds native support for API Gateway logs via the API Gateway stage resource.

Closes #4461

How did you implement it:

Merge in all the resources necessary to setup API Gateway logging via CloudFormation. Set the correct settings in the API Gateway Stage resource.

How can we verify it:

Deploy the following serverless.yml:

service: test

provider:
  name: aws
  runtime: nodejs8.10
  apiGateway:
    logs: true

functions:
  hello:
    handler: handler.hello
    events:
      - http: GET hello

After that curl the endpoint and look into CloudWatch logs for the api-gateway log group.

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@pmuens pmuens added this to the 1.42.0 milestone Apr 26, 2019
@pmuens pmuens self-assigned this Apr 26, 2019
@pmuens pmuens requested review from dschep and eahefnawy April 26, 2019 13:19
@pmuens pmuens changed the title Add support for API Gateway Logs Add support for API Gateway REST API Logs Apr 29, 2019
@pmuens pmuens force-pushed the api-gateway-logs branch 3 times, most recently from 926bec8 to 3b5b5ef Compare May 1, 2019 12:50
@drissamri
Copy link

Cool to see native integration of API GW logs are coming soon! Great work.

Should we expect options to customize the format logging afterwards as well? https://docs.aws.amazon.com/apigateway/latest/developerguide/set-up-logging.html & https://docs.aws.amazon.com/apigateway/api-reference/resource/stage/#format

Copy link
Contributor

@dschep dschep left a comment

Choose a reason for hiding this comment

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

LGTM & worked as expected :shipit:

Copy link
Contributor

@dschep dschep left a comment

Choose a reason for hiding this comment

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

oops. wrong button. approved!

@pmuens
Copy link
Contributor Author

pmuens commented May 6, 2019

Thanks @dschep 👍

@drissamri yes, that's a good idea. The initial implementation was planned to be as simple as possible but we can extend this easily later on.

@pmuens pmuens merged commit 2c97649 into master May 6, 2019
@pmuens pmuens deleted the api-gateway-logs branch May 6, 2019 10:06
@anjo-swe
Copy link

anjo-swe commented May 6, 2019

@pmuens is there already a ticket on your backlog to update the implementation to support custom logging?

@gootdude
Copy link

gootdude commented May 6, 2019

@pmuens please expose the following fields for configuration of the log group and log format

  • AWS::Logs::LogGroup
    • Properties
      • LogGroupName
      • RetentionInDays
  • AWS::ApiGateway::Stage
    • AccessLogSettings
      • Format

Here's our current serverless.yml that we use to create the log group and set retention time as well as the custom format:

resources:
  Resources:
    ABCAccessLogGroup:
      Type: AWS::Logs::LogGroup
      Properties:
        LogGroupName: ABC-${self:provider.stage}
        RetentionInDays: ${file(./config.yml):accessLogRetentionInDays}
    ABCGatewayStage:
      Type: 'AWS::ApiGateway::Stage'
      Properties:
        DeploymentId:
          Ref: __deployment__
        RestApiId:
          Ref: ABCApiGateway
        StageName: ${self:provider.stage}
        TracingEnabled: true
        MethodSettings:
          - HttpMethod: "*"
            ResourcePath: "/*"
            MetricsEnabled: true
            DataTraceEnabled: false
        AccessLogSetting:
          Format: '{"apiId":"$context.apiId","stage":"$context.stage","resourcePath":"$context.resourcePath","requestId":"$context.requestId","awsEndpointRequestId":"$context.awsEndpointRequestId","xrayTraceId":"$context.xrayTraceId","requestTime":"$context.requestTime","requestTimeEpoch":$context.requestTimeEpoch,"httpMethod":"$context.httpMethod","status":"$context.status","path":"$context.path"}'
          DestinationArn:
                Fn::GetAtt:
                  - OneAccessLogGroup
                  - Arn

@pmuens
Copy link
Contributor Author

pmuens commented May 8, 2019

@pmuens is there already a ticket on your backlog to update the implementation to support custom logging?

@anjo-swe no, we haven't created such an issue. Feel free to open one! 👍

@gootdude thanks for the feedback. That sounds reasonable. With this iteration we want to implement the most simple way to enable API Gateway logs. Well built upon this implementation in the upcoming versions. Feel free to add your requirements to the API Gateway logs issue (the one @anjo-swe will create).

Thanks again for looking into this and providing feedback 🙏

@pmuens
Copy link
Contributor Author

pmuens commented May 8, 2019

@anjo-swe @gootdude I just created #6094 to track proposals for future enhancements.

@davidfarinha
Copy link

davidfarinha commented May 9, 2019

When can we expect to see this feature released and is there any way we can use this feature in the meantime? Many thanks

@pmuens
Copy link
Contributor Author

pmuens commented May 9, 2019

@davidfarinha We've just release the new Serverless Framework version which adds support for this.

Should hit the CDNs in a couple of minutes ⏲

@davidfarinha
Copy link

Very nice, great work!

@davidfarinha
Copy link

davidfarinha commented May 10, 2019

I'm using the following serverless config with version 1.42.1

apiGateway:
  logs: true
  apiKeySourceType: HEADER
  minimumCompressionSize: 1024

And even tried re-deploying however still not getting any logs configured on my API Gateway or can't see any log streams created in CloudWatch. Any ideas?

@sdomagala
Copy link
Contributor

@davidfarinha checking history I found out this feature was changed a bit before a release - 1df8598#diff-7ef39ede04c5da8e81c20fe60e80a9ce

Now, format for logs is:

provider:
  logs:
    restApi: true

and it works for me

@davidfarinha
Copy link

Thanks @sdomagala, works for me now.

@ozbillwang
Copy link

Any one tested the feature to true first, then to false?

I enable access log with restApi: true, then I change it to false, and deploy again, but the option is still enabled

@ozbillwang
Copy link

ozbillwang commented May 15, 2019

After enabled the access log, I got the error sometime (2 times), after deploy the change.

CloudFormation - CREATE_FAILED - AWS::Logs::LogGroup - ApiGatewayLogGroup

/aws/api-gateway/serverless-api-dev already exists

I have to manually delete the log group to fix this issue, but that's not right. I will lost old access logs.

@sachinsmc
Copy link

sachinsmc commented May 22, 2019

After enabled the access log, I got the error sometime (2 times), after deploy the change.

CloudFormation - CREATE_FAILED - AWS::Logs::LogGroup - ApiGatewayLogGroup

/aws/api-gateway/serverless-api-dev already exists

I have to manually delete the log group to fix this issue, but that's not right. I will lost old access logs.

Facing same issue as @ozbillwang , first deployment is fine, second one throws

CloudFormation - CREATE_FAILED - AWS::Logs::LogGroup - ApiGatewayLogGroup

/aws/api-gateway/serverless-api-dev already exists

any workaround?

@toxuin
Copy link

toxuin commented Jun 6, 2019

After enabled the access log, I got the error sometime (2 times), after deploy the change.

CloudFormation - CREATE_FAILED - AWS::Logs::LogGroup - ApiGatewayLogGroup

/aws/api-gateway/serverless-api-dev already exists

I have to manually delete the log group to fix this issue, but that's not right. I will lost old access logs.

This is happening to us too.

Furthermore, if you set logging to true you get this error on second deploy. If you NOT set logging to anything - it unsets the existing logging settings and stops ALL logging for the API. (See: #6165)

This is pretty bad.

@ozbillwang
Copy link

I do recommend to rollback this change now and redo it later

@tommedema
Copy link

Where is this feature documented?

@neilh-cogapp
Copy link

@tommedema https://www.serverless.com/framework/docs/providers/aws/events/apigateway/#logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API Gateway REST API Logs