Commit a55b7853 authored by David O'Regan's avatar David O'Regan

Merge branch 'qmnguyen0711/improve-performance-bar' into 'master'

Improve the performance bar

See merge request gitlab-org/gitlab!56830
parents bd1042a3 89d438d4
<script>
import { GlButton, GlModal, GlModalDirective } from '@gitlab/ui';
import { GlButton, GlModal, GlModalDirective, GlSegmentedControl } from '@gitlab/ui';
import { s__ } from '~/locale';
import { sortOrders, sortOrderOptions } from '../constants';
import RequestWarning from './request_warning.vue';
export default {
......@@ -7,6 +10,7 @@ export default {
RequestWarning,
GlButton,
GlModal,
GlSegmentedControl,
},
directives: {
'gl-modal': GlModalDirective,
......@@ -39,6 +43,7 @@ export default {
data() {
return {
openedBacktraces: [],
sortOrder: sortOrders.DURATION,
};
},
computed: {
......@@ -48,13 +53,37 @@ export default {
metricDetails() {
return this.currentRequest.details[this.metric];
},
metricDetailsSummary() {
return {
[s__('Total')]: this.metricDetails.calls,
[s__('PerformanceBar|Total duration')]: this.metricDetails.duration,
...(this.metricDetails.summary || {}),
};
},
metricDetailsLabel() {
return this.metricDetails.duration
? `${this.metricDetails.duration} / ${this.metricDetails.calls}`
: this.metricDetails.calls;
if (this.metricDetails.duration && this.metricDetails.calls) {
return `${this.metricDetails.duration} / ${this.metricDetails.calls}`;
} else if (this.metricDetails.calls) {
return this.metricDetails.calls;
}
return '0';
},
displaySortOrder() {
return (
this.metricDetails.details.length !== 0 &&
this.metricDetails.details.every((item) => item.start)
);
},
detailsList() {
return this.metricDetails.details;
return this.metricDetails.details.map((item, index) => ({ ...item, id: index }));
},
sortedList() {
if (this.sortOrder === sortOrders.CHRONOLOGICAL) {
return this.detailsList.slice().sort(this.sortDetailChronologically);
}
return this.detailsList.slice().sort(this.sortDetailByDuration);
},
warnings() {
return this.metricDetails.warnings || [];
......@@ -82,7 +111,17 @@ export default {
itemHasOpenedBacktrace(toggledIndex) {
return this.openedBacktraces.find((openedIndex) => openedIndex === toggledIndex) >= 0;
},
changeSortOrder(order) {
this.sortOrder = order;
},
sortDetailByDuration(a, b) {
return a.duration < b.duration ? 1 : -1;
},
sortDetailChronologically(a, b) {
return a.start < b.start ? -1 : 1;
},
},
sortOrderOptions,
};
</script>
<template>
......@@ -93,18 +132,41 @@ export default {
data-qa-selector="detailed_metric_content"
>
<gl-button v-gl-modal="modalId" class="gl-mr-2" type="button" variant="link">
<span class="gl-text-blue-300 gl-font-weight-bold">{{ metricDetailsLabel }}</span>
<span
class="gl-text-blue-300 gl-font-weight-bold"
data-testid="performance-bar-details-label"
>
{{ metricDetailsLabel }}
</span>
</gl-button>
<gl-modal :modal-id="modalId" :title="header" size="lg" footer-class="d-none" scrollable>
<div class="gl-display-flex gl-align-items-center gl-justify-content-space-between">
<div class="gl-display-flex gl-align-items-center" data-testid="performance-bar-summary">
<div v-for="(value, name) in metricDetailsSummary" :key="name" class="gl-pr-8">
<div v-if="value" data-testid="performance-bar-summary-item">
<div>{{ name }}</div>
<div class="gl-font-size-h1 gl-font-weight-bold">{{ value }}</div>
</div>
</div>
</div>
<gl-segmented-control
v-if="displaySortOrder"
data-testid="performance-bar-sort-order"
:options="$options.sortOrderOptions"
:checked="sortOrder"
@input="changeSortOrder"
/>
</div>
<hr />
<table class="table gl-table">
<template v-if="detailsList.length">
<tr v-for="(item, index) in detailsList" :key="index">
<td>
<template v-if="sortedList.length">
<tr v-for="item in sortedList" :key="item.id">
<td data-testid="performance-item-duration">
<span v-if="item.duration">{{
sprintf(__('%{duration}ms'), { duration: item.duration })
}}</span>
</td>
<td>
<td data-testid="performance-item-content">
<div>
<div
v-for="(key, keyIndex) in keys"
......@@ -121,12 +183,12 @@ export default {
variant="default"
icon="ellipsis_h"
size="small"
:selected="itemHasOpenedBacktrace(index)"
:selected="itemHasOpenedBacktrace(item.id)"
:aria-label="__('Toggle backtrace')"
@click="toggleBacktrace(index)"
@click="toggleBacktrace(item.id)"
/>
</div>
<pre v-if="itemHasOpenedBacktrace(index)" class="backtrace-row mt-2">{{
<pre v-if="itemHasOpenedBacktrace(item.id)" class="backtrace-row gl-mt-3">{{
item.backtrace
}}</pre>
</div>
......@@ -135,7 +197,7 @@ export default {
</template>
<template v-else>
<tr>
<td>
<td data-testid="performance-bar-empty-detail-notice">
{{ sprintf(__('No %{header} for this request.'), { header: header.toLowerCase() }) }}
</td>
</tr>
......
......@@ -40,7 +40,7 @@ export default {
metric: 'active-record',
title: 'pg',
header: s__('PerformanceBar|SQL queries'),
keys: ['sql', 'cached', 'db_role'],
keys: ['sql', 'cached', 'transaction', 'db_role'],
},
{
metric: 'bullet',
......@@ -69,6 +69,7 @@ export default {
},
{
metric: 'external-http',
title: 'external',
header: s__('PerformanceBar|External Http calls'),
keys: ['label', 'code', 'proxy', 'error'],
},
......@@ -157,15 +158,17 @@ export default {
class="view"
>
<a class="gl-text-blue-300" :href="currentRequest.details.tracing.tracing_url">{{
s__('PerformanceBar|trace')
s__('PerformanceBar|Trace')
}}</a>
</div>
<add-request v-on="$listeners" />
<div v-if="currentRequest.details" id="peek-download" class="view">
<a class="gl-text-blue-300" :download="downloadName" :href="downloadPath">{{
s__('PerformanceBar|Download')
}}</a>
</div>
<a v-if="statsUrl" class="gl-text-blue-300 view" :href="statsUrl">{{
s__('PerformanceBar|Stats')
}}</a>
<request-selector
v-if="currentRequest"
:current-request="currentRequest"
......@@ -173,9 +176,7 @@ export default {
class="ml-auto"
@change-current-request="changeCurrentRequest"
/>
<div v-if="statsUrl" id="peek-stats" class="view">
<a class="gl-text-blue-300" :href="statsUrl">{{ s__('PerformanceBar|Stats') }}</a>
</div>
<add-request v-on="$listeners" />
</div>
</div>
</template>
import { s__ } from '~/locale';
export const sortOrders = {
DURATION: 'duration',
CHRONOLOGICAL: 'chronological',
};
export const sortOrderOptions = [
{
value: sortOrders.DURATION,
text: s__('PerformanceBar|Sort by duration'),
},
{
value: sortOrders.CHRONOLOGICAL,
text: s__('PerformanceBar|Sort chronologically'),
},
];
---
title: Allow details to be sorted chronologically in the performance bar
merge_request: 56830
author:
type: changed
......@@ -11,11 +11,21 @@ module EE
detail = super
if ::Gitlab::Database::LoadBalancing.enable?
detail[:db_role] = ::Gitlab::Database::LoadBalancing.db_role_for_connection(data[:connection])
detail[:db_role] = ::Gitlab::Database::LoadBalancing.db_role_for_connection(data[:connection]).to_s.capitalize
end
detail
end
override :count_summary
def count_summary(item, count)
super
if ::Gitlab::Database::LoadBalancing.enable?
count[item[:db_role]] ||= 0
count[item[:db_role]] += 1
end
end
end
end
end
......
......@@ -6,14 +6,15 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
subject { Peek.views.find { |v| v.instance_of?(Peek::Views::ActiveRecord) } }
let(:connection_replica) { double(:connection_replica) }
let(:connection_primary) { double(:connection_primary) }
let(:connection_primary_1) { double(:connection_primary) }
let(:connection_primary_2) { double(:connection_primary) }
let(:event_1) do
{
name: 'SQL',
sql: 'SELECT * FROM users WHERE id = 10',
cached: false,
connection: connection_primary
connection: connection_primary_1
}
end
......@@ -31,12 +32,15 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
name: 'SQL',
sql: 'UPDATE users SET admin = true WHERE id = 10',
cached: false,
connection: connection_primary
connection: connection_primary_2
}
end
before do
allow(Gitlab::PerformanceBar).to receive(:enabled_for_request?).and_return(true)
allow(connection_replica).to receive(:transaction_open?).and_return(false)
allow(connection_primary_1).to receive(:transaction_open?).and_return(false)
allow(connection_primary_2).to receive(:transaction_open?).and_return(true)
end
context 'when database load balancing is not enabled' do
......@@ -48,22 +52,32 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
end
expect(subject.results).to match(
calls: '3 (1 cached)',
calls: 3,
summary: {
"Cached" => 1,
"In a transaction" => 1
},
duration: '6000.00ms',
warnings: ["active-record duration: 6000.0 over 3000"],
details: contain_exactly(
a_hash_including(
start: be_a(Time),
cached: '',
transaction: '',
duration: 1000.0,
sql: 'SELECT * FROM users WHERE id = 10'
),
a_hash_including(
cached: 'cached',
start: be_a(Time),
cached: 'Cached',
transaction: '',
duration: 2000.0,
sql: 'SELECT * FROM users WHERE id = 10'
),
a_hash_including(
start: be_a(Time),
cached: '',
transaction: 'In a transaction',
duration: 3000.0,
sql: 'UPDATE users SET admin = true WHERE id = 10'
)
......@@ -76,7 +90,8 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
before do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true)
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_replica).and_return(:replica)
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary).and_return(:primary)
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_1).and_return(:primary)
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_2).and_return(:primary)
end
it 'includes db role data' do
......@@ -87,27 +102,39 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
end
expect(subject.results).to match(
calls: '3 (1 cached)',
calls: 3,
summary: {
"Cached" => 1,
"In a transaction" => 1,
"Primary" => 2,
"Replica" => 1
},
duration: '6000.00ms',
warnings: ["active-record duration: 6000.0 over 3000"],
details: contain_exactly(
a_hash_including(
start: be_a(Time),
cached: '',
transaction: '',
duration: 1000.0,
sql: 'SELECT * FROM users WHERE id = 10',
db_role: :primary
db_role: 'Primary'
),
a_hash_including(
cached: 'cached',
start: be_a(Time),
cached: 'Cached',
transaction: '',
duration: 2000.0,
sql: 'SELECT * FROM users WHERE id = 10',
db_role: :replica
db_role: 'Replica'
),
a_hash_including(
start: be_a(Time),
cached: '',
transaction: 'In a transaction',
duration: 3000.0,
sql: 'UPDATE users SET admin = true WHERE id = 10',
db_role: :primary
db_role: 'Primary'
)
)
)
......
......@@ -17,22 +17,32 @@ module Peek
}
}.freeze
def results
super.merge(calls: detailed_calls)
end
def self.thresholds
@thresholds ||= THRESHOLDS.fetch(Rails.env.to_sym, DEFAULT_THRESHOLDS)
end
def results
super.merge(summary: summary)
end
private
def detailed_calls
"#{calls} (#{cached_calls} cached)"
def summary
detail_store.each_with_object({}) do |item, count|
count_summary(item, count)
end
end
def cached_calls
detail_store.count { |item| item[:cached] == 'cached' }
def count_summary(item, count)
if item[:cached].present?
count[item[:cached]] ||= 0
count[item[:cached]] += 1
end
if item[:transaction].present?
count[item[:transaction]] ||= 0
count[item[:transaction]] += 1
end
end
def setup_subscribers
......@@ -45,10 +55,12 @@ module Peek
def generate_detail(start, finish, data)
{
start: start,
duration: finish - start,
sql: data[:sql].strip,
backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller),
cached: data[:cached] ? 'cached' : ''
cached: data[:cached] ? 'Cached' : '',
transaction: data[:connection].transaction_open? ? 'In a transaction' : ''
}
end
end
......
......@@ -22327,10 +22327,19 @@ msgstr ""
msgid "PerformanceBar|SQL queries"
msgstr ""
msgid "PerformanceBar|Sort by duration"
msgstr ""
msgid "PerformanceBar|Sort chronologically"
msgstr ""
msgid "PerformanceBar|Stats"
msgstr ""
msgid "PerformanceBar|trace"
msgid "PerformanceBar|Total duration"
msgstr ""
msgid "PerformanceBar|Trace"
msgstr ""
msgid "Permissions"
......
......@@ -5,14 +5,16 @@ require 'spec_helper'
RSpec.describe Peek::Views::ActiveRecord, :request_store do
subject { Peek.views.find { |v| v.instance_of?(Peek::Views::ActiveRecord) } }
let(:connection) { double(:connection) }
let(:connection_1) { double(:connection) }
let(:connection_2) { double(:connection) }
let(:connection_3) { double(:connection) }
let(:event_1) do
{
name: 'SQL',
sql: 'SELECT * FROM users WHERE id = 10',
cached: false,
connection: connection
connection: connection_1
}
end
......@@ -21,7 +23,7 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
name: 'SQL',
sql: 'SELECT * FROM users WHERE id = 10',
cached: true,
connection: connection
connection: connection_2
}
end
......@@ -30,12 +32,15 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
name: 'SQL',
sql: 'UPDATE users SET admin = true WHERE id = 10',
cached: false,
connection: connection
connection: connection_3
}
end
before do
allow(Gitlab::PerformanceBar).to receive(:enabled_for_request?).and_return(true)
allow(connection_1).to receive(:transaction_open?).and_return(false)
allow(connection_2).to receive(:transaction_open?).and_return(false)
allow(connection_3).to receive(:transaction_open?).and_return(true)
end
it 'subscribes and store data into peek views' do
......@@ -46,22 +51,32 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
end
expect(subject.results).to match(
calls: '3 (1 cached)',
calls: 3,
summary: {
"Cached" => 1,
"In a transaction" => 1
},
duration: '6000.00ms',
warnings: ["active-record duration: 6000.0 over 3000"],
details: contain_exactly(
a_hash_including(
start: be_a(Time),
cached: '',
transaction: '',
duration: 1000.0,
sql: 'SELECT * FROM users WHERE id = 10'
),
a_hash_including(
cached: 'cached',
start: be_a(Time),
cached: 'Cached',
transaction: '',
duration: 2000.0,
sql: 'SELECT * FROM users WHERE id = 10'
),
a_hash_including(
start: be_a(Time),
cached: '',
transaction: 'In a transaction',
duration: 3000.0,
sql: 'UPDATE users SET admin = true WHERE id = 10'
)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment