Skip to content
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .github/workflows/update-release-branch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,21 @@ jobs:
update-release:
runs-on: ubuntu-latest
steps:
- name: Set start timestamp
id: start
run: printf 'timestamp=%(%s)T\n' >> "$GITHUB_OUTPUT"
Comment on lines +30 to +32
Copy link
Member

Choose a reason for hiding this comment

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

Is duration a meaningful metric for this job? Since it runs on an automated schedule at 3AM, I don't think we care if it takes 5, 15, even 30 minutes, just that it succeeds or fails.

GitHub already has a default timeout of 6 hours, so we could never get into a situation where the job runs for 24+ hours and conflicts with subsequent merge jobs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe durations are always useful, particularly when they start being larger/smaller than one expects. You're right, we don't necessarily care whether it's 5 minutes or 30 minutes but we probably care if it's always been 5 minutes and suddenly becomes 30 minutes.

Additionally, we do run this job manually from time to time which means we'd be waiting for it to complete. In that case, I believe we would care more about time.

- uses: actions/checkout@v4
with:
ref: 'main'
fetch-depth: 0
token: ${{ secrets.CI_USER_PAT }}
- name: Configure AWS Credentials
uses: aws-actions/configure-aws-credentials@v4
with:
role-to-assume: ${{ secrets.CI_AWS_ROLE_ARN }}
aws-region: us-west-2
- name: Setup kat
uses: awslabs/aws-kotlin-repo-tools/.github/actions/setup-kat@main
- name: Configure Git
shell: bash
run: |
Expand Down Expand Up @@ -107,3 +117,23 @@ jobs:
echo "pushing changes to release branch";
git push origin release;
fi
- name: Calculate duration
id: end
run: |
printf -v now '%(%s)T'
duration=$(( now - ${{ steps.start.outputs.timestamp }} ))
printf 'duration=$duration\n' >> "$GITHUB_OUTPUT"
- name: Emit metrics
if: always()
Copy link
Member

Choose a reason for hiding this comment

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

question: What does this if do, is it necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it ensures we always run this step regardless of failure or cancelation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It forces this step to always run, even if previous steps failed. By default, every step implicitly has if: success() (meaning only run this step if no previous steps failed) unless explicitly overridden.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Adding a comment explaining why this is here

uses: awslabs/aws-kotlin-repo-tools/.github/actions/emit-metrics
with:
namespace: CI metrics
Copy link
Member

Choose a reason for hiding this comment

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

naming: Is this name what appears in our CloudWatch dashboard, should it be "CI Metrics"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is and it should be.

dimensions: |
Product=aws-sdk-kotlin
Trigger=${{ github.event_name == 'schedule' && 'schedule' || 'manual' }}
metrics: |
ReleaseMergeAttempted:1:Count
ReleaseMergeSucceeded:${{ job.status == 'success' && '1' || '0' }}:Count
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: If the job is canceled or fails before this step runs, would we still be able to emit metrics?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As you noted above, this step will always run regardless of whether previous steps have failed or the job's been canceled.

ReleaseMergeCancelled:${{ job.status == 'cancelled' && '1' || '0' }}:Count
Copy link
Member

Choose a reason for hiding this comment

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

naming suggestion: ReleaseMergeCanceled

ReleaseMergeFailed:${{ job.status == 'failure' && '1' || '0' }}:Count
ReleaseMergeDuration:${{ steps.end.outputs.duration }}:Seconds
Loading