Added filtering for incorrect slo boosting logs #2118
Conversation
k8s-ci-robot
added
release-note-none
k8s-ci-robot
added
the
cncf-cla: no
|
Welcome @chrisThePattyEater! |
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @chrisThePattyEater. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
size/L
chrisThePattyEater
force-pushed
the
master
branch
2 times, most recently
from
5c1e6e8 to
e42e6e2
Compare
| if err == nil { | ||
| return false | ||
| } | ||
| re, compileErr := regexp.Compile(regexPattern) |
There was a problem hiding this comment.
Can you set re as a static var, with regexp.MustCompile?
|
|
||
| // IsGCPOrgViolationError checks if the error is a GCP organization policy violation error. | ||
| func IsGCPOrgViolationError(err error) bool { | ||
| return ErrorContainsRegex(err, "violates constraint constraints/gcp.") |
There was a problem hiding this comment.
Is there a more complex error policy violation field in the response that you can check instead of string error message extraction?
The risk is if the error message from the API is reformatted, this regex matching may fail. If there is a more dependable API type response we could use, I would prefer to use that.
| expectedResult bool | ||
| }{ | ||
| { | ||
| name: "is gcp org violation error", |
There was a problem hiding this comment.
Test name does not match Test function.
chrisThePattyEater
force-pushed
the
master
branch
from
e42e6e2 to
f015679
Compare
k8s-ci-robot
added
size/M
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cemakd, chrisThePattyEater, pwschuurman 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Recently we were out of SLO for the PDCSI CreateSnapshot endpoint. After analysis of the errors contributing to our slo we have identified the issue to be an excessive amount of INTERNAL errors. Further analysis of these internal errors has brought to light two major types of errors that are being incorrectly tagged as internal. These should not be contributing to our slos, they are the following:
Already exists errors should be converted from INTERNAL to ALREADY_EXISTS
Org policy violations should be converted from INTERNAL to FAILED_PRECONDITION
These errors are both returned from the create snapshot gcp endpoint
This change introduces methods that use regex to identify these two types of errors in the response payload and correctly marks them as ALREADY_EXISTS and FAILED_PRECONDITION errors.
Which issue(s) this PR fixes:
Fixes #
https://b.corp.google.com/issues/398008774
Special notes for your reviewer:
Does this PR introduce a user-facing change?: