<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/gitlab/gitlab-ce.git/spec/lib/peek, branch patch-75</title>
<subtitle>gitlab.com: gitlab-org/gitlab-ce.git
</subtitle>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/'/>
<entry>
<title>Merge branch 'fix-peek-on-puma' into 'master'</title>
<updated>2019-09-02T08:43:21+00:00</updated>
<author>
<name>Rémy Coutable</name>
<email>remy@rymai.me</email>
</author>
<published>2019-09-02T08:43:21+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=b7398bc1fdad6c721cfa35ff17cebc2cb00c8240'/>
<id>b7398bc1fdad6c721cfa35ff17cebc2cb00c8240</id>
<content type='text'>
Fix Peek on Puma

Closes #66528

See merge request gitlab-org/gitlab-ce!32213</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Fix Peek on Puma

Closes #66528

See merge request gitlab-org/gitlab-ce!32213</pre>
</div>
</content>
</entry>
<entry>
<title>Make performance bar enabled checks consistent</title>
<updated>2019-08-28T16:25:02+00:00</updated>
<author>
<name>Sean McGivern</name>
<email>sean@gitlab.com</email>
</author>
<published>2019-08-27T11:40:44+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=f9c456bd0c34002952fa4ac812068b38258b108d'/>
<id>f9c456bd0c34002952fa4ac812068b38258b108d</id>
<content type='text'>
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.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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.
</pre>
</div>
</content>
</entry>
<entry>
<title>Return warnings for performance bar from backend</title>
<updated>2019-08-28T15:39:33+00:00</updated>
<author>
<name>Sean McGivern</name>
<email>sean@gitlab.com</email>
</author>
<published>2019-08-06T15:04:43+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=23c1cfcc110b52c1060f35a4a4a7816fb38adc6c'/>
<id>23c1cfcc110b52c1060f35a4a4a7816fb38adc6c</id>
<content type='text'>
For each DetailedView subclass, we add a `warnings` array to:

1. The top-level response.
2. Each individual call under the `details` key.

We use the `.thresholds` hash on the DetailedView to determine what's a
warning. If that hash is empty (the default), then the warnings array
will always be empty.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
For each DetailedView subclass, we add a `warnings` array to:

1. The top-level response.
2. Each individual call under the `details` key.

We use the `.thresholds` hash on the DetailedView to determine what's a
warning. If that hash is empty (the default), then the warnings array
will always be empty.
</pre>
</div>
</content>
</entry>
<entry>
<title>Fix SystemStackError when Peek bar is active with Rugged calls</title>
<updated>2019-07-31T22:47:19+00:00</updated>
<author>
<name>Stan Hu</name>
<email>stanhu@gmail.com</email>
</author>
<published>2019-07-31T22:38:02+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=7a5c4cd0ca692e2fac0d648726b71dcd304602ec'/>
<id>7a5c4cd0ca692e2fac0d648726b71dcd304602ec</id>
<content type='text'>
Peek attempts to serialize results with `to_json`, which calls
`ActiveSupport::JSON`. If an object is passed to `to_json` that contains
instance variables, `ActiveSupport` will attempt to recursively traverse
all variables.

The problem is that we can get into an infinite loop if the instance
references to an instance that references to something else that points
back to the same instance.

To avoid this mess, we just call `to_s` on the object. It appears only
`Gitlab::Git::Repository` and `::Repository` are the culprits here.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/65404
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Peek attempts to serialize results with `to_json`, which calls
`ActiveSupport::JSON`. If an object is passed to `to_json` that contains
instance variables, `ActiveSupport` will attempt to recursively traverse
all variables.

The problem is that we can get into an infinite loop if the instance
references to an instance that references to something else that points
back to the same instance.

To avoid this mess, we just call `to_s` on the object. It appears only
`Gitlab::Git::Repository` and `::Repository` are the culprits here.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/65404
</pre>
</div>
</content>
</entry>
<entry>
<title>Use a base class for Peek views</title>
<updated>2019-07-24T15:57:42+00:00</updated>
<author>
<name>Stan Hu</name>
<email>stanhu@gmail.com</email>
</author>
<published>2019-07-24T14:42:45+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=d7eadcc0f30d3bd005f9dfb160dd0a460b3a8f56'/>
<id>d7eadcc0f30d3bd005f9dfb160dd0a460b3a8f56</id>
<content type='text'>
Introduce a `DetailedView` base class, which is inherited by
the Gitaly, Redis, and Rugged views. This reduces code duplication.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Introduce a `DetailedView` base class, which is inherited by
the Gitaly, Redis, and Rugged views. This reduces code duplication.
</pre>
</div>
</content>
</entry>
<entry>
<title>Hide Rugged data if it doesn't exist</title>
<updated>2019-07-24T05:12:42+00:00</updated>
<author>
<name>Stan Hu</name>
<email>stanhu@gmail.com</email>
</author>
<published>2019-07-24T05:12:42+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=02a27937255f7815ce8d87ea044d4426848f2841'/>
<id>02a27937255f7815ce8d87ea044d4426848f2841</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Add Rugged calls to performance bar</title>
<updated>2019-07-24T04:38:05+00:00</updated>
<author>
<name>Stan Hu</name>
<email>stanhu@gmail.com</email>
</author>
<published>2019-07-21T05:34:46+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=291df05e434f5678c47bce9521ff15748d6c767f'/>
<id>291df05e434f5678c47bce9521ff15748d6c767f</id>
<content type='text'>
This will help diagnose the source of excessive I/O from Rugged
calls. To implement this, we need to obtain the full list of arguments
sent to each request method.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This will help diagnose the source of excessive I/O from Rugged
calls. To implement this, we need to obtain the full list of arguments
sent to each request method.
</pre>
</div>
</content>
</entry>
<entry>
<title>Fix inconsistency in Redis performance bar stats</title>
<updated>2019-07-17T22:11:01+00:00</updated>
<author>
<name>Stan Hu</name>
<email>stanhu@gmail.com</email>
</author>
<published>2019-07-17T19:33:49+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=9dd59df6991b9d82bcbb95bf406194aab8ecf743'/>
<id>9dd59df6991b9d82bcbb95bf406194aab8ecf743</id>
<content type='text'>
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
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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
</pre>
</div>
</content>
</entry>
<entry>
<title>Perform more redactions in Redis performance bar traces</title>
<updated>2019-07-09T14:21:49+00:00</updated>
<author>
<name>Stan Hu</name>
<email>stanhu@gmail.com</email>
</author>
<published>2019-07-09T14:07:41+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=3084c37f3e1e5c1260fbc4a00082300ec0a7b0bd'/>
<id>3084c37f3e1e5c1260fbc4a00082300ec0a7b0bd</id>
<content type='text'>
HMSET and AUTH commands were not properly redacted. This commit
does that and adds a test.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/64309
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
HMSET and AUTH commands were not properly redacted. This commit
does that and adds a test.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/64309
</pre>
</div>
</content>
</entry>
</feed>
