From 7f102819a56b55607e657447b51d2eeb45b2fe94 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Mon, 26 Aug 2019 14:57:59 +0100 Subject: Fix Peek on Puma 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). --- app/views/peek/_bar.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/views') diff --git a/app/views/peek/_bar.html.haml b/app/views/peek/_bar.html.haml index 5228930293c..3c2c4d30535 100644 --- a/app/views/peek/_bar.html.haml +++ b/app/views/peek/_bar.html.haml @@ -1,6 +1,6 @@ - return unless peek_enabled? #js-peek{ data: { env: Peek.env, - request_id: Peek.request_id, + request_id: peek_request_id, peek_url: "#{peek_routes_path}/results" }, class: Peek.env } -- cgit v1.2.1 From f9c456bd0c34002952fa4ac812068b38258b108d Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 27 Aug 2019 12:40:44 +0100 Subject: 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. --- app/views/peek/_bar.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/views') diff --git a/app/views/peek/_bar.html.haml b/app/views/peek/_bar.html.haml index 3c2c4d30535..9725f640be9 100644 --- a/app/views/peek/_bar.html.haml +++ b/app/views/peek/_bar.html.haml @@ -1,4 +1,4 @@ -- return unless peek_enabled? +- return unless performance_bar_enabled? #js-peek{ data: { env: Peek.env, request_id: peek_request_id, -- cgit v1.2.1