Previously, we called the `peek_enabled?` method like so:
prepend_before_action :set_peek_request_id, if: :peek_enabled?
Now we don't have a `set_peek_request_id` method, so we don't need that
line. However, the `peek_enabled?` part had a side-effect: it would also
populate the request store cache for whether the performance bar was
enabled for the current request or not.
This commit makes that side-effect explicit, and replaces all uses of
`peek_enabled?` with the more explicit
`Gitlab::PerformanceBar.enabled_for_request?`. There is one spec that
still sets `SafeRequestStore[:peek_enabled]` directly, because it is
contrasting behaviour with and without a request store enabled.
The upshot is:
1. We still set the value in one place. We make it more explicit that
that's what we're doing.
2. Reading that value uses a consistent method so it's easier to find in
future.
This key is useful to reduce the amount of logic needed on the frontend:
if `has_warnings` is true, then the frontend knows that the request in
question has warnings for some metric.
Peek's `Peek.request_id` method doesn't work well with a multi-threaded
server and concurrent requests, because requests can 'steal' another
request's ID, or unset it before it was due.
The upstream change resolves this; the commit here is just to ensure
that GitLab works with that upstream change, mostly by not using
`Peek.request_id` any more (as the method doesn't exist).
This uses an ActiveRecord subscriber to get queries and calculate the
total query time from that. This means that the total will always be
consistent with the queries in the table. It does however mean that we
could potentially miss some queries that don't go through ActiveRecord.
Making this change also allows us to unify the response JSON a little
bit, making the frontend slightly simpler as a result.
Just as we have backtraces for Gitaly, we should also have backtraces
for SQL calls. This makes it much easier to find the source of the SQL
call and optimize N+1 queries and other performance issues with an
endpoint.
Sherlock::Query generates a backtrace on every call to "new", which is
very slow. Formatter queries are also not displayed properly due to the
lack of "white-space: pre" in the CSS. We took a look at fixing this,
but the produced output is not really better than just displaying
queries on one line.
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/39351
GitLab Bot
Rémy Coutable
Sean McGivern
Stan Hu
Jasper Maes
gfyoung
Michael Kozono
Yorick Peterse
Sean McGivern
Grzegorz Bizon