Skip to content

Fix Cloudwatch query errors in various reports#10875

Merged
zachmargolis merged 12 commits intomainfrom
margolis-cloudwatch-complete-logs
Jun 27, 2024
Merged

Fix Cloudwatch query errors in various reports#10875
zachmargolis merged 12 commits intomainfrom
margolis-cloudwatch-complete-logs

Conversation

@zachmargolis
Copy link
Contributor

This PR fixes that same set of errors for reports from other teams

  • All reports use a "more realistic" stubbing of CW logs that goes all the way to fake AWS APIs, so it goes fully through our wrapper
  • I used my best judgement here and switched ensure_complete_logs: false in a few cases where I think it was not appropriate (such as pre-aggregated queries like | stats or | dedup

@zachmargolis zachmargolis requested review from a team June 26, 2024 16:44
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the query in this file uses | stats so I think the "split over time and combine" approach is not applicable here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the queries in this file uses | stats or | dedup so I think the "split over time and combine" approach is not applicable here

@zachmargolis zachmargolis requested a review from a team June 26, 2024 16:47
- This report uses an aggregate "stats" query, ensure_complete_logs does not
  necessarily make sense when the results are pre-aggregated
- Each query has some sort of aggregation or deduping, so ensure_complete_logs
  would not have expected behavior
changelog: Internal, Reporting, Fix queries across multiple internal reports
@zachmargolis zachmargolis force-pushed the margolis-cloudwatch-complete-logs branch from 1b3fe58 to f645e07 Compare June 26, 2024 16:55
Base automatically changed from margolis-fix-lg99-cloudwatch-query to main June 26, 2024 17:11
Copy link
Contributor

@vrajmohan vrajmohan left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@zachmargolis zachmargolis merged commit 4f56ea3 into main Jun 27, 2024
@zachmargolis zachmargolis deleted the margolis-cloudwatch-complete-logs branch June 27, 2024 16:50
@h-m-m h-m-m requested a review from a team June 27, 2024 19:18
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.

3 participants