diff options
27 files changed, 282 insertions, 224 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index 4b3931a7cab..96ff65b3c49 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -402,7 +402,6 @@ RSpec/RepeatedExample: - 'spec/graphql/gitlab_schema_spec.rb' - 'spec/helpers/users_helper_spec.rb' - 'spec/lib/banzai/filter/autolink_filter_spec.rb' - - 'spec/lib/banzai/filter/issuable_state_filter_spec.rb' - 'spec/lib/gitlab/closing_issue_extractor_spec.rb' - 'spec/lib/gitlab/danger/changelog_spec.rb' - 'spec/lib/gitlab/git/blob_spec.rb' diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 8c8824ae47f..584320a66de 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -208,24 +208,12 @@ class Projects::BlobController < Projects::ApplicationController .last_for_path(@repository, @ref, @path).sha end - def set_code_navigation_build - return if Feature.disabled?(:code_navigation, @project) - - artifact = - Ci::JobArtifact - .for_sha(@blob.commit_id, @project.id) - .for_job_name(Ci::Build::CODE_NAVIGATION_JOB_NAME) - .last - - @code_navigation_build = artifact&.job - end - def show_html environment_params = @repository.branch_exists?(@ref) ? { ref: @ref } : { commit: @commit } environment_params[:find_latest] = true @environment = EnvironmentsFinder.new(@project, current_user, environment_params).execute.last @last_commit = @repository.last_commit_for_path(@commit.id, @blob.path) - set_code_navigation_build + @code_navigation_path = Gitlab::CodeNavigationPath.new(@project, @blob.commit_id).full_json_path_for(@blob.path) render 'show' end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 5b794f7ccf0..74a329dccf4 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -33,8 +33,6 @@ module Ci scheduler_failure: 2 }.freeze - CODE_NAVIGATION_JOB_NAME = 'code_navigation' - has_one :deployment, as: :deployable, class_name: 'Deployment' has_one :resource, class_name: 'Ci::Resource', inverse_of: :build has_many :trace_sections, class_name: 'Ci::BuildTraceSection' diff --git a/app/serializers/diff_file_entity.rb b/app/serializers/diff_file_entity.rb index 45c16aabe9e..26a42d2e9eb 100644 --- a/app/serializers/diff_file_entity.rb +++ b/app/serializers/diff_file_entity.rb @@ -64,6 +64,10 @@ class DiffFileEntity < DiffFileBaseEntity # Used for parallel diffs expose :parallel_diff_lines, using: DiffLineParallelEntity, if: -> (diff_file, options) { parallel_diff_view?(options, diff_file) && diff_file.text? } + expose :code_navigation_path, if: -> (diff_file) { options[:code_navigation_path] } do |diff_file| + options[:code_navigation_path].full_json_path_for(diff_file.new_path) + end + private def parallel_diff_view?(options, diff_file) diff --git a/app/serializers/diffs_entity.rb b/app/serializers/diffs_entity.rb index 02f78180fb0..b709fc7228b 100644 --- a/app/serializers/diffs_entity.rb +++ b/app/serializers/diffs_entity.rb @@ -70,13 +70,21 @@ class DiffsEntity < Grape::Entity expose :diff_files do |diffs, options| submodule_links = Gitlab::SubmoduleLinks.new(merge_request.project.repository) - DiffFileEntity.represent(diffs.diff_files, options.merge(submodule_links: submodule_links)) + code_navigation_path = + Gitlab::CodeNavigationPath.new(merge_request.project, diffs.diff_refs.head_sha) + + DiffFileEntity.represent(diffs.diff_files, + options.merge(submodule_links: submodule_links, code_navigation_path: code_navigation_path)) end expose :merge_request_diffs, using: MergeRequestDiffEntity, if: -> (_, options) { options[:merge_request_diffs]&.any? } do |diffs| options[:merge_request_diffs] end + expose :definition_path_prefix, if: -> (diff_file) { Feature.enabled?(:code_navigation, merge_request.project) } do |diffs| + project_blob_path(merge_request.project, diffs.diff_refs.head_sha) + end + def merge_request options[:merge_request] end diff --git a/app/serializers/paginated_diff_entity.rb b/app/serializers/paginated_diff_entity.rb index 622da926c69..a31c9d70d4b 100644 --- a/app/serializers/paginated_diff_entity.rb +++ b/app/serializers/paginated_diff_entity.rb @@ -10,7 +10,11 @@ class PaginatedDiffEntity < Grape::Entity expose :diff_files do |diffs, options| submodule_links = Gitlab::SubmoduleLinks.new(merge_request.project.repository) - DiffFileEntity.represent(diffs.diff_files, options.merge(submodule_links: submodule_links)) + code_navigation_path = + Gitlab::CodeNavigationPath.new(merge_request.project, diffs.diff_refs.head_sha) + + DiffFileEntity.represent(diffs.diff_files, + options.merge(submodule_links: submodule_links, code_navigation_path: code_navigation_path)) end expose :pagination do diff --git a/app/services/metrics/dashboard/base_service.rb b/app/services/metrics/dashboard/base_service.rb index 3cd7d8437b1..a19f3f78b3d 100644 --- a/app/services/metrics/dashboard/base_service.rb +++ b/app/services/metrics/dashboard/base_service.rb @@ -45,9 +45,30 @@ module Metrics # Returns a new dashboard Hash, supplemented with DB info def process_dashboard - ::Gitlab::Metrics::Dashboard::Processor - .new(project, raw_dashboard, sequence, process_params) - .process + # Get the dashboard from cache/disk before beginning the benchmark. + dashboard = raw_dashboard + processed_dashboard = nil + + benchmark_processing do + processed_dashboard = ::Gitlab::Metrics::Dashboard::Processor + .new(project, dashboard, sequence, process_params) + .process + end + + processed_dashboard + end + + def benchmark_processing + output = nil + + processing_time_seconds = Benchmark.realtime { output = yield } + + if output + processing_time_metric.observe( + processing_time_metric_labels, + processing_time_seconds * 1_000 + ) + end end def process_params @@ -72,6 +93,26 @@ module Metrics def sequence SEQUENCE end + + def processing_time_metric + @processing_time_metric ||= ::Gitlab::Metrics.summary( + :gitlab_metrics_dashboard_processing_time_ms, + 'Metrics dashboard processing time in milliseconds' + ) + end + + def processing_time_metric_labels + { + stages: sequence_string, + service: self.class.name + } + end + + # If @sequence is [STAGES::CommonMetricsInserter, STAGES::CustomMetricsInserter], + # this function will output `CommonMetricsInserter-CustomMetricsInserter`. + def sequence_string + sequence.map { |stage_class| stage_class.to_s.split('::').last }.join('-') + end end end end diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml index 02a327c5a49..8e8a6b847df 100644 --- a/app/views/projects/blob/_blob.html.haml +++ b/app/views/projects/blob/_blob.html.haml @@ -9,9 +9,8 @@ = render "projects/blob/auxiliary_viewer", blob: blob #blob-content-holder.blob-content-holder - - if @code_navigation_build - - code_nav_url = raw_project_job_artifacts_url(@project, @code_navigation_build, path: "lsif/#{blob.path}") - #js-code-navigation{ data: { code_nav_url: "#{code_nav_url}.json", definition_path_prefix: project_blob_path(@project, @ref) } } + - if @code_navigation_path + #js-code-navigation{ data: { code_nav_url: @code_navigation_path, definition_path_prefix: project_blob_path(@project, @ref) } } %article.file-holder = render 'projects/blob/header', blob: blob = render 'projects/blob/content', blob: blob diff --git a/changelogs/unreleased/208890-optimize-notes-and-suggestion-counters.yml b/changelogs/unreleased/208890-optimize-notes-and-suggestion-counters.yml new file mode 100644 index 00000000000..e1424e61e3e --- /dev/null +++ b/changelogs/unreleased/208890-optimize-notes-and-suggestion-counters.yml @@ -0,0 +1,5 @@ +--- +title: Optimize suggestions counters +merge_request: 26443 +author: +type: performance diff --git a/config/feature_categories.yml b/config/feature_categories.yml index b40bbdd9658..f12c0ef466a 100644 --- a/config/feature_categories.yml +++ b/config/feature_categories.yml @@ -71,7 +71,6 @@ - kubernetes_management - language_specific - license_compliance -- live_coding - load_testing - logging - malware_scanning diff --git a/db/migrate/20200330132913_add_index_on_author_id_and_created_at_and_id_to_notes.rb b/db/migrate/20200330132913_add_index_on_author_id_and_created_at_and_id_to_notes.rb new file mode 100644 index 00000000000..3b0a63ee565 --- /dev/null +++ b/db/migrate/20200330132913_add_index_on_author_id_and_created_at_and_id_to_notes.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddIndexOnAuthorIdAndCreatedAtAndIdToNotes < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :notes, [:author_id, :created_at, :id] + remove_concurrent_index :notes, [:author_id, :created_at] + end + + def down + add_concurrent_index :notes, [:author_id, :created_at] + remove_concurrent_index :notes, [:author_id, :created_at, :id] + end +end diff --git a/db/structure.sql b/db/structure.sql index 86a931b6cb4..bbd6312b08a 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9519,7 +9519,7 @@ CREATE INDEX index_namespaces_on_type_partial ON public.namespaces USING btree ( CREATE UNIQUE INDEX index_note_diff_files_on_diff_note_id ON public.note_diff_files USING btree (diff_note_id); -CREATE INDEX index_notes_on_author_id_and_created_at ON public.notes USING btree (author_id, created_at); +CREATE INDEX index_notes_on_author_id_and_created_at_and_id ON public.notes USING btree (author_id, created_at, id); CREATE INDEX index_notes_on_commit_id ON public.notes USING btree (commit_id); @@ -12925,5 +12925,6 @@ COPY "schema_migrations" (version) FROM STDIN; 20200326144443 20200326145443 20200330074719 +20200330132913 \. diff --git a/doc/administration/gitaly/img/praefect_storage_v12_10.png b/doc/administration/gitaly/img/praefect_storage_v12_10.png Binary files differnew file mode 100644 index 00000000000..f60a56fa1fb --- /dev/null +++ b/doc/administration/gitaly/img/praefect_storage_v12_10.png diff --git a/doc/administration/gitaly/praefect.md b/doc/administration/gitaly/praefect.md index a34c6c0b336..32062d0767e 100644 --- a/doc/administration/gitaly/praefect.md +++ b/doc/administration/gitaly/praefect.md @@ -256,9 +256,9 @@ application server, or a Gitaly node. ```ruby # Name of storage hash must match storage name in git_data_dirs on GitLab - # server ('praefect') and in git_data_dirs on Gitaly nodes ('gitaly-1') + # server ('storage_1') and in git_data_dirs on Gitaly nodes ('gitaly-1') praefect['virtual_storages'] = { - 'praefect' => { + 'storage_1' => { 'gitaly-1' => { 'address' => 'tcp://GITALY_HOST:8075', 'token' => 'PRAEFECT_INTERNAL_TOKEN', @@ -430,7 +430,7 @@ documentation](index.md#3-gitaly-server-configuration). gitlab-ctl restart gitaly ``` -**Complete these steps for each Gitaly node!** +**The steps above must be completed for each Gitaly node!** After all Gitaly nodes are configured, you can run the Praefect connection checker to verify Praefect can connect to all Gitaly servers in the Praefect @@ -442,6 +442,34 @@ config. sudo /opt/gitlab/embedded/bin/praefect -config /var/opt/gitlab/praefect/config.toml dial-nodes ``` +1. Enable automatic failover by editing `/etc/gitlab/gitlab.rb`: + + ```ruby + praefect['failover_enabled'] = true + ``` + + When automatic failover is enabled, Praefect checks the health of internal + Gitaly nodes. If the primary has a certain amount of health checks fail, it + will promote one of the secondaries to be primary, and demote the primary to + be a secondary. + + Manual failover is possible by updating `praefect['virtual_storages']` and + nominating a new primary node. + + NOTE: **Note:**: Automatic failover is not yet supported for setups with + multiple Praefect nodes. There is currently no coordination between Praefect + nodes, which could result in two Praefect instances thinking two different + Gitaly nodes are the primary. Follow issue + [#2547](https://gitlab.com/gitlab-org/gitaly/-/issues/2547) for + updates. + +1. Save the changes to `/etc/gitlab/gitlab.rb` and [reconfigure + Praefect](../restart_gitlab.md#omnibus-gitlab-reconfigure): + + ```shell + gitlab-ctl reconfigure + ``` + ### GitLab To complete this section you will need: @@ -560,133 +588,80 @@ Particular attention should be shown to: - Deselect the **default** storage location - Select the **praefect** storage location +  + 1. Verify everything is still working by creating a new project. Check the "Initialize repository with a README" box so that there is content in the repository that viewed. If the project is created, and you can see the README file, it works! -Congratulations! You have configured a highly available Praefect cluster. - -### Failover +### Grafana -There are two ways to do a failover from one internal Gitaly node to another as the primary. Manually, or automatically. - -As an example, in this `config.toml` we have one virtual storage named "default" with two internal Gitaly nodes behind it. -One is deemed the "primary". This means that read and write traffic will go to `internal_storage_0`, and writes -will get replicated to `internal_storage_1`: - -```toml -socket_path = "/path/to/Praefect.socket" - -# failover_enabled will enable automatic failover -failover_enabled = false - -[logging] -format = "json" -level = "info" - -[[virtual_storage]] -name = "default" - -[[virtual_storage.node]] - name = "internal_storage_0" - address = "tcp://localhost:9999" - primary = true - token = "supersecret" +Grafana is included with GitLab, and can be used to monitor your Praefect +cluster. See [Grafana Dashboard +Service](https://docs.gitlab.com/omnibus/settings/grafana.html) +for detailed documentation. -[[virtual_storage.node]] - name = "internal_storage_1" - address = "tcp://localhost:9998" - token = "supersecret" -``` +To get started quickly: -#### Manual failover +1. SSH into the **GitLab** node and login as root: -In order to failover from using one internal Gitaly node to using another, a manual failover step can be used. Unless `failover_enabled` is set to `true` -in the `config.toml`, the only way to fail over from one primary to using another node as the primary is to do a manual failover. + ```shell + sudo -i + ``` -1. Move `primary = true` from the current `[[virtual_storage.node]]` to another node in `/etc/gitlab/gitlab.rb`: +1. Enable the Grafana login form by editing `/etc/gitlab/gitlab.rb`. ```ruby - praefect['virtual_storages'] = { - 'praefect' => { - 'gitaly-1' => { - 'address' => 'tcp://GITALY_HOST:8075', - 'token' => 'PRAEFECT_INTERNAL_TOKEN', - # no longer the primary - }, - 'gitaly-2' => { - 'address' => 'tcp://GITALY_HOST:8075', - 'token' => 'PRAEFECT_INTERNAL_TOKEN', - # this is the new primary - 'primary' => true - }, - 'gitaly-3' => { - 'address' => 'tcp://GITALY_HOST:8075', - 'token' => 'PRAEFECT_INTERNAL_TOKEN', - } - } - } + grafana['disable_login_form'] = false ``` -1. Save the file and [reconfigure GitLab](../restart_gitlab.md#omnibus-gitlab-reconfigure). +1. Save the changes to `/etc/gitlab/gitlab.rb` and [reconfigure + GitLab](../restart_gitlab.md#omnibus-gitlab-reconfigure): -On a restart, Praefect will send write traffic to `internal_storage_1`. `internal_storage_0` is the new secondary now, -and replication jobs will be created to replicate repository data to `internal_storage_0` **from** `internal_storage_1` + ```shell + gitlab-ctl reconfigure + ``` -#### Automatic failover +1. Set the Grafana admin password. This command will prompt you to enter a new + password: -When automatic failover is enabled, Praefect will do automatic detection of the health of internal Gitaly nodes. If the -primary has a certain amount of health checks fail, it will decide to promote one of the secondaries to be primary, and -demote the primary to be a secondary. + ```shell + gitlab-ctl set-grafana-password + ``` -1. To enable automatic failover, edit `/etc/gitlab/gitlab.rb`: +1. In your web browser, open `/-/grafana` (e.g. + `https://gitlab.example.com/-/grafana`) on your GitLab server. - ```ruby - # failover_enabled turns on automatic failover - praefect['failover_enabled'] = true - praefect['virtual_storages'] = { - 'praefect' => { - 'gitaly-1' => { - 'address' => 'tcp://GITALY_HOST:8075', - 'token' => 'PRAEFECT_INTERNAL_TOKEN', - 'primary' => true - }, - 'gitaly-2' => { - 'address' => 'tcp://GITALY_HOST:8075', - 'token' => 'PRAEFECT_INTERNAL_TOKEN' - }, - 'gitaly-3' => { - 'address' => 'tcp://GITALY_HOST:8075', - 'token' => 'PRAEFECT_INTERNAL_TOKEN' - } - } - } - ``` + Login using the password you set, and the username `admin`. -1. Save the file and [reconfigure GitLab](../restart_gitlab.md#omnibus-gitlab-reconfigure). +1. Go to **Explore** and query `gitlab_build_info` to verify that you are + getting metrics from all your machines. -Below is the picture when Praefect starts up with the config.toml above: +Congratulations! You've configured an observable highly available Praefect +cluster. -```mermaid -graph TD - A[Praefect] -->|Mutator RPC| B(internal_storage_0) - B --> |Replication|C[internal_storage_1] -``` +## Automatic failover and leader election -Let's say suddenly `internal_storage_0` goes down. Praefect will detect this and -automatically switch over to `internal_storage_1`, and `internal_storage_0` will serve as a secondary: +Praefect regularly checks the health of each backend Gitaly node. This +information can be used to automatically failover to a new primary node if the +current primary node is found to be unhealthy. -```mermaid -graph TD - A[Praefect] -->|Mutator RPC| B(internal_storage_1) - B --> |Replication|C[internal_storage_0] -``` +- **Manual:** Automatic failover is disabled. The primary node can be + reconfigured in `/etc/gitlab/gitlab.rb` on the Praefect node. Modify the + `praefect['virtual_storages']` field by moving the `primary = true` to promote + a different Gitaly node to primary. In the steps above, `gitaly-1` was set to + the primary. +- **Memory:** Enabled by setting `praefect['failover_enabled'] = true` in + `/etc/gitlab/gitlab.rb` on the Praefect node. If a sufficient number of health + checks fail for the current primary backend Gitaly node, and new primary will + be elected. **Do not use with multiple Praefect nodes!** Using with multiple + Praefect nodes is likely to result in a split brain. +- **PostgreSQL:** Coming soon. See isse + [#2547](https://gitlab.com/gitlab-org/gitaly/-/issues/2547) for updates. -NOTE: **Note:**: Currently this feature is supported for setups that only have 1 Praefect instance. Praefect instances running, -for example behind a load balancer, `failover_enabled` should be disabled. The reason is The reason is because there -is no coordination that currently happens across different Praefect instances, so there could be a situation where -two Praefect instances think two different Gitaly nodes are the primary. +It is likely that we will implement support for Consul, and a cloud native +strategy in the future. ## Backend Node Recovery @@ -711,49 +686,6 @@ The command will return a list of repositories that were found to be inconsistent against the current primary. Each of these inconsistencies will also be logged with an accompanying replication job ID. -## Grafana - -Grafana is included with GitLab, and can be used to monitor your Praefect -cluster. See [Grafana Dashboard -Service](https://docs.gitlab.com/omnibus/settings/grafana.html) -for detailed documentation. - -To get started quickly: - -1. SSH into the **GitLab** node and login as root: - - ```shell - sudo -i - ``` - -1. Enable the Grafana login form by editing `/etc/gitlab/gitlab.rb`. - - ```ruby - grafana['disable_login_form'] = false - ``` - -1. Save the changes to `/etc/gitlab/gitlab.rb` and [reconfigure - GitLab](../restart_gitlab.md#omnibus-gitlab-reconfigure): - - ```shell - gitlab-ctl reconfigure - ``` - -1. Set the Grafana admin password. This command will prompt you to enter a new - password: - - ```shell - gitlab-ctl set-grafana-password - ``` - -1. In your web browser, open `/-/grafana` (e.g. - `https://gitlab.example.com/-/grafana`) on your GitLab server. - - Login using the password you set, and the username `admin`. - -1. Go to **Explore** and query `gitlab_build_info` to verify that you are - getting metrics from all your machines. - ## Migrating existing repositories to Praefect If your GitLab instance already has repositories, these won't be migrated diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index 889c2209a54..0833412ecb8 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -92,6 +92,7 @@ The following metrics are available: | `failed_login_captcha_total` | Gauge | 11.0 | Counter of failed CAPTCHA attempts during login | | | `successful_login_captcha_total` | Gauge | 11.0 | Counter of successful CAPTCHA attempts during login | | | `auto_devops_pipelines_completed_total` | Counter | 12.7 | Counter of completed Auto DevOps pipelines, labeled by status | | +| `gitlab_metrics_dashboard_processing_time_ms` | Summary | 12.10 | Metrics dashboard processing time in milliseconds | service, stages | ## Metrics controlled by a feature flag diff --git a/lib/gitlab/code_navigation_path.rb b/lib/gitlab/code_navigation_path.rb new file mode 100644 index 00000000000..8dd2e9cb1bb --- /dev/null +++ b/lib/gitlab/code_navigation_path.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Gitlab + class CodeNavigationPath + include Gitlab::Utils::StrongMemoize + include Gitlab::Routing + + CODE_NAVIGATION_JOB_NAME = 'code_navigation' + + def initialize(project, commit_sha) + @project = project + @commit_sha = commit_sha + end + + def full_json_path_for(path) + return if Feature.disabled?(:code_navigation, project) + return unless build + + raw_project_job_artifacts_path(project, build, path: "lsif/#{path}.json") + end + + private + + attr_reader :project, :commit_sha + + def build + strong_memoize(:build) do + artifact = ::Ci::JobArtifact + .for_sha(commit_sha, project.id) + .for_job_name(CODE_NAVIGATION_JOB_NAME) + .last + + artifact&.job + end + end + end +end diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 9fdaa728fd7..ad04c6e61e8 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -118,32 +118,6 @@ describe Projects::BlobController do end end end - - context 'when there is an artifact with code navigation data' do - let!(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.id) } - let!(:job) { create(:ci_build, pipeline: pipeline, name: Ci::Build::CODE_NAVIGATION_JOB_NAME) } - let!(:artifact) { create(:ci_job_artifact, :lsif, job: job) } - - let(:id) { 'master/README.md' } - - it 'assigns code_navigation_build variable' do - request - - expect(assigns[:code_navigation_build]).to eq(job) - end - - context 'when code_navigation feature is disabled' do - before do - stub_feature_flags(code_navigation: false) - end - - it 'does not assign code_navigation_build variable' do - request - - expect(assigns[:code_navigation_build]).to be_nil - end - end - end end describe 'GET diff' do diff --git a/spec/lib/banzai/filter/issuable_state_filter_spec.rb b/spec/lib/banzai/filter/issuable_state_filter_spec.rb index cb431df7551..5950b6878ef 100644 --- a/spec/lib/banzai/filter/issuable_state_filter_spec.rb +++ b/spec/lib/banzai/filter/issuable_state_filter_spec.rb @@ -156,20 +156,6 @@ describe Banzai::Filter::IssuableStateFilter do expect(doc.css('a').last.text).to eq(merge_request.to_reference) end - it 'ignores reopened merge request references' do - merge_request = create_merge_request(:opened) - - link = create_link( - merge_request.to_reference, - merge_request: merge_request.id, - reference_type: 'merge_request' - ) - - doc = filter(link, context) - - expect(doc.css('a').last.text).to eq(merge_request.to_reference) - end - it 'ignores locked merge request references' do merge_request = create_merge_request(:locked) diff --git a/spec/lib/gitlab/code_navigation_path_spec.rb b/spec/lib/gitlab/code_navigation_path_spec.rb new file mode 100644 index 00000000000..cafe362c8c7 --- /dev/null +++ b/spec/lib/gitlab/code_navigation_path_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::CodeNavigationPath do + context 'when there is an artifact with code navigation data' do + let(:project) { create(:project, :repository) } + let(:sha) { project.commit.id } + let(:build_name) { Gitlab::CodeNavigationPath::CODE_NAVIGATION_JOB_NAME } + let(:path) { 'lib/app.rb' } + let!(:pipeline) { create(:ci_pipeline, project: project, sha: sha) } + let!(:job) { create(:ci_build, pipeline: pipeline, name: build_name) } + let!(:artifact) { create(:ci_job_artifact, :lsif, job: job) } + + subject { described_class.new(project, sha).full_json_path_for(path) } + + it 'assigns code_navigation_build variable' do + expect(subject).to eq("/#{project.full_path}/-/jobs/#{job.id}/artifacts/raw/lsif/#{path}.json") + end + + context 'when code_navigation feature is disabled' do + before do + stub_feature_flags(code_navigation: false) + end + + it 'does not assign code_navigation_build variable' do + expect(subject).to be_nil + end + end + end +end diff --git a/spec/serializers/diff_file_entity_spec.rb b/spec/serializers/diff_file_entity_spec.rb index 3e341a58a15..e3ecd72b275 100644 --- a/spec/serializers/diff_file_entity_spec.rb +++ b/spec/serializers/diff_file_entity_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' describe DiffFileEntity do include RepoHelpers - let(:project) { create(:project, :repository) } + let_it_be(:project) { create(:project, :repository) } let(:repository) { project.repository } let(:commit) { project.commit(sample_commit.id) } let(:diff_refs) { commit.diff_refs } @@ -21,10 +21,11 @@ describe DiffFileEntity do end context 'when there is a merge request' do + let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:user) { create(:user) } + let(:code_navigation_path) { Gitlab::CodeNavigationPath.new(project, project.commit.sha) } let(:request) { EntityRequest.new(project: project, current_user: user) } - let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } - let(:entity) { described_class.new(diff_file, options.merge(request: request, merge_request: merge_request)) } + let(:entity) { described_class.new(diff_file, options.merge(request: request, merge_request: merge_request, code_navigation_path: code_navigation_path)) } let(:exposed_urls) { %i(edit_path view_path context_lines_path) } it_behaves_like 'diff file entity' @@ -32,6 +33,7 @@ describe DiffFileEntity do it 'exposes additional attributes' do expect(subject).to include(*exposed_urls) expect(subject).to include(:replaced_view_path) + expect(subject).to include(:code_navigation_path) end it 'points all urls to merge request target project' do diff --git a/spec/serializers/diffs_entity_spec.rb b/spec/serializers/diffs_entity_spec.rb index 59acbdac3d0..bb4ac5f9608 100644 --- a/spec/serializers/diffs_entity_spec.rb +++ b/spec/serializers/diffs_entity_spec.rb @@ -23,7 +23,7 @@ describe DiffsEntity do :start_version, :latest_diff, :latest_version_path, :added_lines, :removed_lines, :render_overflow_warning, :email_patch_path, :plain_diff_path, :diff_files, - :merge_request_diffs + :merge_request_diffs, :definition_path_prefix ) end end diff --git a/spec/serializers/diffs_metadata_entity_spec.rb b/spec/serializers/diffs_metadata_entity_spec.rb index 86438bd59d7..a6bf9a7700e 100644 --- a/spec/serializers/diffs_metadata_entity_spec.rb +++ b/spec/serializers/diffs_metadata_entity_spec.rb @@ -28,8 +28,8 @@ describe DiffsMetadataEntity do :start_version, :latest_diff, :latest_version_path, :added_lines, :removed_lines, :render_overflow_warning, :email_patch_path, :plain_diff_path, - :merge_request_diffs, - :context_commits, + :merge_request_diffs, :context_commits, + :definition_path_prefix, # Attributes :diff_files ) diff --git a/spec/services/metrics/dashboard/custom_dashboard_service_spec.rb b/spec/services/metrics/dashboard/custom_dashboard_service_spec.rb index 9458df3dca0..4966b83bbab 100644 --- a/spec/services/metrics/dashboard/custom_dashboard_service_spec.rb +++ b/spec/services/metrics/dashboard/custom_dashboard_service_spec.rb @@ -16,10 +16,20 @@ describe Metrics::Dashboard::CustomDashboardService, :use_clean_rails_memory_sto describe '#get_dashboard' do let(:dashboard_path) { '.gitlab/dashboards/test.yml' } let(:service_params) { [project, user, { environment: environment, dashboard_path: dashboard_path }] } - let(:service_call) { described_class.new(*service_params).get_dashboard } + let(:service_call) { subject.get_dashboard } + + subject { described_class.new(*service_params) } context 'when the dashboard does not exist' do it_behaves_like 'misconfigured dashboard service response', :not_found + + it 'does not update gitlab_metrics_dashboard_processing_time_ms metric', :prometheus do + service_call + metric = subject.send(:processing_time_metric) + labels = subject.send(:processing_time_metric_labels) + + expect(metric.get(labels)).to eq(0) + end end it_behaves_like 'raises error for users with insufficient permissions' @@ -28,6 +38,7 @@ describe Metrics::Dashboard::CustomDashboardService, :use_clean_rails_memory_sto let(:project) { project_with_dashboard(dashboard_path) } it_behaves_like 'valid dashboard service response' + it_behaves_like 'updates gitlab_metrics_dashboard_processing_time_ms metric' it 'caches the unprocessed dashboard for subsequent calls' do expect_any_instance_of(described_class) diff --git a/spec/services/metrics/dashboard/pod_dashboard_service_spec.rb b/spec/services/metrics/dashboard/pod_dashboard_service_spec.rb index 36ca6f882fa..1e62a5504a9 100644 --- a/spec/services/metrics/dashboard/pod_dashboard_service_spec.rb +++ b/spec/services/metrics/dashboard/pod_dashboard_service_spec.rb @@ -36,9 +36,12 @@ describe Metrics::Dashboard::PodDashboardService, :use_clean_rails_memory_store_ describe '#get_dashboard' do let(:dashboard_path) { described_class::DASHBOARD_PATH } let(:service_params) { [project, user, { environment: environment, dashboard_path: dashboard_path }] } - let(:service_call) { described_class.new(*service_params).get_dashboard } + let(:service_call) { subject.get_dashboard } + + subject { described_class.new(*service_params) } it_behaves_like 'valid dashboard service response' it_behaves_like 'caches the unprocessed dashboard for subsequent calls' + it_behaves_like 'updates gitlab_metrics_dashboard_processing_time_ms metric' end end diff --git a/spec/services/metrics/dashboard/self_monitoring_dashboard_service_spec.rb b/spec/services/metrics/dashboard/self_monitoring_dashboard_service_spec.rb index 9ee5b06b410..6c9a380a470 100644 --- a/spec/services/metrics/dashboard/self_monitoring_dashboard_service_spec.rb +++ b/spec/services/metrics/dashboard/self_monitoring_dashboard_service_spec.rb @@ -16,11 +16,14 @@ describe Metrics::Dashboard::SelfMonitoringDashboardService, :use_clean_rails_me describe '#get_dashboard' do let(:service_params) { [project, user, { environment: environment }] } - let(:service_call) { described_class.new(*service_params).get_dashboard } + let(:service_call) { subject.get_dashboard } + + subject { described_class.new(*service_params) } it_behaves_like 'valid dashboard service response' it_behaves_like 'raises error for users with insufficient permissions' it_behaves_like 'caches the unprocessed dashboard for subsequent calls' + it_behaves_like 'updates gitlab_metrics_dashboard_processing_time_ms metric' end describe '.all_dashboard_paths' do diff --git a/spec/services/metrics/dashboard/system_dashboard_service_spec.rb b/spec/services/metrics/dashboard/system_dashboard_service_spec.rb index 1956f9b563b..7d58501ae3f 100644 --- a/spec/services/metrics/dashboard/system_dashboard_service_spec.rb +++ b/spec/services/metrics/dashboard/system_dashboard_service_spec.rb @@ -16,11 +16,14 @@ describe Metrics::Dashboard::SystemDashboardService, :use_clean_rails_memory_sto describe '#get_dashboard' do let(:dashboard_path) { described_class::DASHBOARD_PATH } let(:service_params) { [project, user, { environment: environment, dashboard_path: dashboard_path }] } - let(:service_call) { described_class.new(*service_params).get_dashboard } + let(:service_call) { subject.get_dashboard } + + subject { described_class.new(*service_params) } it_behaves_like 'valid dashboard service response' it_behaves_like 'raises error for users with insufficient permissions' it_behaves_like 'caches the unprocessed dashboard for subsequent calls' + it_behaves_like 'updates gitlab_metrics_dashboard_processing_time_ms metric' context 'when called with a non-system dashboard' do let(:dashboard_path) { 'garbage/dashboard/path' } diff --git a/spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb b/spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb index bcd69b9e4f6..90fcac0e55c 100644 --- a/spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb +++ b/spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb @@ -118,3 +118,13 @@ RSpec.shared_examples 'misconfigured dashboard service response with stepable' d expect(result[:message]).to eq(message) if message end end + +RSpec.shared_examples 'updates gitlab_metrics_dashboard_processing_time_ms metric' do + specify :prometheus do + service_call + metric = subject.send(:processing_time_metric) + labels = subject.send(:processing_time_metric_labels) + + expect(metric.get(labels)).to be > 0 + end +end |
