Show Ajax requests in performance bar
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.
Are there points in the code the reviewer needs to double check?
Yes, lots!
Frontend:
- Is the CSS reasonable?
- 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. - There were no tests previously, but should I add some?
- 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.
- 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?
- Do I need to intercept any non-Axios requests?
- Everything else - I don't really know what I'm doing here!
Backend:
- I remove the other profiling options for simplicity, and just kept 'all'. Is that a problem?
- I replace
peek-host
with our own hostname for two reasons: - This gem basically just does this: https://github.com/jacobbednarz/peek-host/blob/master/lib/peek/views/host.rb
- I needed the value in the JSON response, not the HTML.
- 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?
- Do we want any other features?
Does this MR meet the acceptance criteria?
-
Documentation created/updated -
API support added -
Tests added for this feature/bug - Review
-
Has been reviewed by Frontend -
Has been reviewed by Backend
-
-
Conform by the merge request performance guides -
Conform by the style guides -
Squashed related commits together
What are the relevant issue numbers?
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/43925.
Edited by Robert Speicher