Skip to content

Conversation

saranyailla
Copy link
Member

@saranyailla saranyailla commented Sep 15, 2025

Issue #, if available:

Description of changes:

  • Added lastComponentUploadedTime parameter to both loadProcessingFilesConfig() and loadProcessingFilesConfigDeprecated() to not load stale information from the persisted configuration.
    • Only loads processing file information for files modified after the last component upload time.
    • Prevents loading stale file information from previous LM versions (≤2.3.10)
  • [Improvement] Do not keep processing information of uploaded files in memory.
    • Always removes processing file information for uploaded files.
    • [Fix] Do not add it to processing information in memory at all.
  • [Fix] When deleting a log file, delete it from the log group as well to maintain consistent state.
    • Always remove processing file information regardless of the actual file deletion.

Why is this change necessary:

How was this change tested:

Any additional information or context required to review the change:

Checklist:

  • Updated the README if applicable
  • Updated or added new unit tests
  • Updated or added new integration tests
  • Updated or added new end-to-end tests

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link

github-actions bot commented Sep 19, 2025

Unit Tests Coverage Report

File Coverage Lines Branches
All files 77% 84% 71%
com.aws.greengrass.logmanager.services.DiskSpaceManagementService 84% 93% 75%
com.aws.greengrass.logmanager.CloudWatchLogsUploader 90% 97% 83%
com.aws.greengrass.logmanager.LogManagerService 70% 83% 57%
com.aws.greengrass.logmanager.LogManagerService$CurrentProcessingFileInformation 79% 89% 68%
com.aws.greengrass.logmanager.PositionTrackingBufferedReader 86% 87% 86%
com.aws.greengrass.logmanager.CloudWatchAttemptLogsProcessor 88% 89% 86%
com.aws.greengrass.logmanager.util.ConfigUtil 85% 85% 85%
com.aws.greengrass.logmanager.util.SdkClientWrapper 58% 67% 50%
com.aws.greengrass.logmanager.util.CloudWatchClientFactory 36% 36% 0%
com.aws.greengrass.logmanager.model.CloudWatchAttemptLogInformation 100% 100% 0%
com.aws.greengrass.logmanager.model.EventType 100% 100% 0%
com.aws.greengrass.logmanager.model.ComponentType 100% 100% 0%
com.aws.greengrass.logmanager.model.ProcessingFiles 80% 91% 68%
com.aws.greengrass.logmanager.model.LogFile 85% 78% 92%
com.aws.greengrass.logmanager.model.LogFileGroup 70% 75% 65%

Minimum allowed coverage is 65%

Generated by 🐒 cobertura-action against 4bb27bd

Copy link

github-actions bot commented Sep 19, 2025

Integration Tests Coverage Report

File Coverage Lines Branches
All files 71% 79% 64%
com.aws.greengrass.logmanager.services.DiskSpaceManagementService 46% 56% 37%
com.aws.greengrass.logmanager.CloudWatchLogsUploader 45% 49% 41%
com.aws.greengrass.logmanager.LogManagerService 85% 94% 77%
com.aws.greengrass.logmanager.LogManagerService$CurrentProcessingFileInformation 46% 55% 37%
com.aws.greengrass.logmanager.PositionTrackingBufferedReader 72% 82% 63%
com.aws.greengrass.logmanager.CloudWatchAttemptLogsProcessor 70% 78% 61%
com.aws.greengrass.logmanager.util.ConfigUtil 76% 73% 78%
com.aws.greengrass.logmanager.util.SdkClientWrapper 44% 47% 42%
com.aws.greengrass.logmanager.util.CloudWatchClientFactory 100% 100% 0%
com.aws.greengrass.logmanager.model.CloudWatchAttemptLogInformation 100% 100% 0%
com.aws.greengrass.logmanager.model.EventType 100% 100% 0%
com.aws.greengrass.logmanager.model.ComponentType 100% 100% 0%
com.aws.greengrass.logmanager.model.ProcessingFiles 91% 95% 87%
com.aws.greengrass.logmanager.model.LogFile 43% 47% 38%
com.aws.greengrass.logmanager.model.LogFileGroup 67% 80% 53%

Minimum allowed coverage is 58%

Generated by 🐒 cobertura-action against 4bb27bd

// Must set an MDC adapter for 1.3.8+. https://github.com/qos-ch/logback/issues/709
loggerContext.setMDCAdapter(new NOPMDCAdapter());
loggerContext.setMDCAdapter(new BasicMDCAdapter());
// loggerContext.setMDCAdapter(new NOPMDCAdapter());
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Remove instead of commenting out

import ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy;
import ch.qos.logback.core.util.FileSize;
import org.slf4j.helpers.BasicMDCAdapter;
import org.slf4j.helpers.NOPMDCAdapter;
Copy link
Member

Choose a reason for hiding this comment

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

This import is now unused

@kstribrnAmzn
Copy link
Member

Approving as my comments are mostly nit-picks.

@saranyailla saranyailla merged commit bc5b26d into main Sep 23, 2025
4 checks passed
@saranyailla saranyailla deleted the fix-processing-config branch September 23, 2025 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants