diff options
17 files changed, 173 insertions, 29 deletions
diff --git a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue index cd298e2c692..a76eb747c34 100644 --- a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue +++ b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue @@ -1,6 +1,13 @@ <script> -import { mapActions, mapState } from 'vuex'; -import { GlEmptyState, GlButton, GlLink, GlLoadingIcon, GlTable } from '@gitlab/ui'; +import { mapActions, mapState, mapGetters } from 'vuex'; +import { + GlEmptyState, + GlButton, + GlLink, + GlLoadingIcon, + GlTable, + GlSearchBoxByType, +} from '@gitlab/ui'; import Icon from '~/vue_shared/components/icon.vue'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; import { __ } from '~/locale'; @@ -20,6 +27,7 @@ export default { GlLink, GlLoadingIcon, GlTable, + GlSearchBoxByType, Icon, TimeAgo, }, @@ -48,8 +56,17 @@ export default { required: true, }, }, + data() { + return { + errorSearchQuery: '', + }; + }, computed: { ...mapState(['errors', 'externalUrl', 'loading']), + ...mapGetters(['filterErrorsByTitle']), + filteredErrors() { + return this.errorSearchQuery ? this.filterErrorsByTitle(this.errorSearchQuery) : this.errors; + }, }, created() { if (this.errorTrackingEnabled) { @@ -71,10 +88,17 @@ export default { <gl-loading-icon :size="3" /> </div> <div v-else> - <div class="d-flex justify-content-end"> + <div class="d-flex flex-row justify-content-around bg-secondary border"> + <gl-search-box-by-type + v-model="errorSearchQuery" + class="col-lg-10 m-3 p-0" + :placeholder="__('Search or filter results...')" + type="search" + autofocus + /> <gl-button v-track-event="trackViewInSentryOptions(externalUrl)" - class="my-3 ml-auto" + class="m-3" variant="primary" :href="externalUrl" target="_blank" @@ -84,7 +108,14 @@ export default { </gl-button> </div> - <gl-table :items="errors" :fields="$options.fields" :show-empty="true" fixed stacked="sm"> + <gl-table + class="mt-3" + :items="filteredErrors" + :fields="$options.fields" + :show-empty="true" + fixed + stacked="sm" + > <template slot="HEAD_events" slot-scope="data"> <div class="text-md-right">{{ data.label }}</div> </template> diff --git a/app/assets/javascripts/error_tracking/store/getters.js b/app/assets/javascripts/error_tracking/store/getters.js new file mode 100644 index 00000000000..1a2ec62f79f --- /dev/null +++ b/app/assets/javascripts/error_tracking/store/getters.js @@ -0,0 +1,4 @@ +export const filterErrorsByTitle = state => errorQuery => + state.errors.filter(error => error.title.match(new RegExp(`${errorQuery}`, 'i'))); + +export default () => {}; diff --git a/app/assets/javascripts/error_tracking/store/index.js b/app/assets/javascripts/error_tracking/store/index.js index 3136682fb64..3ba05e22727 100644 --- a/app/assets/javascripts/error_tracking/store/index.js +++ b/app/assets/javascripts/error_tracking/store/index.js @@ -1,6 +1,7 @@ import Vue from 'vue'; import Vuex from 'vuex'; import * as actions from './actions'; +import * as getters from './getters'; import mutations from './mutations'; Vue.use(Vuex); @@ -14,6 +15,7 @@ export const createStore = () => }, actions, mutations, + getters, }); export default createStore(); diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 106ef1b72c1..6770ff37d91 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -156,14 +156,21 @@ class Projects::PipelinesController < Projects::ApplicationController def test_report return unless Feature.enabled?(:junit_pipeline_view, project) - if pipeline_test_report == :error - render json: { status: :error_parsing_report } - return - end + respond_to do |format| + format.html do + render 'show' + end - render json: TestReportSerializer - .new(current_user: @current_user) - .represent(pipeline_test_report) + format.json do + if pipeline_test_report == :error + render json: { status: :error_parsing_report } + else + render json: TestReportSerializer + .new(current_user: @current_user) + .represent(pipeline_test_report) + end + end + end end private diff --git a/app/views/projects/blame/show.html.haml b/app/views/projects/blame/show.html.haml index f2215765974..30fe5622ebd 100644 --- a/app/views/projects/blame/show.html.haml +++ b/app/views/projects/blame/show.html.haml @@ -19,7 +19,7 @@ = author_avatar(commit, size: 36, has_tooltip: false) .commit-row-title %span.item-title.str-truncated-100 - = link_to_markdown commit.title, project_commit_path(@project, commit.id), class: "cdark", title: commit.title + = link_to commit.title, project_commit_path(@project, commit.id), class: "cdark", title: commit.title .float-right = link_to commit.short_id, project_commit_path(@project, commit), class: "commit-sha" diff --git a/changelogs/unreleased/drop-id-of-ci-trace-sectinos.yml b/changelogs/unreleased/drop-id-of-ci-trace-sectinos.yml new file mode 100644 index 00000000000..3071269df2c --- /dev/null +++ b/changelogs/unreleased/drop-id-of-ci-trace-sectinos.yml @@ -0,0 +1,5 @@ +--- +title: Drop `id` column from `ci_build_trace_sections` table +merge_request: 18741 +author: +type: changed diff --git a/changelogs/unreleased/jc-dont-render-commit-links.yml b/changelogs/unreleased/jc-dont-render-commit-links.yml new file mode 100644 index 00000000000..6146e354702 --- /dev/null +++ b/changelogs/unreleased/jc-dont-render-commit-links.yml @@ -0,0 +1,5 @@ +--- +title: Do not render links in commit message on blame page +merge_request: 19128 +author: +type: performance diff --git a/changelogs/unreleased/lm-search-list-of-sentry-errors.yml b/changelogs/unreleased/lm-search-list-of-sentry-errors.yml new file mode 100644 index 00000000000..619c2b0c276 --- /dev/null +++ b/changelogs/unreleased/lm-search-list-of-sentry-errors.yml @@ -0,0 +1,5 @@ +--- +title: Search list of Sentry errors by title in Gitlab +merge_request: 18772 +author: +type: added diff --git a/changelogs/unreleased/sh-gitaly-duration-measurement.yml b/changelogs/unreleased/sh-gitaly-duration-measurement.yml new file mode 100644 index 00000000000..60d00e7d9ab --- /dev/null +++ b/changelogs/unreleased/sh-gitaly-duration-measurement.yml @@ -0,0 +1,5 @@ +--- +title: Fix Gitaly call duration measurements +merge_request: 18785 +author: +type: fixed diff --git a/db/post_migrate/20191017180026_drop_ci_build_trace_sections_id.rb b/db/post_migrate/20191017180026_drop_ci_build_trace_sections_id.rb new file mode 100644 index 00000000000..0405e23b465 --- /dev/null +++ b/db/post_migrate/20191017180026_drop_ci_build_trace_sections_id.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class DropCiBuildTraceSectionsId < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def up + ## + # This column has already been ignored since 12.4 + # See https://gitlab.com/gitlab-org/gitlab/issues/32569 + remove_column :ci_build_trace_sections, :id + end + + def down + ## + # We don't backfill serial ids as it's not used in application code + # and quite expensive process. + add_column :ci_build_trace_sections, :id, :bigint + end +end diff --git a/db/schema.rb b/db/schema.rb index 31bb3d75ea2..dce9d1dc189 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_10_17_094449) do +ActiveRecord::Schema.define(version: 2019_10_17_180026) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -599,7 +599,7 @@ ActiveRecord::Schema.define(version: 2019_10_17_094449) do t.index ["project_id", "name"], name: "index_ci_build_trace_section_names_on_project_id_and_name", unique: true end - create_table "ci_build_trace_sections", id: :serial, force: :cascade do |t| + create_table "ci_build_trace_sections", id: false, force: :cascade do |t| t.integer "project_id", null: false t.datetime "date_start", null: false t.datetime "date_end", null: false diff --git a/doc/user/project/operations/error_tracking.md b/doc/user/project/operations/error_tracking.md index f93c98f0218..50a67bddf01 100644 --- a/doc/user/project/operations/error_tracking.md +++ b/doc/user/project/operations/error_tracking.md @@ -41,5 +41,6 @@ NOTE: **Note:** You will need at least Reporter [permissions](../../permissions.md) to view the Error Tracking list. The Error Tracking list may be found at **Operations > Error Tracking** in your project's sidebar. +Errors can be filtered by title. ![Error Tracking list](img/error_tracking_list.png) diff --git a/doc/user/project/operations/img/error_tracking_list.png b/doc/user/project/operations/img/error_tracking_list.png Binary files differindex 194c7b440a4..79b464e021e 100644 --- a/doc/user/project/operations/img/error_tracking_list.png +++ b/doc/user/project/operations/img/error_tracking_list.png diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index b0f29d22ad4..9e3af00e00d 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -142,18 +142,39 @@ module Gitlab # kwargs.merge(deadline: Time.now + 10) # end # - def self.call(storage, service, rpc, request, remote_storage: nil, timeout: default_timeout) - start = Gitlab::Metrics::System.monotonic_time - request_hash = request.is_a?(Google::Protobuf::MessageExts) ? request.to_h : {} + def self.call(storage, service, rpc, request, remote_storage: nil, timeout: default_timeout, &block) + self.measure_timings(service, rpc, request) do + self.execute(storage, service, rpc, request, remote_storage: remote_storage, timeout: timeout, &block) + end + end + # This method is like GitalyClient.call but should be used with + # Gitaly streaming RPCs. It measures how long the the RPC took to + # produce the full response, not just the initial response. + def self.streaming_call(storage, service, rpc, request, remote_storage: nil, timeout: default_timeout) + self.measure_timings(service, rpc, request) do + response = self.execute(storage, service, rpc, request, remote_storage: remote_storage, timeout: timeout) + + yield(response) + end + end + + def self.execute(storage, service, rpc, request, remote_storage:, timeout:) enforce_gitaly_request_limits(:call) kwargs = request_kwargs(storage, timeout: timeout.to_f, remote_storage: remote_storage) kwargs = yield(kwargs) if block_given? stub(service, storage).__send__(rpc, request, kwargs) # rubocop:disable GitlabSecurity/PublicSend + end + + def self.measure_timings(service, rpc, request) + start = Gitlab::Metrics::System.monotonic_time + + yield ensure duration = Gitlab::Metrics::System.monotonic_time - start + request_hash = request.is_a?(Google::Protobuf::MessageExts) ? request.to_h : {} # Keep track, separately, for the performance bar self.query_time += duration diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index dca55091be6..15318bc817a 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -200,8 +200,9 @@ module Gitlab to: to ) - response = GitalyClient.call(@repository.storage, :commit_service, :commits_between, request, timeout: GitalyClient.medium_timeout) - consume_commits_response(response) + GitalyClient.streaming_call(@repository.storage, :commit_service, :commits_between, request, timeout: GitalyClient.medium_timeout) do |response| + consume_commits_response(response) + end end def diff_stats(left_commit_sha, right_commit_sha) @@ -224,8 +225,9 @@ module Gitlab ) request.order = opts[:order].upcase if opts[:order].present? - response = GitalyClient.call(@repository.storage, :commit_service, :find_all_commits, request, timeout: GitalyClient.medium_timeout) - consume_commits_response(response) + GitalyClient.streaming_call(@repository.storage, :commit_service, :find_all_commits, request, timeout: GitalyClient.medium_timeout) do |response| + consume_commits_response(response) + end end def list_commits_by_oid(oids) @@ -233,8 +235,9 @@ module Gitlab request = Gitaly::ListCommitsByOidRequest.new(repository: @gitaly_repo, oid: oids) - response = GitalyClient.call(@repository.storage, :commit_service, :list_commits_by_oid, request, timeout: GitalyClient.medium_timeout) - consume_commits_response(response) + GitalyClient.streaming_call(@repository.storage, :commit_service, :list_commits_by_oid, request, timeout: GitalyClient.medium_timeout) do |response| + consume_commits_response(response) + end rescue GRPC::NotFound # If no repository is found, happens mainly during testing [] end @@ -249,8 +252,9 @@ module Gitlab offset: offset.to_i ) - response = GitalyClient.call(@repository.storage, :commit_service, :commits_by_message, request, timeout: GitalyClient.medium_timeout) - consume_commits_response(response) + GitalyClient.streaming_call(@repository.storage, :commit_service, :commits_by_message, request, timeout: GitalyClient.medium_timeout) do |response| + consume_commits_response(response) + end end def languages(ref = nil) @@ -323,9 +327,9 @@ module Gitlab request.paths = encode_repeated(Array(options[:path])) if options[:path].present? - response = GitalyClient.call(@repository.storage, :commit_service, :find_commits, request, timeout: GitalyClient.medium_timeout) - - consume_commits_response(response) + GitalyClient.streaming_call(@repository.storage, :commit_service, :find_commits, request, timeout: GitalyClient.medium_timeout) do |response| + consume_commits_response(response) + end end def filter_shas_with_signatures(shas) diff --git a/spec/frontend/error_tracking/store/getters_spec.js b/spec/frontend/error_tracking/store/getters_spec.js new file mode 100644 index 00000000000..371dfae373b --- /dev/null +++ b/spec/frontend/error_tracking/store/getters_spec.js @@ -0,0 +1,33 @@ +import * as getters from '~/error_tracking/store/getters'; + +describe('Error Tracking getters', () => { + let state; + + const mockErrors = [ + { title: 'ActiveModel::MissingAttributeError: missing attribute: encrypted_password' }, + { title: 'Grape::Exceptions::MethodNotAllowed: Grape::Exceptions::MethodNotAllowed' }, + { title: 'NoMethodError: undefined method `sanitize_http_headers=' }, + { title: 'NoMethodError: undefined method `pry' }, + ]; + + beforeEach(() => { + state = { + errors: mockErrors, + }; + }); + + describe('search results', () => { + it('should return errors filtered by words in title matching the query', () => { + const filteredErrors = getters.filterErrorsByTitle(state)('NoMethod'); + + expect(filteredErrors).not.toContainEqual(mockErrors[0]); + expect(filteredErrors.length).toBe(2); + }); + + it('should not return results if there is no matching query', () => { + const filteredErrors = getters.filterErrorsByTitle(state)('GitLab'); + + expect(filteredErrors.length).toBe(0); + }); + }); +}); diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index ea3bb12d049..67fe89f3fbe 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -377,6 +377,8 @@ describe Gitlab::GitalyClient do context 'when the request store is active', :request_store do it 'records call details if a RPC is called' do + expect(described_class).to receive(:measure_timings).and_call_original + gitaly_server.server_version expect(described_class.list_call_details).not_to be_empty |