Skip to content

Conversation

duhminick
Copy link
Contributor

@duhminick duhminick commented Oct 20, 2025

Description of the issue

In #1717, we implemented batching for DescribeLogGroups (DLG) calls using the LogGroupIdentifiers parameter to reduce API throttling when customers have many log groups. However, this batched DLG API is not supported in all cases. When unsupported, the agent fails with an InvalidParameterException error: "Input filter on Log group identifiers is not supported.", preventing log group retention policies from being checked and updated.

Description of changes

This PR adds a fallback mechanism to handle regions where the batched DLG API is not available:

  1. Primary path: Attempts to use the batched DescribeLogGroups API with LogGroupIdentifiers (introduced in Batch DescribeLogGroups calls #1717)

  2. Fallback path: If the batch call fails with the specific unsupported error, falls back to individual DescribeLogGroups calls using LogGroupNamePrefix for each log group. This is the same logic that was in the agent before.

Implementation details:

  • Refactored updateTargetBatch()updateTargets() which orchestrates the batch-first approach
  • Added updateTargetsBatch() for the batched API call (returns error if not supported)
  • Added updateTargetsIteratively() for the fallback individual API calls
  • Added back getRetention() helper to retrieve retention using prefix-based lookup
  • Error detection checks for both InvalidParameterException error code AND exact error message to avoid false positives
  • Logs at DEBUG level when fallback occurs

License

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

Tests

  1. Unit tests
  2. Integration tests

Requirements

Before commiting your code, please do the following steps.

  1. Run make fmt and make fmt-sh
  2. Run make lint

Integration Tests

To run integration tests against this PR, add the ready for testing label.

@duhminick duhminick requested a review from a team as a code owner October 20, 2025 20:40
@duhminick duhminick added the ready for testing Indicates this PR is ready for integration tests to run label Oct 20, 2025
continue
}

if currentRetention != target.Retention && target.Retention > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should skip the target if the target.Retention <= 0. no point in the making the DescribeLogGroups call if we're not going to use it, and we should minimize AWS calls where we can.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

func (m *targetManager) PutRetentionPolicy(target Target) {
// new pusher will call this so start with dlg
if target.Retention > 0 {
m.logger.Debugf("sending log group %v to dlg channel by pusher", target.Group)
m.dlg <- target
}
}

The target map will only have targets with retention greater than 0

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok great. So the target.Retention > 0 check isn't needed but good to have a double check.

@duhminick duhminick merged commit 6a16e3f into main Oct 21, 2025
259 of 262 checks passed
@duhminick duhminick deleted the dominic-dlg-singular branch October 21, 2025 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for testing Indicates this PR is ready for integration tests to run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants