-
Notifications
You must be signed in to change notification settings - Fork 537
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
Added API Gateway v2 HTTP & Websocket support #207
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one! I've added my suggestions. The following comments also apply to this PR:
return &schema.RegistryItem{ | ||
Name: "aws_apigatewayv2_api", | ||
Notes: []string{ | ||
"WebSocket Connection minutes is not yet supported", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can cost components for this:
$ cd infracost/infracost/scripts/price_explorer
$ ./2_find_product_families.sh AmazonApiGateway
List of Product Families for AmazonApiGateway:
Executing query... done
[
"API Calls",
"Amazon API Gateway Cache",
"WebSocket"
]
$ ./3_find_attribute_keys.sh AmazonApiGateway WebSocket
Executing query... done
#####################################
Found 2 different products
------------------
....
"key": "usagetype",
- "value": "USE1-ApiGatewayMessage",
+ "value": "USE1-ApiGatewayMinute",
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alikhajeh1 do you want me to add the cost component in anyway? The reason I left it out was because of the calculation variation on websocket connections. It can be varied per second / minute / hour & the number of connections rate can be per second / minute / hour / day.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robh007 I think it's helpful to see that, even if there is no infracost provider support for it, e.g.:
└─ Connection duration - 1M minutes 0.025 - -
If we add the provider support, would having average_connection_duration
(in minutes) and monthly_connections
help? so the user inputs those and we multiply them to calculate the total connection minutes.
func websocketApiCostComponent(d *schema.ResourceData, u *schema.ResourceData) []*schema.CostComponent { | ||
region := d.Get("region").String() | ||
monthlyMessages := decimal.Zero | ||
messageSize := decimal.NewFromInt(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Messages are metered in 32 KB increments.
Should this default to 32KB as that seems to be the minimum?
|
||
return []*schema.CostComponent{ | ||
{ | ||
Name: "Requests (first 1B message transfers)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name: "Requests (first 1B message transfers)", | |
Name: "Messages (first 1B)", |
The WebSocket APIs
section of https://aws.amazon.com/api-gateway/pricing/ talks about messages, and that's how websocket load tests usually work too (based on number of messages and message size), so I think we can override the Number of Requests
title they use in their pricing table. The usagetype
also uses the word "message".
return []*schema.CostComponent{ | ||
{ | ||
Name: "Requests (first 1B message transfers)", | ||
Unit: "requests", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit: "requests", | |
Unit: "messages", |
Please also add UnitMultiplier: 1000000,
Service: strPtr("AmazonApiGateway"), | ||
ProductFamily: strPtr("WebSocket"), | ||
AttributeFilters: []*schema.AttributeFilter{ | ||
{Key: "description", Value: strPtr("WebSocket Messages")}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't needed
}, | ||
}, | ||
{ | ||
Name: "Requests (over 1B message transfers)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same three comments from above apply here too.
@alikhajeh1 I've updated with the requested changes. I can't assign back to you, I've just managed to un-assign myself from this.... |
New output
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @robh007! I added my suggestions, thanks for your patience as I improve the CONTRIBUTING.md file to clarify the tweaks/wording.
btw, maybe run make fmt
and make lint
to see if that fixes the failed CI build.
Service: strPtr("AmazonApiGateway"), | ||
ProductFamily: strPtr("API Calls"), | ||
AttributeFilters: []*schema.AttributeFilter{ | ||
{Key: "description", Value: strPtr("HTTP API Requests")}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{Key: "description", Value: strPtr("HTTP API Requests")}, |
description
is needed either (descriptions might change if AWS reword things? I think usagetype is the one that users rely on in the billing CSV too)
Service: strPtr("AmazonApiGateway"), | ||
ProductFamily: strPtr("API Calls"), | ||
AttributeFilters: []*schema.AttributeFilter{ | ||
{Key: "description", Value: strPtr("HTTP API Requests")}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{Key: "description", Value: strPtr("HTTP API Requests")}, |
|
||
CostComponent := []*schema.CostComponent{ | ||
{ | ||
Name: "Requests (first 300m)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name: "Requests (first 300m)", | |
Name: "Requests (first 300M)", |
I'll add this to the CONTRIBUTING.md file
- tiers in names: use the K postfix for thousand, M for million, B for billion and T for trillion, e.g. "Requests (first 300M)" and "Messages (first 1B)". Note the capital case letters.
|
||
if apiTierTwo.GreaterThan(decimal.NewFromInt(0)) { | ||
CostComponent = append(CostComponent, &schema.CostComponent{ | ||
Name: "Requests (over 300m)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name: "Requests (over 300m)", | |
Name: "Requests (over 300M)", |
Name: "aws_apigatewayv2_api.http", | ||
CostComponentChecks: []testutil.CostComponentCheck{ | ||
{ | ||
Name: "Requests (first 300m)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name: "Requests (first 300m)", | |
Name: "Requests (first 300M)", |
@robh007 I resolved the conflict with master, I think this PR just needs the |
Objective:
Adds support for
api_gatewayv2_api
Example output:
Pricing details:
The aws_apigatewayv2_api provides 4 pricing components described in https://aws.amazon.com/api-gateway/pricing/. This adds support for the API gateway HTTP API & WebSocket API.