Skip to content

Conversation

@lukaszgryglicki
Copy link
Member

cc @mlehotskylf @ahmedomosanya

Signed-off-by: Lukasz Gryglicki [email protected]

Assisted by OpenAI

Assisted by GitHub Copilot

…og entry in AWS logs

Signed-off-by: Lukasz Gryglicki <[email protected]>

Assisted by [OpenAI](https://platform.openai.com/)

Assisted by [GitHub Copilot](https://github.com/features/copilot)
@coderabbitai
Copy link

coderabbitai bot commented Nov 27, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Two shell utilities for CloudWatch logs: utils/calculate_api_stats.sh was modified to run two region-specific searches with explicit INCL_LOGS filters, merge results, and print unique logGroupName values; utils/find_1st_log_entry.sh was added to fetch first matching log timestamps across regions.

Changes

Cohort / File(s) Summary
Updated log search & merge
utils/calculate_api_stats.sh
Replaced previous region/DTTO search calls with two explicit INCL_LOGS invocations (one for us-east-1, one for us-east-2), outputs written to api-logs-${STAGE}-1.json and api-logs-${STAGE}-2.json, then jq-merged to api-logs-${STAGE}.json; added final line to print unique logGroupName values.
New timestamp lookup script
utils/find_1st_log_entry.sh
New Bash script that iterates a log_groups list (regions + log group names), runs aws logs filter-log-events with start-time 0 and filter-pattern "LG:api-request-path", extracts first event timestamp via jq, formats it to human-readable datetime, and prints results.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Check correctness of INCL_LOGS lists and that intended log groups are covered for each region in calculate_api_stats.sh
  • Verify aws CLI flags, pagination/limits, and jq usage handle empty or missing results gracefully
  • Confirm find_1st_log_entry.sh handles regions/log-group parsing and timestamp formatting (millisecond-to-second conversion) as intended
  • Consider adding basic error handling or exit codes for AWS CLI failures or missing events

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description mentions reviewers and assistance acknowledgments but lacks substantive details about the changes made to the scripts. Add details about what was changed in the log-grepping scripts and what the new script does, even if brief.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main changes: updating log-grepping scripts and adding a new script to find the first API log entry in AWS logs, which matches the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch unicron-cla-stats-from-logs

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
utils/calculate_api_stats.sh (1)

20-20: Refactor JSON parsing to use jq instead of grep.

Extracting JSON field values using grep is fragile and inefficient. Use jq for consistency and robustness.

- cat "api-logs-${STAGE}.json" | grep '"logGroupName": "' | sort | uniq
+ jq -r '.[] | .logGroupName' "api-logs-${STAGE}.json" | sort -u

Note: This assumes the JSON structure has a logGroupName field at the top level or appropriately adjusted based on the actual structure.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1edadbf and 808ee41.

📒 Files selected for processing (2)
  • utils/calculate_api_stats.sh (1 hunks)
  • utils/find_1st_log_entry.sh (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Agent
  • GitHub Check: cypress-functional
  • GitHub Check: build-test-lint

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a new utility script to find the first API log entry across multiple AWS Lambda log groups and updates the API stats calculation script to explicitly filter specific log groups using the INCL_LOGS variable.

Key changes:

  • New script find_1st_log_entry.sh to query the first API request log entry from AWS CloudWatch logs across multiple Lambda functions
  • Modified calculate_api_stats.sh to explicitly specify which log groups to include for each region using the INCL_LOGS environment variable
  • Added diagnostic output to show which log groups were processed

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
utils/find_1st_log_entry.sh New script that queries AWS CloudWatch logs to find the first API log entry across multiple Lambda functions in us-east-1 and us-east-2 regions
utils/calculate_api_stats.sh Updated to explicitly filter log groups using INCL_LOGS variable and added diagnostic output showing which log groups were processed

Co-authored-by: Copilot <[email protected]>
Signed-off-by: Łukasz Gryglicki <[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Łukasz Gryglicki <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
utils/find_1st_log_entry.sh (1)

1-7: Add critical error handling directives (set -e and set -o pipefail).

The script invokes multiple external commands (AWS CLI, jq, awk) but lacks shell safety options to halt on failures. Missing set -e and set -o pipefail means the script will continue even if intermediate commands fail, potentially masking errors or producing incomplete output.

Apply this diff to add robust error handling at the top:

  #!/bin/bash
+ set -e
+ set -o pipefail
+ 
  # Copyright The Linux Foundation and each contributor to LFX.
  # SPDX-License-Identifier: MIT
+ #
+ # find_1st_log_entry.sh
+ #
+ # Purpose:
+ #   Finds and prints the timestamp of the first log entry containing "LG:api-request-path"
+ #   for several AWS Lambda log groups related to CLA backend services.
+ #
+ # Environment Variables:
+ #   STAGE - (optional) The deployment stage (e.g., dev, prod, staging). Defaults to "dev" if unset.
+ #
+ # Usage:
+ #   STAGE=prod ./find_1st_log_entry.sh
+ #   ./find_1st_log_entry.sh           # uses STAGE=dev by default
+ #
  if [ -z "${STAGE}" ]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 808ee41 and a4de28d.

📒 Files selected for processing (1)
  • utils/find_1st_log_entry.sh (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: cypress-functional
  • GitHub Check: build-test-lint
🔇 Additional comments (1)
utils/find_1st_log_entry.sh (1)

8-16: Array structure effectively addresses DRY principle.

The log_groups array is a clean refactor compared to the previous repetitive commands. This approach improves maintainability.

@lukaszgryglicki lukaszgryglicki merged commit f8af0dd into dev Nov 27, 2025
4 of 6 checks passed
@lukaszgryglicki lukaszgryglicki deleted the unicron-cla-stats-from-logs branch November 27, 2025 12:16
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