Log Elasticsearch timeouts (second attempt)
What does this MR do?
We previously implemented the timed_out count in
!53955 (diffs)
but it had to be reverted due to it not being able to handle an empty
response body
gitlab-com/gl-infra/production#3623 (closed)
!54053 (merged) .
It was very easy to reproduce the problem observed in that by simply:
- Visit
Admin > Settings > Preferences - Update the email "Additional Text"
- Save
This problem will actually happen when updating anything in the admin UI.
This was quite surprising as we have tonnes of tests that go down this
code path throughout GitLab. As such it was very strange to not see a
single test failure outside of these QA tests that ran against staging.
It turns out the reason is that this code only runs when
SafeRequestStore is active otherwise the interceptor logic is skipped.
It turns out that none of our tests that execute these code paths had
the SafeRequestStore active. In an effort to reduce the risk of this
happening I've actived the request_store in the helper_spec.rb which
covers many of the Elasticsearch queries we execute which would have
actually caught this problem
Additionally I've updated the unit test with this new specific case and another slightly similar case of error responses just to be safe.
Screenshots (strongly suggested)
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec -
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Related to #301146 (closed)