GoogleCloudPlatform / testgrid

Apache License 2.0
198 stars 68 forks source link

Fixing a panic when recording column headers in alerts. #1273

Closed michelle192837 closed 9 months ago

michelle192837 commented 9 months ago

A recent update to Summarizer records column header values (specifically, the 'extra' or 'custom' column header values) when recording metadata about alerted rows. We missed a case where, if there are fewer values recorded than the current number of custom column headers configured for the tab, we hit an out of index issue. (This is a legitimate case that can happen when new custom column headers are added).

Quick-fixed this by checking the length of column header values before recording them, and an additional unit test to verify this case. (We made this change internally, and I've verified it works already).

google-oss-prow[bot] commented 9 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: airbornepony, michelle192837

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/GoogleCloudPlatform/testgrid/blob/main/OWNERS)~~ [michelle192837] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
listx commented 9 months ago

@michelle192837 Also, are you sure there is no additional context for this change for your future self/others? I find it very rare that a commit doesn't need a corresponding commit description (the usual suspects are typically just typofixes).

michelle192837 commented 9 months ago

@michelle192837 Also, are you sure there is no additional context for this change for your future self/others? I find it very rare that a commit doesn't need a corresponding commit description (the usual suspects are typically just typofixes).

Good point, sorry about that. Updated the description with more details, thanks!

listx commented 9 months ago

/lgtm

Thanks

michelle192837 commented 9 months ago

/hold cancel

Thanks!