diff --git a/ee/app/assets/javascripts/tracing/list/tracing_analytics.vue b/ee/app/assets/javascripts/tracing/list/tracing_analytics.vue index 777092938f173ccb57dbc4e297fb945dd4036535..61812c90ee2c91e562a70e149d1c7424550f14f4 100644 --- a/ee/app/assets/javascripts/tracing/list/tracing_analytics.vue +++ b/ee/app/assets/javascripts/tracing/list/tracing_analytics.vue @@ -1,8 +1,7 @@ <script> import { GlLineChart, GlColumnChart } from '@gitlab/ui/dist/charts'; -import { throttle } from 'lodash'; -import { s__ } from '~/locale'; -import { contentTop } from '~/lib/utils/common_utils'; +import { GlSkeletonLoader } from '@gitlab/ui'; +import { s__, sprintf } from '~/locale'; import { durationNanoToMs } from '../trace_utils'; const intervalToTimestamp = (interval) => new Date(interval * 1000); @@ -13,7 +12,7 @@ const buildVolumeRateData = ({ interval, trace_rate: traceRate = 0 }, volumeData }; const buildErrorRateData = ({ interval, error_rate: errorRate = 0 }, errorData) => { - errorData.push([intervalToTimestamp(interval), toFixed(Math.min(errorRate * 100, 100))]); + errorData.push([intervalToTimestamp(interval), toFixed(errorRate)]); }; const buildDurationData = ( @@ -37,10 +36,11 @@ export default { components: { GlLineChart, GlColumnChart, + GlSkeletonLoader, }, i18n: { durationLabel: s__('Tracing|Duration (ms)'), - errorRateLabel: s__('Tracing|Error rate (%%)'), + errorRateLabel: sprintf(s__('Tracing|Error rate (%%)')), volumeLabel: s__('Tracing|Request rate (req/s)'), }, props: { @@ -48,11 +48,14 @@ export default { type: Array, required: true, }, - }, - data() { - return { - chartHeight: 0, - }; + loading: { + type: Boolean, + required: true, + }, + chartHeight: { + type: Number, + required: true, + }, }, computed: { seriesData() { @@ -162,28 +165,18 @@ export default { }; }, }, - created() { - this.resizeChart(); - - this.resizeThrottled = throttle(() => { - this.resizeChart(); - }, 400); - window.addEventListener('resize', this.resizeThrottled); - }, - beforeDestroy() { - window.removeEventListener('resize', this.resizeThrottled, false); - }, - methods: { - resizeChart() { - const containerHeight = window.innerHeight - contentTop(); - this.chartHeight = Math.max(100, (containerHeight * 20) / 100); - }, - }, + SKELETON_CLASS: 'analytics-chart gl-mx-7 gl-my-4', + CONTAINER_CLASS: 'gl-display-flex gl-flex-direction-row gl-mb-6', }; </script> <template> - <div v-if="analytics.length" class="gl-display-flex gl-flex-direction-row gl-mb-8"> + <div v-if="loading" :class="$options.CONTAINER_CLASS"> + <div :class="$options.SKELETON_CLASS"><gl-skeleton-loader :lines="5" /></div> + <div :class="$options.SKELETON_CLASS"><gl-skeleton-loader :lines="5" /></div> + <div :class="$options.SKELETON_CLASS"><gl-skeleton-loader :lines="5" /></div> + </div> + <div v-else-if="analytics.length" :class="$options.CONTAINER_CLASS"> <div class="analytics-chart"> <gl-column-chart :bars="volumeRateChartData" diff --git a/ee/app/assets/javascripts/tracing/list/tracing_list.vue b/ee/app/assets/javascripts/tracing/list/tracing_list.vue index 13c6a6fe98be73870b8395ba840c4a0f2cfd625d..9cb1c1c6ebb86808dca4ec18ca0b2518342f7981 100644 --- a/ee/app/assets/javascripts/tracing/list/tracing_list.vue +++ b/ee/app/assets/javascripts/tracing/list/tracing_list.vue @@ -1,12 +1,11 @@ <script> import { GlLoadingIcon, GlInfiniteScroll } from '@gitlab/ui'; -import { debounce } from 'lodash'; +import { throttle } from 'lodash'; import { s__ } from '~/locale'; import { createAlert } from '~/alert'; import { visitUrl, joinPaths, queryToObject } from '~/lib/utils/url_utility'; import UrlSync from '~/vue_shared/components/url_sync.vue'; import { contentTop, isMetaClick } from '~/lib/utils/common_utils'; -import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants'; import { DEFAULT_SORTING_OPTION } from '~/observability/constants'; import { queryToFilterObj, @@ -41,13 +40,15 @@ export default { const { sortBy, ...filterQuery } = queryToObject(query, { gatherArrays: true }); return { - loading: false, + loadingTraces: false, + loadingAnalytics: false, traces: [], + analytics: [], filters: queryToFilterObj(filterQuery), nextPageToken: null, - highlightedTraceId: null, sortBy: sortBy || DEFAULT_SORTING_OPTION, - analytics: [], + listHeight: 0, + analyticsChartsHeight: 0, }; }, computed: { @@ -65,42 +66,34 @@ export default { if (this.traces.length > 0) return s__(`Tracing|Showing ${this.traces.length} traces`); return null; }, - listHeight() { - return this.containerHeight - this.chartHeight - TRACING_LIST_VERTICAL_PADDING; - }, - chartHeight() { - return (this.containerHeight * 30) / 100; - }, - containerHeight() { - return window.innerHeight - contentTop(); - }, }, created() { - this.debouncedChartItemOver = debounce(this.chartItemOver, DEFAULT_DEBOUNCE_AND_THROTTLE_MS); - this.fetchTraces(true); + this.fetchTraces(); + this.fetchAnalytics(); + }, + mounted() { + this.resize(); + this.resizeThrottled = throttle(() => { + this.resize(); + }, 400); + window.addEventListener('resize', this.resizeThrottled); + }, + beforeDestroy() { + window.removeEventListener('resize', this.resizeThrottled, false); }, methods: { - async fetchTraces(updateAnalytics = false) { - this.loading = true; - + async fetchTraces() { + this.loadingTraces = true; try { - const apiRequests = [ - this.observabilityClient.fetchTraces({ - filters: this.filters, - pageToken: this.nextPageToken, - pageSize: PAGE_SIZE, - sortBy: this.sortBy, - }), - ]; - if (updateAnalytics) { - apiRequests.push( - this.observabilityClient.fetchTracesAnalytics({ filters: this.filters }), - ); - } - - const [tracesResults, analyticsResults] = await Promise.all(apiRequests); - this.analytics = analyticsResults; - const { traces, next_page_token: nextPageToken } = tracesResults; + const { + traces, + next_page_token: nextPageToken, + } = await this.observabilityClient.fetchTraces({ + filters: this.filters, + pageToken: this.nextPageToken, + pageSize: PAGE_SIZE, + sortBy: this.sortBy, + }); this.traces = [...this.traces, ...traces]; if (nextPageToken) { this.nextPageToken = nextPageToken; @@ -110,7 +103,21 @@ export default { message: s__('Tracing|Failed to load traces.'), }); } finally { - this.loading = false; + this.loadingTraces = false; + } + }, + async fetchAnalytics() { + this.loadingAnalytics = true; + try { + this.analytics = await this.observabilityClient.fetchTracesAnalytics({ + filters: this.filters, + }); + } catch { + createAlert({ + message: s__('Tracing|Failed to load tracing analytics.'), + }); + } finally { + this.loadingAnalytics = false; } }, onTraceClicked({ traceId, clickEvent = {} }) { @@ -121,7 +128,9 @@ export default { this.filters = filterTokensToFilterObj(filterTokens); this.nextPageToken = null; this.traces = []; - this.fetchTraces(true); + this.analytics = []; + this.fetchTraces(); + this.fetchAnalytics(); }, onSort(sortBy) { this.sortBy = sortBy; @@ -132,25 +141,11 @@ export default { bottomReached() { this.fetchTraces(); }, - chartItemSelected({ traceId }) { - this.onTraceClicked({ traceId }); - }, - chartItemOver({ traceId }) { - const index = this.traces.findIndex((x) => x.trace_id === traceId); - if (index >= 0) { - this.highlightedTraceId = traceId; - this.scrollToRow(index); - } - }, - scrollToRow(index) { - const tbody = this.$refs.tableList.$el.querySelector('tbody'); - const row = tbody.querySelectorAll('tr')[index]; - if (row) { - this.$refs.infiniteScroll.scrollTo({ top: row.offsetTop, behavior: 'smooth' }); - } - }, - chartItemOut() { - this.highlightedTraceId = null; + resize() { + const containerHeight = window.innerHeight - contentTop(); + this.analyticsChartsHeight = Math.max(100, (containerHeight * 20) / 100); + this.listHeight = + containerHeight - this.analyticsChartsHeight - TRACING_LIST_VERTICAL_PADDING; }, }, }; @@ -158,7 +153,7 @@ export default { <template> <div class="gl-px-8"> - <div v-if="loading && traces.length === 0" class="gl-py-5"> + <div v-if="loadingTraces && traces.length === 0" class="gl-py-5"> <gl-loading-icon size="lg" /> </div> @@ -172,7 +167,12 @@ export default { @sort="onSort" /> - <tracing-analytics v-if="analytics && analytics.length" :analytics="analytics" /> + <tracing-analytics + v-if="traces.length" + :analytics="analytics" + :loading="loadingAnalytics" + :chart-height="analyticsChartsHeight" + /> <gl-infinite-scroll ref="infiniteScroll" @@ -181,16 +181,11 @@ export default { @bottomReached="bottomReached" > <template #items> - <tracing-table-list - ref="tableList" - :traces="traces" - :highlighted-trace-id="highlightedTraceId" - @trace-clicked="onTraceClicked" - /> + <tracing-table-list ref="tableList" :traces="traces" @trace-clicked="onTraceClicked" /> </template> <template #default> - <gl-loading-icon v-if="loading" size="md" /> + <gl-loading-icon v-if="loadingTraces" size="md" /> <span v-else data-testid="tracing-infinite-scrolling-legend">{{ infiniteScrollLegend }}</span> diff --git a/ee/spec/frontend/tracing/list/tracing_analytics_spec.js b/ee/spec/frontend/tracing/list/tracing_analytics_spec.js index 7c22c61415fd0a8fbed98a4c25b4f1fcdb7f6e19..fa4b76b65d68eb0334717cf597f0a3f61388ae4d 100644 --- a/ee/spec/frontend/tracing/list/tracing_analytics_spec.js +++ b/ee/spec/frontend/tracing/list/tracing_analytics_spec.js @@ -1,8 +1,7 @@ import { GlLineChart, GlColumnChart } from '@gitlab/ui/dist/charts'; -import { nextTick } from 'vue'; +import { GlSkeletonLoader } from '@gitlab/ui'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import TracingAnalytics from 'ee/tracing/list/tracing_analytics.vue'; -import * as commonUtils from '~/lib/utils/common_utils'; describe('TracingAnalytics', () => { let wrapper; @@ -40,10 +39,18 @@ describe('TracingAnalytics', () => { }, ]; - const mountComponent = (analytics = mockAnalytics) => { + const TEST_CHART_HEIGHT = 123; + + const mountComponent = ({ + analytics = mockAnalytics, + loading = false, + chartHeight = TEST_CHART_HEIGHT, + } = {}) => { wrapper = shallowMountExtended(TracingAnalytics, { propsData: { analytics, + loading, + chartHeight, }, }); }; @@ -53,6 +60,28 @@ describe('TracingAnalytics', () => { }); const findChart = () => wrapper.findComponent(TracingAnalytics); + const findSkeleton = () => wrapper.findComponent(GlSkeletonLoader); + + describe('skeleton', () => { + it('does not render the skeleton if not loading', () => { + mountComponent({ loading: false }); + + expect(findSkeleton().exists()).toBe(false); + }); + it('renders the skeleton if loading', () => { + mountComponent({ loading: true }); + + expect(findSkeleton().exists()).toBe(true); + }); + }); + + it('renders nothing if analytics is empty', () => { + mountComponent({ analytics: [] }); + + expect(findSkeleton().exists()).toBe(false); + expect(findChart().findComponent(GlColumnChart).exists()).toBe(false); + expect(findChart().findComponent(GlLineChart).exists()).toBe(false); + }); describe('volume chart', () => { it('renders a column chart with volume data', () => { @@ -69,9 +98,9 @@ describe('TracingAnalytics', () => { it('renders a line chart with error data', () => { const chart = findChart().findComponent(GlLineChart); expect(chart.props('data')[0].data).toEqual([ - [new Date('2024-01-28T15:43:00.000Z'), '100.00'], + [new Date('2024-01-28T15:43:00.000Z'), '1.20'], [new Date('2024-01-28T15:44:00.000Z'), '0.00'], - [new Date('2024-01-28T15:45:00.000Z'), '23.42'], + [new Date('2024-01-28T15:45:00.000Z'), '0.23'], ]); }); }); @@ -89,40 +118,15 @@ describe('TracingAnalytics', () => { describe('height', () => { it('sets the chart height to 20% of the container height', () => { - jest.spyOn(commonUtils, 'contentTop').mockReturnValue(200); - window.innerHeight = 1000; - - mountComponent(); - - const chart = findChart().findComponent(GlColumnChart); - expect(chart.props('height')).toBe(160); - }); - - it('sets the min height to 100px', () => { - jest.spyOn(commonUtils, 'contentTop').mockReturnValue(20); - window.innerHeight = 200; - mountComponent(); - const chart = findChart().findComponent(GlColumnChart); - expect(chart.props('height')).toBe(100); - }); - - it('resize the chart on window resize', async () => { - jest.spyOn(commonUtils, 'contentTop').mockReturnValue(200); - window.innerHeight = 1000; - - mountComponent(); - - expect(wrapper.findComponent(GlColumnChart).props('height')).toBe(160); - - jest.spyOn(commonUtils, 'contentTop').mockReturnValue(200); - window.innerHeight = 800; - window.dispatchEvent(new Event('resize')); - - await nextTick(); - - expect(wrapper.findComponent(GlColumnChart).props('height')).toBe(120); + expect(findChart().findComponent(GlColumnChart).props('height')).toBe(TEST_CHART_HEIGHT); + expect(findChart().findAllComponents(GlLineChart).at(0).props('height')).toBe( + TEST_CHART_HEIGHT, + ); + expect(findChart().findAllComponents(GlLineChart).at(1).props('height')).toBe( + TEST_CHART_HEIGHT, + ); }); }); }); diff --git a/ee/spec/frontend/tracing/list/tracing_list_spec.js b/ee/spec/frontend/tracing/list/tracing_list_spec.js index bcaf5c7a9bdcc9be004ec8831671f88aa0d02a42..d0969874936c1a66c94d38e68476a928ee1ef08f 100644 --- a/ee/spec/frontend/tracing/list/tracing_list_spec.js +++ b/ee/spec/frontend/tracing/list/tracing_list_spec.js @@ -12,6 +12,7 @@ import * as urlUtility from '~/lib/utils/url_utility'; import UrlSync from '~/vue_shared/components/url_sync.vue'; import setWindowLocation from 'helpers/set_window_location_helper'; import { createMockClient } from 'helpers/mock_observability_client'; +import * as commonUtils from '~/lib/utils/common_utils'; jest.mock('~/alert'); @@ -69,105 +70,119 @@ describe('TracingList', () => { await waitForPromises(); }; - beforeEach(() => { + beforeEach(async () => { observabilityClientMock = createMockClient(); observabilityClientMock.fetchTraces.mockResolvedValue(mockResponse); observabilityClientMock.fetchTracesAnalytics.mockResolvedValue(mockAnalytics); + + await mountComponent(); }); - describe('fetching data', () => { - beforeEach(async () => { + it('fetches traces', () => { + expect(observabilityClientMock.fetchTraces).toHaveBeenCalled(); + }); + + it('renders the loading icon while fetching traces', async () => { + observabilityClientMock.fetchTraces.mockReturnValue(new Promise(() => {})); + await mountComponent(); + expect(findLoadingIcon().exists()).toBe(true); + }); + + it('renders the trace list with filtered search', () => { + expect(findLoadingIcon().exists()).toBe(false); + expect(findTableList().exists()).toBe(true); + expect(findFilteredSearch().exists()).toBe(true); + expect(findUrlSync().exists()).toBe(true); + expect(findTableList().props('traces')).toEqual(mockResponse.traces); + }); + + describe('analytics', () => { + it('fetches analytics', () => { + expect(observabilityClientMock.fetchTracesAnalytics).toHaveBeenCalled(); + }); + + it('renders the analytics component', () => { + expect(findAnalytics().exists()).toBe(true); + expect(findAnalytics().props('analytics')).toEqual(mockAnalytics); + expect(findAnalytics().props('loading')).toBe(false); + }); + + it('does not render the analytics component if there is no traces', async () => { + observabilityClientMock.fetchTraces.mockResolvedValue([]); await mountComponent(); + expect(findAnalytics().exists()).toBe(false); }); - it('fetches data and renders the trace list with filtered search', () => { - expect(observabilityClientMock.fetchTraces).toHaveBeenCalled(); - expect(observabilityClientMock.fetchTracesAnalytics).toHaveBeenCalled(); - expect(findLoadingIcon().exists()).toBe(false); + it('sets loading prop while fetching analytics', async () => { + observabilityClientMock.fetchTracesAnalytics.mockReturnValue(new Promise(() => {})); + await mountComponent(); + expect(findAnalytics().exists()).toBe(true); + expect(findAnalytics().props('loading')).toBe(true); expect(findTableList().exists()).toBe(true); - expect(findFilteredSearch().exists()).toBe(true); - expect(findUrlSync().exists()).toBe(true); - expect(findTableList().props('traces')).toEqual(mockResponse.traces); + expect(findLoadingIcon().exists()).toBe(false); }); - describe('analytics', () => { - it('renders the analytics component', () => { - expect(findAnalytics().exists()).toBe(true); - expect(findAnalytics().props('analytics')).toEqual(mockAnalytics); - }); + describe('chart height', () => { + it('sets the chart height to 20% of the container height', async () => { + jest.spyOn(commonUtils, 'contentTop').mockReturnValue(200); + window.innerHeight = 1000; - it('does not render the analytics component if there is no data', async () => { - observabilityClientMock.fetchTracesAnalytics.mockResolvedValue([]); await mountComponent(); - expect(findAnalytics().exists()).toBe(false); - }); - it('does not render the analytics component if the call fails', async () => { - observabilityClientMock.fetchTracesAnalytics.mockRejectedValue('error'); - await mountComponent(); - expect(findAnalytics().exists()).toBe(false); + expect(findAnalytics().props('chartHeight')).toBe(160); }); - }); - describe('traces and analytics API requests', () => { - let resolveFetchTraces; - let resolveFetchTracesAnalytics; - beforeEach(async () => { - const fetchTracesPromise = new Promise((resolve) => { - resolveFetchTraces = resolve; - }); - const fetchTracesAnalyticsPromise = new Promise((resolve) => { - resolveFetchTracesAnalytics = resolve; - }); - observabilityClientMock.fetchTraces.mockReturnValue(fetchTracesPromise); - observabilityClientMock.fetchTracesAnalytics.mockReturnValue(fetchTracesAnalyticsPromise); + it('sets the min height to 100px', async () => { + jest.spyOn(commonUtils, 'contentTop').mockReturnValue(20); + window.innerHeight = 200; await mountComponent(); - }); - it('fetches traces and analytics in parallel', () => { - expect(observabilityClientMock.fetchTracesAnalytics).toHaveBeenCalled(); - expect(observabilityClientMock.fetchTraces).toHaveBeenCalled(); + expect(findAnalytics().props('chartHeight')).toBe(100); }); - it('waits for both requests to complete', async () => { - expect(findLoadingIcon().exists()).toBe(true); + it('resize the chart on window resize', async () => { + jest.spyOn(commonUtils, 'contentTop').mockReturnValue(200); + window.innerHeight = 1000; - resolveFetchTraces(); - await waitForPromises(); + await mountComponent(); - expect(findLoadingIcon().exists()).toBe(true); + expect(findAnalytics().props('chartHeight')).toBe(160); - resolveFetchTracesAnalytics(); - await waitForPromises(); + jest.spyOn(commonUtils, 'contentTop').mockReturnValue(200); + window.innerHeight = 800; + window.dispatchEvent(new Event('resize')); - expect(findLoadingIcon().exists()).toBe(false); + await nextTick(); + + expect(findAnalytics().props('chartHeight')).toBe(120); }); }); - describe('on trace-clicked', () => { - let visitUrlMock; - beforeEach(() => { - setWindowLocation('base_path'); - visitUrlMock = jest.spyOn(urlUtility, 'visitUrl').mockReturnValue({}); - }); + }); - it('redirects to the details url', () => { - findTableList().vm.$emit('trace-clicked', { traceId: 'test-trace-id' }); + describe('on trace-clicked', () => { + let visitUrlMock; + beforeEach(() => { + setWindowLocation('base_path'); + visitUrlMock = jest.spyOn(urlUtility, 'visitUrl').mockReturnValue({}); + }); - expect(visitUrlMock).toHaveBeenCalledTimes(1); - expect(visitUrlMock).toHaveBeenCalledWith('/base_path/test-trace-id', false); - }); + it('redirects to the details url', () => { + findTableList().vm.$emit('trace-clicked', { traceId: 'test-trace-id' }); - it('opens a new tab if clicked with meta key', () => { - findTableList().vm.$emit('trace-clicked', { - traceId: 'test-trace-id', - clickEvent: { metaKey: true }, - }); + expect(visitUrlMock).toHaveBeenCalledTimes(1); + expect(visitUrlMock).toHaveBeenCalledWith('/base_path/test-trace-id', false); + }); - expect(visitUrlMock).toHaveBeenCalledTimes(1); - expect(visitUrlMock).toHaveBeenCalledWith('/base_path/test-trace-id', true); + it('opens a new tab if clicked with meta key', () => { + findTableList().vm.$emit('trace-clicked', { + traceId: 'test-trace-id', + clickEvent: { metaKey: true }, }); + + expect(visitUrlMock).toHaveBeenCalledTimes(1); + expect(visitUrlMock).toHaveBeenCalledWith('/base_path/test-trace-id', true); }); }); @@ -283,6 +298,7 @@ describe('TracingList', () => { describe('on search submit', () => { beforeEach(async () => { observabilityClientMock.fetchTracesAnalytics.mockReset(); + observabilityClientMock.fetchTracesAnalytics.mockReturnValue(mockAnalytics); await setFilters({ period: [{ operator: '=', value: '12h' }], service: [{ operator: '=', value: 'frontend' }], @@ -516,6 +532,7 @@ describe('TracingList', () => { await bottomReached(); expect(observabilityClientMock.fetchTracesAnalytics).not.toHaveBeenCalled(); + expect(findAnalytics().exists()).toBe(true); }); it('does not update the next_page_token if missing - i.e. it reached the last page', async () => { @@ -633,16 +650,17 @@ describe('TracingList', () => { expect(createAlert).toHaveBeenLastCalledWith({ message: 'Failed to load traces.' }); expect(findTableList().exists()).toBe(true); expect(findTableList().props('traces')).toEqual([]); + expect(findAnalytics().exists()).toBe(false); }); - it('if fetchTracesAnalytics fails, it renders an alert and empty list', async () => { + it('if fetchTracesAnalytics fails, it renders an alert', async () => { observabilityClientMock.fetchTracesAnalytics.mockRejectedValue('error'); await mountComponent(); - expect(createAlert).toHaveBeenLastCalledWith({ message: 'Failed to load traces.' }); - expect(findTableList().exists()).toBe(true); - expect(findTableList().props('traces')).toEqual([]); + expect(createAlert).toHaveBeenLastCalledWith({ + message: 'Failed to load tracing analytics.', + }); }); }); }); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9e21ab8c05cc44a80105b0c54769f3921f1c47ea..09307787bdac1590ca201c0587ce2c0144fbff02 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -52266,6 +52266,9 @@ msgstr "" msgid "Tracing|Failed to load traces." msgstr "" +msgid "Tracing|Failed to load tracing analytics." +msgstr "" + msgid "Tracing|Filter traces" msgstr ""