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

Enable setting retention policies for created log groups #50

Closed
drboyer opened this issue Jun 10, 2020 · 8 comments · Fixed by #76
Closed

Enable setting retention policies for created log groups #50

drboyer opened this issue Jun 10, 2020 · 8 comments · Fixed by #76
Labels
enhancement New feature or request

Comments

@drboyer
Copy link

drboyer commented Jun 10, 2020

Currently, log groups created by this plugin do not have a retention period set, meaning logs forwarded using this plugin will never expire from CloudWatch. But you may want to set a lower retention period for cost-saving or compliance reasons. I'm proposing adding a new configuration option named like retentionInDays that would be applied to log groups created if auto_create_group is set to true.

I may try to open a PR about this myself - I think I understand where to make the change. But wanted to open a feature request ticket first to facilitate any discussion.

One potential point of discussion - if we add this setting, should we only apply it to created log groups? Or should it be applied to the log group this plugin sends to regardless? I proposed only created groups to avoid this plugin making changes to existing infrastructure.

@PettitWesley
Copy link
Contributor

I proposed only created groups to avoid this plugin making changes to existing infrastructure.

I think this makes sense 👍

@hencrice hencrice added enhancement New feature or request and removed feature-request labels Jun 29, 2020
@hencrice
Copy link
Contributor

hencrice commented Jun 29, 2020

@drboyer Hmm there's a slight chance we might not be able to distinguish between log groups created by us v.s. by others.

The CreateLogGroup API does not allow us to set RetentionInDays directly, so we will resort to making 2 API calls: CreateLogGroup followed by PutRetentionPolicy. If Fluent Bit is forcefully killed during those API calls, then it's possible for the revived Fluent Bit process to treat the log group as "existing not created by me", hence not applying retention policy. Might be acceptable though.

Are you still up for it?

@davidnewhall
Copy link
Contributor

More likely than the above scenario is:

  • Fluent-bit create the log group, but the operator failed to provide permissions to set retention, so the retention policy never gets set.

I ran into this once or twice while developing a patch for this issue.

@luisamador
Copy link

This is currenly not working yet. The README.me file states that the log group retention policy can be controlled by the setting "cloudWatch.logRetentionDays", if you do set that setting it has no effect whatsoever and the resulting log group retention policy is set to "Never Expire". Would be great to fix this.

@PettitWesley
Copy link
Contributor

@luisamador Can you send us more details- the code will always set the retention: https://github.com/aws/amazon-cloudwatch-logs-for-fluent-bit/blob/mainline/cloudwatch/cloudwatch.go#L585

The retention is provided with the log_retention_days parameter:

[OUTPUT]
    Name cloudwatch
    Match   *
    region us-east-1
    log_group_name fluent-bit-cloudwatch
    log_stream_prefix from-fluent-bit-
    auto_create_group true
    log_retention_days 1

@PettitWesley
Copy link
Contributor

You must make sure to set auto_create_group true

@luisamador
Copy link

luisamador commented Jan 28, 2021

@PettitWesley sorry I shouldn't have posted here I think as my issue has to do more with the Helm chart version. See my issue here with an example to replicate my issue: aws/eks-charts#436

@mattduguid
Copy link

seeing this issue here -> aws/eks-charts#436 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants