Skip to content

Show Ajax requests in performance bar

Sean McGivern requested to merge ajax-requests-in-performance-bar into master

What does this MR do?

First, rewrites the performance bar to use Vue, rather than HAML + jQuery from upstream, with some customisations we made separately.

Then, add recording of any AJAX requests on the page, to allow us to see results from those, too.

Performance_bar_demo

Are there points in the code the reviewer needs to double check?

Yes, lots!

Frontend:

  1. Is the CSS reasonable?
  2. The upstreamPerformanceBar component is a bit hacky because I think rewriting https://gitlab.com/gitlab-org/gitlab-ce/blob/master/vendor/assets/javascripts/peek.performance_bar.js in Vue is probably hard (at least, it would be for me). I can create an issue to address that in future, though.
  3. There were no tests previously, but should I add some?
  4. The jQuery is because the line profiler modal is inserted into the page by the backend when a particular query param is set. If there's a better way to do this, I'd love to remove the import.
  5. I kept only the first two requests per URL, to avoid the polling that we do from consuming more and more memory with these responses. Does that sound OK?
  6. Do I need to intercept any non-Axios requests?
  7. Everything else - I don't really know what I'm doing here!

Backend:

  1. I remove the other profiling options for simplicity, and just kept 'all'. Is that a problem?
  2. I replace peek-host with our own hostname for two reasons:
  3. This gem basically just does this: https://github.com/jacobbednarz/peek-host/blob/master/lib/peek/views/host.rb
  4. I needed the value in the JSON response, not the HTML.
  5. This MR is already pretty big, so I didn't investigate further, but I think we could change https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/peek/rblineprof/custom_controller_helpers.rb to store the results in Redis, rather than injecting them into the page. That would mean we could get a profile for other requests too. Should I create an issue for that?
  6. Do we want any other features?

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/43925.

Edited by Robert Speicher

Merge request reports