Fixes for nginx test when running against a fresh project #364
Conversation
integration_test/gce/gce_testing.go
Outdated
| strings.Contains(err.Error(), "failed to look up metric workload.googleapis.com") | ||
| // The error doesn't always look the same, hopefully looking for "code = NotFound" will catch all variations. | ||
| strings.Contains(err.Error(), "failed to look up metric workload.googleapis.com") || | ||
| strings.Contains(err.Error(), "code = NotFound") |
There was a problem hiding this comment.
Is it possible to check the code directly instead of checking the string version?
There was a problem hiding this comment.
(with grouchiness) this api doesn't seem to document what kind of error object is being returned, and looking at the code didn't help either: https://github.com/googleapis/google-cloud-go/blob/monitoring/v1.2.0/monitoring/apiv3/metric_client.go#L362
it could be some kind of gax-ian error object, or it could be decided by c.metricClient.ListTimeSeries, which just links back to itself, very unhelpful.
I agree that matching on the error string is gross but don't have a better solution at the moment. suggestions welcome.
There was a problem hiding this comment.
I would expect it returns an error compatible with status.FromError, which returns a Status object that has a Code() method that you can compare against codes.NotFound.
I would also expect "failed to look up metric" to be paired with a code (perhaps even NotFound), though maybe you want to explicitly match on workload.googleapis.com for some reason?
There was a problem hiding this comment.
OK I changed it to just look for NotFound (and Internal). Much cleaner and hopefully more robust to tiny changes in the error message. It seems to work.
martijnvans
force-pushed
the
martijnvans-fix-nginx
branch
from
26a7c71 to
e2e351e
Compare
See #363.
This PR was tested with the following command:
PROJECT=[REDACTED]
TRANSFERS_BUCKET=[REDACTED]
GOOGLE_APPLICATION_CREDENTIALS="${HOME}/credentials.json"
ZONE=us-central1-b
PLATFORMS=debian-10,centos-7
SCRIPTS_DIR=third_party_apps_data
go test -v third_party_apps_test.go -test.parallel=1000 -tags=integration_test -timeout=4
And, it didn't pass, for various reasons. Mostly that my test file is at head but it's testing the stable agent, which is substantially behind head. But I believe the failures were unrelated to this change. (and nginx passed.) The interesting part of this change was changing the retry logic for metrics queries, and none of the failures that I saw were due to a metrics query not being retried when it was supposed to.