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 Cloudwatch Logs Output #127

Merged
merged 1 commit into from
Nov 27, 2020
Merged

Add Cloudwatch Logs Output #127

merged 1 commit into from
Nov 27, 2020

Conversation

cpanato
Copy link
Member

@cpanato cpanato commented Nov 23, 2020

What type of PR is this?
/kind feature

Any specific area of the project related to this PR?
/area outputs

What this PR does / why we need it:
Add Cloudwatch Logs Output

I still need to run some final manual tests to validate everything, but I would like to get some feedback if is missing something

Which issue(s) this PR fixes:

Fixes #46

Special notes for your reviewer:

@poiana
Copy link

poiana commented Nov 23, 2020

@cpanato: The label(s) area/outputs cannot be applied, because the repository doesn't have them

In response to this:

What type of PR is this?
/kind feature

Any specific area of the project related to this PR?
/area outputs

What this PR does / why we need it:
Add Cloudwatch Logs Output

I still need to run some final manual tests to validate everything, but I would like to get some feedback if is missing something

Which issue(s) this PR fixes:

Fixes #46

Special notes for your reviewer:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@poiana poiana added kind/feature New feature or request dco-signoff: yes labels Nov 23, 2020
@poiana poiana added the size/L label Nov 23, 2020
types/types.go Outdated Show resolved Hide resolved
Copy link
Member Author

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

done, thanks for your review @KeisukeYamashita

@KeisukeYamashita
Copy link
Contributor

Thank you for updating 🙏

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
config_example.yaml Outdated Show resolved Hide resolved
config_example.yaml Outdated Show resolved Hide resolved
outputs/aws.go Outdated Show resolved Hide resolved
outputs/aws.go Outdated Show resolved Hide resolved
@cpanato
Copy link
Member Author

cpanato commented Nov 24, 2020

working 🎉
Screenshot 2020-11-24 at 11 59 12

@cpanato
Copy link
Member Author

cpanato commented Nov 24, 2020

Some ideas: we can create the logstream if is not set.

@Issif
Copy link
Member

Issif commented Nov 24, 2020

Some ideas: we can create the logstream if is not set.

Ok, that's a good idea. If falcosidekick can't do it (lack of IAM rights), the error should be explicit

@cpanato
Copy link
Member Author

cpanato commented Nov 24, 2020

@Issif ok, give me a minute to refactor this code or do you prefer in a followup?

@Issif
Copy link
Member

Issif commented Nov 24, 2020

@Issif ok, give me a minute to refactor this code or do you prefer in a followup?

Better to do all here, as it's a new Output

@cpanato cpanato changed the title Add Cloudwatch Logs Output WIP Add Cloudwatch Logs Output Nov 24, 2020
@cpanato cpanato changed the title WIP Add Cloudwatch Logs Output Add Cloudwatch Logs Output Nov 24, 2020
@cpanato
Copy link
Member Author

cpanato commented Nov 24, 2020

ok, now if the LogStream is not set it try to create a stream and reuse that stream for the other calls

2020/11/24 13:24:14 [INFO]  : Enabled Outputs : AWSCLOUDWATCHLOGS
2020/11/24 13:24:14 [INFO]  : Falco Sidekick is up and listening on port 2801
2020/11/24 13:30:14 [INFO]  : AWS CloudWatchLogs - Log Stream not configured creating one called falcosidekick-1606220656756025000
2020/11/24 13:24:17 [INFO]  : AWS CloudWatchLogs - Send Log OK ({
  NextSequenceToken: "49610214922965016038758914111270014127777750205229476802"
})
2020/11/24 13:30:46 [INFO]  : AWS Refreshing token for LogGroup: carlos-test LogStream: falcosidekick-1606220656756025000
2020/11/24 13:24:23 [INFO]  : AWS CloudWatchLogs - Send Log OK ({
  NextSequenceToken: "49610214922965016038758914113696328247744311371181632450"
})

main.go Outdated Show resolved Hide resolved
outputs/aws.go Outdated Show resolved Hide resolved
Copy link
Member

@Issif Issif left a comment

Choose a reason for hiding this comment

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

See my comment about Stream creation

Signed-off-by: Carlos Panato <ctadeu@gmail.com>
outputs/aws.go Show resolved Hide resolved
@poiana poiana added the lgtm label Nov 27, 2020
@poiana
Copy link

poiana commented Nov 27, 2020

LGTM label has been added.

Git tree hash: 86d499622f9e0310e67ecec4a1dbd89f542652a0

@poiana
Copy link

poiana commented Nov 27, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Issif

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

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.

Add Cloudwatch Logs Output
4 participants