Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Allow user to automatically delete CloudWatch logs in batch that are > 14 days old #56

Merged
merged 8 commits into from
Sep 10, 2020

Conversation

jikawa-az
Copy link
Contributor

@jikawa-az jikawa-az commented Sep 1, 2020

Closes #54
AWS PutLogEvents will not upload a batch log if the logs in the batch are older than 14 days. Therefore we will allow the user to specify an option in FileManagerStrategy to automatically delete 14 day old logs from the batch.

In an effort to minimize runtime, we will first upload the correctly batched logs before deleting any remaining logs over 14 days old.

Related PR opened in cloudwatchlogs-ros1 to allow user to set delete_stale_data option in FileManagerStrategyOptions:
Add option for FileManagerStrategy delete stale data #64

Signed-off-by: Jesse Ikawa jikawa@amazon.com

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Signed-off-by: Jesse Ikawa <jikawa@amazon.com>
@codecov
Copy link

codecov bot commented Sep 1, 2020

Codecov Report

Merging #56 into master will increase coverage by 0.15%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #56      +/-   ##
==========================================
+ Coverage   82.48%   82.64%   +0.15%     
==========================================
  Files          57       57              
  Lines        2398     2420      +22     
==========================================
+ Hits         1978     2000      +22     
  Misses        420      420              
Flag Coverage Δ
#ROS_1 82.64% <100.00%> (+0.15%) ⬆️
#ROS_2 84.78% <100.00%> (+0.13%) ⬆️
#dashing 84.78% <100.00%> (+0.13%) ⬆️
#kinetic 82.93% <100.00%> (+0.15%) ⬆️
#melodic 82.62% <100.00%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...de/cloudwatch_logs_common/utils/log_file_manager.h 60.00% <ø> (ø)
...ile_management/file_upload/file_manager_strategy.h 100.00% <ø> (ø)
...udwatch_logs_common/src/utils/log_file_manager.cpp 93.18% <100.00%> (+0.49%) ⬆️
...ude/cloudwatch_metrics_common/cloudwatch_options.h 100.00% <100.00%> (ø)
...ent/include/file_management/file_manager_options.h 100.00% <100.00%> (ø)
...include/file_management/file_upload/file_manager.h 40.35% <100.00%> (+17.62%) ⬆️
...file_management/file_upload/file_upload_streamer.h 70.90% <100.00%> (+0.53%) ⬆️
...nagement/src/file_upload/file_manager_strategy.cpp 87.14% <100.00%> (+0.09%) ⬆️
...test/file_management/file_upload_streamer_test.cpp 92.85% <100.00%> (+0.14%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 588acba...4d3f341. Read the comment docs.

Signed-off-by: Jesse Ikawa <jikawa@amazon.com>
@jikawa-az jikawa-az changed the title move docstring and don't break loop User option for 2-weeks old logs may be discarded Sep 1, 2020
@jikawa-az jikawa-az force-pushed the jikawa/2-week-logs branch 3 times, most recently from 8ed5672 to 9bcaf27 Compare September 3, 2020 01:14
Signed-off-by: Jesse Ikawa <jikawa@amazon.com>
@jikawa-az jikawa-az marked this pull request as ready for review September 3, 2020 17:41
Copy link
Contributor

@dabonnie dabonnie left a comment

Choose a reason for hiding this comment

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

I think the changes in the CloudWatch Logs specific parts are fine, but I'm not sure how this affects CloudWatch Metrics.

@jikawa-az jikawa-az force-pushed the jikawa/2-week-logs branch 2 times, most recently from 6109759 to 64bd082 Compare September 4, 2020 05:16
Signed-off-by: Jesse Ikawa <jikawa@amazon.com>
@jikawa-az jikawa-az changed the title User option for 2-weeks old logs may be discarded Allow user to automatically discard CloudWatch logs in batch that are > 14 days old Sep 4, 2020
@dabonnie dabonnie requested a review from mm318 September 4, 2020 20:59
@jikawa-az jikawa-az marked this pull request as draft September 5, 2020 07:15
@jikawa-az jikawa-az force-pushed the jikawa/2-week-logs branch 2 times, most recently from 5ea3729 to df4748f Compare September 8, 2020 20:11
Signed-off-by: Jesse Ikawa <jikawa@amazon.com>
Signed-off-by: Jesse Ikawa <jikawa@amazon.com>
Signed-off-by: Jesse Ikawa <jikawa@amazon.com>
@jikawa-az jikawa-az changed the title Allow user to automatically discard CloudWatch logs in batch that are > 14 days old Allow user to automatically delete CloudWatch logs in batch that are > 14 days old Sep 10, 2020
Copy link
Contributor

@dabonnie dabonnie left a comment

Choose a reason for hiding this comment

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

Some minor changes but otherwise it looks good.

cloudwatch_logs_common/src/utils/log_file_manager.cpp Outdated Show resolved Hide resolved
cloudwatch_logs_common/src/utils/log_file_manager.cpp Outdated Show resolved Hide resolved
Signed-off-by: Jesse Ikawa <jikawa@amazon.com>
@jikawa-az jikawa-az merged commit 888a93f into master Sep 10, 2020
@jikawa-az jikawa-az deleted the jikawa/2-week-logs branch September 10, 2020 20:04
@jikawa-az jikawa-az self-assigned this Sep 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request older than 2-weeks old may be discarded
3 participants