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.

Sign up for GitHub

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

Jump to bottom

Add detailed metrics to HttpApi #8510

Merged
merged 3 commits into from Nov 20, 2020
Merged

Add detailed metrics to HttpApi #8510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6afb7ef
Add detailed metrics to HttpApi
BaptistG Nov 16, 2020
84c3d52
Take PR feedbacks
BaptistG Nov 17, 2020
4c0796a
Use existing runServerless for tests
BaptistG Nov 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/providers/aws/events/http-api.md
View file
Open in desktop
Expand Up @@ -271,3 +271,15 @@ provider:
httpApi:
payload: '1.0'
```

### Detailed Metrics

With HTTP API we may configure detailed metrics that can be used setup monitoring and alerting in Cloudwatch.

Detailed Metrics can be turned on with:

```yaml
provider:
httpApi:
metrics: true
```
10 changes: 9 additions & 1 deletion lib/plugins/aws/package/compile/events/httpApi/index.js
View file
Open in desktop
Expand Up @@ -130,7 +130,11 @@ class HttpApiEvents {
ApiId: { Ref: this.provider.naming.getHttpApiLogicalId() },
StageName: '$default',
AutoDeploy: true,
DefaultRouteSettings: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's create this configuration only if provider.httpApi.metrics is provided (technically provider.httpApi.metrics != null)

Copy link
Contributor Author

@BaptistG BaptistG Nov 17, 2020 •
edited

Choose a reason for hiding this comment

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

Hey @medikoo , that's what I was initially going for but when testing I noticed that if you've set DefaultRouteSettings.DetailedMetricsEnabled to true, the only way to revert the change is to explicitely set it to false. If you just remove the DefaultRouteSettings in the CF template it will not deactivate detailed metrics.

(1) This activates metrics

"HttpApiStage": {
      "Type": "AWS::ApiGatewayV2::Stage",
      "Properties": {
        "ApiId": {
          "Ref": "HttpApi"
        },
        "StageName": "$default",
        "AutoDeploy": true,
        "DefaultRouteSettings": {
           "DetailedMetricsEnabled": true
        }
}

(2) This doesnt deactivate metrics if you've deployed (1) before

"HttpApiStage": {
      "Type": "AWS::ApiGatewayV2::Stage",
      "Properties": {
        "ApiId": {
          "Ref": "HttpApi"
        },
        "StageName": "$default",
        "AutoDeploy": true,
}

(3) This deactivates metrics if (1) was deployed previously, if it wasn't, it is equivalent to (2)

"HttpApiStage": {
      "Type": "AWS::ApiGatewayV2::Stage",
      "Properties": {
        "ApiId": {
          "Ref": "HttpApi"
        },
        "StageName": "$default",
        "AutoDeploy": true,
        "DefaultRouteSettings": {
           "DetailedMetricsEnabled": true
        }
}

So if I make it a no setting, users will have to use the console to deactivate detailed metrics or will have to explicitely set metrics to false which might be confusing. Let me know what you think

Copy link
Contributor

Choose a reason for hiding this comment

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

@BaptistG thanks for clarification. Indeed in such case it makes big sense to always configure DefaultRouteSettings. Still let's comment in that reasoning (so it's not accidentally removed with some cleanup)

DetailedMetricsEnabled: this.config.metrics,
},
};

const resource = (this.cfTemplate.Resources[this.provider.naming.getHttpApiStageLogicalId()] = {
Type: 'AWS::ApiGatewayV2::Stage',
Properties: properties,
Expand Down Expand Up @@ -226,7 +230,11 @@ Object.defineProperties(
const routes = new Map();
const providerConfig = this.serverless.service.provider;
const userConfig = providerConfig.httpApi || {};
this.config = { routes, id: userConfig.id };
this.config = {
routes,
id: userConfig.id,
metrics: userConfig.metrics || false,
};
let cors = null;
let shouldFillCorsMethods = false;
const userCors = userConfig.cors;
Expand Down
12 changes: 11 additions & 1 deletion lib/plugins/aws/package/compile/events/httpApi/index.test.js
View file
Open in desktop
Expand Up @@ -60,6 +60,10 @@ describe('HttpApiEvents', () => {
expect(resource.Properties.StageName).to.equal('$default');
expect(resource.Properties.AutoDeploy).to.equal(true);
});
it('Should not have detailed metrics when not asked to', () => {
const resource = cfResources[naming.getHttpApiStageLogicalId()];
expect(resource.Properties.DefaultRouteSettings.DetailedMetricsEnabled).to.equal(false);
});
it('Should configure output', () => {
const output = cfOutputs.HttpApiUrl;
expect(output).to.have.property('Value');
Expand Down Expand Up @@ -460,14 +464,15 @@ describe('HttpApiEvents', () => {
});
});

describe('Access logs', () => {
describe('Access logs and detailed metrics', () => {
let cfResources;
let naming;
before(() =>
runServerless({
fixture: 'httpApi',
configExt: {
provider: {
httpApi: { metrics: true },
logs: {
httpApi: true,
},
Expand All @@ -492,6 +497,11 @@ describe('HttpApiEvents', () => {

expect(stageResourceProps.AccessLogSettings).to.have.property('Format');
});

it('Should configure detailed metrics', () => {
const resource = cfResources[naming.getHttpApiStageLogicalId()];
expect(resource.Properties.DefaultRouteSettings.DetailedMetricsEnabled).to.equal(true);
});
});

describe('External HTTP API', () => {
Expand Down
1 change: 1 addition & 0 deletions lib/plugins/aws/provider/awsProvider.js
View file
Open in desktop
Expand Up @@ -698,6 +698,7 @@ class AwsProvider {
},
name: { type: 'string' },
payload: { type: 'string' },
metrics: { type: 'boolean' },
},
additionalProperties: false,
},
Expand Down