feat: Support AWS usage marketplace#1770
Conversation
erezrokah
left a comment
There was a problem hiding this comment.
Looks good, added a questions and comments.
How can I test this?
| } | ||
| // If user wants to use the AWS Marketplace for billing, don't even try to communicate with CQ API | ||
| if isAWSMarketplace() { | ||
| cfg, err := awsConfig.LoadDefaultConfig(context.TODO()) |
There was a problem hiding this comment.
What's the reason for using context.TODO() in favor of context.Background()? Maybe we should add timeout as well for loading the config?
Also, can we put this logic in a setupAWSMarketplace function as NewUsageClient is getting quite long
There was a problem hiding this comment.
context.TODO() is supposed to be used when the surrounding function doesn't have a context available to use, so that is why I used it here. I am not sure the value of a timeout would add here.
I have moved the logic to setupAWSMarketplace in 566bbc0
| u.awsMarketPlaceClient = marketplacemetering.NewFromConfig(cfg) | ||
| u.teamName = "AWS_MARKETPLACE" | ||
| // This needs to be larger than normal, because we can only send a single usage record per second (from each compute node) | ||
| u.batchLimit = 1000000000 |
There was a problem hiding this comment.
I'm not sure this is necessary, but would need to verify. I think how it should work is that there is a minimum flush time that should come into effect (by default this is 10s)
There was a problem hiding this comment.
I think there is a race condition, if a batch is flushed and within the same second the sync ends then the timestamp will have the same value as timestamps are truncated to the second
There was a problem hiding this comment.
Ah, good catch... ok this should work for now but we should probably try and think of a better solution in the long term, otherwise we won't count any records if the sync is abruptly stopped
erezrokah
left a comment
There was a problem hiding this comment.
Code looks great, how can I test this?
| } | ||
|
|
||
| func isAWSMarketplace() bool { | ||
| return os.Getenv("CQ_AWS_MARKETPLACE_CONTAINER") == "true" |
There was a problem hiding this comment.
Where these envs are being set? If it's something user settable (and not a published image of some sort) strconv.ParseBool would've been better, but I'm guessing it's an image?
| //go:generate mockgen -package=mocks -destination=../premium/mocks/marketplacemetering.go -source=usage.go AWSMarketplaceClientInterface | ||
| type AWSMarketplaceClientInterface interface { | ||
| MeterUsage(ctx context.Context, params *marketplacemetering.MeterUsageInput, optFns ...func(*marketplacemetering.Options)) (*marketplacemetering.MeterUsageOutput, error) | ||
| } |
There was a problem hiding this comment.
This was added to ensure that we could mock the AWS API calls like we do the API Server calls
Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
🤖 I have created a release *beep* *boop* --- ## [4.58.0](v4.57.1...v4.58.0) (2024-08-02) ### Features * Support AWS usage marketplace ([#1770](#1770)) ([1eb6d1a](1eb6d1a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This adds support for using CloudQuery plugins as part of the AWS Marketplace by changing the metering implementation to use the Marketplace instead, if specified via environment variables.