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

(feature/0457-support-amazon-cloudfront-function): Support Amazon Clo… #1127

Conversation

jolo-dev
Copy link
Contributor

@jolo-dev jolo-dev commented Oct 22, 2023

Collect Amazon CloudFront function #457

Solution

Add to Cloudfront Functions a new file functions.go

Changes Made

  • new file provider/aws/cloudfront/functions.go

How to Test

Create a new Cloudfront functions: https://us-east-1.console.aws.amazon.com/cloudfront/v3/home?region=us-east-1#/functions/create

Screenshots

Screenshot 2023-10-22 at 14 45 32

Notes

[Any additional notes or information that you would like to share with the reviewers.]

Checklist

  • Code follows the contributing guidelines
  • Changes have been thoroughly tested
  • Documentation has been updated, if necessary
  • Any dependencies have been added to the project, if necessary

Reviewers

tempRegion := client.AWSClient.Region
client.AWSClient.Region = "us-east-1"
cloudwatchClient := cloudwatch.NewFromConfig(*client.AWSClient)
client.AWSClient.Region = tempRegion
Copy link
Contributor

Choose a reason for hiding this comment

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

🙌

@jolo-dev
Copy link
Contributor Author

@Traxmaxx the dashboard fails because of the missing Toast component: https://discord.com/channels/932683789384183808/933353577102061669/1165250794417836072

@Traxmaxx
Copy link
Contributor

@Traxmaxx the dashboard fails because of the missing Toast component: https://discord.com/channels/932683789384183808/933353577102061669/1165250794417836072

Hey thanks! I'm aware and try to get someone from the maintainers to help me review open PRs to get the fix in asap!

@mlabouardy mlabouardy added this to the v3.1.3 milestone Oct 24, 2023
@bishal7679
Copy link
Contributor

bishal7679 commented Oct 25, 2023

@mlabouardy This changes has already been raised by this PR #1131 #1123
and had to close it due to checks failing!

@AvineshTripathi
Copy link
Collaborator

Hey @bishal7679 your PR also adds something some details to distributions, I would recommend you to review this PR to see if it matches to what your changes are and we can remove those from your PR. However we will make sure you get added as a collaborator for this PR.

This is just to make sure both get credits for the amazing work, however i can only do that when both you and @jolo-dev are ready for it. Thanks!

@bishal7679
Copy link
Contributor

bishal7679 commented Oct 25, 2023

Hey @bishal7679 your PR also adds something some details to distributions, I would recommend you to review this PR to see if it matches to what your changes are and we can remove those from your PR. However we will make sure you get added as a collaborator for this PR.

This is just to make sure both get credits for the amazing work, however i can only do that when both you and @jolo-dev are ready for it. Thanks!

@AvineshTripathi I completely support this! Wouldn't it be good?? get PR #1131 merged first [PS:- might get complicated for merge conflict] after then this PR #1127 could be addon since both are adding same file functions.go and in #1131 dynamic pricing and more metrics being used and #1127 its invocation metric can be added there to make the calculation more efficient??
WDYT @jolo-dev ?

@jolo-dev
Copy link
Contributor Author

@AvineshTripathi I believe the one of #1131 is a bit more sophisticated. However, there might be a few changes needed. He got the Duration which I couldn't find. @bishal7679 Have you tried and that worked?
L138 and L158.
And I am not sure about the Namespace of L92.

@AvineshTripathi
Copy link
Collaborator

Great! I keep this to you'll and let me know how you want to proceed or need any assitance.

Copy link
Contributor

@bishal7679 bishal7679 left a comment

Choose a reason for hiding this comment

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

Hope it helps!
REF :- metricname

providers/aws/cloudfront/functions.go Outdated Show resolved Hide resolved
providers/aws/cloudfront/functions.go Outdated Show resolved Hide resolved
providers/aws/cloudfront/functions.go Outdated Show resolved Hide resolved
jolo-dev and others added 4 commits October 28, 2023 17:05
Co-authored-by: Bishal Das  <70086051+bishal7679@users.noreply.github.com>
Co-authored-by: Bishal Das  <70086051+bishal7679@users.noreply.github.com>
Co-authored-by: Bishal Das  <70086051+bishal7679@users.noreply.github.com>
providers/aws/cloudfront/functions.go Outdated Show resolved Hide resolved
providers/aws/cloudfront/functions.go Show resolved Hide resolved
jolo-dev and others added 2 commits October 29, 2023 09:36
Co-authored-by: Azanul Haque <42029519+Azanul@users.noreply.github.com>
Co-authored-by: Azanul Haque <42029519+Azanul@users.noreply.github.com>
Copy link
Contributor

@bishal7679 bishal7679 left a comment

Choose a reason for hiding this comment

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

LGTM!

@mlabouardy mlabouardy merged commit c136d61 into tailwarden:develop Oct 30, 2023
@mlabouardy mlabouardy added aws and removed hold labels Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants