diff --git a/app/assets/javascripts/performance_bar/components/performance_bar_app.vue b/app/assets/javascripts/performance_bar/components/performance_bar_app.vue
index 710f49b833c894501553fe515d73530980002f23..0f744e858f2b09916735d5c14db28e6b3de22ea9 100644
--- a/app/assets/javascripts/performance_bar/components/performance_bar_app.vue
+++ b/app/assets/javascripts/performance_bar/components/performance_bar_app.vue
@@ -134,6 +134,7 @@ export default {
methods: {
changeCurrentRequest(newRequestId) {
this.currentRequest = newRequestId;
+ this.$emit('change-request', newRequestId);
},
flamegraphPath(mode) {
return mergeUrlParams(
diff --git a/app/assets/javascripts/performance_bar/components/request_selector.vue b/app/assets/javascripts/performance_bar/components/request_selector.vue
index a46ac620f486a96c63d09edb16b971509d6de838..ffc22c2113dce1fb73d43b13a2aab7faaca12e7a 100644
--- a/app/assets/javascripts/performance_bar/components/request_selector.vue
+++ b/app/assets/javascripts/performance_bar/components/request_selector.vue
@@ -1,15 +1,5 @@
<script>
-import { GlPopover, GlSafeHtmlDirective } from '@gitlab/ui';
-import { glEmojiTag } from '~/emoji';
-import { n__ } from '~/locale';
-
export default {
- components: {
- GlPopover,
- },
- directives: {
- SafeHtml: GlSafeHtmlDirective,
- },
props: {
currentRequest: {
type: Object,
@@ -25,27 +15,11 @@ export default {
currentRequestId: this.currentRequest.id,
};
},
- computed: {
- requestsWithWarnings() {
- return this.requests.filter((request) => request.hasWarnings);
- },
- warningMessage() {
- return n__(
- '%d request with warnings',
- '%d requests with warnings',
- this.requestsWithWarnings.length,
- );
- },
- },
watch: {
currentRequestId(newRequestId) {
this.$emit('change-current-request', newRequestId);
},
},
- methods: {
- glEmojiTag,
- },
- safeHtmlConfig: { ADD_TAGS: ['gl-emoji'] },
};
</script>
<template>
@@ -58,19 +32,7 @@ export default {
data-qa-selector="request_dropdown_option"
>
{{ request.truncatedUrl }}
- <span v-if="request.hasWarnings">(!)</span>
</option>
</select>
- <span v-if="requestsWithWarnings.length" class="gl-cursor-default">
- <span
- id="performance-bar-request-selector-warning"
- v-safe-html:[$options.safeHtmlConfig]="glEmojiTag('warning')"
- ></span>
- <gl-popover
- placement="bottom"
- target="performance-bar-request-selector-warning"
- :content="warningMessage"
- />
- </span>
</div>
</template>
diff --git a/app/assets/javascripts/performance_bar/index.js b/app/assets/javascripts/performance_bar/index.js
index eae424ae4aef18d5b832cae91e431defc63e98c6..e7f84eacdcaca4715a9c4641d1ff12b898adcdc6 100644
--- a/app/assets/javascripts/performance_bar/index.js
+++ b/app/assets/javascripts/performance_bar/index.js
@@ -1,5 +1,6 @@
import '../webpack';
+import { isEmpty } from 'lodash';
import Vue from 'vue';
import axios from '~/lib/utils/axios_utils';
import { numberToHumanSize } from '~/lib/utils/number_utils';
@@ -37,9 +38,10 @@ const initPerformanceBar = (el) => {
};
},
mounted() {
- PerformanceBarService.registerInterceptor(this.peekUrl, this.loadRequestDetails);
+ PerformanceBarService.registerInterceptor(this.peekUrl, this.addRequest);
- this.loadRequestDetails(this.requestId, window.location.href);
+ this.addRequest(this.requestId, window.location.href);
+ this.loadRequestDetails(this.requestId);
},
beforeDestroy() {
PerformanceBarService.removeInterceptor();
@@ -51,26 +53,32 @@ const initPerformanceBar = (el) => {
// want to trace the request.
axios.get(urlOrRequestId);
} else {
- this.loadRequestDetails(urlOrRequestId, urlOrRequestId);
+ this.addRequest(urlOrRequestId, urlOrRequestId);
}
},
- loadRequestDetails(requestId, requestUrl) {
+ addRequest(requestId, requestUrl) {
if (!this.store.canTrackRequest(requestUrl)) {
return;
}
this.store.addRequest(requestId, requestUrl);
+ },
+ loadRequestDetails(requestId) {
+ const request = this.store.findRequest(requestId);
+
+ if (request && isEmpty(request.details)) {
+ return PerformanceBarService.fetchRequestDetails(this.peekUrl, requestId)
+ .then((res) => {
+ this.store.addRequestDetails(requestId, res.data);
+ if (this.requestId === requestId) this.collectFrontendPerformanceMetrics();
+ })
+ .catch(() =>
+ // eslint-disable-next-line no-console
+ console.warn(`Error getting performance bar results for ${requestId}`),
+ );
+ }
- PerformanceBarService.fetchRequestDetails(this.peekUrl, requestId)
- .then((res) => {
- this.store.addRequestDetails(requestId, res.data);
-
- if (this.requestId === requestId) this.collectFrontendPerformanceMetrics();
- })
- .catch(() =>
- // eslint-disable-next-line no-console
- console.warn(`Error getting performance bar results for ${requestId}`),
- );
+ return Promise.resolve();
},
collectFrontendPerformanceMetrics() {
if (performance) {
@@ -143,6 +151,7 @@ const initPerformanceBar = (el) => {
},
on: {
'add-request': this.addRequestManually,
+ 'change-request': this.loadRequestDetails,
},
});
},
diff --git a/app/assets/javascripts/performance_bar/stores/performance_bar_store.js b/app/assets/javascripts/performance_bar/stores/performance_bar_store.js
index 51a8eb5ca69a55e35dadd8225fe3671f10d6d5e6..5a69960e4d93df1ddde4f6d67b6fbd38bf9aacfd 100644
--- a/app/assets/javascripts/performance_bar/stores/performance_bar_store.js
+++ b/app/assets/javascripts/performance_bar/stores/performance_bar_store.js
@@ -12,7 +12,6 @@ export default class PerformanceBarStore {
url: requestUrl,
truncatedUrl: shortUrl,
details: {},
- hasWarnings: false,
});
}
@@ -27,7 +26,6 @@ export default class PerformanceBarStore {
const request = this.findRequest(requestId);
request.details = requestDetails.data;
- request.hasWarnings = requestDetails.has_warnings;
return request;
}
diff --git a/doc/administration/monitoring/performance/img/performance_bar_request_selector_warning.png b/doc/administration/monitoring/performance/img/performance_bar_request_selector_warning.png
deleted file mode 100644
index 3872c8b41b2d4f518e903d542408aabd51d4ce05..0000000000000000000000000000000000000000
Binary files a/doc/administration/monitoring/performance/img/performance_bar_request_selector_warning.png and /dev/null differ
diff --git a/doc/administration/monitoring/performance/performance_bar.md b/doc/administration/monitoring/performance/performance_bar.md
index 55ec7faf75b7d6ac650b2752ada30472c4982d7a..93098e43544d9ebb5dad2a6b2b7e105cc24285e8 100644
--- a/doc/administration/monitoring/performance/performance_bar.md
+++ b/doc/administration/monitoring/performance/performance_bar.md
@@ -95,18 +95,14 @@ For non-administrators to display the performance bar, it must be
## Request warnings
+> [Warning icon in the request selector removed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82187) in GitLab 14.9.
+
Requests that exceed predefined limits display a warning **{warning}** icon and
explanation next to the metric. In this example, the Gitaly call duration
exceeded the threshold.
-If any requests on the current page generated warnings, the warning icon displays
-next to the **Requests** selector menu. In this selector menu, an exclamation `(!)`
-appears next to requests with warnings.
-
-
-
## Enable the performance bar for non-administrators
The performance bar is disabled by default for non-administrators. To enable it
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 696412c13caa1a5874e9aec09060c8cb3131aa25..5f040ea020347a457d253f30d6741a5f546dfb18 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -365,11 +365,6 @@ msgid_plural "%d projects selected"
msgstr[0] ""
msgstr[1] ""
-msgid "%d request with warnings"
-msgid_plural "%d requests with warnings"
-msgstr[0] ""
-msgstr[1] ""
-
msgid "%d second"
msgid_plural "%d seconds"
msgstr[0] ""
diff --git a/spec/frontend/performance_bar/components/performance_bar_app_spec.js b/spec/frontend/performance_bar/components/performance_bar_app_spec.js
index 67a4259a8e3a6ad595579fbfff5eaab036925506..2c9ab4bf78d5e598be7729cfead65544f1febf69 100644
--- a/spec/frontend/performance_bar/components/performance_bar_app_spec.js
+++ b/spec/frontend/performance_bar/components/performance_bar_app_spec.js
@@ -18,4 +18,15 @@ describe('performance bar app', () => {
it('sets the class to match the environment', () => {
expect(wrapper.element.getAttribute('class')).toContain('development');
});
+
+ describe('changeCurrentRequest', () => {
+ it('emits a change-request event', () => {
+ expect(wrapper.emitted('change-request')).toBeUndefined();
+
+ wrapper.vm.changeCurrentRequest('123');
+
+ expect(wrapper.emitted('change-request')).toBeDefined();
+ expect(wrapper.emitted('change-request')[0]).toEqual(['123']);
+ });
+ });
});
diff --git a/spec/frontend/performance_bar/components/request_selector_spec.js b/spec/frontend/performance_bar/components/request_selector_spec.js
deleted file mode 100644
index 9cc8c5e73f4555714bbd61ebce431994fefa1dae..0000000000000000000000000000000000000000
--- a/spec/frontend/performance_bar/components/request_selector_spec.js
+++ /dev/null
@@ -1,31 +0,0 @@
-import { shallowMount } from '@vue/test-utils';
-import RequestSelector from '~/performance_bar/components/request_selector.vue';
-
-describe('request selector', () => {
- const requests = [
- {
- id: 'warningReq',
- url: 'https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/1/discussions.json',
- truncatedUrl: 'discussions.json',
- hasWarnings: true,
- },
- ];
-
- const wrapper = shallowMount(RequestSelector, {
- propsData: {
- requests,
- currentRequest: requests[0],
- },
- });
-
- it('has a warning icon if any requests have warnings', () => {
- expect(wrapper.find('span > gl-emoji').element.dataset.name).toEqual('warning');
- });
-
- it('adds a warning glyph to requests with warnings', () => {
- const requestValue = wrapper.find('[value="warningReq"]').text();
-
- expect(requestValue).toContain('discussions.json');
- expect(requestValue).toContain('(!)');
- });
-});
diff --git a/spec/frontend/performance_bar/index_spec.js b/spec/frontend/performance_bar/index_spec.js
index 819b2bcbacfc1c4c0d4e844d0cf1cb58ba3ea386..91cb46002be0e5f211cf73ce41fdbcdecb753fb7 100644
--- a/spec/frontend/performance_bar/index_spec.js
+++ b/spec/frontend/performance_bar/index_spec.js
@@ -51,7 +51,7 @@ describe('performance bar wrapper', () => {
mock.restore();
});
- describe('loadRequestDetails', () => {
+ describe('addRequest', () => {
beforeEach(() => {
jest.spyOn(vm.store, 'addRequest');
});
@@ -59,26 +59,46 @@ describe('performance bar wrapper', () => {
it('does nothing if the request cannot be tracked', () => {
jest.spyOn(vm.store, 'canTrackRequest').mockImplementation(() => false);
- vm.loadRequestDetails('123', 'https://gitlab.com/');
+ vm.addRequest('123', 'https://gitlab.com/');
expect(vm.store.addRequest).not.toHaveBeenCalled();
});
it('adds the request immediately', () => {
- vm.loadRequestDetails('123', 'https://gitlab.com/');
+ vm.addRequest('123', 'https://gitlab.com/');
expect(vm.store.addRequest).toHaveBeenCalledWith('123', 'https://gitlab.com/');
});
+ });
- it('makes an HTTP request for the request details', () => {
+ describe('loadRequestDetails', () => {
+ beforeEach(() => {
jest.spyOn(PerformanceBarService, 'fetchRequestDetails');
+ });
- vm.loadRequestDetails('456', 'https://gitlab.com/');
+ it('makes an HTTP request for the request details', () => {
+ vm.addRequest('456', 'https://gitlab.com/');
+ vm.loadRequestDetails('456');
expect(PerformanceBarService.fetchRequestDetails).toHaveBeenCalledWith(
'/-/peek/results',
'456',
);
});
+
+ it('does not make a request if request was not added', () => {
+ vm.loadRequestDetails('456');
+
+ expect(PerformanceBarService.fetchRequestDetails).not.toHaveBeenCalled();
+ });
+
+ it('makes an HTTP request only once for the same request', async () => {
+ vm.addRequest('456', 'https://gitlab.com/');
+ await vm.loadRequestDetails('456');
+
+ vm.loadRequestDetails('456');
+
+ expect(PerformanceBarService.fetchRequestDetails).toHaveBeenCalledTimes(1);
+ });
});
});