Commit Graph

6 Commits

Author SHA1 Message Date
Sean McGivern f9c456bd0c Make performance bar enabled checks consistent
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.
2019-08-28 17:25:02 +01:00
Stan Hu a43c84e6ab Only track Redis calls if Peek is enabled
In dev environments, Sidekiq was encountering the message:

Circular dependency detected while autoloading constant Gitlab::Profiler

This saves some overhead during normal usage.
2019-08-02 23:48:14 -07:00
Sean McGivern 18cdc5ba6c Remove line profiler from performance bar
1. The output isn't great. It can be hard to find hotspots and, even
   when you do find them, to find why those are hotspots.
2. It uses some jQuery-specific frontend code which we can remove now
   that we don't have this any more.
3. It's only possible to profile the initial request, not any subsequent
   AJAX requests.
2019-07-30 10:00:52 +01:00
Sean McGivern ad1c71663f Replace peek-pg with our own implementation
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.
2019-07-26 14:37:26 +01:00
Stan Hu d7eadcc0f3 Use a base class for Peek views
Introduce a `DetailedView` base class, which is inherited by
the Gitaly, Redis, and Rugged views. This reduces code duplication.
2019-07-24 08:57:42 -07:00
Stan Hu 9dd59df699 Fix inconsistency in Redis performance bar stats
peek-redis resets its counters at the start of an ActionController
notification (`start_processing.action_controller`), which causes it to
miss some Redis queries that precede it, such as the database load
balancer and Rack Attack queries. This produces inconsistencies in the
performance bar between the number of calls and their durations with the
actual calls in the detailed view.

We fix this by getting rid of peek-redis in favor of consolidating all
logic into the `RedisDetailed` view, which tracks Redis queries using
`RequestStore`. This has the nice property of removing thread-specific
counters as well.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/64707
2019-07-17 15:11:01 -07:00