From d0582f7e713169aa8b80520816d45fe7512e645a Mon Sep 17 00:00:00 2001 From: Piotr Wosiek Date: Wed, 27 Feb 2019 15:03:33 +0100 Subject: Add .NET Core YAML template --- lib/gitlab/ci/templates/dotNET-Core.yml | 75 +++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 lib/gitlab/ci/templates/dotNET-Core.yml diff --git a/lib/gitlab/ci/templates/dotNET-Core.yml b/lib/gitlab/ci/templates/dotNET-Core.yml new file mode 100644 index 00000000000..c12c82877f4 --- /dev/null +++ b/lib/gitlab/ci/templates/dotNET-Core.yml @@ -0,0 +1,75 @@ +# This is a simple example illustrating how to build and test .NET Core project +# with GitLab Continuous Integration / Continuous Delivery. +# +# Structure of a sample project would look like this: +# +# Project +# ├── src +# │ └── ConsoleApp +# │ └── Program.cs +# └── test +# └── UnitTests +# └── BasicUnitTests.cs + +# Specify the Docker image +# +# Instead of installing .NET Core SDK manually, a docker image is used +# with already pre-installed .NET Core SDK. +# The 'latest' tag targets the latest available version of .NET Core SDK image. +# If preferred, you can explicitly specify version of .NET Core e.g. using '2.2-sdk' tag. +# +# See other available tags for .NET Core: https://hub.docker.com/r/microsoft/dotnet +# Learn more about Docker tags: https://docs.docker.com/glossary/?term=tag +# and the Docker itself: https://opensource.com/resources/what-docker +image: microsoft/dotnet:latest + +# Define stage list +# +# In this example there are only two stages. +# Initially, the project will be built and then tested. +stages: + - build + - test + +build: + stage: build +# Restore project dependencies +# +# Before building the project all dependencies (e.g. third-party NuGet packages) +# must be restored. +# +# Jobs on GitLab.com's Shared Runners are executed on autoscaled machines. +# Each machine is used only once (for security reasons) and after this it is removed. +# What that means is that before every job a dependency restore must be performed +# because restored dependencies are removed along with machines. There are ways +# to transfer restored packages and other output binaries, but this example +# does not cover that. +# +# Learn more about GitLab job artifacts: https://docs.gitlab.com/ee/user/project/pipelines/job_artifacts.html + before_script: + - 'dotnet restore' +# Build all projects discovered from solution file. +# +# Note: this may fail in case you have at least one not .NET Core based project +# defined in your solution file e.g. WCF service, which is based on .NET Framework +# not .NET Core. In such scenario you will need to build .NET Core project +# by explicitly specifying a relative path to the directory where it is located +# e.g. 'dotnet build ./src/ConsoleApp' +# Only one project path can be passed as a parameter to 'dotnet build' command. + script: + - 'dotnet build' + +unit tests: + stage: test +# Despite the fact that the project was already built and restored, +# a dependency restore must be performed again. + before_script: + - 'dotnet restore' +# Run the tests +# +# You can either run tests only for specific project (like shown below) +# or run tests for all test projects that are defined in a solution file +# with 'dotnet test'. You may want to define separate jobs for separate +# testing projects e.g. IntegrationTests, UnitTests etc. + script: + - 'dotnet test ./test/UnitTests' -- cgit v1.2.1 From fdc52b5fd00570cf62559771d0a5ec99891de664 Mon Sep 17 00:00:00 2001 From: Piotr Wosiek Date: Wed, 13 Mar 2019 16:46:48 +0100 Subject: Improve note about building not .NET Core based projects --- lib/gitlab/ci/templates/dotNET-Core.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/gitlab/ci/templates/dotNET-Core.yml b/lib/gitlab/ci/templates/dotNET-Core.yml index c12c82877f4..88c7d23e7aa 100644 --- a/lib/gitlab/ci/templates/dotNET-Core.yml +++ b/lib/gitlab/ci/templates/dotNET-Core.yml @@ -50,11 +50,11 @@ build: - 'dotnet restore' # Build all projects discovered from solution file. # -# Note: this may fail in case you have at least one not .NET Core based project -# defined in your solution file e.g. WCF service, which is based on .NET Framework -# not .NET Core. In such scenario you will need to build .NET Core project -# by explicitly specifying a relative path to the directory where it is located -# e.g. 'dotnet build ./src/ConsoleApp' +# Note: this will fail if you have any projects in your solution that are not +# .NET Core based projects e.g. WCF service, which is based on .NET Framework, +# not .NET Core. In such scenario you will need to build every .NET Core based +# project by explicitly specifying a relative path to the directory +# where it is located e.g. 'dotnet build ./src/ConsoleApp'. # Only one project path can be passed as a parameter to 'dotnet build' command. script: - 'dotnet build' -- cgit v1.2.1 From 2ad6230aa0f8ccf50a922ac3b4b9a264eb09e05b Mon Sep 17 00:00:00 2001 From: Piotr Wosiek Date: Wed, 13 Mar 2019 16:51:09 +0100 Subject: Remove the diagram of project directories structure --- lib/gitlab/ci/templates/dotNET-Core.yml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lib/gitlab/ci/templates/dotNET-Core.yml b/lib/gitlab/ci/templates/dotNET-Core.yml index 88c7d23e7aa..0f96b040e3b 100644 --- a/lib/gitlab/ci/templates/dotNET-Core.yml +++ b/lib/gitlab/ci/templates/dotNET-Core.yml @@ -1,15 +1,5 @@ # This is a simple example illustrating how to build and test .NET Core project # with GitLab Continuous Integration / Continuous Delivery. -# -# Structure of a sample project would look like this: -# -# Project -# ├── src -# │ └── ConsoleApp -# │ └── Program.cs -# └── test -# └── UnitTests -# └── BasicUnitTests.cs # Specify the Docker image # -- cgit v1.2.1 From b7ba3d903273b8009a066934e8f4bd1ef7a133fa Mon Sep 17 00:00:00 2001 From: Piotr Wosiek Date: Wed, 13 Mar 2019 16:59:16 +0100 Subject: Make the testing section more generic --- lib/gitlab/ci/templates/dotNET-Core.yml | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/gitlab/ci/templates/dotNET-Core.yml b/lib/gitlab/ci/templates/dotNET-Core.yml index 0f96b040e3b..3afac7bfff0 100644 --- a/lib/gitlab/ci/templates/dotNET-Core.yml +++ b/lib/gitlab/ci/templates/dotNET-Core.yml @@ -49,7 +49,7 @@ build: script: - 'dotnet build' -unit tests: +tests: stage: test # Despite the fact that the project was already built and restored, # a dependency restore must be performed again. @@ -57,9 +57,11 @@ unit tests: - 'dotnet restore' # Run the tests # -# You can either run tests only for specific project (like shown below) -# or run tests for all test projects that are defined in a solution file -# with 'dotnet test'. You may want to define separate jobs for separate -# testing projects e.g. IntegrationTests, UnitTests etc. +# You can either run tests for all test projects that are defined in your solution +# with 'dotnet test' or run tests only for specific project by specifying +# a relative path to the directory where it is located e.g. 'dotnet test ./test/UnitTests'. +# +# You may want to define separate testing jobs for different types of testing +# e.g. integration tests, unit tests etc. script: - - 'dotnet test ./test/UnitTests' + - 'dotnet test' -- cgit v1.2.1 From b00fe08dbdaf5f1d5ea440004662236643d12605 Mon Sep 17 00:00:00 2001 From: Piotr Wosiek Date: Wed, 20 Mar 2019 17:10:13 +0100 Subject: Update .NET Core YAML template - add cache Update the template showing how to cache restored dependencies, add few variables, reduce before_script part, add and improve descriptions. --- lib/gitlab/ci/templates/dotNET-Core.yml | 99 +++++++++++++++++++++++++-------- 1 file changed, 77 insertions(+), 22 deletions(-) diff --git a/lib/gitlab/ci/templates/dotNET-Core.yml b/lib/gitlab/ci/templates/dotNET-Core.yml index 3afac7bfff0..15319bafc69 100644 --- a/lib/gitlab/ci/templates/dotNET-Core.yml +++ b/lib/gitlab/ci/templates/dotNET-Core.yml @@ -1,7 +1,7 @@ # This is a simple example illustrating how to build and test .NET Core project # with GitLab Continuous Integration / Continuous Delivery. -# Specify the Docker image +# ### Specify the Docker image # # Instead of installing .NET Core SDK manually, a docker image is used # with already pre-installed .NET Core SDK. @@ -13,32 +13,89 @@ # and the Docker itself: https://opensource.com/resources/what-docker image: microsoft/dotnet:latest -# Define stage list + +# ### Define variables +# +variables: +# 1) Name of directory where restore and build objects are stored. + OBJECTS_DIRECTORY: 'obj' +# 2) Name of directory used for keeping restored dependencies. + NUGET_PACKAGES_DIRECTORY: '.nuget' +# 3) A relative path to the source code from project repository root. +# NOTE: Please edit this path so it matches the structure of your project! + SOURCE_CODE_PATH: '*/*/' + + +# ### Define stage list # # In this example there are only two stages. # Initially, the project will be built and then tested. stages: - build - test + + +# ### Define global cache rule +# +# Before building the project, all dependencies (e.g. third-party NuGet packages) +# must be restored. Jobs on GitLab.com's Shared Runners are executed on autoscaled machines. +# Each machine is used only once (for security reasons) and after that it is removed. +# What that means is that before every job a dependency restore must be performed +# because restored dependencies are removed along with machines. Fortunately, +# GitLab provides cache mechanism with the aim of keeping restored dependencies +# for other jobs. In this example dependencies are restored only once +# and then passed over to the next jobs. +# +# With global cache rule, cached dependencies will be downloaded before every job +# and then unpacked to the paths as specified below. +cache: + paths: +# Specify three paths that should be cached: +# +# 1) Main JSON file holding information about package dependency tree, packages versions, +# frameworks etc. It also holds information where to the dependencies were restored, +# so next time a 'dotnet build' is executed, the build engine will know +# where to look for already downloaded dependencies. + - '$SOURCE_CODE_PATH$OBJECTS_DIRECTORY/project.assets.json' +# 2) Other NuGet and MSBuild related files. Also needed. + - '$SOURCE_CODE_PATH$OBJECTS_DIRECTORY/*.csproj.nuget.*' +# 3) Path to the directory where restored dependencies are kept. + - '$NUGET_PACKAGES_DIRECTORY' + policy: pull # Only download the cache, don't upload it after the job is completed. + build: stage: build -# Restore project dependencies -# -# Before building the project all dependencies (e.g. third-party NuGet packages) -# must be restored. +# +# ### Restore project dependencies # -# Jobs on GitLab.com's Shared Runners are executed on autoscaled machines. -# Each machine is used only once (for security reasons) and after this it is removed. -# What that means is that before every job a dependency restore must be performed -# because restored dependencies are removed along with machines. There are ways -# to transfer restored packages and other output binaries, but this example -# does not cover that. +# NuGet packages by default are restored to '.nuget/packages' directory +# in the user's home directory. That directory is out of scope of GitLab caching. +# To get around this a custom path can be specified using '--packages ' option +# for 'dotnet restore' command. In this example a temporary directory is created +# in the root of project repository, so it's content can be cached. # -# Learn more about GitLab job artifacts: https://docs.gitlab.com/ee/user/project/pipelines/job_artifacts.html +# Learn more about GitLab cache: https://docs.gitlab.com/ee/ci/caching/index.html before_script: - - 'dotnet restore' -# Build all projects discovered from solution file. + - 'dotnet restore --packages $NUGET_PACKAGES_DIRECTORY' +# Override global cache rule for uploading. + cache: + paths: + - '$SOURCE_CODE_PATH$OBJECTS_DIRECTORY/project.assets.json' + - '$SOURCE_CODE_PATH$OBJECTS_DIRECTORY/*.csproj.nuget.*' + - '$NUGET_PACKAGES_DIRECTORY' +# +# 'pull-push' policy means that latest cache will be downloaded (if exists) +# before executing the job, and a newer version will be uploaded afterwards. +# Such setting saves time when there are no changes in referenced third-party +# packages. For example if you run a pipeline with changes in your code, +# but with no changes within third-party packages which your project is using, +# then project restore will happen in next to no time as all required dependencies +# will already be there — unzipped from cache. 'pull-push' policy is a default +# cache policy, you do not have to specify it explicitly. + policy: pull-push +# +# ### Build all projects discovered from solution file. # # Note: this will fail if you have any projects in your solution that are not # .NET Core based projects e.g. WCF service, which is based on .NET Framework, @@ -47,15 +104,13 @@ build: # where it is located e.g. 'dotnet build ./src/ConsoleApp'. # Only one project path can be passed as a parameter to 'dotnet build' command. script: - - 'dotnet build' + - 'dotnet build --no-restore' + tests: stage: test -# Despite the fact that the project was already built and restored, -# a dependency restore must be performed again. - before_script: - - 'dotnet restore' -# Run the tests +# +# ### Run the tests # # You can either run tests for all test projects that are defined in your solution # with 'dotnet test' or run tests only for specific project by specifying @@ -64,4 +119,4 @@ tests: # You may want to define separate testing jobs for different types of testing # e.g. integration tests, unit tests etc. script: - - 'dotnet test' + - 'dotnet test --no-restore' -- cgit v1.2.1 From ab9aa8210872070f06bad33df82717bd59ea7e58 Mon Sep 17 00:00:00 2001 From: Piotr Wosiek Date: Fri, 22 Mar 2019 01:09:27 +0100 Subject: Update .NET Core YAML template - improve caching Revise logic of dependency caching, add cache key, make before script global. --- lib/gitlab/ci/templates/dotNET-Core.yml | 49 +++++++++++++++------------------ 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/lib/gitlab/ci/templates/dotNET-Core.yml b/lib/gitlab/ci/templates/dotNET-Core.yml index 15319bafc69..08295106c44 100644 --- a/lib/gitlab/ci/templates/dotNET-Core.yml +++ b/lib/gitlab/ci/templates/dotNET-Core.yml @@ -43,29 +43,36 @@ stages: # What that means is that before every job a dependency restore must be performed # because restored dependencies are removed along with machines. Fortunately, # GitLab provides cache mechanism with the aim of keeping restored dependencies -# for other jobs. In this example dependencies are restored only once -# and then passed over to the next jobs. +# for other jobs. This example shows how to configure cache to pass over restored +# dependencies for re-use. # # With global cache rule, cached dependencies will be downloaded before every job # and then unpacked to the paths as specified below. cache: +# Per-stage and per-branch caching. + key: "$CI_JOB_STAGE-$CI_COMMIT_REF_SLUG" paths: # Specify three paths that should be cached: # # 1) Main JSON file holding information about package dependency tree, packages versions, -# frameworks etc. It also holds information where to the dependencies were restored, -# so next time a 'dotnet build' is executed, the build engine will know -# where to look for already downloaded dependencies. +# frameworks etc. It also holds information where to the dependencies were restored. - '$SOURCE_CODE_PATH$OBJECTS_DIRECTORY/project.assets.json' # 2) Other NuGet and MSBuild related files. Also needed. - '$SOURCE_CODE_PATH$OBJECTS_DIRECTORY/*.csproj.nuget.*' # 3) Path to the directory where restored dependencies are kept. - '$NUGET_PACKAGES_DIRECTORY' - policy: pull # Only download the cache, don't upload it after the job is completed. - +# +# 'pull-push' policy means that latest cache will be downloaded (if exists) +# before executing the job, and a newer version will be uploaded afterwards. +# Such setting saves time when there are no changes in referenced third-party +# packages. For example if you run a pipeline with changes in your code, +# but with no changes within third-party packages which your project is using, +# then project restore will happen in next to no time as all required dependencies +# will already be there — unzipped from cache. 'pull-push' policy is a default +# cache policy, you do not have to specify it explicitly. + policy: pull-push + -build: - stage: build # # ### Restore project dependencies # @@ -76,24 +83,12 @@ build: # in the root of project repository, so it's content can be cached. # # Learn more about GitLab cache: https://docs.gitlab.com/ee/ci/caching/index.html - before_script: - - 'dotnet restore --packages $NUGET_PACKAGES_DIRECTORY' -# Override global cache rule for uploading. - cache: - paths: - - '$SOURCE_CODE_PATH$OBJECTS_DIRECTORY/project.assets.json' - - '$SOURCE_CODE_PATH$OBJECTS_DIRECTORY/*.csproj.nuget.*' - - '$NUGET_PACKAGES_DIRECTORY' -# -# 'pull-push' policy means that latest cache will be downloaded (if exists) -# before executing the job, and a newer version will be uploaded afterwards. -# Such setting saves time when there are no changes in referenced third-party -# packages. For example if you run a pipeline with changes in your code, -# but with no changes within third-party packages which your project is using, -# then project restore will happen in next to no time as all required dependencies -# will already be there — unzipped from cache. 'pull-push' policy is a default -# cache policy, you do not have to specify it explicitly. - policy: pull-push +before_script: + - 'dotnet restore --packages $NUGET_PACKAGES_DIRECTORY' + + +build: + stage: build # # ### Build all projects discovered from solution file. # -- cgit v1.2.1 From fd1bc613b4aa1bb4f580240d50fc1abf40a3d1d7 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 21 Apr 2019 16:16:33 -0700 Subject: Disable password autocomplete in mirror repository form Chrome and other browsers ignore `autocomplete=false` and `autocomplete=off` now. This likely caused the mirroring section to expand whenever a user clicked "Expand" in the protected branches settings. https://developers.google.com/web/fundamentals/design-and-ux/input/forms/?hl=en recommends using `new-password` as the new field to prevent automatic filling. This is similar to https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/8477. Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/11237 --- app/views/projects/mirrors/_mirror_repos.html.haml | 4 ++-- changelogs/unreleased/sh-fix-autocomplete-mirror-repo.yml | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/sh-fix-autocomplete-mirror-repo.yml diff --git a/app/views/projects/mirrors/_mirror_repos.html.haml b/app/views/projects/mirrors/_mirror_repos.html.haml index c031815200b..a1ec2c887c2 100644 --- a/app/views/projects/mirrors/_mirror_repos.html.haml +++ b/app/views/projects/mirrors/_mirror_repos.html.haml @@ -11,7 +11,7 @@ = link_to _('Read more'), help_page_path('workflow/repository_mirroring'), target: '_blank' .settings-content - = form_for @project, url: project_mirror_path(@project), html: { class: 'gl-show-field-errors js-mirror-form', autocomplete: 'false', data: mirrors_form_data_attributes } do |f| + = form_for @project, url: project_mirror_path(@project), html: { class: 'gl-show-field-errors js-mirror-form', autocomplete: 'new-password', data: mirrors_form_data_attributes } do |f| .panel.panel-default .panel-heading %h3.panel-title= _('Mirror a repository') @@ -20,7 +20,7 @@ .form-group.has-feedback = label_tag :url, _('Git repository URL'), class: 'label-light' - = text_field_tag :url, nil, class: 'form-control js-mirror-url js-repo-url qa-mirror-repository-url-input', placeholder: _('Input your repository URL'), required: true, pattern: "(#{protocols}):\/\/.+" + = text_field_tag :url, nil, class: 'form-control js-mirror-url js-repo-url qa-mirror-repository-url-input', placeholder: _('Input your repository URL'), required: true, pattern: "(#{protocols}):\/\/.+", autocomplete: 'new-password' = render 'projects/mirrors/instructions' diff --git a/changelogs/unreleased/sh-fix-autocomplete-mirror-repo.yml b/changelogs/unreleased/sh-fix-autocomplete-mirror-repo.yml new file mode 100644 index 00000000000..e855684bab1 --- /dev/null +++ b/changelogs/unreleased/sh-fix-autocomplete-mirror-repo.yml @@ -0,0 +1,5 @@ +--- +title: Disable password autocomplete in mirror repository form +merge_request: 27542 +author: +type: fixed -- cgit v1.2.1 From 113c7af4c295df678af41daf1ed76b58304a393c Mon Sep 17 00:00:00 2001 From: rpereira2 Date: Mon, 22 Apr 2019 23:57:29 +0530 Subject: Refactor operations controller spec - Move specs into a shared_context so that they can be reused. - Move common specs out of a more specific context. --- .../settings/operations_controller_spec.rb | 222 +++++++++++---------- 1 file changed, 117 insertions(+), 105 deletions(-) diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index 02a392f23c2..7ec5ebf3ce9 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/spec/controllers/projects/settings/operations_controller_spec.rb @@ -11,66 +11,142 @@ describe Projects::Settings::OperationsController do project.add_maintainer(user) end - context 'error tracking' do - describe 'GET #show' do - it 'renders show template' do + describe 'GET #show' do + it 'renders show template' do + get :show, params: project_params(project) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + end + + context 'with insufficient permissions' do + before do + project.add_reporter(user) + end + + it 'renders 404' do get :show, params: project_params(project) - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:show) + expect(response).to have_gitlab_http_status(:not_found) end + end - context 'with existing setting' do - let!(:error_tracking_setting) do - create(:project_error_tracking_setting, project: project) - end + context 'as an anonymous user' do + before do + sign_out(user) + end - it 'loads existing setting' do - get :show, params: project_params(project) + it 'redirects to signup page' do + get :show, params: project_params(project) - expect(controller.helpers.error_tracking_setting) - .to eq(error_tracking_setting) - end + expect(response).to redirect_to(new_user_session_path) end + end + end - context 'without an existing setting' do - it 'builds a new setting' do - get :show, params: project_params(project) + describe 'PATCH #update' do + context 'with insufficient permissions' do + before do + project.add_reporter(user) + end - expect(controller.helpers.error_tracking_setting).to be_new_record + it 'renders 404' do + patch :update, params: project_params(project) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'as an anonymous user' do + before do + sign_out(user) + end + + it 'redirects to signup page' do + patch :update, params: project_params(project) + + expect(response).to redirect_to(new_user_session_path) + end + end + end + + shared_context 'PATCH #update' do + let(:operations_update_service) { instance_double(::Projects::Operations::UpdateService) } + let(:operations_url) { project_settings_operations_url(project) } + + let(:permitted_params) do + ActionController::Parameters.new(params).permit! + end + + context 'format json' do + context 'when update succeeds' do + before do + stub_operations_update_service_returning(status: :success) + end + + it 'returns success status' do + patch :update, + params: project_params(project, params), + format: :json + + expect(::Projects::Operations::UpdateService) + .to have_received(:new).with(project, user, permitted_params) + expect(operations_update_service).to have_received(:execute) + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq('status' => 'success') + expect(flash[:notice]).to eq('Your changes have been saved') end end - context 'with insufficient permissions' do + context 'when update fails' do before do - project.add_reporter(user) + stub_operations_update_service_returning( + status: :error, + message: 'error message' + ) end - it 'renders 404' do - get :show, params: project_params(project) + it 'returns error' do + patch :update, + params: project_params(project, params), + format: :json - expect(response).to have_gitlab_http_status(:not_found) + expect(::Projects::Operations::UpdateService) + .to have_received(:new).with(project, user, permitted_params) + expect(operations_update_service).to have_received(:execute) + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).not_to be_nil end end + end + end - context 'as an anonymous user' do - before do - sign_out(user) + context 'error tracking' do + describe 'GET #show' do + context 'with existing setting' do + let!(:error_tracking_setting) do + create(:project_error_tracking_setting, project: project) + end + + it 'loads existing setting' do + get :show, params: project_params(project) + + expect(controller.helpers.error_tracking_setting) + .to eq(error_tracking_setting) end + end - it 'redirects to signup page' do + context 'without an existing setting' do + it 'builds a new setting' do get :show, params: project_params(project) - expect(response).to redirect_to(new_user_session_path) + expect(controller.helpers.error_tracking_setting).to be_new_record end end end describe 'PATCH #update' do - let(:operations_update_service) { spy(:operations_update_service) } - let(:operations_url) { project_settings_operations_url(project) } - - let(:error_tracking_params) do + let(:params) do { error_tracking_setting_attributes: { enabled: '1', @@ -86,79 +162,7 @@ describe Projects::Settings::OperationsController do } end - let(:error_tracking_permitted) do - ActionController::Parameters.new(error_tracking_params).permit! - end - - context 'format json' do - context 'when update succeeds' do - before do - stub_operations_update_service_returning(status: :success) - end - - it 'returns success status' do - patch :update, - params: project_params(project, error_tracking_params), - format: :json - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to eq('status' => 'success') - expect(flash[:notice]).to eq('Your changes have been saved') - end - end - - context 'when update fails' do - before do - stub_operations_update_service_returning( - status: :error, - message: 'error message' - ) - end - - it 'returns error' do - patch :update, - params: project_params(project, error_tracking_params), - format: :json - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).not_to be_nil - end - end - end - - context 'with insufficient permissions' do - before do - project.add_reporter(user) - end - - it 'renders 404' do - patch :update, params: project_params(project) - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'as an anonymous user' do - before do - sign_out(user) - end - - it 'redirects to signup page' do - patch :update, params: project_params(project) - - expect(response).to redirect_to(new_user_session_path) - end - end - end - - private - - def stub_operations_update_service_returning(return_value = {}) - expect(::Projects::Operations::UpdateService) - .to receive(:new).with(project, user, error_tracking_permitted) - .and_return(operations_update_service) - expect(operations_update_service).to receive(:execute) - .and_return(return_value) + it_behaves_like 'PATCH #update' end end @@ -171,4 +175,12 @@ describe Projects::Settings::OperationsController do project: params } end + + def stub_operations_update_service_returning(return_value = {}) + allow(::Projects::Operations::UpdateService) + .to receive(:new).with(project, user, permitted_params) + .and_return(operations_update_service) + allow(operations_update_service).to receive(:execute) + .and_return(return_value) + end end -- cgit v1.2.1 From 814232ed8930e1c2e7be792b27cd995822fde86a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aaron=20Br=C3=BClisauer?= Date: Tue, 23 Apr 2019 08:51:30 +0000 Subject: fix include example no leading slash at project reference --- doc/ci/yaml/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 2e85e34f17b..54fc7865dd1 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -2061,7 +2061,7 @@ from another project: ```yaml include: - template: Bash.gitlab-ci.yml - - project: /group/my-project + - project: group/my-project file: /templates/docker-workflow.yml ``` -- cgit v1.2.1 From 9f9bb16cdfb07fc975e1bcf50d900c50837836b3 Mon Sep 17 00:00:00 2001 From: rpereira2 Date: Wed, 24 Apr 2019 11:56:31 +0530 Subject: Move shared context to top of spec file - Rename the shared_context - Use expect in stub_operations_update_service_returning. --- .../settings/operations_controller_spec.rb | 112 ++++++++++----------- 1 file changed, 52 insertions(+), 60 deletions(-) diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index 7ec5ebf3ce9..7df63266f71 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/spec/controllers/projects/settings/operations_controller_spec.rb @@ -11,6 +11,57 @@ describe Projects::Settings::OperationsController do project.add_maintainer(user) end + shared_context 'PATCHable' do + let(:operations_update_service) { instance_double(::Projects::Operations::UpdateService) } + let(:operations_url) { project_settings_operations_url(project) } + + let(:permitted_params) do + ActionController::Parameters.new(params).permit! + end + + context 'format json' do + context 'when update succeeds' do + it 'returns success status' do + stub_operations_update_service_returning(status: :success) + + patch :update, + params: project_params(project, params), + format: :json + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq('status' => 'success') + expect(flash[:notice]).to eq('Your changes have been saved') + end + end + + context 'when update fails' do + it 'returns error' do + stub_operations_update_service_returning( + status: :error, + message: 'error message' + ) + + patch :update, + params: project_params(project, params), + format: :json + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('error message') + end + end + end + + private + + def stub_operations_update_service_returning(return_value = {}) + expect(::Projects::Operations::UpdateService) + .to receive(:new).with(project, user, permitted_params) + .and_return(operations_update_service) + expect(operations_update_service).to receive(:execute) + .and_return(return_value) + end + end + describe 'GET #show' do it 'renders show template' do get :show, params: project_params(project) @@ -70,57 +121,6 @@ describe Projects::Settings::OperationsController do end end - shared_context 'PATCH #update' do - let(:operations_update_service) { instance_double(::Projects::Operations::UpdateService) } - let(:operations_url) { project_settings_operations_url(project) } - - let(:permitted_params) do - ActionController::Parameters.new(params).permit! - end - - context 'format json' do - context 'when update succeeds' do - before do - stub_operations_update_service_returning(status: :success) - end - - it 'returns success status' do - patch :update, - params: project_params(project, params), - format: :json - - expect(::Projects::Operations::UpdateService) - .to have_received(:new).with(project, user, permitted_params) - expect(operations_update_service).to have_received(:execute) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to eq('status' => 'success') - expect(flash[:notice]).to eq('Your changes have been saved') - end - end - - context 'when update fails' do - before do - stub_operations_update_service_returning( - status: :error, - message: 'error message' - ) - end - - it 'returns error' do - patch :update, - params: project_params(project, params), - format: :json - - expect(::Projects::Operations::UpdateService) - .to have_received(:new).with(project, user, permitted_params) - expect(operations_update_service).to have_received(:execute) - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).not_to be_nil - end - end - end - end - context 'error tracking' do describe 'GET #show' do context 'with existing setting' do @@ -162,7 +162,7 @@ describe Projects::Settings::OperationsController do } end - it_behaves_like 'PATCH #update' + it_behaves_like 'PATCHable' end end @@ -175,12 +175,4 @@ describe Projects::Settings::OperationsController do project: params } end - - def stub_operations_update_service_returning(return_value = {}) - allow(::Projects::Operations::UpdateService) - .to receive(:new).with(project, user, permitted_params) - .and_return(operations_update_service) - allow(operations_update_service).to receive(:execute) - .and_return(return_value) - end end -- cgit v1.2.1 From cad7a7bb24e16513101f89792379ce9a1cbe3a04 Mon Sep 17 00:00:00 2001 From: rpereira2 Date: Thu, 11 Apr 2019 20:09:58 +0530 Subject: Change the schema of the common_metrics.yml - Change it to the new dashboard syntax. --- db/importers/common_metrics_importer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/importers/common_metrics_importer.rb b/db/importers/common_metrics_importer.rb index 195bde8f34a..f40be12719a 100644 --- a/db/importers/common_metrics_importer.rb +++ b/db/importers/common_metrics_importer.rb @@ -38,7 +38,7 @@ module Importers attr_reader :content def initialize(filename = 'common_metrics.yml') - @content = YAML.load_file(Rails.root.join('config', 'prometheus', filename)) + @content = YAML.load_file(Rails.root.join('config', 'prometheus', filename)).first end def execute -- cgit v1.2.1 From 6735f4c705ac6a08ac0b5e847c9f197b81c8502a Mon Sep 17 00:00:00 2001 From: rpereira2 Date: Mon, 15 Apr 2019 14:59:59 +0530 Subject: Make dashboard a top level key - Since each yml file can hold only one dashboard. --- db/importers/common_metrics_importer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/importers/common_metrics_importer.rb b/db/importers/common_metrics_importer.rb index f40be12719a..195bde8f34a 100644 --- a/db/importers/common_metrics_importer.rb +++ b/db/importers/common_metrics_importer.rb @@ -38,7 +38,7 @@ module Importers attr_reader :content def initialize(filename = 'common_metrics.yml') - @content = YAML.load_file(Rails.root.join('config', 'prometheus', filename)).first + @content = YAML.load_file(Rails.root.join('config', 'prometheus', filename)) end def execute -- cgit v1.2.1 From a2920682ec307b9aa830903014139948cdbb9b1f Mon Sep 17 00:00:00 2001 From: syasonik Date: Thu, 11 Apr 2019 13:58:18 +0800 Subject: Add inital dashboard endpoint support --- .../projects/environments_controller.rb | 14 ++ app/services/metrics_dashboard_service.rb | 60 +++++ config/prometheus/system_dashboard.yml | 274 +++++++++++++++++++++ config/routes/project.rb | 1 + 4 files changed, 349 insertions(+) create mode 100644 app/services/metrics_dashboard_service.rb create mode 100644 config/prometheus/system_dashboard.yml diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index e35f34be23c..41fe70052ce 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -12,6 +12,8 @@ class Projects::EnvironmentsController < Projects::ApplicationController before_action :expire_etag_cache, only: [:index] before_action only: [:metrics, :additional_metrics] do push_frontend_feature_flag(:metrics_time_window) + push_frontend_feature_flag(:environment_metrics_use_prometheus_endpoint) + push_frontend_feature_flag(:environment_metrics_show_multiple_dashboards) end def index @@ -156,6 +158,18 @@ class Projects::EnvironmentsController < Projects::ApplicationController end end + def metrics_dashboard + access_denied! unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, project) + + respond_to do |format| + format.json do + dashboard = MetricsDashboardService.new(@project).find(params[:dashboard]) + + render json: dashboard, status: :ok + end + end + end + def search respond_to do |format| format.json do diff --git a/app/services/metrics_dashboard_service.rb b/app/services/metrics_dashboard_service.rb new file mode 100644 index 00000000000..532f5697be8 --- /dev/null +++ b/app/services/metrics_dashboard_service.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +# Searches a projects repository for a metrics dashboard and formats the output. +# Expects any custom dashboards will be located in `.gitlab/dashboards` +class MetricsDashboardService + DASHBOARD_ROOT = ".gitlab/dashboards" + DASHBOARD_EXTENSION = '.yml' + + SYSTEM_DASHBOARD_NAME = 'system_dashboard' + SYSTEM_DASHBOARD_ROOT = "config/prometheus" + SYSTEM_DASHBOARD_PATH = Rails.root.join(SYSTEM_DASHBOARD_ROOT, "#{SYSTEM_DASHBOARD_NAME}#{DASHBOARD_EXTENSION}") + + def initialize(project) + @project = project + end + + # Returns a DB-supplemented json representation of a dashboard config file. + # + # param: dashboard_name [String] Filename of dashboard w/o an extension. + # If not provided, the system dashboard will be returned. + def find(dashboard_name = nil) + unless Feature.enabled?(:environment_metrics_show_multiple_dashboards, @project) + return process_dashboard(system_dashboard) + end + + dashboard = Rails.cache.fetch(cache_key(dashboard_name)) do + dashboard_name ? project_dashboard(dashboard) : system_dashboard + end + + process_dashboard(dashboard) + end + + private + + # Returns the base metrics shipped with every GitLab service. + def system_dashboard + YAML.load_file(SYSTEM_DASHBOARD_PATH) + end + + # Searches the project repo for a custom-defined dashboard. + def project_dashboard(dashboard_name) + Gitlab::Template::Finders::RepoTemplateFinder.new( + project, + DASHBOARD_ROOT, + DASHBOARD_EXTENSION + ).find(dashboard_name).read + end + + def cache_key(dashboard_name) + return "metrics_dashboard_#{SYSTEM_DASHBOARD_NAME}" unless dashboard_name + + "project_#{@project.id}_metrics_dashboard_#{dashboard_name}" + end + + # TODO: "Processing" the dashboard needs to include several steps such as + # inserting metric ids and alert information. + def process_dashboard(dashboard) + dashboard.to_json + end +end diff --git a/config/prometheus/system_dashboard.yml b/config/prometheus/system_dashboard.yml new file mode 100644 index 00000000000..694d6531034 --- /dev/null +++ b/config/prometheus/system_dashboard.yml @@ -0,0 +1,274 @@ +dashboard: 'System Metrics' +order: 0 +panel_groups: + # NGINX Ingress metrics for pre-0.16.0 versions + - group: Response metrics (NGINX Ingress VTS) + priority: 10 + panels: + - type: area-chart + title: "Throughput" + y_label: "Requests / Sec" + metrics: + - id: response_metrics_nginx_ingress_throughput_status_code + query_range: 'sum(rate(nginx_upstream_responses_total{upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) by (status_code)' + unit: req / sec + label: Status Code + required_metrics: + - nginx_upstream_responses_total + series: + - label: status_code + when: + - value: 2xx + color: green + - value: 4xx + color: orange + - value: 5xx + color: red + - type: area-chart + title: "Latency" + y_label: "Latency (ms)" + metrics: + - id: response_metrics_nginx_ingress_latency_pod_average + query_range: 'avg(nginx_upstream_response_msecs_avg{upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"})' + label: Pod average + unit: ms + required_metrics: + - nginx_upstream_response_msecs_avg + - type: area-chart + title: "HTTP Error Rate" + y_label: "HTTP Errors" + metrics: + - id: response_metrics_nginx_ingress_http_error_rate + query_range: 'sum(rate(nginx_upstream_responses_total{status_code="5xx", upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) / sum(rate(nginx_upstream_responses_total{upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) * 100' + label: 5xx Errors + unit: "%" + required_metrics: + - nginx_upstream_responses_total + # NGINX Ingress metrics for post-0.16.0 versions + - group: Response metrics (NGINX Ingress) + priority: 10 + panels: + - type: area-chart + title: "Throughput" + y_label: "Requests / Sec" + metrics: + - id: response_metrics_nginx_ingress_16_throughput_status_code + query_range: 'sum(label_replace(rate(nginx_ingress_controller_requests{namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m]), "status_code", "${1}xx", "status", "(.)..")) by (status_code)' + unit: req / sec + required_metrics: + - nginx_ingress_controller_requests + label: Status Code + series: + - label: status_code + when: + - value: 2xx + color: green + - value: 3xx + color: blue + - value: 4xx + color: orange + - value: 5xx + color: red + - type: area-chart + title: "Latency" + y_label: "Latency (ms)" + metrics: + - id: response_metrics_nginx_ingress_16_latency_pod_average + query_range: 'sum(rate(nginx_ingress_controller_ingress_upstream_latency_seconds_sum{namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m])) / sum(rate(nginx_ingress_controller_ingress_upstream_latency_seconds_count{namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m])) * 1000' + label: Pod average + unit: ms + required_metrics: + - nginx_ingress_controller_ingress_upstream_latency_seconds_sum + - type: area-chart + title: "HTTP Error Rate" + y_label: "HTTP Errors" + metrics: + - id: response_metrics_nginx_ingress_16_http_error_rate + query_range: 'sum(rate(nginx_ingress_controller_requests{status=~"5.*",namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m])) / sum(rate(nginx_ingress_controller_requests{namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m])) * 100' + label: 5xx Errors + unit: "%" + required_metrics: + - nginx_ingress_controller_requests + - group: Response metrics (HA Proxy) + priority: 10 + panels: + - type: area-chart + title: "Throughput" + y_label: "Requests / Sec" + metrics: + - id: response_metrics_ha_proxy_throughput_status_code + query_range: 'sum(rate(haproxy_frontend_http_requests_total{%{environment_filter}}[2m])) by (code)' + unit: req / sec + label: Status Code + required_metrics: + - haproxy_frontend_http_requests_total + series: + - label: status_code + when: + - value: 2xx + color: green + - value: 4xx + color: yellow + - value: 5xx + color: red + - type: area-chart + title: "HTTP Error Rate" + y_label: "Error Rate (%)" + metrics: + - id: response_metrics_ha_proxy_http_error_rate + query_range: 'sum(rate(haproxy_frontend_http_responses_total{code="5xx",%{environment_filter}}[2m])) / sum(rate(haproxy_frontend_http_responses_total{%{environment_filter}}[2m]))' + label: HTTP Errors + unit: "%" + required_metrics: + - haproxy_frontend_http_responses_total + - group: Response metrics (AWS ELB) + priority: 10 + panels: + - type: area-chart + title: "Throughput" + y_label: "Requests / Sec" + metrics: + - id: response_metrics_aws_elb_throughput_requests + query_range: 'sum(aws_elb_request_count_sum{%{environment_filter}}) / 60' + label: Total + unit: req / sec + required_metrics: + - aws_elb_request_count_sum + - type: area-chart + title: "Latency" + y_label: "Latency (ms)" + metrics: + - id: response_metrics_aws_elb_latency_average + query_range: 'avg(aws_elb_latency_average{%{environment_filter}}) * 1000' + label: Average + unit: ms + required_metrics: + - aws_elb_latency_average + - type: area-chart + title: "HTTP Error Rate" + y_label: "Error Rate (%)" + metrics: + - id: response_metrics_aws_elb_http_error_rate + query_range: 'sum(aws_elb_httpcode_backend_5_xx_sum{%{environment_filter}}) / sum(aws_elb_request_count_sum{%{environment_filter}})' + label: HTTP Errors + unit: "%" + required_metrics: + - aws_elb_request_count_sum + - aws_elb_httpcode_backend_5_xx_sum + - group: Response metrics (NGINX) + priority: 10 + panels: + - type: area-chart + title: "Throughput" + y_label: "Requests / Sec" + metrics: + - id: response_metrics_nginx_throughput_status_code + query_range: 'sum(rate(nginx_server_requests{server_zone!="*", server_zone!="_", %{environment_filter}}[2m])) by (code)' + unit: req / sec + required_metrics: + - nginx_server_requests + label: Status Code + series: + - label: status_code + when: + - value: 2xx + color: green + - value: 4xx + color: orange + - value: 5xx + color: red + - type: area-chart + title: "Latency" + y_label: "Latency (ms)" + metrics: + - id: response_metrics_nginx_latency + query_range: 'avg(nginx_server_requestMsec{%{environment_filter}})' + label: Upstream + unit: ms + required_metrics: + - nginx_server_requestMsec + - type: area-chart + title: "HTTP Error Rate" + y_label: "HTTP 500 Errors / Sec" + metrics: + - id: response_metrics_nginx_http_error_rate + query_range: 'sum(rate(nginx_server_requests{code="5xx", %{environment_filter}}[2m]))' + label: HTTP Errors + unit: "errors / sec" + required_metrics: + - nginx_server_requests + - group: System metrics (Kubernetes) + priority: 5 + panels: + - type: area-chart + title: "Memory Usage (Total)" + y_label: "Total Memory Used" + metrics: + - id: system_metrics_kubernetes_container_memory_total + query_range: 'avg(sum(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-(.*)",namespace="%{kube_namespace}"}) by (job)) without (job) /1024/1024/1024' + label: Total + unit: GB + required_metrics: + - container_memory_usage_bytes + - type: area-chart + title: "Core Usage (Total)" + y_label: "Total Cores" + metrics: + - id: system_metrics_kubernetes_container_cores_total + query_range: 'avg(sum(rate(container_cpu_usage_seconds_total{container_name!="POD",pod_name=~"^%{ci_environment_slug}-(.*)",namespace="%{kube_namespace}"}[15m])) by (job)) without (job)' + label: Total + unit: "cores" + required_metrics: + - container_cpu_usage_seconds_total + - type: area-chart + title: "Memory Usage (Pod Average)" + y_label: "Memory Used per Pod" + metrics: + - id: system_metrics_kubernetes_container_memory_average + query_range: 'avg(sum(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-([^c].*|c([^a]|a([^n]|n([^a]|a([^r]|r[^y])))).*|)-(.*)",namespace="%{kube_namespace}"}) by (job)) without (job) / count(avg(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-([^c].*|c([^a]|a([^n]|n([^a]|a([^r]|r[^y])))).*|)-(.*)",namespace="%{kube_namespace}"}) without (job)) /1024/1024' + label: Pod average + unit: MB + required_metrics: + - container_memory_usage_bytes + - type: area-chart + title: "Canary: Memory Usage (Pod Average)" + y_label: "Memory Used per Pod" + metrics: + - id: system_metrics_kubernetes_container_memory_average_canary + query_range: 'avg(sum(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-canary-(.*)",namespace="%{kube_namespace}"}) by (job)) without (job) / count(avg(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-canary-(.*)",namespace="%{kube_namespace}"}) without (job)) /1024/1024' + label: Pod average + unit: MB + required_metrics: + - container_memory_usage_bytes + track: canary + - type: area-chart + title: "Core Usage (Pod Average)" + y_label: "Cores per Pod" + metrics: + - id: system_metrics_kubernetes_container_core_usage + query_range: 'avg(sum(rate(container_cpu_usage_seconds_total{container_name!="POD",pod_name=~"^%{ci_environment_slug}-([^c].*|c([^a]|a([^n]|n([^a]|a([^r]|r[^y])))).*|)-(.*)",namespace="%{kube_namespace}"}[15m])) by (job)) without (job) / count(sum(rate(container_cpu_usage_seconds_total{container_name!="POD",pod_name=~"^%{ci_environment_slug}-([^c].*|c([^a]|a([^n]|n([^a]|a([^r]|r[^y])))).*|)-(.*)",namespace="%{kube_namespace}"}[15m])) by (pod_name))' + label: Pod average + unit: "cores" + required_metrics: + - container_cpu_usage_seconds_total + - type: area-chart + title: "Canary: Core Usage (Pod Average)" + y_label: "Cores per Pod" + metrics: + - id: system_metrics_kubernetes_container_core_usage_canary + query_range: 'avg(sum(rate(container_cpu_usage_seconds_total{container_name!="POD",pod_name=~"^%{ci_environment_slug}-canary-(.*)",namespace="%{kube_namespace}"}[15m])) by (job)) without (job) / count(sum(rate(container_cpu_usage_seconds_total{container_name!="POD",pod_name=~"^%{ci_environment_slug}-canary-(.*)",namespace="%{kube_namespace}"}[15m])) by (pod_name))' + label: Pod average + unit: "cores" + track: canary + required_metrics: + - container_cpu_usage_seconds_total + - type: area-chart + title: "Knative function invocations" + y_label: "Invocations" + metrics: + - id: system_metrics_knative_function_invocation_count + query_range: 'floor(sum(rate(istio_revision_request_count{destination_configuration="%{function_name}", destination_namespace="%{kube_namespace}"}[1m])*30))' + label: invocations / minute + unit: requests + required_metrics: + - istio_revision_request_count diff --git a/config/routes/project.rb b/config/routes/project.rb index 93d168fc595..f7841bbe595 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -218,6 +218,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :terminal get :metrics get :additional_metrics + get :metrics_dashboard get '/terminal.ws/authorize', to: 'environments#terminal_websocket_authorize', constraints: { format: nil } get '/prometheus/api/v1/*proxy_path', to: 'environments/prometheus_api#proxy' -- cgit v1.2.1 From 185ec80751c665bdde6f5494d7cb51acbb2784a4 Mon Sep 17 00:00:00 2001 From: syasonik Date: Thu, 11 Apr 2019 15:04:22 +0800 Subject: Save multi-dashboard logic for another MR --- .../projects/environments_controller.rb | 2 +- app/services/metrics_dashboard_service.rb | 41 ++++------------------ 2 files changed, 7 insertions(+), 36 deletions(-) diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 41fe70052ce..ea25ae22bcf 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -163,7 +163,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController respond_to do |format| format.json do - dashboard = MetricsDashboardService.new(@project).find(params[:dashboard]) + dashboard = MetricsDashboardService.new.get_dashboard render json: dashboard, status: :ok end diff --git a/app/services/metrics_dashboard_service.rb b/app/services/metrics_dashboard_service.rb index 532f5697be8..2f5c6cb4532 100644 --- a/app/services/metrics_dashboard_service.rb +++ b/app/services/metrics_dashboard_service.rb @@ -1,31 +1,13 @@ # frozen_string_literal: true -# Searches a projects repository for a metrics dashboard and formats the output. -# Expects any custom dashboards will be located in `.gitlab/dashboards` +# Fetches the metrics dashboard layout and supplemented the output with DB info. class MetricsDashboardService - DASHBOARD_ROOT = ".gitlab/dashboards" - DASHBOARD_EXTENSION = '.yml' - SYSTEM_DASHBOARD_NAME = 'system_dashboard' - SYSTEM_DASHBOARD_ROOT = "config/prometheus" - SYSTEM_DASHBOARD_PATH = Rails.root.join(SYSTEM_DASHBOARD_ROOT, "#{SYSTEM_DASHBOARD_NAME}#{DASHBOARD_EXTENSION}") - - def initialize(project) - @project = project - end + SYSTEM_DASHBOARD_PATH = Rails.root.join("config/prometheus", "#{SYSTEM_DASHBOARD_NAME}.yml") # Returns a DB-supplemented json representation of a dashboard config file. - # - # param: dashboard_name [String] Filename of dashboard w/o an extension. - # If not provided, the system dashboard will be returned. - def find(dashboard_name = nil) - unless Feature.enabled?(:environment_metrics_show_multiple_dashboards, @project) - return process_dashboard(system_dashboard) - end - - dashboard = Rails.cache.fetch(cache_key(dashboard_name)) do - dashboard_name ? project_dashboard(dashboard) : system_dashboard - end + def get_dashboard + dashboard = Rails.cache.fetch(cache_key) { system_dashboard } process_dashboard(dashboard) end @@ -37,19 +19,8 @@ class MetricsDashboardService YAML.load_file(SYSTEM_DASHBOARD_PATH) end - # Searches the project repo for a custom-defined dashboard. - def project_dashboard(dashboard_name) - Gitlab::Template::Finders::RepoTemplateFinder.new( - project, - DASHBOARD_ROOT, - DASHBOARD_EXTENSION - ).find(dashboard_name).read - end - - def cache_key(dashboard_name) - return "metrics_dashboard_#{SYSTEM_DASHBOARD_NAME}" unless dashboard_name - - "project_#{@project.id}_metrics_dashboard_#{dashboard_name}" + def cache_key + "metrics_dashboard_#{SYSTEM_DASHBOARD_NAME}" end # TODO: "Processing" the dashboard needs to include several steps such as -- cgit v1.2.1 From 7eb796e6b1b1c6e3ceeb85746bc7d9e8f9a519de Mon Sep 17 00:00:00 2001 From: syasonik Date: Thu, 11 Apr 2019 20:36:47 +0800 Subject: Inject dashboard response with db info --- .../metrics_dashboard_processing_service.rb | 88 ++++++++++++++++++++++ app/services/metrics_dashboard_service.rb | 8 +- 2 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 app/services/metrics_dashboard_processing_service.rb diff --git a/app/services/metrics_dashboard_processing_service.rb b/app/services/metrics_dashboard_processing_service.rb new file mode 100644 index 00000000000..2bfeccc2644 --- /dev/null +++ b/app/services/metrics_dashboard_processing_service.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +class MetricsDashboardProcessingService + DEFAULT_PANEL_TYPE = 'area-chart' + + def initialize(dashboard, project) + @dashboard = dashboard.deep_transform_keys(&:to_sym) + @project = project + end + + def process + insert_persisted_metrics! + insert_metric_ids! + @dashboard.to_json + end + + private + + # Inserts project-specific metrics into the dashboard config. + # If there are no project-specific metrics, this will have no effect. + def insert_persisted_metrics! + @project.prometheus_metrics.each do |persisted_metric| + group = find_or_create_group(@dashboard[:panel_groups], persisted_metric) + panel = find_or_create_panel(group[:panels], persisted_metric) + find_or_create_metric(panel[:metrics], persisted_metric) + end + end + + # For each metric in the dashboard config, attempts to find a corresponding + # persisted record. If found, includes the record id in the config. + def insert_metric_ids! + @dashboard[:panel_groups].each do |group| + group[:panels].each do |panel| + panel[:metrics].each do |metric| + metric_record = common_metrics.find {|m| m.identifier == metric[:id] } + metric[:metric_id] = metric_record.id if metric_record + end + end + end + end + + def common_metrics + @common_metrics ||= ::PrometheusMetric.common + end + + def find_or_create_group(panel_groups, metric) + target_group = panel_groups.find { |group| group[:group] == metric.group_title } + + unless target_group + target_group = { + group: metric.group_title, + priority: metric.priority, + panels: [] + } + panel_groups << target_group + end + + target_group + end + + def find_or_create_panel(panels, metric) + panel_identifiers = [DEFAULT_PANEL_TYPE, metric.title, metric.y_label] + target_panel = panels.find { |panel| panel.values_at(:type, :title, :y_label) == panel_identifiers } + + unless target_panel + target_panel = { + type: DEFAULT_PANEL_TYPE, + title: metric.title, + y_label: metric.y_label, + metrics: [] + } + panels << target_panel + end + + target_panel + end + + def find_or_create_metric(metrics, metric) + target_metric = metrics.find { |m| m[:id] == metric.identifier } + + unless target_metric + target_metric = metric.queries.first.merge(metric_id: metric.id) + metrics << target_metric + end + + target_metric + end +end diff --git a/app/services/metrics_dashboard_service.rb b/app/services/metrics_dashboard_service.rb index 2f5c6cb4532..3502c1ca221 100644 --- a/app/services/metrics_dashboard_service.rb +++ b/app/services/metrics_dashboard_service.rb @@ -3,7 +3,11 @@ # Fetches the metrics dashboard layout and supplemented the output with DB info. class MetricsDashboardService SYSTEM_DASHBOARD_NAME = 'system_dashboard' - SYSTEM_DASHBOARD_PATH = Rails.root.join("config/prometheus", "#{SYSTEM_DASHBOARD_NAME}.yml") + SYSTEM_DASHBOARD_PATH = Rails.root.join('config', 'prometheus', "#{SYSTEM_DASHBOARD_NAME}.yml") + + def initialize(project) + @project = project + end # Returns a DB-supplemented json representation of a dashboard config file. def get_dashboard @@ -26,6 +30,6 @@ class MetricsDashboardService # TODO: "Processing" the dashboard needs to include several steps such as # inserting metric ids and alert information. def process_dashboard(dashboard) - dashboard.to_json + MetricsDashboardProcessingService.new(dashboard, @project).process end end -- cgit v1.2.1 From 18183f404e492bcdb9818e2fb29b50dafdbca247 Mon Sep 17 00:00:00 2001 From: syasonik Date: Tue, 16 Apr 2019 15:00:51 +0800 Subject: Use existing common metrics --- .../projects/environments_controller.rb | 1 - app/services/metrics_dashboard_service.rb | 2 +- config/prometheus/system_dashboard.yml | 274 --------------------- 3 files changed, 1 insertion(+), 276 deletions(-) delete mode 100644 config/prometheus/system_dashboard.yml diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index ea25ae22bcf..b10f0c7b7db 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -13,7 +13,6 @@ class Projects::EnvironmentsController < Projects::ApplicationController before_action only: [:metrics, :additional_metrics] do push_frontend_feature_flag(:metrics_time_window) push_frontend_feature_flag(:environment_metrics_use_prometheus_endpoint) - push_frontend_feature_flag(:environment_metrics_show_multiple_dashboards) end def index diff --git a/app/services/metrics_dashboard_service.rb b/app/services/metrics_dashboard_service.rb index 3502c1ca221..92aeafa5eea 100644 --- a/app/services/metrics_dashboard_service.rb +++ b/app/services/metrics_dashboard_service.rb @@ -2,7 +2,7 @@ # Fetches the metrics dashboard layout and supplemented the output with DB info. class MetricsDashboardService - SYSTEM_DASHBOARD_NAME = 'system_dashboard' + SYSTEM_DASHBOARD_NAME = 'common_metrics' SYSTEM_DASHBOARD_PATH = Rails.root.join('config', 'prometheus', "#{SYSTEM_DASHBOARD_NAME}.yml") def initialize(project) diff --git a/config/prometheus/system_dashboard.yml b/config/prometheus/system_dashboard.yml deleted file mode 100644 index 694d6531034..00000000000 --- a/config/prometheus/system_dashboard.yml +++ /dev/null @@ -1,274 +0,0 @@ -dashboard: 'System Metrics' -order: 0 -panel_groups: - # NGINX Ingress metrics for pre-0.16.0 versions - - group: Response metrics (NGINX Ingress VTS) - priority: 10 - panels: - - type: area-chart - title: "Throughput" - y_label: "Requests / Sec" - metrics: - - id: response_metrics_nginx_ingress_throughput_status_code - query_range: 'sum(rate(nginx_upstream_responses_total{upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) by (status_code)' - unit: req / sec - label: Status Code - required_metrics: - - nginx_upstream_responses_total - series: - - label: status_code - when: - - value: 2xx - color: green - - value: 4xx - color: orange - - value: 5xx - color: red - - type: area-chart - title: "Latency" - y_label: "Latency (ms)" - metrics: - - id: response_metrics_nginx_ingress_latency_pod_average - query_range: 'avg(nginx_upstream_response_msecs_avg{upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"})' - label: Pod average - unit: ms - required_metrics: - - nginx_upstream_response_msecs_avg - - type: area-chart - title: "HTTP Error Rate" - y_label: "HTTP Errors" - metrics: - - id: response_metrics_nginx_ingress_http_error_rate - query_range: 'sum(rate(nginx_upstream_responses_total{status_code="5xx", upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) / sum(rate(nginx_upstream_responses_total{upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) * 100' - label: 5xx Errors - unit: "%" - required_metrics: - - nginx_upstream_responses_total - # NGINX Ingress metrics for post-0.16.0 versions - - group: Response metrics (NGINX Ingress) - priority: 10 - panels: - - type: area-chart - title: "Throughput" - y_label: "Requests / Sec" - metrics: - - id: response_metrics_nginx_ingress_16_throughput_status_code - query_range: 'sum(label_replace(rate(nginx_ingress_controller_requests{namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m]), "status_code", "${1}xx", "status", "(.)..")) by (status_code)' - unit: req / sec - required_metrics: - - nginx_ingress_controller_requests - label: Status Code - series: - - label: status_code - when: - - value: 2xx - color: green - - value: 3xx - color: blue - - value: 4xx - color: orange - - value: 5xx - color: red - - type: area-chart - title: "Latency" - y_label: "Latency (ms)" - metrics: - - id: response_metrics_nginx_ingress_16_latency_pod_average - query_range: 'sum(rate(nginx_ingress_controller_ingress_upstream_latency_seconds_sum{namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m])) / sum(rate(nginx_ingress_controller_ingress_upstream_latency_seconds_count{namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m])) * 1000' - label: Pod average - unit: ms - required_metrics: - - nginx_ingress_controller_ingress_upstream_latency_seconds_sum - - type: area-chart - title: "HTTP Error Rate" - y_label: "HTTP Errors" - metrics: - - id: response_metrics_nginx_ingress_16_http_error_rate - query_range: 'sum(rate(nginx_ingress_controller_requests{status=~"5.*",namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m])) / sum(rate(nginx_ingress_controller_requests{namespace="%{kube_namespace}",ingress=~".*%{ci_environment_slug}.*"}[2m])) * 100' - label: 5xx Errors - unit: "%" - required_metrics: - - nginx_ingress_controller_requests - - group: Response metrics (HA Proxy) - priority: 10 - panels: - - type: area-chart - title: "Throughput" - y_label: "Requests / Sec" - metrics: - - id: response_metrics_ha_proxy_throughput_status_code - query_range: 'sum(rate(haproxy_frontend_http_requests_total{%{environment_filter}}[2m])) by (code)' - unit: req / sec - label: Status Code - required_metrics: - - haproxy_frontend_http_requests_total - series: - - label: status_code - when: - - value: 2xx - color: green - - value: 4xx - color: yellow - - value: 5xx - color: red - - type: area-chart - title: "HTTP Error Rate" - y_label: "Error Rate (%)" - metrics: - - id: response_metrics_ha_proxy_http_error_rate - query_range: 'sum(rate(haproxy_frontend_http_responses_total{code="5xx",%{environment_filter}}[2m])) / sum(rate(haproxy_frontend_http_responses_total{%{environment_filter}}[2m]))' - label: HTTP Errors - unit: "%" - required_metrics: - - haproxy_frontend_http_responses_total - - group: Response metrics (AWS ELB) - priority: 10 - panels: - - type: area-chart - title: "Throughput" - y_label: "Requests / Sec" - metrics: - - id: response_metrics_aws_elb_throughput_requests - query_range: 'sum(aws_elb_request_count_sum{%{environment_filter}}) / 60' - label: Total - unit: req / sec - required_metrics: - - aws_elb_request_count_sum - - type: area-chart - title: "Latency" - y_label: "Latency (ms)" - metrics: - - id: response_metrics_aws_elb_latency_average - query_range: 'avg(aws_elb_latency_average{%{environment_filter}}) * 1000' - label: Average - unit: ms - required_metrics: - - aws_elb_latency_average - - type: area-chart - title: "HTTP Error Rate" - y_label: "Error Rate (%)" - metrics: - - id: response_metrics_aws_elb_http_error_rate - query_range: 'sum(aws_elb_httpcode_backend_5_xx_sum{%{environment_filter}}) / sum(aws_elb_request_count_sum{%{environment_filter}})' - label: HTTP Errors - unit: "%" - required_metrics: - - aws_elb_request_count_sum - - aws_elb_httpcode_backend_5_xx_sum - - group: Response metrics (NGINX) - priority: 10 - panels: - - type: area-chart - title: "Throughput" - y_label: "Requests / Sec" - metrics: - - id: response_metrics_nginx_throughput_status_code - query_range: 'sum(rate(nginx_server_requests{server_zone!="*", server_zone!="_", %{environment_filter}}[2m])) by (code)' - unit: req / sec - required_metrics: - - nginx_server_requests - label: Status Code - series: - - label: status_code - when: - - value: 2xx - color: green - - value: 4xx - color: orange - - value: 5xx - color: red - - type: area-chart - title: "Latency" - y_label: "Latency (ms)" - metrics: - - id: response_metrics_nginx_latency - query_range: 'avg(nginx_server_requestMsec{%{environment_filter}})' - label: Upstream - unit: ms - required_metrics: - - nginx_server_requestMsec - - type: area-chart - title: "HTTP Error Rate" - y_label: "HTTP 500 Errors / Sec" - metrics: - - id: response_metrics_nginx_http_error_rate - query_range: 'sum(rate(nginx_server_requests{code="5xx", %{environment_filter}}[2m]))' - label: HTTP Errors - unit: "errors / sec" - required_metrics: - - nginx_server_requests - - group: System metrics (Kubernetes) - priority: 5 - panels: - - type: area-chart - title: "Memory Usage (Total)" - y_label: "Total Memory Used" - metrics: - - id: system_metrics_kubernetes_container_memory_total - query_range: 'avg(sum(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-(.*)",namespace="%{kube_namespace}"}) by (job)) without (job) /1024/1024/1024' - label: Total - unit: GB - required_metrics: - - container_memory_usage_bytes - - type: area-chart - title: "Core Usage (Total)" - y_label: "Total Cores" - metrics: - - id: system_metrics_kubernetes_container_cores_total - query_range: 'avg(sum(rate(container_cpu_usage_seconds_total{container_name!="POD",pod_name=~"^%{ci_environment_slug}-(.*)",namespace="%{kube_namespace}"}[15m])) by (job)) without (job)' - label: Total - unit: "cores" - required_metrics: - - container_cpu_usage_seconds_total - - type: area-chart - title: "Memory Usage (Pod Average)" - y_label: "Memory Used per Pod" - metrics: - - id: system_metrics_kubernetes_container_memory_average - query_range: 'avg(sum(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-([^c].*|c([^a]|a([^n]|n([^a]|a([^r]|r[^y])))).*|)-(.*)",namespace="%{kube_namespace}"}) by (job)) without (job) / count(avg(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-([^c].*|c([^a]|a([^n]|n([^a]|a([^r]|r[^y])))).*|)-(.*)",namespace="%{kube_namespace}"}) without (job)) /1024/1024' - label: Pod average - unit: MB - required_metrics: - - container_memory_usage_bytes - - type: area-chart - title: "Canary: Memory Usage (Pod Average)" - y_label: "Memory Used per Pod" - metrics: - - id: system_metrics_kubernetes_container_memory_average_canary - query_range: 'avg(sum(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-canary-(.*)",namespace="%{kube_namespace}"}) by (job)) without (job) / count(avg(container_memory_usage_bytes{container_name!="POD",pod_name=~"^%{ci_environment_slug}-canary-(.*)",namespace="%{kube_namespace}"}) without (job)) /1024/1024' - label: Pod average - unit: MB - required_metrics: - - container_memory_usage_bytes - track: canary - - type: area-chart - title: "Core Usage (Pod Average)" - y_label: "Cores per Pod" - metrics: - - id: system_metrics_kubernetes_container_core_usage - query_range: 'avg(sum(rate(container_cpu_usage_seconds_total{container_name!="POD",pod_name=~"^%{ci_environment_slug}-([^c].*|c([^a]|a([^n]|n([^a]|a([^r]|r[^y])))).*|)-(.*)",namespace="%{kube_namespace}"}[15m])) by (job)) without (job) / count(sum(rate(container_cpu_usage_seconds_total{container_name!="POD",pod_name=~"^%{ci_environment_slug}-([^c].*|c([^a]|a([^n]|n([^a]|a([^r]|r[^y])))).*|)-(.*)",namespace="%{kube_namespace}"}[15m])) by (pod_name))' - label: Pod average - unit: "cores" - required_metrics: - - container_cpu_usage_seconds_total - - type: area-chart - title: "Canary: Core Usage (Pod Average)" - y_label: "Cores per Pod" - metrics: - - id: system_metrics_kubernetes_container_core_usage_canary - query_range: 'avg(sum(rate(container_cpu_usage_seconds_total{container_name!="POD",pod_name=~"^%{ci_environment_slug}-canary-(.*)",namespace="%{kube_namespace}"}[15m])) by (job)) without (job) / count(sum(rate(container_cpu_usage_seconds_total{container_name!="POD",pod_name=~"^%{ci_environment_slug}-canary-(.*)",namespace="%{kube_namespace}"}[15m])) by (pod_name))' - label: Pod average - unit: "cores" - track: canary - required_metrics: - - container_cpu_usage_seconds_total - - type: area-chart - title: "Knative function invocations" - y_label: "Invocations" - metrics: - - id: system_metrics_knative_function_invocation_count - query_range: 'floor(sum(rate(istio_revision_request_count{destination_configuration="%{function_name}", destination_namespace="%{kube_namespace}"}[1m])*30))' - label: invocations / minute - unit: requests - required_metrics: - - istio_revision_request_count -- cgit v1.2.1 From 3d302879f431aed213c0b9d6308c7dff9a9c3958 Mon Sep 17 00:00:00 2001 From: syasonik Date: Tue, 16 Apr 2019 15:01:43 +0800 Subject: Add unit tests and fix broken endpoint --- .../projects/environments_controller.rb | 4 +- .../metrics_dashboard_processing_service.rb | 58 ++++++++++++++++------ .../projects/environments_controller_spec.rb | 24 +++++++++ .../metrics_dashboard_processing_service.yml | 36 ++++++++++++++ .../metrics_dashboard_processing_service_spec.rb | 56 +++++++++++++++++++++ spec/services/metrics_dashboard_service_spec.rb | 22 ++++++++ 6 files changed, 183 insertions(+), 17 deletions(-) create mode 100644 spec/fixtures/services/metrics_dashboard_processing_service.yml create mode 100644 spec/services/metrics_dashboard_processing_service_spec.rb create mode 100644 spec/services/metrics_dashboard_service_spec.rb diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index b10f0c7b7db..da35d8d37f7 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -158,11 +158,11 @@ class Projects::EnvironmentsController < Projects::ApplicationController end def metrics_dashboard - access_denied! unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, project) + render_403 and return unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, project) respond_to do |format| format.json do - dashboard = MetricsDashboardService.new.get_dashboard + dashboard = MetricsDashboardService.new(@project).get_dashboard render json: dashboard, status: :ok end diff --git a/app/services/metrics_dashboard_processing_service.rb b/app/services/metrics_dashboard_processing_service.rb index 2bfeccc2644..6dc8db00934 100644 --- a/app/services/metrics_dashboard_processing_service.rb +++ b/app/services/metrics_dashboard_processing_service.rb @@ -9,40 +9,68 @@ class MetricsDashboardProcessingService end def process - insert_persisted_metrics! insert_metric_ids! + sort_groups! + sort_panels! + insert_project_metrics! @dashboard.to_json end private + # ------- Processing Steps ----------- + + # For each metric in the dashboard config, attempts to find a corresponding + # database record. If found, includes the record's id in the dashboard config. + def insert_metric_ids! + for_metrics do |metric| + metric_record = common_metrics.find { |m| m.identifier == metric[:id] } + metric[:metric_id] = metric_record.id if metric_record + end + end + # Inserts project-specific metrics into the dashboard config. # If there are no project-specific metrics, this will have no effect. - def insert_persisted_metrics! - @project.prometheus_metrics.each do |persisted_metric| - group = find_or_create_group(@dashboard[:panel_groups], persisted_metric) - panel = find_or_create_panel(group[:panels], persisted_metric) - find_or_create_metric(panel[:metrics], persisted_metric) + def insert_project_metrics! + project_metrics.each do |project_metric| + group = find_or_create_group(@dashboard[:panel_groups], project_metric) + panel = find_or_create_panel(group[:panels], project_metric) + find_or_create_metric(panel[:metrics], project_metric) end end - # For each metric in the dashboard config, attempts to find a corresponding - # persisted record. If found, includes the record id in the config. - def insert_metric_ids! + # Sorts the groups in the dashboard by the :priority key + def sort_groups! + @dashboard[:panel_groups] = @dashboard[:panel_groups].sort_by { |group| group[:priority] } + end + + # Sorts the panels in the dashboard by the :weight key + def sort_panels! @dashboard[:panel_groups].each do |group| - group[:panels].each do |panel| - panel[:metrics].each do |metric| - metric_record = common_metrics.find {|m| m.identifier == metric[:id] } - metric[:metric_id] = metric_record.id if metric_record - end - end + group[:panels] = group[:panels].sort_by { |panel| panel[:weight] } end end + # ------- Processing Helpers ----------- + + def project_metrics + @project.prometheus_metrics + end + def common_metrics @common_metrics ||= ::PrometheusMetric.common end + def for_metrics + @dashboard[:panel_groups].each do |panel_group| + panel_group[:panels].each do |panel| + panel[:metrics].each do |metric| + yield metric + end + end + end + end + def find_or_create_group(panel_groups, metric) target_group = panel_groups.find { |group| group[:group] == metric.group_title } diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 75158f2e8e0..02441385da0 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -461,6 +461,30 @@ describe Projects::EnvironmentsController do end end + describe 'metrics_dashboard' do + context 'when prometheus endpoint is disabled' do + before do + stub_feature_flags(environment_metrics_use_prometheus_endpoint: false) + end + + it 'responds with status code 403' do + get :metrics_dashboard, params: environment_params(format: :json) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when prometheus endpoint is enabled' do + it 'returns a json representation of the environment dashboard' do + get :metrics_dashboard, params: environment_params(format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to include('dashboard', 'order', 'panel_groups') + expect(json_response['panel_groups']).to all( include('group', 'priority', 'panels') ) + end + end + end + describe 'GET #search' do before do create(:environment, name: 'staging', project: project) diff --git a/spec/fixtures/services/metrics_dashboard_processing_service.yml b/spec/fixtures/services/metrics_dashboard_processing_service.yml new file mode 100644 index 00000000000..ebfe06da6db --- /dev/null +++ b/spec/fixtures/services/metrics_dashboard_processing_service.yml @@ -0,0 +1,36 @@ +dashboard: 'Test Dashboard' +order: 1 +panel_groups: +- group: Group A + priority: 10 + panels: + - title: "Super Chart A1" + type: "area-chart" + y_label: "y_label" + weight: 2 + metrics: + - id: metric_a1 + query_range: 'query' + unit: unit + label: Legend Label + - title: "Super Chart A2" + type: "area-chart" + y_label: "y_label" + weight: 1 + metrics: + - id: metric_a2 + query_range: 'query' + label: Legend Label + unit: unit +- group: Group B + priority: 1 + panels: + - title: "Super Chart B" + type: "area-chart" + y_label: "y_label" + weight: 1 + metrics: + - id: metric_b + query_range: 'query' + unit: unit + label: Legend Label diff --git a/spec/services/metrics_dashboard_processing_service_spec.rb b/spec/services/metrics_dashboard_processing_service_spec.rb new file mode 100644 index 00000000000..658ff1dab49 --- /dev/null +++ b/spec/services/metrics_dashboard_processing_service_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +describe MetricsDashboardProcessingService do + let(:project) { build(:project) } + let(:dashboard_yml) { YAML.load_file('spec/fixtures/services/metrics_dashboard_processing_service.yml') } + + describe 'process' do + let(:dashboard) { JSON.parse(described_class.new(dashboard_yml, project).process, symbolize_names: true) } + + context 'when dashboard config corresponds to common metrics' do + let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') } + + it 'inserts metric ids into the config' do + target_metric = all_metrics.find { |metric| metric[:id] == 'metric_a1' } + + expect(target_metric).to include(:metric_id) + end + end + + context 'when the project has associated metrics' do + let!(:project_metric) { create(:prometheus_metric, project: project) } + + it 'includes project-specific metrics' do + + project_metric_details = { + query_range: project_metric.query, + unit: project_metric.unit, + label: project_metric.legend, + metric_id: project_metric.id, + } + + expect(all_metrics).to include project_metric_details + end + + it 'includes project metrics at the end of the config' do + expected_metrics_order = ['metric_b', 'metric_a2', 'metric_a1', nil] + actual_metrics_order = all_metrics.map { |m| m[:id] } + + expect(actual_metrics_order).to eq expected_metrics_order + end + end + + it 'orders groups by priority and panels by weight' do + expected_metrics_order = ['metric_b', 'metric_a2', 'metric_a1'] + actual_metrics_order = all_metrics.map { |m| m[:id] } + + expect(actual_metrics_order).to eq expected_metrics_order + end + end + + def all_metrics + dashboard[:panel_groups].map do |group| + group[:panels].map { |panel| panel[:metrics] } + end.flatten + end +end diff --git a/spec/services/metrics_dashboard_service_spec.rb b/spec/services/metrics_dashboard_service_spec.rb new file mode 100644 index 00000000000..9b7994fa34f --- /dev/null +++ b/spec/services/metrics_dashboard_service_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe MetricsDashboardService, :use_clean_rails_memory_store_caching do + let(:project) { build(:project) } + + describe 'get_dashboard' do + it 'returns a json representation of the environment dashboard' do + dashboard = described_class.new(project).get_dashboard + json = JSON.parse(dashboard, symbolize_names: true) + + expect(json).to include(:dashboard, :order, :panel_groups) + expect(json[:panel_groups]).to all( include(:group, :priority, :panels) ) + end + + it 'caches the dashboard for subsequent calls' do + expect(YAML).to receive(:load_file).once.and_call_original + + described_class.new(project).get_dashboard + described_class.new(project).get_dashboard + end + end +end -- cgit v1.2.1 From c9253c3eeb29cf163b0bd387ceb44aa7629be347 Mon Sep 17 00:00:00 2001 From: syasonik Date: Tue, 16 Apr 2019 15:15:57 +0800 Subject: Rubocop cleanup --- app/controllers/projects/environments_controller.rb | 2 +- spec/services/metrics_dashboard_processing_service_spec.rb | 7 +++---- spec/services/metrics_dashboard_service_spec.rb | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index da35d8d37f7..db0253a9183 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -158,7 +158,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController end def metrics_dashboard - render_403 and return unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, project) + render_403 && return unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, project) respond_to do |format| format.json do diff --git a/spec/services/metrics_dashboard_processing_service_spec.rb b/spec/services/metrics_dashboard_processing_service_spec.rb index 658ff1dab49..917ae582afe 100644 --- a/spec/services/metrics_dashboard_processing_service_spec.rb +++ b/spec/services/metrics_dashboard_processing_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe MetricsDashboardProcessingService do - let(:project) { build(:project) } + let(:project) { build(:project) } let(:dashboard_yml) { YAML.load_file('spec/fixtures/services/metrics_dashboard_processing_service.yml') } describe 'process' do @@ -21,12 +21,11 @@ describe MetricsDashboardProcessingService do let!(:project_metric) { create(:prometheus_metric, project: project) } it 'includes project-specific metrics' do - project_metric_details = { query_range: project_metric.query, unit: project_metric.unit, label: project_metric.legend, - metric_id: project_metric.id, + metric_id: project_metric.id } expect(all_metrics).to include project_metric_details @@ -41,7 +40,7 @@ describe MetricsDashboardProcessingService do end it 'orders groups by priority and panels by weight' do - expected_metrics_order = ['metric_b', 'metric_a2', 'metric_a1'] + expected_metrics_order = %w('metric_b metric_a2 metric_a1') actual_metrics_order = all_metrics.map { |m| m[:id] } expect(actual_metrics_order).to eq expected_metrics_order diff --git a/spec/services/metrics_dashboard_service_spec.rb b/spec/services/metrics_dashboard_service_spec.rb index 9b7994fa34f..db59ec3a3a4 100644 --- a/spec/services/metrics_dashboard_service_spec.rb +++ b/spec/services/metrics_dashboard_service_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe MetricsDashboardService, :use_clean_rails_memory_store_caching do - let(:project) { build(:project) } + let(:project) { build(:project) } describe 'get_dashboard' do it 'returns a json representation of the environment dashboard' do -- cgit v1.2.1 From d307f446d807b03ddc7d55705df5602c275e61fb Mon Sep 17 00:00:00 2001 From: syasonik Date: Tue, 16 Apr 2019 15:38:11 +0800 Subject: Remove obsolete TODO comment --- app/services/metrics_dashboard_service.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/services/metrics_dashboard_service.rb b/app/services/metrics_dashboard_service.rb index 92aeafa5eea..b72095744b2 100644 --- a/app/services/metrics_dashboard_service.rb +++ b/app/services/metrics_dashboard_service.rb @@ -27,8 +27,6 @@ class MetricsDashboardService "metrics_dashboard_#{SYSTEM_DASHBOARD_NAME}" end - # TODO: "Processing" the dashboard needs to include several steps such as - # inserting metric ids and alert information. def process_dashboard(dashboard) MetricsDashboardProcessingService.new(dashboard, @project).process end -- cgit v1.2.1 From 35c412327cd0a0897d4d81aa80c88782c98fdd89 Mon Sep 17 00:00:00 2001 From: syasonik Date: Tue, 16 Apr 2019 17:09:10 +0800 Subject: Refactor dashboard proccesing into stages --- .../projects/environments_controller.rb | 2 +- app/services/metrics_dashboard_service.rb | 33 ---------- .../metrics_dashboard/common_metrics_inserter.rb | 32 ++++++++++ lib/gitlab/metrics_dashboard/processor.rb | 20 +++++++ .../metrics_dashboard/project_metrics_inserter.rb | 70 ++++++++++++++++++++++ lib/gitlab/metrics_dashboard/service.rb | 37 ++++++++++++ lib/gitlab/metrics_dashboard/sorter.rb | 26 ++++++++ .../lib/gitlab/metrics_dashboard/processor_spec.rb | 55 +++++++++++++++++ spec/lib/gitlab/metrics_dashboard/service_spec.rb | 22 +++++++ .../metrics_dashboard_processing_service_spec.rb | 55 ----------------- spec/services/metrics_dashboard_service_spec.rb | 22 ------- 11 files changed, 263 insertions(+), 111 deletions(-) delete mode 100644 app/services/metrics_dashboard_service.rb create mode 100644 lib/gitlab/metrics_dashboard/common_metrics_inserter.rb create mode 100644 lib/gitlab/metrics_dashboard/processor.rb create mode 100644 lib/gitlab/metrics_dashboard/project_metrics_inserter.rb create mode 100644 lib/gitlab/metrics_dashboard/service.rb create mode 100644 lib/gitlab/metrics_dashboard/sorter.rb create mode 100644 spec/lib/gitlab/metrics_dashboard/processor_spec.rb create mode 100644 spec/lib/gitlab/metrics_dashboard/service_spec.rb delete mode 100644 spec/services/metrics_dashboard_processing_service_spec.rb delete mode 100644 spec/services/metrics_dashboard_service_spec.rb diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index db0253a9183..69ceeab5b99 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -162,7 +162,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController respond_to do |format| format.json do - dashboard = MetricsDashboardService.new(@project).get_dashboard + dashboard = Gitlab::MetricsDashboard::Service.new(@project).get_dashboard render json: dashboard, status: :ok end diff --git a/app/services/metrics_dashboard_service.rb b/app/services/metrics_dashboard_service.rb deleted file mode 100644 index b72095744b2..00000000000 --- a/app/services/metrics_dashboard_service.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -# Fetches the metrics dashboard layout and supplemented the output with DB info. -class MetricsDashboardService - SYSTEM_DASHBOARD_NAME = 'common_metrics' - SYSTEM_DASHBOARD_PATH = Rails.root.join('config', 'prometheus', "#{SYSTEM_DASHBOARD_NAME}.yml") - - def initialize(project) - @project = project - end - - # Returns a DB-supplemented json representation of a dashboard config file. - def get_dashboard - dashboard = Rails.cache.fetch(cache_key) { system_dashboard } - - process_dashboard(dashboard) - end - - private - - # Returns the base metrics shipped with every GitLab service. - def system_dashboard - YAML.load_file(SYSTEM_DASHBOARD_PATH) - end - - def cache_key - "metrics_dashboard_#{SYSTEM_DASHBOARD_NAME}" - end - - def process_dashboard(dashboard) - MetricsDashboardProcessingService.new(dashboard, @project).process - end -end diff --git a/lib/gitlab/metrics_dashboard/common_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/common_metrics_inserter.rb new file mode 100644 index 00000000000..5bc83e6266e --- /dev/null +++ b/lib/gitlab/metrics_dashboard/common_metrics_inserter.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Gitlab + module MetricsDashboard + class CommonMetricsInserter + class << self + # For each metric in the dashboard config, attempts to find a corresponding + # database record. If found, includes the record's id in the dashboard config. + def transform!(dashboard, _project) + common_metrics = ::PrometheusMetric.common + + for_metrics(dashboard) do |metric| + metric_record = common_metrics.find { |m| m.identifier == metric[:id] } + metric[:metric_id] = metric_record.id if metric_record + end + end + + private + + def for_metrics(dashboard) + dashboard[:panel_groups].each do |panel_group| + panel_group[:panels].each do |panel| + panel[:metrics].each do |metric| + yield metric + end + end + end + end + end + end + end +end diff --git a/lib/gitlab/metrics_dashboard/processor.rb b/lib/gitlab/metrics_dashboard/processor.rb new file mode 100644 index 00000000000..9e254aa2b37 --- /dev/null +++ b/lib/gitlab/metrics_dashboard/processor.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Gitlab + module MetricsDashboard + class Processor + STAGES = [CommonMetricsInserter, Sorter, ProjectMetricsInserter] + + def initialize(dashboard, project) + @dashboard = dashboard.deep_transform_keys(&:to_sym) + @project = project + end + + def process + STAGES.each { |stage| stage.transform!(@dashboard, @project) } + + @dashboard.to_json + end + end + end +end diff --git a/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb new file mode 100644 index 00000000000..67300c79e57 --- /dev/null +++ b/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +module Gitlab + module MetricsDashboard + class ProjectMetricsInserter + DEFAULT_PANEL_TYPE = 'area-chart' + + class << self + # Inserts project-specific metrics into the dashboard config. + # If there are no project-specific metrics, this will have no effect. + def transform!(dashboard, project) + project.prometheus_metrics.each do |project_metric| + group = find_or_create(:panel_group, dashboard[:panel_groups], project_metric) + panel = find_or_create(:panel, group[:panels], project_metric) + find_or_create(:metric, panel[:metrics], project_metric) + end + end + + # Looks for an instance of the named resource corresponding to the provided + # metric object. If unavailable, inserts one. + # @param name [Symbol, String] One of :panel_group, :panel, or :metric + # @param existing_resources [Array] + # @param metric [PrometheusMetric] + def find_or_create(name, existing_resources, metric) + target = self.send("find_#{name}", existing_resources, metric) + return target if target + + target = self.send("new_#{name}", metric) + existing_resources << target + + target + end + + def find_panel_group(panel_groups, metric) + panel_groups.find { |group| group[:group] == metric.group_title } + end + + def find_panel(panels, metric) + panel_identifiers = [DEFAULT_PANEL_TYPE, metric.title, metric.y_label] + target_panel = panels.find { |panel| panel.values_at(:type, :title, :y_label) == panel_identifiers } + end + + def find_metric(metrics, metric) + metrics.find { |m| m[:id] == metric.identifier } + end + + def new_panel_group(metric) + { + group: metric.group_title, + priority: metric.priority, + panels: [] + } + end + + def new_panel(metric) + { + type: DEFAULT_PANEL_TYPE, + title: metric.title, + y_label: metric.y_label, + metrics: [] + } + end + + def new_metric(metric) + metric.queries.first.merge(metric_id: metric.id) + end + end + end + end +end diff --git a/lib/gitlab/metrics_dashboard/service.rb b/lib/gitlab/metrics_dashboard/service.rb new file mode 100644 index 00000000000..22e1771cbea --- /dev/null +++ b/lib/gitlab/metrics_dashboard/service.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +# Fetches the metrics dashboard layout and supplemented the output with DB info. +module Gitlab + module MetricsDashboard + class Service + SYSTEM_DASHBOARD_NAME = 'common_metrics' + SYSTEM_DASHBOARD_PATH = Rails.root.join('config', 'prometheus', "#{SYSTEM_DASHBOARD_NAME}.yml") + + def initialize(project) + @project = project + end + + # Returns a DB-supplemented json representation of a dashboard config file. + def get_dashboard + dashboard = Rails.cache.fetch(cache_key) { system_dashboard } + + process_dashboard(dashboard) + end + + private + + # Returns the base metrics shipped with every GitLab service. + def system_dashboard + YAML.load_file(SYSTEM_DASHBOARD_PATH) + end + + def cache_key + "metrics_dashboard_#{SYSTEM_DASHBOARD_NAME}" + end + + def process_dashboard(dashboard) + Processor.new(dashboard, @project).process + end + end + end +end diff --git a/lib/gitlab/metrics_dashboard/sorter.rb b/lib/gitlab/metrics_dashboard/sorter.rb new file mode 100644 index 00000000000..ba30c1e4656 --- /dev/null +++ b/lib/gitlab/metrics_dashboard/sorter.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Gitlab + module MetricsDashboard + class Sorter + class << self + def transform!(dashboard, _project) + sort_groups!(dashboard) + sort_panels!(dashboard) + end + + # Sorts the groups in the dashboard by the :priority key + def sort_groups!(dashboard) + dashboard[:panel_groups] = dashboard[:panel_groups].sort_by { |group| group[:priority] } + end + + # Sorts the panels in the dashboard by the :weight key + def sort_panels!(dashboard) + dashboard[:panel_groups].each do |group| + group[:panels] = group[:panels].sort_by { |panel| panel[:weight] } + end + end + end + end + end +end diff --git a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb new file mode 100644 index 00000000000..de1702d3314 --- /dev/null +++ b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Gitlab::MetricsDashboard::Processor do + let(:project) { build(:project) } + let(:dashboard_yml) { YAML.load_file('spec/fixtures/services/metrics_dashboard_processing_service.yml') } + + describe 'process' do + let(:dashboard) { JSON.parse(described_class.new(dashboard_yml, project).process, symbolize_names: true) } + + context 'when dashboard config corresponds to common metrics' do + let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') } + + it 'inserts metric ids into the config' do + target_metric = all_metrics.find { |metric| metric[:id] == 'metric_a1' } + + expect(target_metric).to include(:metric_id) + end + end + + context 'when the project has associated metrics' do + let!(:project_metric) { create(:prometheus_metric, project: project) } + + it 'includes project-specific metrics' do + project_metric_details = { + query_range: project_metric.query, + unit: project_metric.unit, + label: project_metric.legend, + metric_id: project_metric.id + } + + expect(all_metrics).to include project_metric_details + end + + it 'includes project metrics at the end of the config' do + expected_metrics_order = ['metric_b', 'metric_a2', 'metric_a1', nil] + actual_metrics_order = all_metrics.map { |m| m[:id] } + + expect(actual_metrics_order).to eq expected_metrics_order + end + end + + it 'orders groups by priority and panels by weight' do + expected_metrics_order = %w(metric_b metric_a2 metric_a1) + actual_metrics_order = all_metrics.map { |m| m[:id] } + + expect(actual_metrics_order).to eq expected_metrics_order + end + end + + def all_metrics + dashboard[:panel_groups].map do |group| + group[:panels].map { |panel| panel[:metrics] } + end.flatten + end +end diff --git a/spec/lib/gitlab/metrics_dashboard/service_spec.rb b/spec/lib/gitlab/metrics_dashboard/service_spec.rb new file mode 100644 index 00000000000..e4b7b35763a --- /dev/null +++ b/spec/lib/gitlab/metrics_dashboard/service_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Gitlab::MetricsDashboard::Service, :use_clean_rails_memory_store_caching do + let(:project) { build(:project) } + + describe 'get_dashboard' do + it 'returns a json representation of the environment dashboard' do + dashboard = described_class.new(project).get_dashboard + json = JSON.parse(dashboard, symbolize_names: true) + + expect(json).to include(:dashboard, :order, :panel_groups) + expect(json[:panel_groups]).to all( include(:group, :priority, :panels) ) + end + + it 'caches the dashboard for subsequent calls' do + expect(YAML).to receive(:load_file).once.and_call_original + + described_class.new(project).get_dashboard + described_class.new(project).get_dashboard + end + end +end diff --git a/spec/services/metrics_dashboard_processing_service_spec.rb b/spec/services/metrics_dashboard_processing_service_spec.rb deleted file mode 100644 index 917ae582afe..00000000000 --- a/spec/services/metrics_dashboard_processing_service_spec.rb +++ /dev/null @@ -1,55 +0,0 @@ -require 'spec_helper' - -describe MetricsDashboardProcessingService do - let(:project) { build(:project) } - let(:dashboard_yml) { YAML.load_file('spec/fixtures/services/metrics_dashboard_processing_service.yml') } - - describe 'process' do - let(:dashboard) { JSON.parse(described_class.new(dashboard_yml, project).process, symbolize_names: true) } - - context 'when dashboard config corresponds to common metrics' do - let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') } - - it 'inserts metric ids into the config' do - target_metric = all_metrics.find { |metric| metric[:id] == 'metric_a1' } - - expect(target_metric).to include(:metric_id) - end - end - - context 'when the project has associated metrics' do - let!(:project_metric) { create(:prometheus_metric, project: project) } - - it 'includes project-specific metrics' do - project_metric_details = { - query_range: project_metric.query, - unit: project_metric.unit, - label: project_metric.legend, - metric_id: project_metric.id - } - - expect(all_metrics).to include project_metric_details - end - - it 'includes project metrics at the end of the config' do - expected_metrics_order = ['metric_b', 'metric_a2', 'metric_a1', nil] - actual_metrics_order = all_metrics.map { |m| m[:id] } - - expect(actual_metrics_order).to eq expected_metrics_order - end - end - - it 'orders groups by priority and panels by weight' do - expected_metrics_order = %w('metric_b metric_a2 metric_a1') - actual_metrics_order = all_metrics.map { |m| m[:id] } - - expect(actual_metrics_order).to eq expected_metrics_order - end - end - - def all_metrics - dashboard[:panel_groups].map do |group| - group[:panels].map { |panel| panel[:metrics] } - end.flatten - end -end diff --git a/spec/services/metrics_dashboard_service_spec.rb b/spec/services/metrics_dashboard_service_spec.rb deleted file mode 100644 index db59ec3a3a4..00000000000 --- a/spec/services/metrics_dashboard_service_spec.rb +++ /dev/null @@ -1,22 +0,0 @@ -require 'spec_helper' - -describe MetricsDashboardService, :use_clean_rails_memory_store_caching do - let(:project) { build(:project) } - - describe 'get_dashboard' do - it 'returns a json representation of the environment dashboard' do - dashboard = described_class.new(project).get_dashboard - json = JSON.parse(dashboard, symbolize_names: true) - - expect(json).to include(:dashboard, :order, :panel_groups) - expect(json[:panel_groups]).to all( include(:group, :priority, :panels) ) - end - - it 'caches the dashboard for subsequent calls' do - expect(YAML).to receive(:load_file).once.and_call_original - - described_class.new(project).get_dashboard - described_class.new(project).get_dashboard - end - end -end -- cgit v1.2.1 From 1596e5556030ab3990a7d0117bf7d4306417c052 Mon Sep 17 00:00:00 2001 From: syasonik Date: Tue, 16 Apr 2019 17:16:51 +0800 Subject: Cleanup misnamed or unnecessary files --- .../metrics_dashboard_processing_service.rb | 116 --------------------- .../gitlab/metrics_dashboard/sample_dashboard.yml | 36 +++++++ .../metrics_dashboard_processing_service.yml | 36 ------- .../lib/gitlab/metrics_dashboard/processor_spec.rb | 2 +- 4 files changed, 37 insertions(+), 153 deletions(-) delete mode 100644 app/services/metrics_dashboard_processing_service.rb create mode 100644 spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml delete mode 100644 spec/fixtures/services/metrics_dashboard_processing_service.yml diff --git a/app/services/metrics_dashboard_processing_service.rb b/app/services/metrics_dashboard_processing_service.rb deleted file mode 100644 index 6dc8db00934..00000000000 --- a/app/services/metrics_dashboard_processing_service.rb +++ /dev/null @@ -1,116 +0,0 @@ -# frozen_string_literal: true - -class MetricsDashboardProcessingService - DEFAULT_PANEL_TYPE = 'area-chart' - - def initialize(dashboard, project) - @dashboard = dashboard.deep_transform_keys(&:to_sym) - @project = project - end - - def process - insert_metric_ids! - sort_groups! - sort_panels! - insert_project_metrics! - @dashboard.to_json - end - - private - - # ------- Processing Steps ----------- - - # For each metric in the dashboard config, attempts to find a corresponding - # database record. If found, includes the record's id in the dashboard config. - def insert_metric_ids! - for_metrics do |metric| - metric_record = common_metrics.find { |m| m.identifier == metric[:id] } - metric[:metric_id] = metric_record.id if metric_record - end - end - - # Inserts project-specific metrics into the dashboard config. - # If there are no project-specific metrics, this will have no effect. - def insert_project_metrics! - project_metrics.each do |project_metric| - group = find_or_create_group(@dashboard[:panel_groups], project_metric) - panel = find_or_create_panel(group[:panels], project_metric) - find_or_create_metric(panel[:metrics], project_metric) - end - end - - # Sorts the groups in the dashboard by the :priority key - def sort_groups! - @dashboard[:panel_groups] = @dashboard[:panel_groups].sort_by { |group| group[:priority] } - end - - # Sorts the panels in the dashboard by the :weight key - def sort_panels! - @dashboard[:panel_groups].each do |group| - group[:panels] = group[:panels].sort_by { |panel| panel[:weight] } - end - end - - # ------- Processing Helpers ----------- - - def project_metrics - @project.prometheus_metrics - end - - def common_metrics - @common_metrics ||= ::PrometheusMetric.common - end - - def for_metrics - @dashboard[:panel_groups].each do |panel_group| - panel_group[:panels].each do |panel| - panel[:metrics].each do |metric| - yield metric - end - end - end - end - - def find_or_create_group(panel_groups, metric) - target_group = panel_groups.find { |group| group[:group] == metric.group_title } - - unless target_group - target_group = { - group: metric.group_title, - priority: metric.priority, - panels: [] - } - panel_groups << target_group - end - - target_group - end - - def find_or_create_panel(panels, metric) - panel_identifiers = [DEFAULT_PANEL_TYPE, metric.title, metric.y_label] - target_panel = panels.find { |panel| panel.values_at(:type, :title, :y_label) == panel_identifiers } - - unless target_panel - target_panel = { - type: DEFAULT_PANEL_TYPE, - title: metric.title, - y_label: metric.y_label, - metrics: [] - } - panels << target_panel - end - - target_panel - end - - def find_or_create_metric(metrics, metric) - target_metric = metrics.find { |m| m[:id] == metric.identifier } - - unless target_metric - target_metric = metric.queries.first.merge(metric_id: metric.id) - metrics << target_metric - end - - target_metric - end -end diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml b/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml new file mode 100644 index 00000000000..ebfe06da6db --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml @@ -0,0 +1,36 @@ +dashboard: 'Test Dashboard' +order: 1 +panel_groups: +- group: Group A + priority: 10 + panels: + - title: "Super Chart A1" + type: "area-chart" + y_label: "y_label" + weight: 2 + metrics: + - id: metric_a1 + query_range: 'query' + unit: unit + label: Legend Label + - title: "Super Chart A2" + type: "area-chart" + y_label: "y_label" + weight: 1 + metrics: + - id: metric_a2 + query_range: 'query' + label: Legend Label + unit: unit +- group: Group B + priority: 1 + panels: + - title: "Super Chart B" + type: "area-chart" + y_label: "y_label" + weight: 1 + metrics: + - id: metric_b + query_range: 'query' + unit: unit + label: Legend Label diff --git a/spec/fixtures/services/metrics_dashboard_processing_service.yml b/spec/fixtures/services/metrics_dashboard_processing_service.yml deleted file mode 100644 index ebfe06da6db..00000000000 --- a/spec/fixtures/services/metrics_dashboard_processing_service.yml +++ /dev/null @@ -1,36 +0,0 @@ -dashboard: 'Test Dashboard' -order: 1 -panel_groups: -- group: Group A - priority: 10 - panels: - - title: "Super Chart A1" - type: "area-chart" - y_label: "y_label" - weight: 2 - metrics: - - id: metric_a1 - query_range: 'query' - unit: unit - label: Legend Label - - title: "Super Chart A2" - type: "area-chart" - y_label: "y_label" - weight: 1 - metrics: - - id: metric_a2 - query_range: 'query' - label: Legend Label - unit: unit -- group: Group B - priority: 1 - panels: - - title: "Super Chart B" - type: "area-chart" - y_label: "y_label" - weight: 1 - metrics: - - id: metric_b - query_range: 'query' - unit: unit - label: Legend Label diff --git a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb index de1702d3314..0b3ee90d3e8 100644 --- a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::MetricsDashboard::Processor do let(:project) { build(:project) } - let(:dashboard_yml) { YAML.load_file('spec/fixtures/services/metrics_dashboard_processing_service.yml') } + let(:dashboard_yml) { YAML.load_file('spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml') } describe 'process' do let(:dashboard) { JSON.parse(described_class.new(dashboard_yml, project).process, symbolize_names: true) } -- cgit v1.2.1 From b1773bf8b741ffc52e2699848e42aa0a054c9e6e Mon Sep 17 00:00:00 2001 From: rpereira2 Date: Tue, 16 Apr 2019 16:13:08 +0530 Subject: Fix rubocop failures - Add 3 functions called find_or_create_panel, find_or_create_panel_group, and find_or_create_metric to avoid having to use 'send'. - Remove an unused variable. - Freeze a constant array. --- lib/gitlab/metrics_dashboard/processor.rb | 2 +- .../metrics_dashboard/project_metrics_inserter.rb | 57 ++++++++++++++++------ lib/gitlab/metrics_dashboard/sorter.rb | 2 + 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/lib/gitlab/metrics_dashboard/processor.rb b/lib/gitlab/metrics_dashboard/processor.rb index 9e254aa2b37..cfeb0ddb468 100644 --- a/lib/gitlab/metrics_dashboard/processor.rb +++ b/lib/gitlab/metrics_dashboard/processor.rb @@ -3,7 +3,7 @@ module Gitlab module MetricsDashboard class Processor - STAGES = [CommonMetricsInserter, Sorter, ProjectMetricsInserter] + STAGES = [CommonMetricsInserter, Sorter, ProjectMetricsInserter].freeze def initialize(dashboard, project) @dashboard = dashboard.deep_transform_keys(&:to_sym) diff --git a/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb index 67300c79e57..3903bd39912 100644 --- a/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb +++ b/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb @@ -10,25 +10,54 @@ module Gitlab # If there are no project-specific metrics, this will have no effect. def transform!(dashboard, project) project.prometheus_metrics.each do |project_metric| - group = find_or_create(:panel_group, dashboard[:panel_groups], project_metric) - panel = find_or_create(:panel, group[:panels], project_metric) - find_or_create(:metric, panel[:metrics], project_metric) + group = find_or_create_panel_group(dashboard[:panel_groups], project_metric) + panel = find_or_create_panel(group[:panels], project_metric) + find_or_create_metric(panel[:metrics], project_metric) end end - # Looks for an instance of the named resource corresponding to the provided - # metric object. If unavailable, inserts one. - # @param name [Symbol, String] One of :panel_group, :panel, or :metric - # @param existing_resources [Array] + private + + # Looks for a panel corresponding to the provided metric object. + # If unavailable, inserts one. + # @param panels [Array] + # @param metric [PrometheusMetric] + def find_or_create_panel(panels, metric) + panel = find_panel(panels, metric) + return panel if panel + + panel = new_panel(metric) + panels << panel + + panel + end + + # Looks for a panel_group corresponding to the provided metric object. + # If unavailable, inserts one. + # @param panel_groups [Array] + # @param metric [PrometheusMetric] + def find_or_create_panel_group(panel_groups, metric) + panel_group = find_panel_group(panel_groups, metric) + return panel_group if panel_group + + panel_group = new_panel_group(metric) + panel_groups << panel_group + + panel_group + end + + # Looks for a metric corresponding to the provided metric object. + # If unavailable, inserts one. + # @param metrics [Array] # @param metric [PrometheusMetric] - def find_or_create(name, existing_resources, metric) - target = self.send("find_#{name}", existing_resources, metric) - return target if target + def find_or_create_metric(metrics, metric) + target_metric = find_metric(metrics, metric) + return target_metric if target_metric - target = self.send("new_#{name}", metric) - existing_resources << target + target_metric = new_metric(metric) + metrics << target_metric - target + target_metric end def find_panel_group(panel_groups, metric) @@ -37,7 +66,7 @@ module Gitlab def find_panel(panels, metric) panel_identifiers = [DEFAULT_PANEL_TYPE, metric.title, metric.y_label] - target_panel = panels.find { |panel| panel.values_at(:type, :title, :y_label) == panel_identifiers } + panels.find { |panel| panel.values_at(:type, :title, :y_label) == panel_identifiers } end def find_metric(metrics, metric) diff --git a/lib/gitlab/metrics_dashboard/sorter.rb b/lib/gitlab/metrics_dashboard/sorter.rb index ba30c1e4656..1d28fc8bd3a 100644 --- a/lib/gitlab/metrics_dashboard/sorter.rb +++ b/lib/gitlab/metrics_dashboard/sorter.rb @@ -9,6 +9,8 @@ module Gitlab sort_panels!(dashboard) end + private + # Sorts the groups in the dashboard by the :priority key def sort_groups!(dashboard) dashboard[:panel_groups] = dashboard[:panel_groups].sort_by { |group| group[:priority] } -- cgit v1.2.1 From 0007a42a7bcc7bcee3dd10a3132dc96478b77e80 Mon Sep 17 00:00:00 2001 From: rpereira2 Date: Tue, 16 Apr 2019 21:07:47 +0530 Subject: Correct the order of groups and panels - Order groups by descending order of priority. - Order panels by descending order of weight. - Perform sorting after adding project/custom metrics. --- lib/gitlab/metrics_dashboard/processor.rb | 2 +- lib/gitlab/metrics_dashboard/sorter.rb | 4 +-- .../gitlab/metrics_dashboard/sample_dashboard.yml | 4 +-- .../lib/gitlab/metrics_dashboard/processor_spec.rb | 39 +++++++++++----------- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/lib/gitlab/metrics_dashboard/processor.rb b/lib/gitlab/metrics_dashboard/processor.rb index cfeb0ddb468..518e0123220 100644 --- a/lib/gitlab/metrics_dashboard/processor.rb +++ b/lib/gitlab/metrics_dashboard/processor.rb @@ -3,7 +3,7 @@ module Gitlab module MetricsDashboard class Processor - STAGES = [CommonMetricsInserter, Sorter, ProjectMetricsInserter].freeze + STAGES = [CommonMetricsInserter, ProjectMetricsInserter, Sorter].freeze def initialize(dashboard, project) @dashboard = dashboard.deep_transform_keys(&:to_sym) diff --git a/lib/gitlab/metrics_dashboard/sorter.rb b/lib/gitlab/metrics_dashboard/sorter.rb index 1d28fc8bd3a..9a8f87fcb6e 100644 --- a/lib/gitlab/metrics_dashboard/sorter.rb +++ b/lib/gitlab/metrics_dashboard/sorter.rb @@ -13,13 +13,13 @@ module Gitlab # Sorts the groups in the dashboard by the :priority key def sort_groups!(dashboard) - dashboard[:panel_groups] = dashboard[:panel_groups].sort_by { |group| group[:priority] } + dashboard[:panel_groups] = dashboard[:panel_groups].sort_by { |group| group[:priority] }.reverse end # Sorts the panels in the dashboard by the :weight key def sort_panels!(dashboard) dashboard[:panel_groups].each do |group| - group[:panels] = group[:panels].sort_by { |panel| panel[:weight] } + group[:panels] = group[:panels].sort_by { |panel| panel[:weight] }.reverse end end end diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml b/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml index ebfe06da6db..7b6527ea715 100644 --- a/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml +++ b/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml @@ -7,7 +7,7 @@ panel_groups: - title: "Super Chart A1" type: "area-chart" y_label: "y_label" - weight: 2 + weight: 1 metrics: - id: metric_a1 query_range: 'query' @@ -16,7 +16,7 @@ panel_groups: - title: "Super Chart A2" type: "area-chart" y_label: "y_label" - weight: 1 + weight: 2 metrics: - id: metric_a2 query_range: 'query' diff --git a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb index 0b3ee90d3e8..973b3e2e13a 100644 --- a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb @@ -18,38 +18,39 @@ describe Gitlab::MetricsDashboard::Processor do end context 'when the project has associated metrics' do - let!(:project_metric) { create(:prometheus_metric, project: project) } + let!(:project_response_metric) { create(:prometheus_metric, project: project, group: :response) } + let!(:project_system_metric) { create(:prometheus_metric, project: project, group: :system) } + let!(:project_business_metric) { create(:prometheus_metric, project: project, group: :business) } it 'includes project-specific metrics' do - project_metric_details = { - query_range: project_metric.query, - unit: project_metric.unit, - label: project_metric.legend, - metric_id: project_metric.id - } - - expect(all_metrics).to include project_metric_details + expect(all_metrics).to include get_metric_details(project_system_metric) + expect(all_metrics).to include get_metric_details(project_response_metric) + expect(all_metrics).to include get_metric_details(project_business_metric) end - it 'includes project metrics at the end of the config' do - expected_metrics_order = ['metric_b', 'metric_a2', 'metric_a1', nil] - actual_metrics_order = all_metrics.map { |m| m[:id] } + it 'orders groups by priority and panels by weight' do + expected_metrics_order = ['metric_a2', 'metric_a1', 'metric_b', project_business_metric.id, project_response_metric.id, project_system_metric.id] + actual_metrics_order = all_metrics.map { |m| m[:id] || m[:metric_id] } expect(actual_metrics_order).to eq expected_metrics_order end end - - it 'orders groups by priority and panels by weight' do - expected_metrics_order = %w(metric_b metric_a2 metric_a1) - actual_metrics_order = all_metrics.map { |m| m[:id] } - - expect(actual_metrics_order).to eq expected_metrics_order - end end + private + def all_metrics dashboard[:panel_groups].map do |group| group[:panels].map { |panel| panel[:metrics] } end.flatten end + + def get_metric_details(metric) + { + query_range: metric.query, + unit: metric.unit, + label: metric.legend, + metric_id: metric.id + } + end end -- cgit v1.2.1 From 64a1fab788b5f5e92626d96795d68799401536b4 Mon Sep 17 00:00:00 2001 From: rpereira2 Date: Tue, 16 Apr 2019 21:38:04 +0530 Subject: Add frozen_string_literal magic comment --- spec/lib/gitlab/metrics_dashboard/processor_spec.rb | 2 ++ spec/lib/gitlab/metrics_dashboard/service_spec.rb | 2 ++ 2 files changed, 4 insertions(+) diff --git a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb index 973b3e2e13a..bd6af18cbc0 100644 --- a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Gitlab::MetricsDashboard::Processor do diff --git a/spec/lib/gitlab/metrics_dashboard/service_spec.rb b/spec/lib/gitlab/metrics_dashboard/service_spec.rb index e4b7b35763a..416b84687a9 100644 --- a/spec/lib/gitlab/metrics_dashboard/service_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/service_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Gitlab::MetricsDashboard::Service, :use_clean_rails_memory_store_caching do -- cgit v1.2.1 From b209fb6fb21369b381e4a538f2e46ef1b6579aed Mon Sep 17 00:00:00 2001 From: syasonik Date: Wed, 17 Apr 2019 14:47:50 +0800 Subject: Minor optimization and code clarity --- .../metrics_dashboard/project_metrics_inserter.rb | 28 +++++++++++----------- lib/gitlab/metrics_dashboard/sorter.rb | 4 ++-- .../lib/gitlab/metrics_dashboard/processor_spec.rb | 9 ++++++- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb index 3903bd39912..f4463645d2c 100644 --- a/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb +++ b/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb @@ -18,20 +18,6 @@ module Gitlab private - # Looks for a panel corresponding to the provided metric object. - # If unavailable, inserts one. - # @param panels [Array] - # @param metric [PrometheusMetric] - def find_or_create_panel(panels, metric) - panel = find_panel(panels, metric) - return panel if panel - - panel = new_panel(metric) - panels << panel - - panel - end - # Looks for a panel_group corresponding to the provided metric object. # If unavailable, inserts one. # @param panel_groups [Array] @@ -46,6 +32,20 @@ module Gitlab panel_group end + # Looks for a panel corresponding to the provided metric object. + # If unavailable, inserts one. + # @param panels [Array] + # @param metric [PrometheusMetric] + def find_or_create_panel(panels, metric) + panel = find_panel(panels, metric) + return panel if panel + + panel = new_panel(metric) + panels << panel + + panel + end + # Looks for a metric corresponding to the provided metric object. # If unavailable, inserts one. # @param metrics [Array] diff --git a/lib/gitlab/metrics_dashboard/sorter.rb b/lib/gitlab/metrics_dashboard/sorter.rb index 9a8f87fcb6e..59b21d207e1 100644 --- a/lib/gitlab/metrics_dashboard/sorter.rb +++ b/lib/gitlab/metrics_dashboard/sorter.rb @@ -13,13 +13,13 @@ module Gitlab # Sorts the groups in the dashboard by the :priority key def sort_groups!(dashboard) - dashboard[:panel_groups] = dashboard[:panel_groups].sort_by { |group| group[:priority] }.reverse + dashboard[:panel_groups] = dashboard[:panel_groups].sort_by { |group| -group[:priority].to_i } end # Sorts the panels in the dashboard by the :weight key def sort_panels!(dashboard) dashboard[:panel_groups].each do |group| - group[:panels] = group[:panels].sort_by { |panel| panel[:weight] }.reverse + group[:panels] = group[:panels].sort_by { |panel| -panel[:weight].to_i } end end end diff --git a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb index bd6af18cbc0..9cf5b00e915 100644 --- a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb @@ -31,7 +31,14 @@ describe Gitlab::MetricsDashboard::Processor do end it 'orders groups by priority and panels by weight' do - expected_metrics_order = ['metric_a2', 'metric_a1', 'metric_b', project_business_metric.id, project_response_metric.id, project_system_metric.id] + expected_metrics_order = [ + 'metric_a2', # group priority 10, panel weight 2 + 'metric_a1', # group priority 10, panel weight 1 + 'metric_b', # group priority 1, panel weight 1 + project_business_metric.id, # group priority 0, panel weight nil (0) + project_response_metric.id, # group priority -5, panel weight nil (0) + project_system_metric.id, # group priority -10, panel weight nil (0) + ] actual_metrics_order = all_metrics.map { |m| m[:id] || m[:metric_id] } expect(actual_metrics_order).to eq expected_metrics_order -- cgit v1.2.1 From 4a9002cee3c80a9de24f01a5eb313ff17d2e4931 Mon Sep 17 00:00:00 2001 From: rpereira2 Date: Wed, 17 Apr 2019 15:16:11 +0530 Subject: Inherit from BaseService Change MetricsDashboard::Service to inherit from BaseService so that it can reuse methods like initialize, success, error. --- app/controllers/projects/environments_controller.rb | 16 +++++++++++++--- lib/gitlab/metrics_dashboard/service.rb | 12 +++++------- .../controllers/projects/environments_controller_spec.rb | 4 ++-- spec/lib/gitlab/metrics_dashboard/service_spec.rb | 10 ++++++---- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 69ceeab5b99..25815b92a0f 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -162,9 +162,19 @@ class Projects::EnvironmentsController < Projects::ApplicationController respond_to do |format| format.json do - dashboard = Gitlab::MetricsDashboard::Service.new(@project).get_dashboard - - render json: dashboard, status: :ok + result = Gitlab::MetricsDashboard::Service.new(@project).get_dashboard + + if result[:status] == :success + render status: :ok, json: { + status: :success, + dashboard: result[:dashboard] + } + else + render status: result[:http_status] || :bad_request, json: { + message: result[:message], + status: result[:status] + } + end end end end diff --git a/lib/gitlab/metrics_dashboard/service.rb b/lib/gitlab/metrics_dashboard/service.rb index 22e1771cbea..521a3914e9e 100644 --- a/lib/gitlab/metrics_dashboard/service.rb +++ b/lib/gitlab/metrics_dashboard/service.rb @@ -3,19 +3,17 @@ # Fetches the metrics dashboard layout and supplemented the output with DB info. module Gitlab module MetricsDashboard - class Service + class Service < ::BaseService SYSTEM_DASHBOARD_NAME = 'common_metrics' SYSTEM_DASHBOARD_PATH = Rails.root.join('config', 'prometheus', "#{SYSTEM_DASHBOARD_NAME}.yml") - def initialize(project) - @project = project - end - # Returns a DB-supplemented json representation of a dashboard config file. def get_dashboard - dashboard = Rails.cache.fetch(cache_key) { system_dashboard } + dashboard_string = Rails.cache.fetch(cache_key) { system_dashboard } + + dashboard = JSON.parse(process_dashboard(dashboard_string)) - process_dashboard(dashboard) + success(dashboard: dashboard) end private diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 02441385da0..6c3aad55622 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -479,8 +479,8 @@ describe Projects::EnvironmentsController do get :metrics_dashboard, params: environment_params(format: :json) expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to include('dashboard', 'order', 'panel_groups') - expect(json_response['panel_groups']).to all( include('group', 'priority', 'panels') ) + expect(json_response.keys).to contain_exactly('dashboard', 'status') + expect(json_response['dashboard']).to be_an_instance_of(Hash) end end end diff --git a/spec/lib/gitlab/metrics_dashboard/service_spec.rb b/spec/lib/gitlab/metrics_dashboard/service_spec.rb index 416b84687a9..f9574889314 100644 --- a/spec/lib/gitlab/metrics_dashboard/service_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/service_spec.rb @@ -7,11 +7,13 @@ describe Gitlab::MetricsDashboard::Service, :use_clean_rails_memory_store_cachin describe 'get_dashboard' do it 'returns a json representation of the environment dashboard' do - dashboard = described_class.new(project).get_dashboard - json = JSON.parse(dashboard, symbolize_names: true) + result = described_class.new(project).get_dashboard - expect(json).to include(:dashboard, :order, :panel_groups) - expect(json[:panel_groups]).to all( include(:group, :priority, :panels) ) + expect(result.keys).to contain_exactly(:dashboard, :status) + expect(result[:status]).to eq(:success) + + expect(result[:dashboard]).to include('dashboard', 'order', 'panel_groups') + expect(result[:dashboard]['panel_groups']).to all( include('group', 'priority', 'panels') ) end it 'caches the dashboard for subsequent calls' do -- cgit v1.2.1 From c2818d6b23543f288db3ae5464c3f3b473522836 Mon Sep 17 00:00:00 2001 From: rpereira2 Date: Wed, 17 Apr 2019 15:21:42 +0530 Subject: Add schema validation spec Validate the schema of the dashboard that is loaded by Gitlab::MetricsDashboard::Service. --- .../gitlab/metrics_dashboard/schemas/dashboard.json | 17 +++++++++++++++++ .../gitlab/metrics_dashboard/schemas/metrics.json | 16 ++++++++++++++++ .../metrics_dashboard/schemas/panel_groups.json | 17 +++++++++++++++++ .../lib/gitlab/metrics_dashboard/schemas/panels.json | 20 ++++++++++++++++++++ spec/lib/gitlab/metrics_dashboard/service_spec.rb | 5 +++-- 5 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json create mode 100644 spec/fixtures/lib/gitlab/metrics_dashboard/schemas/metrics.json create mode 100644 spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panel_groups.json create mode 100644 spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panels.json diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json new file mode 100644 index 00000000000..4d1ab8615eb --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json @@ -0,0 +1,17 @@ +{ + "type": "object", + "required": [ + "dashboard", + "order", + "panel_groups" + ], + "properties": { + "dashboard": { "type": "string" }, + "order": { "type": "number" }, + "panel_groups": { + "type": "array", + "items": { "$ref": "spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panel_groups.json" } + } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/metrics.json b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/metrics.json new file mode 100644 index 00000000000..4da1004dedd --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/metrics.json @@ -0,0 +1,16 @@ +{ + "type": "object", + "required": [ + "unit", + "label" + ], + "properties": { + "id": { "type": "string" }, + "query_range": { "type": "string" }, + "query": { "type": "string" }, + "unit": { "type": "string" }, + "label": { "type": "string" }, + "track": { "type": "string" } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panel_groups.json b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panel_groups.json new file mode 100644 index 00000000000..d7a390adcdc --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panel_groups.json @@ -0,0 +1,17 @@ +{ + "type": "object", + "required": [ + "group", + "priority", + "panels" + ], + "properties": { + "group": { "type": "string" }, + "priority": { "type": "number" }, + "panels": { + "type": "array", + "items": { "$ref": "panels.json" } + } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panels.json b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panels.json new file mode 100644 index 00000000000..1548daacd64 --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panels.json @@ -0,0 +1,20 @@ +{ + "type": "object", + "required": [ + "title", + "y_label", + "weight", + "metrics" + ], + "properties": { + "title": { "type": "string" }, + "type": { "type": "string" }, + "y_label": { "type": "string" }, + "weight": { "type": "number" }, + "metrics": { + "type": "array", + "items": { "$ref": "metrics.json" } + } + }, + "additionalProperties": false +} diff --git a/spec/lib/gitlab/metrics_dashboard/service_spec.rb b/spec/lib/gitlab/metrics_dashboard/service_spec.rb index f9574889314..127fc28bcd6 100644 --- a/spec/lib/gitlab/metrics_dashboard/service_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/service_spec.rb @@ -6,14 +6,15 @@ describe Gitlab::MetricsDashboard::Service, :use_clean_rails_memory_store_cachin let(:project) { build(:project) } describe 'get_dashboard' do + let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics_dashboard/schemas/dashboard.json')) } + it 'returns a json representation of the environment dashboard' do result = described_class.new(project).get_dashboard expect(result.keys).to contain_exactly(:dashboard, :status) expect(result[:status]).to eq(:success) - expect(result[:dashboard]).to include('dashboard', 'order', 'panel_groups') - expect(result[:dashboard]['panel_groups']).to all( include('group', 'priority', 'panels') ) + expect(JSON::Validator.fully_validate(dashboard_schema, result[:dashboard])).to be_empty end it 'caches the dashboard for subsequent calls' do -- cgit v1.2.1 From f029c3dd39a5b74c50bb9804a7f175b48cda0305 Mon Sep 17 00:00:00 2001 From: rpereira2 Date: Wed, 17 Apr 2019 15:35:11 +0530 Subject: Make query or query_range required in metrics Either the query or query_range attribute needs to be present in the metrics yml. The yml is invalid if both are present or neither is present. --- spec/fixtures/lib/gitlab/metrics_dashboard/schemas/metrics.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/metrics.json b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/metrics.json index 4da1004dedd..2d0af57ec2c 100644 --- a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/metrics.json +++ b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/metrics.json @@ -4,6 +4,10 @@ "unit", "label" ], + "oneOf": [ + { "required": ["query"] }, + { "required": ["query_range"] } + ], "properties": { "id": { "type": "string" }, "query_range": { "type": "string" }, -- cgit v1.2.1 From 671f698845339563852abcb0b9607632fd8076a6 Mon Sep 17 00:00:00 2001 From: syasonik Date: Wed, 17 Apr 2019 19:37:42 +0800 Subject: Bring in line with EE needs --- .../projects/environments_controller.rb | 2 +- .../metrics_dashboard/common_metrics_inserter.rb | 32 ------- lib/gitlab/metrics_dashboard/processor.rb | 17 ++-- .../metrics_dashboard/project_metrics_inserter.rb | 99 ---------------------- lib/gitlab/metrics_dashboard/service.rb | 4 +- lib/gitlab/metrics_dashboard/sorter.rb | 28 ------ lib/gitlab/metrics_dashboard/stages/base_stage.rb | 39 +++++++++ .../stages/common_metrics_inserter.rb | 21 +++++ .../stages/project_metrics_inserter.rb | 97 +++++++++++++++++++++ lib/gitlab/metrics_dashboard/stages/sorter.rb | 28 ++++++ .../lib/gitlab/metrics_dashboard/processor_spec.rb | 5 +- spec/lib/gitlab/metrics_dashboard/service_spec.rb | 7 +- 12 files changed, 208 insertions(+), 171 deletions(-) delete mode 100644 lib/gitlab/metrics_dashboard/common_metrics_inserter.rb delete mode 100644 lib/gitlab/metrics_dashboard/project_metrics_inserter.rb delete mode 100644 lib/gitlab/metrics_dashboard/sorter.rb create mode 100644 lib/gitlab/metrics_dashboard/stages/base_stage.rb create mode 100644 lib/gitlab/metrics_dashboard/stages/common_metrics_inserter.rb create mode 100644 lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb create mode 100644 lib/gitlab/metrics_dashboard/stages/sorter.rb diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 25815b92a0f..52736bcf3f8 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -162,7 +162,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController respond_to do |format| format.json do - result = Gitlab::MetricsDashboard::Service.new(@project).get_dashboard + result = Gitlab::MetricsDashboard::Service.new(@project, @current_user, environment: environment).get_dashboard if result[:status] == :success render status: :ok, json: { diff --git a/lib/gitlab/metrics_dashboard/common_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/common_metrics_inserter.rb deleted file mode 100644 index 5bc83e6266e..00000000000 --- a/lib/gitlab/metrics_dashboard/common_metrics_inserter.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module MetricsDashboard - class CommonMetricsInserter - class << self - # For each metric in the dashboard config, attempts to find a corresponding - # database record. If found, includes the record's id in the dashboard config. - def transform!(dashboard, _project) - common_metrics = ::PrometheusMetric.common - - for_metrics(dashboard) do |metric| - metric_record = common_metrics.find { |m| m.identifier == metric[:id] } - metric[:metric_id] = metric_record.id if metric_record - end - end - - private - - def for_metrics(dashboard) - dashboard[:panel_groups].each do |panel_group| - panel_group[:panels].each do |panel| - panel[:metrics].each do |metric| - yield metric - end - end - end - end - end - end - end -end diff --git a/lib/gitlab/metrics_dashboard/processor.rb b/lib/gitlab/metrics_dashboard/processor.rb index 518e0123220..14ee12049d0 100644 --- a/lib/gitlab/metrics_dashboard/processor.rb +++ b/lib/gitlab/metrics_dashboard/processor.rb @@ -3,17 +3,24 @@ module Gitlab module MetricsDashboard class Processor - STAGES = [CommonMetricsInserter, ProjectMetricsInserter, Sorter].freeze - - def initialize(dashboard, project) + def initialize(dashboard, project, environment) @dashboard = dashboard.deep_transform_keys(&:to_sym) @project = project + @environment = environment + end + + def stages + @stages ||= [ + Stages::CommonMetricsInserter, + Stages::ProjectMetricsInserter, + Stages::Sorter + ].freeze end def process - STAGES.each { |stage| stage.transform!(@dashboard, @project) } + stages.each { |stage| stage.new(@dashboard, @project, @environment).transform! } - @dashboard.to_json + @dashboard end end end diff --git a/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb deleted file mode 100644 index f4463645d2c..00000000000 --- a/lib/gitlab/metrics_dashboard/project_metrics_inserter.rb +++ /dev/null @@ -1,99 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module MetricsDashboard - class ProjectMetricsInserter - DEFAULT_PANEL_TYPE = 'area-chart' - - class << self - # Inserts project-specific metrics into the dashboard config. - # If there are no project-specific metrics, this will have no effect. - def transform!(dashboard, project) - project.prometheus_metrics.each do |project_metric| - group = find_or_create_panel_group(dashboard[:panel_groups], project_metric) - panel = find_or_create_panel(group[:panels], project_metric) - find_or_create_metric(panel[:metrics], project_metric) - end - end - - private - - # Looks for a panel_group corresponding to the provided metric object. - # If unavailable, inserts one. - # @param panel_groups [Array] - # @param metric [PrometheusMetric] - def find_or_create_panel_group(panel_groups, metric) - panel_group = find_panel_group(panel_groups, metric) - return panel_group if panel_group - - panel_group = new_panel_group(metric) - panel_groups << panel_group - - panel_group - end - - # Looks for a panel corresponding to the provided metric object. - # If unavailable, inserts one. - # @param panels [Array] - # @param metric [PrometheusMetric] - def find_or_create_panel(panels, metric) - panel = find_panel(panels, metric) - return panel if panel - - panel = new_panel(metric) - panels << panel - - panel - end - - # Looks for a metric corresponding to the provided metric object. - # If unavailable, inserts one. - # @param metrics [Array] - # @param metric [PrometheusMetric] - def find_or_create_metric(metrics, metric) - target_metric = find_metric(metrics, metric) - return target_metric if target_metric - - target_metric = new_metric(metric) - metrics << target_metric - - target_metric - end - - def find_panel_group(panel_groups, metric) - panel_groups.find { |group| group[:group] == metric.group_title } - end - - def find_panel(panels, metric) - panel_identifiers = [DEFAULT_PANEL_TYPE, metric.title, metric.y_label] - panels.find { |panel| panel.values_at(:type, :title, :y_label) == panel_identifiers } - end - - def find_metric(metrics, metric) - metrics.find { |m| m[:id] == metric.identifier } - end - - def new_panel_group(metric) - { - group: metric.group_title, - priority: metric.priority, - panels: [] - } - end - - def new_panel(metric) - { - type: DEFAULT_PANEL_TYPE, - title: metric.title, - y_label: metric.y_label, - metrics: [] - } - end - - def new_metric(metric) - metric.queries.first.merge(metric_id: metric.id) - end - end - end - end -end diff --git a/lib/gitlab/metrics_dashboard/service.rb b/lib/gitlab/metrics_dashboard/service.rb index 521a3914e9e..01e61b257e2 100644 --- a/lib/gitlab/metrics_dashboard/service.rb +++ b/lib/gitlab/metrics_dashboard/service.rb @@ -11,7 +11,7 @@ module Gitlab def get_dashboard dashboard_string = Rails.cache.fetch(cache_key) { system_dashboard } - dashboard = JSON.parse(process_dashboard(dashboard_string)) + dashboard = process_dashboard(dashboard_string) success(dashboard: dashboard) end @@ -28,7 +28,7 @@ module Gitlab end def process_dashboard(dashboard) - Processor.new(dashboard, @project).process + Processor.new(dashboard, project, params[:environment]).process end end end diff --git a/lib/gitlab/metrics_dashboard/sorter.rb b/lib/gitlab/metrics_dashboard/sorter.rb deleted file mode 100644 index 59b21d207e1..00000000000 --- a/lib/gitlab/metrics_dashboard/sorter.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module MetricsDashboard - class Sorter - class << self - def transform!(dashboard, _project) - sort_groups!(dashboard) - sort_panels!(dashboard) - end - - private - - # Sorts the groups in the dashboard by the :priority key - def sort_groups!(dashboard) - dashboard[:panel_groups] = dashboard[:panel_groups].sort_by { |group| -group[:priority].to_i } - end - - # Sorts the panels in the dashboard by the :weight key - def sort_panels!(dashboard) - dashboard[:panel_groups].each do |group| - group[:panels] = group[:panels].sort_by { |panel| -panel[:weight].to_i } - end - end - end - end - end -end diff --git a/lib/gitlab/metrics_dashboard/stages/base_stage.rb b/lib/gitlab/metrics_dashboard/stages/base_stage.rb new file mode 100644 index 00000000000..72085e0c09e --- /dev/null +++ b/lib/gitlab/metrics_dashboard/stages/base_stage.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Gitlab + module MetricsDashboard + module Stages + class BaseStage + DEFAULT_PANEL_TYPE = 'area-chart' + + attr_reader :dashboard, :project, :environment + + def initialize(dashboard, project, environment) + @dashboard = dashboard + @project = project + @environment = environment + end + + # Entry-point to the stage + # @param dashboard [Hash] + # @param project [Project] + # @param environment [Environment] + def transform! + raise NotImplementedError + end + + protected + + def for_metrics + dashboard[:panel_groups].each do |panel_group| + panel_group[:panels].each do |panel| + panel[:metrics].each do |metric| + yield metric + end + end + end + end + end + end + end +end diff --git a/lib/gitlab/metrics_dashboard/stages/common_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/stages/common_metrics_inserter.rb new file mode 100644 index 00000000000..e85bdb2700b --- /dev/null +++ b/lib/gitlab/metrics_dashboard/stages/common_metrics_inserter.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Gitlab + module MetricsDashboard + module Stages + class CommonMetricsInserter < BaseStage + # For each metric in the dashboard config, attempts to + # find a corresponding database record. If found, + # includes the record's id in the dashboard config. + def transform! + common_metrics = ::PrometheusMetric.common + + for_metrics do |metric| + metric_record = common_metrics.find { |m| m.identifier == metric[:id] } + metric[:metric_id] = metric_record.id if metric_record + end + end + end + end + end +end diff --git a/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb new file mode 100644 index 00000000000..7b694c6e5fa --- /dev/null +++ b/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +module Gitlab + module MetricsDashboard + module Stages + class ProjectMetricsInserter < BaseStage + # Inserts project-specific metrics into the dashboard config. + # If there are no project-specific metrics, this will have no effect. + def transform! + project.prometheus_metrics.each do |project_metric| + group = find_or_create_panel_group(dashboard[:panel_groups], project_metric) + panel = find_or_create_panel(group[:panels], project_metric) + find_or_create_metric(panel[:metrics], project_metric) + end + end + + private + + # Looks for a panel_group corresponding to the provided metric object. + # If unavailable, inserts one. + # @param panel_groups [Array] + # @param metric [PrometheusMetric] + def find_or_create_panel_group(panel_groups, metric) + panel_group = find_panel_group(panel_groups, metric) + return panel_group if panel_group + + panel_group = new_panel_group(metric) + panel_groups << panel_group + + panel_group + end + + # Looks for a panel corresponding to the provided metric object. + # If unavailable, inserts one. + # @param panels [Array] + # @param metric [PrometheusMetric] + def find_or_create_panel(panels, metric) + panel = find_panel(panels, metric) + return panel if panel + + panel = new_panel(metric) + panels << panel + + panel + end + + # Looks for a metric corresponding to the provided metric object. + # If unavailable, inserts one. + # @param metrics [Array] + # @param metric [PrometheusMetric] + def find_or_create_metric(metrics, metric) + target_metric = find_metric(metrics, metric) + return target_metric if target_metric + + target_metric = new_metric(metric) + metrics << target_metric + + target_metric + end + + def find_panel_group(panel_groups, metric) + panel_groups.find { |group| group[:group] == metric.group_title } + end + + def find_panel(panels, metric) + panel_identifiers = [DEFAULT_PANEL_TYPE, metric.title, metric.y_label] + panels.find { |panel| panel.values_at(:type, :title, :y_label) == panel_identifiers } + end + + def find_metric(metrics, metric) + metrics.find { |m| m[:id] == metric.identifier } + end + + def new_panel_group(metric) + { + group: metric.group_title, + priority: metric.priority, + panels: [] + } + end + + def new_panel(metric) + { + type: DEFAULT_PANEL_TYPE, + title: metric.title, + y_label: metric.y_label, + metrics: [] + } + end + + def new_metric(metric) + metric.queries.first.merge(metric_id: metric.id) + end + end + end + end +end diff --git a/lib/gitlab/metrics_dashboard/stages/sorter.rb b/lib/gitlab/metrics_dashboard/stages/sorter.rb new file mode 100644 index 00000000000..74b596038fe --- /dev/null +++ b/lib/gitlab/metrics_dashboard/stages/sorter.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Gitlab + module MetricsDashboard + module Stages + class Sorter < BaseStage + def transform! + sort_groups! + sort_panels! + end + + private + + # Sorts the groups in the dashboard by the :priority key + def sort_groups! + dashboard[:panel_groups] = dashboard[:panel_groups].sort_by { |group| -group[:priority].to_i } + end + + # Sorts the panels in the dashboard by the :weight key + def sort_panels! + dashboard[:panel_groups].each do |group| + group[:panels] = group[:panels].sort_by { |panel| -panel[:weight].to_i } + end + end + end + end + end +end diff --git a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb index 9cf5b00e915..bc5f6527ad7 100644 --- a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb @@ -4,10 +4,12 @@ require 'spec_helper' describe Gitlab::MetricsDashboard::Processor do let(:project) { build(:project) } + let(:environment) { build(:environment) } let(:dashboard_yml) { YAML.load_file('spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml') } describe 'process' do - let(:dashboard) { JSON.parse(described_class.new(dashboard_yml, project).process, symbolize_names: true) } + let(:process_params) { [dashboard_yml, project, environment] } + let(:dashboard) { described_class.new(*process_params).process } context 'when dashboard config corresponds to common metrics' do let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') } @@ -16,6 +18,7 @@ describe Gitlab::MetricsDashboard::Processor do target_metric = all_metrics.find { |metric| metric[:id] == 'metric_a1' } expect(target_metric).to include(:metric_id) + expect(target_metric[:metric_id]).to eq(common_metric.id) end end diff --git a/spec/lib/gitlab/metrics_dashboard/service_spec.rb b/spec/lib/gitlab/metrics_dashboard/service_spec.rb index 127fc28bcd6..710c71fd6bd 100644 --- a/spec/lib/gitlab/metrics_dashboard/service_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/service_spec.rb @@ -4,12 +4,13 @@ require 'spec_helper' describe Gitlab::MetricsDashboard::Service, :use_clean_rails_memory_store_caching do let(:project) { build(:project) } + let(:environment) { build(:environment) } describe 'get_dashboard' do let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics_dashboard/schemas/dashboard.json')) } it 'returns a json representation of the environment dashboard' do - result = described_class.new(project).get_dashboard + result = described_class.new(project, environment).get_dashboard expect(result.keys).to contain_exactly(:dashboard, :status) expect(result[:status]).to eq(:success) @@ -20,8 +21,8 @@ describe Gitlab::MetricsDashboard::Service, :use_clean_rails_memory_store_cachin it 'caches the dashboard for subsequent calls' do expect(YAML).to receive(:load_file).once.and_call_original - described_class.new(project).get_dashboard - described_class.new(project).get_dashboard + described_class.new(project, environment).get_dashboard + described_class.new(project, environment).get_dashboard end end end -- cgit v1.2.1 From 9a9cb2534ca931bf02bae9f86201129bdb6fa3c3 Mon Sep 17 00:00:00 2001 From: syasonik Date: Wed, 17 Apr 2019 19:45:06 +0800 Subject: Minor line length cleanup --- lib/gitlab/metrics_dashboard/processor.rb | 3 ++- .../stages/project_metrics_inserter.rb | 17 +++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/gitlab/metrics_dashboard/processor.rb b/lib/gitlab/metrics_dashboard/processor.rb index 14ee12049d0..3b8ab27a07b 100644 --- a/lib/gitlab/metrics_dashboard/processor.rb +++ b/lib/gitlab/metrics_dashboard/processor.rb @@ -18,7 +18,8 @@ module Gitlab end def process - stages.each { |stage| stage.new(@dashboard, @project, @environment).transform! } + stage_params = [@dashboard, @project, @environment] + stages.each { |stage| stage.new(*stage_params).transform! } @dashboard end diff --git a/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb index 7b694c6e5fa..af59e6f5910 100644 --- a/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb +++ b/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb @@ -4,8 +4,9 @@ module Gitlab module MetricsDashboard module Stages class ProjectMetricsInserter < BaseStage - # Inserts project-specific metrics into the dashboard config. - # If there are no project-specific metrics, this will have no effect. + # Inserts project-specific metrics into the dashboard + # config. If there are no project-specific metrics, + # this will have no effect. def transform! project.prometheus_metrics.each do |project_metric| group = find_or_create_panel_group(dashboard[:panel_groups], project_metric) @@ -16,8 +17,8 @@ module Gitlab private - # Looks for a panel_group corresponding to the provided metric object. - # If unavailable, inserts one. + # Looks for a panel_group corresponding to the + # provided metric object. If unavailable, inserts one. # @param panel_groups [Array] # @param metric [PrometheusMetric] def find_or_create_panel_group(panel_groups, metric) @@ -30,8 +31,8 @@ module Gitlab panel_group end - # Looks for a panel corresponding to the provided metric object. - # If unavailable, inserts one. + # Looks for a panel corresponding to the provided + # metric object. If unavailable, inserts one. # @param panels [Array] # @param metric [PrometheusMetric] def find_or_create_panel(panels, metric) @@ -44,8 +45,8 @@ module Gitlab panel end - # Looks for a metric corresponding to the provided metric object. - # If unavailable, inserts one. + # Looks for a metric corresponding to the provided + # metric object. If unavailable, inserts one. # @param metrics [Array] # @param metric [PrometheusMetric] def find_or_create_metric(metrics, metric) -- cgit v1.2.1 From 57e0d474c24dec779c6794a29cd84f8cef38d9a7 Mon Sep 17 00:00:00 2001 From: syasonik Date: Wed, 17 Apr 2019 20:53:32 +0800 Subject: Reduce congnitive complexity --- app/controllers/projects/environments_controller.rb | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 52736bcf3f8..1aa4ac6017c 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -165,16 +165,15 @@ class Projects::EnvironmentsController < Projects::ApplicationController result = Gitlab::MetricsDashboard::Service.new(@project, @current_user, environment: environment).get_dashboard if result[:status] == :success - render status: :ok, json: { - status: :success, - dashboard: result[:dashboard] - } + status_code = :ok + details = { dashboard: result[:dashboard] } else - render status: result[:http_status] || :bad_request, json: { - message: result[:message], - status: result[:status] - } + status_code = result[:http_status] || :bad_request + details = { message: result[:message] } end + + render status: status_code, + json: { status: result[:status] }.merge(details) end end end -- cgit v1.2.1 From d9b83ff36a8203de01fe27e139101f3d8c31e36b Mon Sep 17 00:00:00 2001 From: syasonik Date: Wed, 17 Apr 2019 21:00:27 +0800 Subject: Utilize renamed dashboard ordering attribute --- spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml | 2 +- spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml b/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml index 7b6527ea715..c2d3d3d8aca 100644 --- a/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml +++ b/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml @@ -1,5 +1,5 @@ dashboard: 'Test Dashboard' -order: 1 +priority: 1 panel_groups: - group: Group A priority: 10 diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json index 4d1ab8615eb..865aebe4a1e 100644 --- a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json +++ b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json @@ -2,7 +2,7 @@ "type": "object", "required": [ "dashboard", - "order", + "priority", "panel_groups" ], "properties": { -- cgit v1.2.1 From ed87159a71703111ffcedbd2e4248755057c1d8d Mon Sep 17 00:00:00 2001 From: syasonik Date: Wed, 17 Apr 2019 22:12:21 +0800 Subject: Finish updating dashboard schema --- spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json index 865aebe4a1e..2d7601b64b1 100644 --- a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json +++ b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json @@ -7,7 +7,7 @@ ], "properties": { "dashboard": { "type": "string" }, - "order": { "type": "number" }, + "priority": { "type": "number" }, "panel_groups": { "type": "array", "items": { "$ref": "spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panel_groups.json" } -- cgit v1.2.1 From 1edbf97e25018203e1a0ad6da7e50cadbda66337 Mon Sep 17 00:00:00 2001 From: syasonik Date: Wed, 17 Apr 2019 23:59:31 +0800 Subject: Try to reduce complexity again --- app/controllers/projects/environments_controller.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 1aa4ac6017c..145b0a6cade 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -158,22 +158,16 @@ class Projects::EnvironmentsController < Projects::ApplicationController end def metrics_dashboard - render_403 && return unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, project) + render_403 && return unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, @project) respond_to do |format| format.json do result = Gitlab::MetricsDashboard::Service.new(@project, @current_user, environment: environment).get_dashboard - if result[:status] == :success - status_code = :ok - details = { dashboard: result[:dashboard] } - else - status_code = result[:http_status] || :bad_request - details = { message: result[:message] } - end + ok_status = :ok if result[:status] == :success + status = ok_status || result[:http_status] || :bad_request - render status: status_code, - json: { status: result[:status] }.merge(details) + render status: status, json: result end end end -- cgit v1.2.1 From 655fae4242e88d71756578970d19c3db42ed63e1 Mon Sep 17 00:00:00 2001 From: syasonik Date: Thu, 18 Apr 2019 15:41:34 +0800 Subject: Reduce cognitivty complexity more --- app/controllers/projects/environments_controller.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 145b0a6cade..3f345204241 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -164,10 +164,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController format.json do result = Gitlab::MetricsDashboard::Service.new(@project, @current_user, environment: environment).get_dashboard - ok_status = :ok if result[:status] == :success - status = ok_status || result[:http_status] || :bad_request - - render status: status, json: result + render_metrics_dashboard_response(result) end end end @@ -212,6 +209,13 @@ class Projects::EnvironmentsController < Projects::ApplicationController params.require([:start, :end]) end + def render_metrics_dashboard_response(result) + ok_status = :ok if result[:status] == :success + status = ok_status || result[:http_status] || :bad_request + + render status: status, json: result + end + def search_environment_names return [] unless params[:query] -- cgit v1.2.1 From c1c0fb66937dcea326cb70528373ce6ab822d25a Mon Sep 17 00:00:00 2001 From: syasonik Date: Fri, 19 Apr 2019 18:12:54 +0800 Subject: Make EE interactions and transformations cleaner --- .../projects/environments_controller.rb | 2 +- lib/gitlab/metrics_dashboard/processor.rb | 25 ++++++++++++++-------- lib/gitlab/metrics_dashboard/service.rb | 2 +- lib/gitlab/metrics_dashboard/stages/base_stage.rb | 9 ++++---- .../stages/common_metrics_inserter.rb | 4 ++-- .../stages/project_metrics_inserter.rb | 2 +- lib/gitlab/metrics_dashboard/stages/sorter.rb | 10 ++++----- .../lib/gitlab/metrics_dashboard/processor_spec.rb | 4 ++-- 8 files changed, 32 insertions(+), 26 deletions(-) diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 3f345204241..e285011469c 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -10,7 +10,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController before_action :environment, only: [:show, :edit, :update, :stop, :terminal, :terminal_websocket_authorize, :metrics] before_action :verify_api_request!, only: :terminal_websocket_authorize before_action :expire_etag_cache, only: [:index] - before_action only: [:metrics, :additional_metrics] do + before_action only: [:metrics, :additional_metrics, :metrics_dashboard] do push_frontend_feature_flag(:metrics_time_window) push_frontend_feature_flag(:environment_metrics_use_prometheus_endpoint) end diff --git a/lib/gitlab/metrics_dashboard/processor.rb b/lib/gitlab/metrics_dashboard/processor.rb index 3b8ab27a07b..ef9d75947f0 100644 --- a/lib/gitlab/metrics_dashboard/processor.rb +++ b/lib/gitlab/metrics_dashboard/processor.rb @@ -2,26 +2,33 @@ module Gitlab module MetricsDashboard + # Responsible for processesing a dashboard hash, inserting + # relevantDB records & sorting for proper rendering in + # the UI. These includes shared metric info, custom metrics + # info, and alerts (only in EE). class Processor - def initialize(dashboard, project, environment) - @dashboard = dashboard.deep_transform_keys(&:to_sym) + def initialize(project, environment) @project = project @environment = environment end - def stages - @stages ||= [ + def sequence + [ Stages::CommonMetricsInserter, Stages::ProjectMetricsInserter, Stages::Sorter - ].freeze + ] end - def process - stage_params = [@dashboard, @project, @environment] - stages.each { |stage| stage.new(*stage_params).transform! } + # Returns a new dashboard hash with the results of + # running transforms on the dashboard. + def process(dashboard) + dashboard = dashboard.deep_transform_keys(&:to_sym) - @dashboard + stage_params = [@project, @environment] + sequence.each { |stage| stage.new(*stage_params).transform!(dashboard) } + + dashboard end end end diff --git a/lib/gitlab/metrics_dashboard/service.rb b/lib/gitlab/metrics_dashboard/service.rb index 01e61b257e2..a65f01ca54e 100644 --- a/lib/gitlab/metrics_dashboard/service.rb +++ b/lib/gitlab/metrics_dashboard/service.rb @@ -28,7 +28,7 @@ module Gitlab end def process_dashboard(dashboard) - Processor.new(dashboard, project, params[:environment]).process + Processor.new(project, params[:environment]).process(dashboard) end end end diff --git a/lib/gitlab/metrics_dashboard/stages/base_stage.rb b/lib/gitlab/metrics_dashboard/stages/base_stage.rb index 72085e0c09e..bdbf0c196cc 100644 --- a/lib/gitlab/metrics_dashboard/stages/base_stage.rb +++ b/lib/gitlab/metrics_dashboard/stages/base_stage.rb @@ -6,10 +6,9 @@ module Gitlab class BaseStage DEFAULT_PANEL_TYPE = 'area-chart' - attr_reader :dashboard, :project, :environment + attr_reader :project, :environment - def initialize(dashboard, project, environment) - @dashboard = dashboard + def initialize(project, environment) @project = project @environment = environment end @@ -18,13 +17,13 @@ module Gitlab # @param dashboard [Hash] # @param project [Project] # @param environment [Environment] - def transform! + def transform!(_dashboard) raise NotImplementedError end protected - def for_metrics + def for_metrics(dashboard) dashboard[:panel_groups].each do |panel_group| panel_group[:panels].each do |panel| panel[:metrics].each do |metric| diff --git a/lib/gitlab/metrics_dashboard/stages/common_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/stages/common_metrics_inserter.rb index e85bdb2700b..ef70347c6b2 100644 --- a/lib/gitlab/metrics_dashboard/stages/common_metrics_inserter.rb +++ b/lib/gitlab/metrics_dashboard/stages/common_metrics_inserter.rb @@ -7,10 +7,10 @@ module Gitlab # For each metric in the dashboard config, attempts to # find a corresponding database record. If found, # includes the record's id in the dashboard config. - def transform! + def transform!(dashboard) common_metrics = ::PrometheusMetric.common - for_metrics do |metric| + for_metrics(dashboard) do |metric| metric_record = common_metrics.find { |m| m.identifier == metric[:id] } metric[:metric_id] = metric_record.id if metric_record end diff --git a/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb index af59e6f5910..8edb21c89c1 100644 --- a/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb +++ b/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb @@ -7,7 +7,7 @@ module Gitlab # Inserts project-specific metrics into the dashboard # config. If there are no project-specific metrics, # this will have no effect. - def transform! + def transform!(dashboard) project.prometheus_metrics.each do |project_metric| group = find_or_create_panel_group(dashboard[:panel_groups], project_metric) panel = find_or_create_panel(group[:panels], project_metric) diff --git a/lib/gitlab/metrics_dashboard/stages/sorter.rb b/lib/gitlab/metrics_dashboard/stages/sorter.rb index 74b596038fe..a2429e65efa 100644 --- a/lib/gitlab/metrics_dashboard/stages/sorter.rb +++ b/lib/gitlab/metrics_dashboard/stages/sorter.rb @@ -4,20 +4,20 @@ module Gitlab module MetricsDashboard module Stages class Sorter < BaseStage - def transform! - sort_groups! - sort_panels! + def transform!(dashboard) + sort_groups!(dashboard) + sort_panels!(dashboard) end private # Sorts the groups in the dashboard by the :priority key - def sort_groups! + def sort_groups!(dashboard) dashboard[:panel_groups] = dashboard[:panel_groups].sort_by { |group| -group[:priority].to_i } end # Sorts the panels in the dashboard by the :weight key - def sort_panels! + def sort_panels!(dashboard) dashboard[:panel_groups].each do |group| group[:panels] = group[:panels].sort_by { |panel| -panel[:weight].to_i } end diff --git a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb index bc5f6527ad7..1bd905989fe 100644 --- a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb @@ -8,8 +8,8 @@ describe Gitlab::MetricsDashboard::Processor do let(:dashboard_yml) { YAML.load_file('spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml') } describe 'process' do - let(:process_params) { [dashboard_yml, project, environment] } - let(:dashboard) { described_class.new(*process_params).process } + let(:process_params) { [project, environment] } + let(:dashboard) { described_class.new(*process_params).process(dashboard_yml) } context 'when dashboard config corresponds to common metrics' do let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') } -- cgit v1.2.1 From 131494f26f042c7b7ced1d9abeb405986c524d92 Mon Sep 17 00:00:00 2001 From: syasonik Date: Fri, 19 Apr 2019 18:37:48 +0800 Subject: Refactor metrics_dashboard response conditionals --- app/controllers/projects/environments_controller.rb | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index e285011469c..36b9bb101af 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -159,12 +159,13 @@ class Projects::EnvironmentsController < Projects::ApplicationController def metrics_dashboard render_403 && return unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, @project) + result = Gitlab::MetricsDashboard::Service.new(@project, @current_user, environment: environment).get_dashboard respond_to do |format| - format.json do - result = Gitlab::MetricsDashboard::Service.new(@project, @current_user, environment: environment).get_dashboard - - render_metrics_dashboard_response(result) + if result[:status] == :success + format.json { render status: :ok, json: result } + else + format.json { render status: result[:http_status], json: result } end end end @@ -209,13 +210,6 @@ class Projects::EnvironmentsController < Projects::ApplicationController params.require([:start, :end]) end - def render_metrics_dashboard_response(result) - ok_status = :ok if result[:status] == :success - status = ok_status || result[:http_status] || :bad_request - - render status: status, json: result - end - def search_environment_names return [] unless params[:query] -- cgit v1.2.1 From 25f957711dac1d401982c18da439580b2e9912c9 Mon Sep 17 00:00:00 2001 From: syasonik Date: Fri, 19 Apr 2019 19:24:46 +0800 Subject: Remove extra space --- app/controllers/projects/environments_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 36b9bb101af..29aab7baa60 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -163,7 +163,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController respond_to do |format| if result[:status] == :success - format.json { render status: :ok, json: result } + format.json { render status: :ok, json: result } else format.json { render status: result[:http_status], json: result } end -- cgit v1.2.1 From a08d4cad90f98169339a3793d18f1bae4e46ad83 Mon Sep 17 00:00:00 2001 From: syasonik Date: Mon, 22 Apr 2019 14:23:35 +0800 Subject: Defend against dashboard errors, rework sequence --- .../projects/environments_controller.rb | 2 +- lib/gitlab/metrics_dashboard/processor.rb | 22 ++++++++++-------- lib/gitlab/metrics_dashboard/service.rb | 3 +++ lib/gitlab/metrics_dashboard/stages/base_stage.rb | 20 +++++++++++++++++ .../stages/project_metrics_inserter.rb | 6 +++++ lib/gitlab/metrics_dashboard/stages/sorter.rb | 4 ++++ .../projects/environments_controller_spec.rb | 13 +++++++++++ .../lib/gitlab/metrics_dashboard/processor_spec.rb | 26 ++++++++++++++++++++++ spec/lib/gitlab/metrics_dashboard/service_spec.rb | 16 +++++++++++++ 9 files changed, 102 insertions(+), 10 deletions(-) diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 29aab7baa60..04f9782b158 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -158,7 +158,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController end def metrics_dashboard - render_403 && return unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, @project) + return render_403 unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, @project) result = Gitlab::MetricsDashboard::Service.new(@project, @current_user, environment: environment).get_dashboard respond_to do |format| diff --git a/lib/gitlab/metrics_dashboard/processor.rb b/lib/gitlab/metrics_dashboard/processor.rb index ef9d75947f0..8fa95f1ee2d 100644 --- a/lib/gitlab/metrics_dashboard/processor.rb +++ b/lib/gitlab/metrics_dashboard/processor.rb @@ -3,23 +3,21 @@ module Gitlab module MetricsDashboard # Responsible for processesing a dashboard hash, inserting - # relevantDB records & sorting for proper rendering in + # relevant DB records & sorting for proper rendering in # the UI. These includes shared metric info, custom metrics # info, and alerts (only in EE). class Processor + SEQUENCE = [ + Stages::CommonMetricsInserter, + Stages::ProjectMetricsInserter, + Stages::Sorter + ].freeze + def initialize(project, environment) @project = project @environment = environment end - def sequence - [ - Stages::CommonMetricsInserter, - Stages::ProjectMetricsInserter, - Stages::Sorter - ] - end - # Returns a new dashboard hash with the results of # running transforms on the dashboard. def process(dashboard) @@ -30,6 +28,12 @@ module Gitlab dashboard end + + private + + def sequence + SEQUENCE + end end end end diff --git a/lib/gitlab/metrics_dashboard/service.rb b/lib/gitlab/metrics_dashboard/service.rb index a65f01ca54e..84f174c7d1b 100644 --- a/lib/gitlab/metrics_dashboard/service.rb +++ b/lib/gitlab/metrics_dashboard/service.rb @@ -14,6 +14,8 @@ module Gitlab dashboard = process_dashboard(dashboard_string) success(dashboard: dashboard) + rescue Gitlab::MetricsDashboard::Stages::BaseStage::DashboardLayoutError => e + error(e.message, :unprocessable_entity) end private @@ -27,6 +29,7 @@ module Gitlab "metrics_dashboard_#{SYSTEM_DASHBOARD_NAME}" end + # Returns a new dashboard Hash, supplemented with DB info def process_dashboard(dashboard) Processor.new(project, params[:environment]).process(dashboard) end diff --git a/lib/gitlab/metrics_dashboard/stages/base_stage.rb b/lib/gitlab/metrics_dashboard/stages/base_stage.rb index bdbf0c196cc..df49126a07b 100644 --- a/lib/gitlab/metrics_dashboard/stages/base_stage.rb +++ b/lib/gitlab/metrics_dashboard/stages/base_stage.rb @@ -4,6 +4,8 @@ module Gitlab module MetricsDashboard module Stages class BaseStage + DashboardLayoutError = Class.new(StandardError) + DEFAULT_PANEL_TYPE = 'area-chart' attr_reader :project, :environment @@ -23,9 +25,27 @@ module Gitlab protected + def missing_panel_groups! + raise DashboardLayoutError.new('Top-level key :panel_groups must be an array') + end + + def missing_panels! + raise DashboardLayoutError.new('Each "panel_group" must define an array :panels') + end + + def missing_metrics! + raise DashboardLayoutError.new('Each "panel" must define an array :metrics') + end + def for_metrics(dashboard) + missing_panel_groups! unless dashboard[:panel_groups].is_a?(Array) + dashboard[:panel_groups].each do |panel_group| + missing_panels! unless panel_group[:panels].is_a?(Array) + panel_group[:panels].each do |panel| + missing_metrics! unless panel[:metrics].is_a?(Array) + panel[:metrics].each do |metric| yield metric end diff --git a/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb index 8edb21c89c1..ab33ee75cce 100644 --- a/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb +++ b/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb @@ -60,15 +60,21 @@ module Gitlab end def find_panel_group(panel_groups, metric) + return unless panel_groups + panel_groups.find { |group| group[:group] == metric.group_title } end def find_panel(panels, metric) + return unless panels + panel_identifiers = [DEFAULT_PANEL_TYPE, metric.title, metric.y_label] panels.find { |panel| panel.values_at(:type, :title, :y_label) == panel_identifiers } end def find_metric(metrics, metric) + return unless metrics + metrics.find { |m| m[:id] == metric.identifier } end diff --git a/lib/gitlab/metrics_dashboard/stages/sorter.rb b/lib/gitlab/metrics_dashboard/stages/sorter.rb index a2429e65efa..ca310c9637a 100644 --- a/lib/gitlab/metrics_dashboard/stages/sorter.rb +++ b/lib/gitlab/metrics_dashboard/stages/sorter.rb @@ -5,6 +5,8 @@ module Gitlab module Stages class Sorter < BaseStage def transform!(dashboard) + missing_panel_groups! unless dashboard[:panel_groups].is_a? Array + sort_groups!(dashboard) sort_panels!(dashboard) end @@ -19,6 +21,8 @@ module Gitlab # Sorts the panels in the dashboard by the :weight key def sort_panels!(dashboard) dashboard[:panel_groups].each do |group| + missing_panels! unless group[:panels].is_a? Array + group[:panels] = group[:panels].sort_by { |panel| -panel[:weight].to_i } end end diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 6c3aad55622..b43698a6ef7 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -482,6 +482,19 @@ describe Projects::EnvironmentsController do expect(json_response.keys).to contain_exactly('dashboard', 'status') expect(json_response['dashboard']).to be_an_instance_of(Hash) end + + context 'when the dashboard could not be provided' do + before do + allow(YAML).to receive(:load_file).and_return({}) + end + + it 'returns an error response' do + get :metrics_dashboard, params: environment_params(format: :json) + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response.keys).to contain_exactly('message', 'status', 'http_status') + end + end end end diff --git a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb index 1bd905989fe..9a4897944c6 100644 --- a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb @@ -47,6 +47,32 @@ describe Gitlab::MetricsDashboard::Processor do expect(actual_metrics_order).to eq expected_metrics_order end end + + shared_examples_for 'errors with message' do |expected_message| + it 'raises a DashboardLayoutError' do + error_class = Gitlab::MetricsDashboard::Stages::BaseStage::DashboardLayoutError + + expect { dashboard }.to raise_error(error_class, expected_message) + end + end + + context 'when the dashboard is missing panel_groups' do + let(:dashboard_yml) { {} } + + it_behaves_like 'errors with message', 'Top-level key :panel_groups must be an array' + end + + context 'when the dashboard contains a panel_group which is missing panels' do + let(:dashboard_yml) { { panel_groups: [{}] } } + + it_behaves_like 'errors with message', 'Each "panel_group" must define an array :panels' + end + + context 'when the dashboard contains a panel which is missing metrics' do + let(:dashboard_yml) { { panel_groups: [{ panels: [{}] }] } } + + it_behaves_like 'errors with message', 'Each "panel" must define an array :metrics' + end end private diff --git a/spec/lib/gitlab/metrics_dashboard/service_spec.rb b/spec/lib/gitlab/metrics_dashboard/service_spec.rb index 710c71fd6bd..38c7942f250 100644 --- a/spec/lib/gitlab/metrics_dashboard/service_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/service_spec.rb @@ -24,5 +24,21 @@ describe Gitlab::MetricsDashboard::Service, :use_clean_rails_memory_store_cachin described_class.new(project, environment).get_dashboard described_class.new(project, environment).get_dashboard end + + context 'when the dashboard is configured incorrectly' do + let(:bad_dashboard) { {} } + + before do + allow(described_class).to receive(:system_dashboard).and_return(bad_dashboard) + end + + it 'returns an appropriate message and status code' do + result = described_class.new(project, environment).get_dashboard + + expect(result.keys).to contain_exactly(:message, :http_status, :status) + expect(result[:status]).to eq(:error) + expect(result[:status]).to eq(:unprocessable_entity) + end + end end end -- cgit v1.2.1 From 9fedbea78e9d28809b2e68d4effab00cf14573f9 Mon Sep 17 00:00:00 2001 From: syasonik Date: Mon, 22 Apr 2019 14:41:38 +0800 Subject: Fix broken spec --- spec/lib/gitlab/metrics_dashboard/service_spec.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/spec/lib/gitlab/metrics_dashboard/service_spec.rb b/spec/lib/gitlab/metrics_dashboard/service_spec.rb index 38c7942f250..d2b8231fbfb 100644 --- a/spec/lib/gitlab/metrics_dashboard/service_spec.rb +++ b/spec/lib/gitlab/metrics_dashboard/service_spec.rb @@ -26,10 +26,8 @@ describe Gitlab::MetricsDashboard::Service, :use_clean_rails_memory_store_cachin end context 'when the dashboard is configured incorrectly' do - let(:bad_dashboard) { {} } - before do - allow(described_class).to receive(:system_dashboard).and_return(bad_dashboard) + allow(YAML).to receive(:load_file).and_return({}) end it 'returns an appropriate message and status code' do @@ -37,7 +35,7 @@ describe Gitlab::MetricsDashboard::Service, :use_clean_rails_memory_store_cachin expect(result.keys).to contain_exactly(:message, :http_status, :status) expect(result[:status]).to eq(:error) - expect(result[:status]).to eq(:unprocessable_entity) + expect(result[:http_status]).to eq(:unprocessable_entity) end end end -- cgit v1.2.1 From dde709912f0c186d569ec5fb3d1986b8184d81f1 Mon Sep 17 00:00:00 2001 From: syasonik Date: Mon, 22 Apr 2019 15:50:43 +0800 Subject: Rubocop --- app/controllers/projects/environments_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 04f9782b158..30cbe35edf5 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -159,6 +159,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController def metrics_dashboard return render_403 unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, @project) + result = Gitlab::MetricsDashboard::Service.new(@project, @current_user, environment: environment).get_dashboard respond_to do |format| -- cgit v1.2.1 From 254c49afcacea7ae543162b30696703384433487 Mon Sep 17 00:00:00 2001 From: Andrew Fontaine Date: Thu, 18 Apr 2019 14:24:46 -0400 Subject: Backport Environments Dashboard SCSS Changes --- app/assets/stylesheets/components/dashboard_skeleton.scss | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/components/dashboard_skeleton.scss b/app/assets/stylesheets/components/dashboard_skeleton.scss index 9775c329922..a104d035a9a 100644 --- a/app/assets/stylesheets/components/dashboard_skeleton.scss +++ b/app/assets/stylesheets/components/dashboard_skeleton.scss @@ -11,7 +11,7 @@ } &-body { - height: 120px; + min-height: 120px; &-warning { background-color: $orange-50; @@ -22,10 +22,8 @@ } } - &-time-ago { - &-icon { - color: $gray-500; - } + &-icon { + color: $gray-500; } &-footer { -- cgit v1.2.1 From 41ff0d1e08c0f38ac3de8eb66e5635ecb772173c Mon Sep 17 00:00:00 2001 From: Sam Bigelow Date: Tue, 23 Apr 2019 11:23:33 -0400 Subject: Init MR Popovers onmount of system note Fixing a bug where system notes had no MR Popovers --- app/assets/javascripts/vue_shared/components/notes/system_note.vue | 4 ++++ .../unreleased/60855-mr-popover-is-not-attached-in-system-notes.yml | 5 +++++ spec/javascripts/vue_shared/components/notes/system_note_spec.js | 6 ++++++ 3 files changed, 15 insertions(+) create mode 100644 changelogs/unreleased/60855-mr-popover-is-not-attached-in-system-notes.yml diff --git a/app/assets/javascripts/vue_shared/components/notes/system_note.vue b/app/assets/javascripts/vue_shared/components/notes/system_note.vue index acc179b3834..3c86b7e4c61 100644 --- a/app/assets/javascripts/vue_shared/components/notes/system_note.vue +++ b/app/assets/javascripts/vue_shared/components/notes/system_note.vue @@ -22,6 +22,7 @@ import noteHeader from '~/notes/components/note_header.vue'; import Icon from '~/vue_shared/components/icon.vue'; import TimelineEntryItem from './timeline_entry_item.vue'; import { spriteIcon } from '../../../lib/utils/common_utils'; +import initMRPopovers from '~/mr_popover/'; const MAX_VISIBLE_COMMIT_LIST_COUNT = 3; @@ -71,6 +72,9 @@ export default { ); }, }, + mounted() { + initMRPopovers(this.$el.querySelectorAll('.gfm-merge_request')); + }, }; diff --git a/changelogs/unreleased/60855-mr-popover-is-not-attached-in-system-notes.yml b/changelogs/unreleased/60855-mr-popover-is-not-attached-in-system-notes.yml new file mode 100644 index 00000000000..f7017ddf3dd --- /dev/null +++ b/changelogs/unreleased/60855-mr-popover-is-not-attached-in-system-notes.yml @@ -0,0 +1,5 @@ +--- +title: Fix bug where system note MR has no popover +merge_request: 27589 +author: +type: fixed diff --git a/spec/javascripts/vue_shared/components/notes/system_note_spec.js b/spec/javascripts/vue_shared/components/notes/system_note_spec.js index adcb1c858aa..5b4ca20940a 100644 --- a/spec/javascripts/vue_shared/components/notes/system_note_spec.js +++ b/spec/javascripts/vue_shared/components/notes/system_note_spec.js @@ -5,8 +5,10 @@ import createStore from '~/notes/stores'; describe('system note component', () => { let vm; let props; + let initMRPopoversSpy; beforeEach(() => { + initMRPopoversSpy = spyOnDependency(issueSystemNote, 'initMRPopovers'); props = { note: { id: '1424', @@ -56,4 +58,8 @@ describe('system note component', () => { it('removes wrapping paragraph from note HTML', () => { expect(vm.$el.querySelector('.system-note-message').innerHTML).toEqual('closed'); }); + + it('should initMRPopovers onMount', () => { + expect(initMRPopoversSpy).toHaveBeenCalled(); + }); }); -- cgit v1.2.1 From ca6e946f0f50d997ecad7d3758fe362cf0fae5ce Mon Sep 17 00:00:00 2001 From: Nathan Friend Date: Wed, 24 Apr 2019 12:19:15 -0300 Subject: Only show message when MR is open This commit fixes the bug that was causing the "target branch has "advanced" error message to display after an MR was closed. --- .../vue_merge_request_widget/mr_widget_options.vue | 3 ++- ...ly-show-target-branch-advanced-error-before-merge.yml | 6 ++++++ spec/javascripts/vue_mr_widget/mr_widget_options_spec.js | 16 ++++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/60808-only-show-target-branch-advanced-error-before-merge.yml diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue index aa4ecb0aac3..705ee05e29f 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue +++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue @@ -119,7 +119,8 @@ export default { }, showTargetBranchAdvancedError() { return Boolean( - this.mr.pipeline && + this.mr.isOpen && + this.mr.pipeline && this.mr.pipeline.target_sha && this.mr.pipeline.target_sha !== this.mr.targetBranchSha, ); diff --git a/changelogs/unreleased/60808-only-show-target-branch-advanced-error-before-merge.yml b/changelogs/unreleased/60808-only-show-target-branch-advanced-error-before-merge.yml new file mode 100644 index 00000000000..b340f8408f3 --- /dev/null +++ b/changelogs/unreleased/60808-only-show-target-branch-advanced-error-before-merge.yml @@ -0,0 +1,6 @@ +--- +title: Only show the "target branch has advanced" message when the merge request is + open +merge_request: 27588 +author: +type: fixed diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js index 690fcd3e224..a0628fdcebe 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -228,6 +228,7 @@ describe('mrWidgetOptions', () => { describe('showTargetBranchAdvancedError', () => { describe(`when the pipeline's target_sha property doesn't exist`, () => { beforeEach(done => { + Vue.set(vm.mr, 'isOpen', true); Vue.set(vm.mr.pipeline, 'target_sha', undefined); Vue.set(vm.mr, 'targetBranchSha', 'abcd'); vm.$nextTick(done); @@ -240,6 +241,7 @@ describe('mrWidgetOptions', () => { describe(`when the pipeline's target_sha matches the target branch's sha`, () => { beforeEach(done => { + Vue.set(vm.mr, 'isOpen', true); Vue.set(vm.mr.pipeline, 'target_sha', 'abcd'); Vue.set(vm.mr, 'targetBranchSha', 'abcd'); vm.$nextTick(done); @@ -250,8 +252,22 @@ describe('mrWidgetOptions', () => { }); }); + describe(`when the merge request is not open`, () => { + beforeEach(done => { + Vue.set(vm.mr, 'isOpen', false); + Vue.set(vm.mr.pipeline, 'target_sha', 'abcd'); + Vue.set(vm.mr, 'targetBranchSha', 'bcde'); + vm.$nextTick(done); + }); + + it('should be false', () => { + expect(vm.showTargetBranchAdvancedError).toEqual(false); + }); + }); + describe(`when the pipeline's target_sha does not match the target branch's sha`, () => { beforeEach(done => { + Vue.set(vm.mr, 'isOpen', true); Vue.set(vm.mr.pipeline, 'target_sha', 'abcd'); Vue.set(vm.mr, 'targetBranchSha', 'bcde'); vm.$nextTick(done); -- cgit v1.2.1 From 68dae77b05986c25b6651c0479d6cff791d2154d Mon Sep 17 00:00:00 2001 From: Steve Azzopardi Date: Wed, 24 Apr 2019 17:31:37 +0200 Subject: Use constants for assertion for helm chart test We are duplicating the value from the constant `Clusters::Applications::Runner::VERSION` inside of the tests which results into developers having to update the tests as well when they want to upgrade the Helm chart used for GitLab Runner --- spec/models/clusters/applications/runner_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index de406211a5b..b66acf13135 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -24,7 +24,7 @@ describe Clusters::Applications::Runner do it 'is initialized with 4 arguments' do expect(subject.name).to eq('runner') expect(subject.chart).to eq('runner/gitlab-runner') - expect(subject.version).to eq('0.4.0') + expect(subject.version).to eq(Clusters::Applications::Runner::VERSION) expect(subject).to be_rbac expect(subject.repository).to eq('https://charts.gitlab.io') expect(subject.files).to eq(gitlab_runner.files) @@ -42,7 +42,7 @@ describe Clusters::Applications::Runner do let(:gitlab_runner) { create(:clusters_applications_runner, :errored, runner: ci_runner, version: '0.1.13') } it 'is initialized with the locked version' do - expect(subject.version).to eq('0.4.0') + expect(subject.version).to eq(Clusters::Applications::Runner::VERSION) end end end -- cgit v1.2.1 From 9649c2ef474701646e74bfd5134613528018a6c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 18 Apr 2019 21:18:20 +0200 Subject: Organize better Review Apps and QA jobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also, play manual jobs once dependency jobs are done instead of polling for the dependent jobs to be finished. Signed-off-by: Rémy Coutable --- .gitlab-ci.yml | 2 + .gitlab/ci/cng.gitlab-ci.yml | 2 +- .gitlab/ci/frontend.gitlab-ci.yml | 4 ++ .gitlab/ci/qa.gitlab-ci.yml | 11 ++- .gitlab/ci/review.gitlab-ci.yml | 117 +++++++++++++++---------------- scripts/review_apps/review-apps.sh | 97 -------------------------- scripts/utils.sh | 136 +++++++++++++++++++++++++++++++++++-- 7 files changed, 201 insertions(+), 168 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4d30efccb5c..f1573dba32a 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -28,6 +28,8 @@ stages: - prepare - merge - test + - review + - qa - post-test - pages - post-cleanup diff --git a/.gitlab/ci/cng.gitlab-ci.yml b/.gitlab/ci/cng.gitlab-ci.yml index e15f8ed91e0..c384bcdcdfc 100644 --- a/.gitlab/ci/cng.gitlab-ci.yml +++ b/.gitlab/ci/cng.gitlab-ci.yml @@ -9,7 +9,7 @@ cloud-native-image: cache: {} when: manual script: - - gem install gitlab --no-document + - install_gitlab_gem - CNG_PROJECT_PATH="gitlab-org/build/CNG" BUILD_TRIGGER_TOKEN=$CI_JOB_TOKEN ./scripts/trigger-build cng only: - tags@gitlab-org/gitlab-ce diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index fd179f77e26..bfefd42c52d 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -38,6 +38,10 @@ gitlab:assets:compile: - bundle exec rake gitlab:assets:compile - time scripts/build_assets_image - scripts/clean-old-cached-assets + # Play dependent manual jobs + - install_api_client_dependencies_with_apt + - play_job "review-build-cng" || true # this job might not exist so ignore the failure if it cannot be played + - play_job "schedule:review-build-cng" || true # this job might not exist so ignore the failure if it cannot be played artifacts: name: webpack-report expire_in: 31d diff --git a/.gitlab/ci/qa.gitlab-ci.yml b/.gitlab/ci/qa.gitlab-ci.yml index 07b38c9aa85..85c6409186e 100644 --- a/.gitlab/ci/qa.gitlab-ci.yml +++ b/.gitlab/ci/qa.gitlab-ci.yml @@ -1,20 +1,17 @@ package-and-qa: image: ruby:2.5-alpine - stage: test + stage: qa + when: manual before_script: [] dependencies: [] cache: {} variables: GIT_DEPTH: "1" - API_TOKEN: "${GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN}" retry: 0 script: - - apk add --update openssl curl jq - - gem install gitlab --no-document - - source ./scripts/review_apps/review-apps.sh - - wait_for_job_to_be_done "gitlab:assets:compile" + - source scripts/utils.sh + - install_gitlab_gem - ./scripts/trigger-build omnibus - when: manual only: - /.+/@gitlab-org/gitlab-ce - /.+/@gitlab-org/gitlab-ee diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index 9cfb50eeefc..cc5d6060716 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -26,12 +26,10 @@ extends: .dedicated-runner <<: *review-only image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-charts-build-base - stage: test cache: {} dependencies: [] - environment: &review-environment - name: review/${CI_COMMIT_REF_NAME} - url: https://gitlab-${CI_ENVIRONMENT_SLUG}.${REVIEW_APPS_DOMAIN} + before_script: + - source scripts/utils.sh .review-docker: &review-docker <<: *review-base @@ -42,18 +40,13 @@ - gitlab-org - docker variables: &review-docker-variables - GIT_DEPTH: "1" DOCKER_DRIVER: overlay2 DOCKER_HOST: tcp://docker:2375 LATEST_QA_IMAGE: "gitlab/${CI_PROJECT_NAME}-qa:nightly" QA_IMAGE: "${CI_REGISTRY}/${CI_PROJECT_PATH}/gitlab/${CI_PROJECT_NAME}-qa:${CI_COMMIT_REF_SLUG}" - before_script: [] build-qa-image: <<: *review-docker - variables: - <<: *review-docker-variables - GIT_DEPTH: "20" stage: prepare script: - time docker build --cache-from ${LATEST_QA_IMAGE} --tag ${QA_IMAGE} ./qa/ @@ -63,16 +56,14 @@ build-qa-image: .review-build-cng-base: &review-build-cng-base image: ruby:2.5-alpine stage: test - before_script: [] + when: manual + before_script: + - source scripts/utils.sh + - install_api_client_dependencies_with_apk + - install_gitlab_gem dependencies: [] cache: {} - variables: - API_TOKEN: "${GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN}" script: - - apk add --update openssl curl jq - - gem install gitlab --no-document - - source ./scripts/review_apps/review-apps.sh - - wait_for_job_to_be_done "gitlab:assets:compile" - BUILD_TRIGGER_TOKEN=$REVIEW_APPS_BUILD_TRIGGER_TOKEN ./scripts/trigger-build cng review-build-cng: @@ -85,26 +76,32 @@ schedule:review-build-cng: .review-deploy-base: &review-deploy-base <<: *review-base + stage: review retry: 2 allow_failure: true variables: HOST_SUFFIX: "${CI_ENVIRONMENT_SLUG}" DOMAIN: "-${CI_ENVIRONMENT_SLUG}.${REVIEW_APPS_DOMAIN}" GITLAB_HELM_CHART_REF: "master" - API_TOKEN: "${GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN}" - environment: - <<: *review-environment + environment: &review-environment + name: review/${CI_COMMIT_REF_NAME} + url: https://gitlab-${CI_ENVIRONMENT_SLUG}.${REVIEW_APPS_DOMAIN} on_stop: review-stop before_script: - export GITLAB_SHELL_VERSION=$( review_app_url.txt + - source scripts/utils.sh + - install_api_client_dependencies_with_apk + - source scripts/review_apps/review-apps.sh script: - - wait_for_job_to_be_done "review-build-cng" - perform_review_app_deployment + artifacts: + paths: + - review_app_url.txt + expire_in: 2 days + when: always review-deploy: <<: *review-deploy-base @@ -113,15 +110,29 @@ schedule:review-deploy: <<: *review-deploy-base <<: *review-schedules-only script: - - wait_for_job_to_be_done "schedule:review-build-cng" - perform_review_app_deployment +review-stop: + <<: *review-base + stage: review + when: manual + allow_failure: true + variables: + GIT_DEPTH: "1" + environment: + <<: *review-environment + action: stop + script: + - source scripts/review_apps/review-apps.sh + - delete + - cleanup + .review-qa-base: &review-qa-base <<: *review-docker + stage: qa allow_failure: true variables: <<: *review-docker-variables - API_TOKEN: "${GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN}" QA_ARTIFACTS_DIR: "${CI_PROJECT_DIR}/qa" QA_CAN_TEST_GIT_PROTOCOL_V2: "false" GITLAB_USERNAME: "root" @@ -131,40 +142,45 @@ schedule:review-deploy: GITHUB_ACCESS_TOKEN: "${REVIEW_APPS_QA_GITHUB_ACCESS_TOKEN}" EE_LICENSE: "${REVIEW_APPS_EE_LICENSE}" QA_DEBUG: "true" + dependencies: + - review-deploy artifacts: paths: - ./qa/gitlab-qa-run-* expire_in: 7 days when: always before_script: - - echo "${QA_IMAGE}" + - export CI_ENVIRONMENT_URL="$(cat review_app_url.txt)" - echo "${CI_ENVIRONMENT_URL}" - - apk update && apk add curl jq - - source ./scripts/review_apps/review-apps.sh + - echo "${QA_IMAGE}" + - source scripts/utils.sh + - install_api_client_dependencies_with_apk - gem install gitlab-qa --no-document ${GITLAB_QA_VERSION:+ --version ${GITLAB_QA_VERSION}} review-qa-smoke: <<: *review-qa-base retry: 2 script: - - wait_for_job_to_be_done "review-deploy" - gitlab-qa Test::Instance::Smoke "${QA_IMAGE}" "${CI_ENVIRONMENT_URL}" review-qa-all: <<: *review-qa-base + when: manual script: - - wait_for_job_to_be_done "review-deploy" - gitlab-qa Test::Instance::Any "${QA_IMAGE}" "${CI_ENVIRONMENT_URL}" - when: manual .review-performance-base: &review-performance-base <<: *review-qa-base - script: - - wait_for_job_to_be_done "review-deploy" + stage: qa + before_script: + - export CI_ENVIRONMENT_URL="$(cat review_app_url.txt)" + - echo "${CI_ENVIRONMENT_URL}" - mkdir -p gitlab-exporter - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/master/index.js - - mkdir sitespeed-results - - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io sitespeedio/sitespeed.io:6.3.1 --plugins.add ./gitlab-exporter --outputFolder sitespeed-results "$CI_ENVIRONMENT_URL" + - mkdir -p sitespeed-results + script: + - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io sitespeedio/sitespeed.io:6.3.1 --plugins.add ./gitlab-exporter --outputFolder sitespeed-results "${CI_ENVIRONMENT_URL}" + after_script: - mv sitespeed-results/data/performance.json performance.json artifacts: paths: @@ -175,42 +191,27 @@ review-qa-all: review-performance: <<: *review-performance-base -review-stop: - <<: *review-base - extends: .single-script-job-dedicated-runner - image: registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-charts-build-base - allow_failure: true - variables: - SCRIPT_NAME: "review_apps/review-apps.sh" - when: manual - environment: - <<: *review-environment - action: stop - script: - - source $(basename "${SCRIPT_NAME}") - - delete - - cleanup +schedule:review-performance: + <<: *review-performance-base + <<: *review-schedules-only + dependencies: + - schedule:review-deploy schedule:review-cleanup: <<: *review-base <<: *review-schedules-only - stage: build + stage: review allow_failure: true variables: GIT_DEPTH: "1" environment: name: review/auto-cleanup before_script: - - gem install gitlab --no-document + - source scripts/utils.sh + - install_gitlab_gem script: - ruby -rrubygems scripts/review_apps/automated_cleanup.rb -schedule:review-performance: - <<: *review-performance-base - <<: *review-schedules-only - script: - - wait_for_job_to_be_done "schedule:review-deploy" - danger-review: extends: .dedicated-pull-cache-job image: registry.gitlab.com/gitlab-org/gitlab-build-images:danger diff --git a/scripts/review_apps/review-apps.sh b/scripts/review_apps/review-apps.sh index b55ce1af55e..8be22dc0278 100755 --- a/scripts/review_apps/review-apps.sh +++ b/scripts/review_apps/review-apps.sh @@ -1,26 +1,6 @@ [[ "$TRACE" ]] && set -x export TILLER_NAMESPACE="$KUBE_NAMESPACE" -function echoerr() { - local header="${2}" - - if [ -n "${header}" ]; then - printf "\n\033[0;31m** %s **\n\033[0m" "${1}" >&2; - else - printf "\033[0;31m%s\n\033[0m" "${1}" >&2; - fi -} - -function echoinfo() { - local header="${2}" - - if [ -n "${header}" ]; then - printf "\n\033[0;33m** %s **\n\033[0m" "${1}" >&2; - else - printf "\033[0;33m%s\n\033[0m" "${1}" >&2; - fi -} - function deployExists() { local namespace="${1}" local deploy="${2}" @@ -328,80 +308,3 @@ function add_license() { puts "License added"; ' } - -function get_job_id() { - local job_name="${1}" - local query_string="${2:+&${2}}" - - local max_page=3 - local page=1 - - while true; do - local url="https://gitlab.com/api/v4/projects/${CI_PROJECT_ID}/pipelines/${CI_PIPELINE_ID}/jobs?per_page=100&page=${page}${query_string}" - echoinfo "GET ${url}" - - local job_id - job_id=$(curl --silent --show-error --header "PRIVATE-TOKEN: ${API_TOKEN}" "${url}" | jq "map(select(.name == \"${job_name}\")) | map(.id) | last") - [[ "${job_id}" == "null" && "${page}" -lt "$max_page" ]] || break - - let "page++" - done - - if [[ "${job_id}" == "" ]]; then - echoerr "The '${job_name}' job ID couldn't be retrieved!" - else - echoinfo "The '${job_name}' job ID is ${job_id}" - echo "${job_id}" - fi -} - -function play_job() { - local job_name="${1}" - local job_id - job_id=$(get_job_id "${job_name}" "scope=manual"); - if [ -z "${job_id}" ]; then return; fi - - local url="https://gitlab.com/api/v4/projects/${CI_PROJECT_ID}/jobs/${job_id}/play" - echoinfo "POST ${url}" - - local job_url - job_url=$(curl --silent --show-error --request POST --header "PRIVATE-TOKEN: ${API_TOKEN}" "${url}" | jq ".web_url") - echoinfo "Manual job '${job_name}' started at: ${job_url}" -} - -function wait_for_job_to_be_done() { - local job_name="${1}" - local query_string="${2}" - local job_id - job_id=$(get_job_id "${job_name}" "${query_string}") - if [ -z "${job_id}" ]; then return; fi - - echoinfo "Waiting for the '${job_name}' job to finish..." - - local url="https://gitlab.com/api/v4/projects/${CI_PROJECT_ID}/jobs/${job_id}" - echoinfo "GET ${url}" - - # In case the job hasn't finished yet. Keep trying until the job times out. - local interval=30 - local elapsed_seconds=0 - while true; do - local job_status - job_status=$(curl --silent --show-error --header "PRIVATE-TOKEN: ${API_TOKEN}" "${url}" | jq ".status" | sed -e s/\"//g) - [[ "${job_status}" == "pending" || "${job_status}" == "running" ]] || break - - printf "." - let "elapsed_seconds+=interval" - sleep ${interval} - done - - local elapsed_minutes=$((elapsed_seconds / 60)) - echoinfo "Waited '${job_name}' for ${elapsed_minutes} minutes." - - if [[ "${job_status}" == "failed" ]]; then - echoerr "The '${job_name}' failed." - elif [[ "${job_status}" == "manual" ]]; then - echoinfo "The '${job_name}' is manual." - else - echoinfo "The '${job_name}' passed." - fi -} diff --git a/scripts/utils.sh b/scripts/utils.sh index 2d2ba115563..4a6567b8a62 100644 --- a/scripts/utils.sh +++ b/scripts/utils.sh @@ -1,4 +1,4 @@ -retry() { +function retry() { if eval "$@"; then return 0 fi @@ -13,15 +13,15 @@ retry() { return 1 } -setup_db_user_only() { +function setup_db_user_only() { if [ "$GITLAB_DATABASE" = "postgresql" ]; then - . scripts/create_postgres_user.sh + source scripts/create_postgres_user.sh else - . scripts/create_mysql_user.sh + source scripts/create_mysql_user.sh fi } -setup_db() { +function setup_db() { setup_db_user_only bundle exec rake db:drop db:create db:schema:load db:migrate @@ -30,3 +30,129 @@ setup_db() { bundle exec rake add_limits_mysql fi } + +function install_api_client_dependencies_with_apk() { + apk add --update openssl curl jq +} + +function install_api_client_dependencies_with_apt() { + apt update && apt install jq -y +} + +function install_gitlab_gem() { + gem install gitlab --no-document +} + +function echoerr() { + local header="${2}" + + if [ -n "${header}" ]; then + printf "\n\033[0;31m** %s **\n\033[0m" "${1}" >&2; + else + printf "\033[0;31m%s\n\033[0m" "${1}" >&2; + fi +} + +function echoinfo() { + local header="${2}" + + if [ -n "${header}" ]; then + printf "\n\033[0;33m** %s **\n\033[0m" "${1}" >&2; + else + printf "\033[0;33m%s\n\033[0m" "${1}" >&2; + fi +} + +function get_job_id() { + local job_name="${1}" + local query_string="${2:+&${2}}" + local api_token="${API_TOKEN-${GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN}}" + if [ -z "${api_token}" ]; then + echoerr "Please provide an API token with \$API_TOKEN or \$GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN." + return + fi + + local max_page=3 + local page=1 + + while true; do + local url="https://gitlab.com/api/v4/projects/${CI_PROJECT_ID}/pipelines/${CI_PIPELINE_ID}/jobs?per_page=100&page=${page}${query_string}" + echoinfo "GET ${url}" + + local job_id + job_id=$(curl --silent --show-error --header "PRIVATE-TOKEN: ${api_token}" "${url}" | jq "map(select(.name == \"${job_name}\")) | map(.id) | last") + [[ "${job_id}" == "null" && "${page}" -lt "$max_page" ]] || break + + let "page++" + done + + if [[ "${job_id}" == "" ]]; then + echoerr "The '${job_name}' job ID couldn't be retrieved!" + else + echoinfo "The '${job_name}' job ID is ${job_id}" + echo "${job_id}" + fi +} + +function play_job() { + local job_name="${1}" + local job_id + job_id=$(get_job_id "${job_name}" "scope=manual"); + if [ -z "${job_id}" ]; then return; fi + + local api_token="${API_TOKEN-${GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN}}" + if [ -z "${api_token}" ]; then + echoerr "Please provide an API token with \$API_TOKEN or \$GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN." + return + fi + + local url="https://gitlab.com/api/v4/projects/${CI_PROJECT_ID}/jobs/${job_id}/play" + echoinfo "POST ${url}" + + local job_url + job_url=$(curl --silent --show-error --request POST --header "PRIVATE-TOKEN: ${api_token}" "${url}" | jq ".web_url") + echoinfo "Manual job '${job_name}' started at: ${job_url}" +} + +function wait_for_job_to_be_done() { + local job_name="${1}" + local query_string="${2}" + local job_id + job_id=$(get_job_id "${job_name}" "${query_string}") + if [ -z "${job_id}" ]; then return; fi + + local api_token="${API_TOKEN-${GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN}}" + if [ -z "${api_token}" ]; then + echoerr "Please provide an API token with \$API_TOKEN or \$GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN." + return + fi + + echoinfo "Waiting for the '${job_name}' job to finish..." + + local url="https://gitlab.com/api/v4/projects/${CI_PROJECT_ID}/jobs/${job_id}" + echoinfo "GET ${url}" + + # In case the job hasn't finished yet. Keep trying until the job times out. + local interval=30 + local elapsed_seconds=0 + while true; do + local job_status + job_status=$(curl --silent --show-error --header "PRIVATE-TOKEN: ${api_token}" "${url}" | jq ".status" | sed -e s/\"//g) + [[ "${job_status}" == "pending" || "${job_status}" == "running" ]] || break + + printf "." + let "elapsed_seconds+=interval" + sleep ${interval} + done + + local elapsed_minutes=$((elapsed_seconds / 60)) + echoinfo "Waited '${job_name}' for ${elapsed_minutes} minutes." + + if [[ "${job_status}" == "failed" ]]; then + echoerr "The '${job_name}' failed." + elif [[ "${job_status}" == "manual" ]]; then + echoinfo "The '${job_name}' is manual." + else + echoinfo "The '${job_name}' passed." + fi +} -- cgit v1.2.1 From a374131b1e21a629f3755aaaaf61f973e2154177 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 19 Apr 2019 14:04:44 +0200 Subject: Update development Review Apps documentation with the latest changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- .../img/review_apps_cicd_architecture.png | Bin 73240 -> 136431 bytes doc/development/testing_guide/review_apps.md | 162 +++++++++++---------- 2 files changed, 82 insertions(+), 80 deletions(-) diff --git a/doc/development/testing_guide/img/review_apps_cicd_architecture.png b/doc/development/testing_guide/img/review_apps_cicd_architecture.png index 87e472076f3..1ee28d3db91 100644 Binary files a/doc/development/testing_guide/img/review_apps_cicd_architecture.png and b/doc/development/testing_guide/img/review_apps_cicd_architecture.png differ diff --git a/doc/development/testing_guide/review_apps.md b/doc/development/testing_guide/review_apps.md index 55ca502f84a..7ad33f05f77 100644 --- a/doc/development/testing_guide/review_apps.md +++ b/doc/development/testing_guide/review_apps.md @@ -6,7 +6,7 @@ Review Apps are automatically deployed by each pipeline, both in ## How does it work? -### CD/CD architecture diagram +### CI/CD architecture diagram ![Review Apps CI/CD architecture](img/review_apps_cicd_architecture.png) @@ -14,23 +14,29 @@ Review Apps are automatically deployed by each pipeline, both in Show mermaid source 
 graph TD
-    B1 -.->|2. once gitlab:assets:compile is done,
triggers a CNG-mirror pipeline and wait for it to be done| A2
-    C1 -.->|2. once review-build-cng is done,
Helm deploys the Review App using the Cloud
Native images built by the CNG-mirror pipeline| A3
-
-subgraph gitlab-ce/ee `test` stage
-    A1[gitlab:assets:compile]
-    B1[review-build-cng] -->|1. wait for| A1
-    C1[review-deploy] -->|1. wait for| B1
-    D1[review-qa-smoke] -->|1. wait for| C1
-    D1[review-qa-smoke] -.->|2. once review-deploy is done| E1>gitlab-qa runs the smoke
suite against the Review App]
+    build-qa-image -.->|once the `prepare` stage is done| gitlab:assets:compile
+    review-build-cng -->|triggers a CNG-mirror pipeline and wait for it to be done| CNG-mirror
+    review-build-cng -.->|once the `test` stage is done| review-deploy
+    review-deploy -.->|once the `review` stage is done| review-qa-smoke
+
+subgraph 1. gitlab-ce/ee `prepare` stage
+    build-qa-image
     end
 
-subgraph CNG-mirror pipeline
-    A2>Cloud Native images are built];
+subgraph 2. gitlab-ce/ee `test` stage
+    gitlab:assets:compile -->|plays dependent job once done| review-build-cng
     end
 
-subgraph GCP `gitlab-review-apps` project
-    A3>"Cloud Native images are deployed to the
`review-apps-ce` or `review-apps-ee` Kubernetes (GKE) cluster"];
+subgraph 3. gitlab-ce/ee `review` stage
+    review-deploy["review-deploy

Helm deploys the Review App using the Cloud
Native images built by the CNG-mirror pipeline.

Cloud Native images are deployed to the `review-apps-ce` or `review-apps-ee`
Kubernetes (GKE) cluster, in the GCP `gitlab-review-apps` project."]
+    end
+
+subgraph 4. gitlab-ce/ee `qa` stage
+    review-qa-smoke[review-qa-smoke

gitlab-qa runs the smoke suite against the Review App.]
+    end
+
+subgraph CNG-mirror pipeline
+    CNG-mirror>Cloud Native images are built];
     end
@@ -38,40 +44,37 @@ subgraph GCP `gitlab-review-apps` project ### Detailed explanation 1. On every [pipeline][gitlab-pipeline] during the `test` stage, the - [`review-build-cng`][review-build-cng] and - [`review-deploy`][review-deploy] jobs are automatically started. - - The [`review-deploy`][review-deploy] job waits for the - [`review-build-cng`][review-build-cng] job to finish. - - The [`review-build-cng`][review-build-cng] job waits for the - [`gitlab:assets:compile`][gitlab:assets:compile] job to finish since the - [`CNG-mirror`][cng-mirror] pipeline triggered in the following step depends on it. -1. Once the [`gitlab:assets:compile`][gitlab:assets:compile] job is done, - [`review-build-cng`][review-build-cng] [triggers a pipeline][cng-pipeline] - in the [`CNG-mirror`][cng-mirror] project. - - The [`CNG-mirror`][cng-pipeline] pipeline creates the Docker images of - each component (e.g. `gitlab-rails-ee`, `gitlab-shell`, `gitaly` etc.) - based on the commit from the [GitLab pipeline][gitlab-pipeline] and store - them in its [registry][cng-mirror-registry]. - - We use the [`CNG-mirror`][cng-mirror] project so that the `CNG`, (**C**loud - **N**ative **G**itLab), project's registry is not overloaded with a - lot of transient Docker images. -1. Once the [`review-build-cng`][review-build-cng] job is done, the - [`review-deploy`][review-deploy] job deploys the Review App using - [the official GitLab Helm chart][helm-chart] to the - [`review-apps-ce`][review-apps-ce] / [`review-apps-ee`][review-apps-ee] - Kubernetes cluster on GCP. - - The actual scripts used to deploy the Review App can be found at - [`scripts/review_apps/review-apps.sh`][review-apps.sh]. - - These scripts are basically - [our official Auto DevOps scripts][Auto-DevOps.gitlab-ci.yml] where the - default CNG images are overridden with the images built and stored in the - [`CNG-mirror` project's registry][cng-mirror-registry]. - - Since we're using [the official GitLab Helm chart][helm-chart], this means - you get a dedicated environment for your branch that's very close to what - it would look in production. + [`gitlab:assets:compile`][gitlab:assets:compile] job is automatically started. + - Once it's done, it starts the [`review-build-cng`][review-build-cng] + manual job since the [`CNG-mirror`][cng-mirror] pipeline triggered in the + following step depends on it. +1. The [`review-build-cng`][review-build-cng] job [triggers a pipeline][cng-mirror-pipeline] + in the [`CNG-mirror`][cng-mirror] project. + - The [`CNG-mirror`][cng-mirror-pipeline] pipeline creates the Docker images of + each component (e.g. `gitlab-rails-ee`, `gitlab-shell`, `gitaly` etc.) + based on the commit from the [GitLab pipeline][gitlab-pipeline] and stores + them in its [registry][cng-mirror-registry]. + - We use the [`CNG-mirror`][cng-mirror] project so that the `CNG`, (**C**loud + **N**ative **G**itLab), project's registry is not overloaded with a + lot of transient Docker images. + - Note that the official CNG images are built by the `cloud-native-image` + job, which runs only for tags, and triggers itself a [`CNG`][cng] pipeline. +1. Once the `test` stage is done, the [`review-deploy`][review-deploy] job + deploys the Review App using [the official GitLab Helm chart][helm-chart] to + the [`review-apps-ce`][review-apps-ce] / [`review-apps-ee`][review-apps-ee] + Kubernetes cluster on GCP. + - The actual scripts used to deploy the Review App can be found at + [`scripts/review_apps/review-apps.sh`][review-apps.sh]. + - These scripts are basically + [our official Auto DevOps scripts][Auto-DevOps.gitlab-ci.yml] where the + default CNG images are overridden with the images built and stored in the + [`CNG-mirror` project's registry][cng-mirror-registry]. + - Since we're using [the official GitLab Helm chart][helm-chart], this means + you get a dedicated environment for your branch that's very close to what + it would look in production. 1. Once the [`review-deploy`][review-deploy] job succeeds, you should be able to - use your Review App thanks to the direct link to it from the MR widget. To log - into the Review App, see "Log into my Review App?" below. + use your Review App thanks to the direct link to it from the MR widget. To log + into the Review App, see "Log into my Review App?" below. **Additional notes:** @@ -82,71 +85,69 @@ subgraph GCP `gitlab-review-apps` project - If the Review App deployment fails, you can simply retry it (there's no need to run the [`review-stop`][gitlab-ci-yml] job first). - The manual [`review-stop`][gitlab-ci-yml] in the `test` stage can be used to - stop a Review App manually, and is also started by GitLab once a branch is - deleted. -- Review Apps are cleaned up regularly using a pipeline schedule that runs + stop a Review App manually, and is also started by GitLab once a merge + request's branch is deleted after being merged. +- Review Apps are cleaned up regularly via a pipeline schedule that runs the [`schedule:review-cleanup`][gitlab-ci-yml] job. ## QA runs -On every [pipeline][gitlab-pipeline] during the `test` stage, the -`review-qa-smoke` job is automatically started: it runs the QA smoke suite. -You can also manually start the `review-qa-all`: it runs the QA full suite. +On every [pipeline][gitlab-pipeline] in the `qa` stage (which comes after the +`review` stage), the `review-qa-smoke` job is automatically started and it runs +the QA smoke suite. -Note that both jobs first wait for the `review-deploy` job to be finished. +You can also manually start the `review-qa-all`: it runs the full QA suite. ## Performance Metrics -On every [pipeline][gitlab-pipeline] during the `test` stage, the +On every [pipeline][gitlab-pipeline] in the `qa` stage, the `review-performance` job is automatically started: this job does basic -browser performance testing using [Sitespeed.io Container](https://docs.gitlab.com/ee/user/project/merge_requests/browser_performance_testing.html) . +browser performance testing using a +[Sitespeed.io Container](https://docs.gitlab.com/ee/user/project/merge_requests/browser_performance_testing.html). -This job waits for the `review-deploy` job to be finished. +## How to: -## How to? - -### Log into my Review App? +### Log into my Review App The default username is `root` and its password can be found in the 1Password secure note named **gitlab-{ce,ee} Review App's root password**. -### Enable a feature flag for my Review App? +### Enable a feature flag for my Review App 1. Open your Review App and log in as documented above. 1. Create a personal access token. 1. Enable the feature flag using the [Feature flag API](../../api/features.md). -### Find my Review App slug? +### Find my Review App slug 1. Open the `review-deploy` job. 1. Look for `Checking for previous deployment of review-*`. 1. For instance for `Checking for previous deployment of review-qa-raise-e-12chm0`, - your Review App slug would be `review-qa-raise-e-12chm0` in this case. + your Review App slug would be `review-qa-raise-e-12chm0` in this case. -### Run a Rails console? +### Run a Rails console 1. [Filter Workloads by your Review App slug](https://console.cloud.google.com/kubernetes/workload?project=gitlab-review-apps) - , e.g. `review-29951-issu-id2qax`. -1. Find and open the `task-runner` Deployment, e.g. `review-29951-issu-id2qax-task-runner`. -1. Click on the Pod in the "Managed pods" section, e.g. `review-29951-issu-id2qax-task-runner-d5455cc8-2lsvz`. + , e.g. `review-qa-raise-e-12chm0`. +1. Find and open the `task-runner` Deployment, e.g. `review-qa-raise-e-12chm0-task-runner`. +1. Click on the Pod in the "Managed pods" section, e.g. `review-qa-raise-e-12chm0-task-runner-d5455cc8-2lsvz`. 1. Click on the `KUBECTL` dropdown, then `Exec` -> `task-runner`. 1. Replace `-c task-runner -- ls` with `-it -- gitlab-rails console` from the - default command or - - Run `kubectl exec --namespace review-apps-ce review-29951-issu-id2qax-task-runner-d5455cc8-2lsvz -it -- gitlab-rails console` - and - - Replace `review-apps-ce` with `review-apps-ee` if the Review App - is running EE, and - - Replace `review-29951-issu-id2qax-task-runner-d5455cc8-2lsvz` - with your Pod's name. + default command or + - Run `kubectl exec --namespace review-apps-ce review-qa-raise-e-12chm0-task-runner-d5455cc8-2lsvz -it -- gitlab-rails console` and + - Replace `review-apps-ce` with `review-apps-ee` if the Review App + is running EE, and + - Replace `review-qa-raise-e-12chm0-task-runner-d5455cc8-2lsvz` + with your Pod's name. -### Dig into a Pod's logs? +### Dig into a Pod's logs -1. [Filter Workloads by your Review App slug](https://console.cloud.google.com/kubernetes/workload?project=gitlab-review-apps) - , e.g. `review-1979-1-mul-dnvlhv`. +1. [Filter Workloads by your Review App slug](https://console.cloud.google.com/kubernetes/workload?project=gitlab-review-apps), + e.g. `review-qa-raise-e-12chm0`. 1. Find and open the `migrations` Deployment, e.g. - `review-1979-1-mul-dnvlhv-migrations.1`. + `review-qa-raise-e-12chm0-migrations.1`. 1. Click on the Pod in the "Managed pods" section, e.g. - `review-1979-1-mul-dnvlhv-migrations.1-nqwtx`. + `review-qa-raise-e-12chm0-migrations.1-nqwtx`. 1. Click on the `Container logs` link. ## Frequently Asked Questions @@ -182,7 +183,8 @@ find a way to limit it to only us.** [review-build-cng]: https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/149511623 [review-deploy]: https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/149511624 [cng-mirror]: https://gitlab.com/gitlab-org/build/CNG-mirror -[cng-pipeline]: https://gitlab.com/gitlab-org/build/CNG-mirror/pipelines/44364657 +[cng]: https://gitlab.com/gitlab-org/build/CNG +[cng-mirror-pipeline]: https://gitlab.com/gitlab-org/build/CNG-mirror/pipelines/44364657 [cng-mirror-registry]: https://gitlab.com/gitlab-org/build/CNG-mirror/container_registry [helm-chart]: https://gitlab.com/charts/gitlab/ [review-apps-ce]: https://console.cloud.google.com/kubernetes/clusters/details/us-central1-a/review-apps-ce?project=gitlab-review-apps -- cgit v1.2.1 From 4a5c48c47cccd8ce2b6bd6912ecff925122778f0 Mon Sep 17 00:00:00 2001 From: syasonik Date: Thu, 25 Apr 2019 11:03:50 +0800 Subject: Move MetricsDashboard to Metrics::Dashboard --- .../projects/environments_controller.rb | 2 +- lib/gitlab/metrics/dashboard/processor.rb | 41 ++++++++ lib/gitlab/metrics/dashboard/service.rb | 40 ++++++++ lib/gitlab/metrics/dashboard/stages/base_stage.rb | 60 ++++++++++++ .../dashboard/stages/common_metrics_inserter.rb | 23 +++++ .../dashboard/stages/project_metrics_inserter.rb | 106 +++++++++++++++++++++ lib/gitlab/metrics/dashboard/stages/sorter.rb | 34 +++++++ lib/gitlab/metrics_dashboard/processor.rb | 39 -------- lib/gitlab/metrics_dashboard/service.rb | 38 -------- lib/gitlab/metrics_dashboard/stages/base_stage.rb | 58 ----------- .../stages/common_metrics_inserter.rb | 21 ---- .../stages/project_metrics_inserter.rb | 104 -------------------- lib/gitlab/metrics_dashboard/stages/sorter.rb | 32 ------- .../gitlab/metrics/dashboard/sample_dashboard.yml | 36 +++++++ .../metrics/dashboard/schemas/dashboard.json | 13 +++ .../gitlab/metrics/dashboard/schemas/metrics.json | 20 ++++ .../metrics/dashboard/schemas/panel_groups.json | 17 ++++ .../gitlab/metrics/dashboard/schemas/panels.json | 20 ++++ .../gitlab/metrics_dashboard/sample_dashboard.yml | 36 ------- .../metrics_dashboard/schemas/dashboard.json | 17 ---- .../gitlab/metrics_dashboard/schemas/metrics.json | 20 ---- .../metrics_dashboard/schemas/panel_groups.json | 17 ---- .../gitlab/metrics_dashboard/schemas/panels.json | 20 ---- .../lib/gitlab/metrics/dashboard/processor_spec.rb | 94 ++++++++++++++++++ spec/lib/gitlab/metrics/dashboard/service_spec.rb | 42 ++++++++ .../lib/gitlab/metrics_dashboard/processor_spec.rb | 94 ------------------ spec/lib/gitlab/metrics_dashboard/service_spec.rb | 42 -------- 27 files changed, 547 insertions(+), 539 deletions(-) create mode 100644 lib/gitlab/metrics/dashboard/processor.rb create mode 100644 lib/gitlab/metrics/dashboard/service.rb create mode 100644 lib/gitlab/metrics/dashboard/stages/base_stage.rb create mode 100644 lib/gitlab/metrics/dashboard/stages/common_metrics_inserter.rb create mode 100644 lib/gitlab/metrics/dashboard/stages/project_metrics_inserter.rb create mode 100644 lib/gitlab/metrics/dashboard/stages/sorter.rb delete mode 100644 lib/gitlab/metrics_dashboard/processor.rb delete mode 100644 lib/gitlab/metrics_dashboard/service.rb delete mode 100644 lib/gitlab/metrics_dashboard/stages/base_stage.rb delete mode 100644 lib/gitlab/metrics_dashboard/stages/common_metrics_inserter.rb delete mode 100644 lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb delete mode 100644 lib/gitlab/metrics_dashboard/stages/sorter.rb create mode 100644 spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml create mode 100644 spec/fixtures/lib/gitlab/metrics/dashboard/schemas/dashboard.json create mode 100644 spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json create mode 100644 spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panel_groups.json create mode 100644 spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panels.json delete mode 100644 spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml delete mode 100644 spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json delete mode 100644 spec/fixtures/lib/gitlab/metrics_dashboard/schemas/metrics.json delete mode 100644 spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panel_groups.json delete mode 100644 spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panels.json create mode 100644 spec/lib/gitlab/metrics/dashboard/processor_spec.rb create mode 100644 spec/lib/gitlab/metrics/dashboard/service_spec.rb delete mode 100644 spec/lib/gitlab/metrics_dashboard/processor_spec.rb delete mode 100644 spec/lib/gitlab/metrics_dashboard/service_spec.rb diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 30cbe35edf5..1f619c8fd2f 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -160,7 +160,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController def metrics_dashboard return render_403 unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, @project) - result = Gitlab::MetricsDashboard::Service.new(@project, @current_user, environment: environment).get_dashboard + result = Gitlab::Metrics::Dashboard::Service.new(@project, @current_user, environment: environment).get_dashboard respond_to do |format| if result[:status] == :success diff --git a/lib/gitlab/metrics/dashboard/processor.rb b/lib/gitlab/metrics/dashboard/processor.rb new file mode 100644 index 00000000000..36de1d033f2 --- /dev/null +++ b/lib/gitlab/metrics/dashboard/processor.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Gitlab + module Metrics + module Dashboard + # Responsible for processesing a dashboard hash, inserting + # relevant DB records & sorting for proper rendering in + # the UI. These includes shared metric info, custom metrics + # info, and alerts (only in EE). + class Processor + SEQUENCE = [ + Stages::CommonMetricsInserter, + Stages::ProjectMetricsInserter, + Stages::Sorter + ].freeze + + def initialize(project, environment) + @project = project + @environment = environment + end + + # Returns a new dashboard hash with the results of + # running transforms on the dashboard. + def process(dashboard) + dashboard = dashboard.deep_symbolize_keys + + stage_params = [@project, @environment] + sequence.each { |stage| stage.new(*stage_params).transform!(dashboard) } + + dashboard + end + + private + + def sequence + SEQUENCE + end + end + end + end +end diff --git a/lib/gitlab/metrics/dashboard/service.rb b/lib/gitlab/metrics/dashboard/service.rb new file mode 100644 index 00000000000..966d7279aef --- /dev/null +++ b/lib/gitlab/metrics/dashboard/service.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +# Fetches the metrics dashboard layout and supplemented the output with DB info. +module Gitlab + module Metrics + module Dashboard + class Service < ::BaseService + SYSTEM_DASHBOARD_NAME = 'common_metrics' + SYSTEM_DASHBOARD_PATH = Rails.root.join('config', 'prometheus', "#{SYSTEM_DASHBOARD_NAME}.yml") + + # Returns a DB-supplemented json representation of a dashboard config file. + def get_dashboard + dashboard_string = Rails.cache.fetch(cache_key) { system_dashboard } + + dashboard = process_dashboard(dashboard_string) + + success(dashboard: dashboard) + rescue Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardLayoutError => e + error(e.message, :unprocessable_entity) + end + + private + + # Returns the base metrics shipped with every GitLab service. + def system_dashboard + YAML.load_file(SYSTEM_DASHBOARD_PATH) + end + + def cache_key + "metrics_dashboard_#{SYSTEM_DASHBOARD_NAME}" + end + + # Returns a new dashboard Hash, supplemented with DB info + def process_dashboard(dashboard) + Processor.new(project, params[:environment]).process(dashboard) + end + end + end + end +end diff --git a/lib/gitlab/metrics/dashboard/stages/base_stage.rb b/lib/gitlab/metrics/dashboard/stages/base_stage.rb new file mode 100644 index 00000000000..daecb2698ae --- /dev/null +++ b/lib/gitlab/metrics/dashboard/stages/base_stage.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module Gitlab + module Metrics + module Dashboard + module Stages + class BaseStage + DashboardLayoutError = Class.new(StandardError) + + DEFAULT_PANEL_TYPE = 'area-chart' + + attr_reader :project, :environment + + def initialize(project, environment) + @project = project + @environment = environment + end + + # Entry-point to the stage + # @param dashboard [Hash] + # @param project [Project] + # @param environment [Environment] + def transform!(_dashboard) + raise NotImplementedError + end + + protected + + def missing_panel_groups! + raise DashboardLayoutError.new('Top-level key :panel_groups must be an array') + end + + def missing_panels! + raise DashboardLayoutError.new('Each "panel_group" must define an array :panels') + end + + def missing_metrics! + raise DashboardLayoutError.new('Each "panel" must define an array :metrics') + end + + def for_metrics(dashboard) + missing_panel_groups! unless dashboard[:panel_groups].is_a?(Array) + + dashboard[:panel_groups].each do |panel_group| + missing_panels! unless panel_group[:panels].is_a?(Array) + + panel_group[:panels].each do |panel| + missing_metrics! unless panel[:metrics].is_a?(Array) + + panel[:metrics].each do |metric| + yield metric + end + end + end + end + end + end + end + end +end diff --git a/lib/gitlab/metrics/dashboard/stages/common_metrics_inserter.rb b/lib/gitlab/metrics/dashboard/stages/common_metrics_inserter.rb new file mode 100644 index 00000000000..20e6b98e20b --- /dev/null +++ b/lib/gitlab/metrics/dashboard/stages/common_metrics_inserter.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Gitlab + module Metrics + module Dashboard + module Stages + class CommonMetricsInserter < BaseStage + # For each metric in the dashboard config, attempts to + # find a corresponding database record. If found, + # includes the record's id in the dashboard config. + def transform!(dashboard) + common_metrics = ::PrometheusMetric.common + + for_metrics(dashboard) do |metric| + metric_record = common_metrics.find { |m| m.identifier == metric[:id] } + metric[:metric_id] = metric_record.id if metric_record + end + end + end + end + end + end +end diff --git a/lib/gitlab/metrics/dashboard/stages/project_metrics_inserter.rb b/lib/gitlab/metrics/dashboard/stages/project_metrics_inserter.rb new file mode 100644 index 00000000000..0dc87f1ec16 --- /dev/null +++ b/lib/gitlab/metrics/dashboard/stages/project_metrics_inserter.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +module Gitlab + module Metrics + module Dashboard + module Stages + class ProjectMetricsInserter < BaseStage + # Inserts project-specific metrics into the dashboard + # config. If there are no project-specific metrics, + # this will have no effect. + def transform!(dashboard) + project.prometheus_metrics.each do |project_metric| + group = find_or_create_panel_group(dashboard[:panel_groups], project_metric) + panel = find_or_create_panel(group[:panels], project_metric) + find_or_create_metric(panel[:metrics], project_metric) + end + end + + private + + # Looks for a panel_group corresponding to the + # provided metric object. If unavailable, inserts one. + # @param panel_groups [Array] + # @param metric [PrometheusMetric] + def find_or_create_panel_group(panel_groups, metric) + panel_group = find_panel_group(panel_groups, metric) + return panel_group if panel_group + + panel_group = new_panel_group(metric) + panel_groups << panel_group + + panel_group + end + + # Looks for a panel corresponding to the provided + # metric object. If unavailable, inserts one. + # @param panels [Array] + # @param metric [PrometheusMetric] + def find_or_create_panel(panels, metric) + panel = find_panel(panels, metric) + return panel if panel + + panel = new_panel(metric) + panels << panel + + panel + end + + # Looks for a metric corresponding to the provided + # metric object. If unavailable, inserts one. + # @param metrics [Array] + # @param metric [PrometheusMetric] + def find_or_create_metric(metrics, metric) + target_metric = find_metric(metrics, metric) + return target_metric if target_metric + + target_metric = new_metric(metric) + metrics << target_metric + + target_metric + end + + def find_panel_group(panel_groups, metric) + return unless panel_groups + + panel_groups.find { |group| group[:group] == metric.group_title } + end + + def find_panel(panels, metric) + return unless panels + + panel_identifiers = [DEFAULT_PANEL_TYPE, metric.title, metric.y_label] + panels.find { |panel| panel.values_at(:type, :title, :y_label) == panel_identifiers } + end + + def find_metric(metrics, metric) + return unless metrics + + metrics.find { |m| m[:id] == metric.identifier } + end + + def new_panel_group(metric) + { + group: metric.group_title, + priority: metric.priority, + panels: [] + } + end + + def new_panel(metric) + { + type: DEFAULT_PANEL_TYPE, + title: metric.title, + y_label: metric.y_label, + metrics: [] + } + end + + def new_metric(metric) + metric.queries.first.merge(metric_id: metric.id) + end + end + end + end + end +end diff --git a/lib/gitlab/metrics/dashboard/stages/sorter.rb b/lib/gitlab/metrics/dashboard/stages/sorter.rb new file mode 100644 index 00000000000..9c4183fff99 --- /dev/null +++ b/lib/gitlab/metrics/dashboard/stages/sorter.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module Gitlab + module Metrics + module Dashboard + module Stages + class Sorter < BaseStage + def transform!(dashboard) + missing_panel_groups! unless dashboard[:panel_groups].is_a? Array + + sort_groups!(dashboard) + sort_panels!(dashboard) + end + + private + + # Sorts the groups in the dashboard by the :priority key + def sort_groups!(dashboard) + dashboard[:panel_groups] = dashboard[:panel_groups].sort_by { |group| -group[:priority].to_i } + end + + # Sorts the panels in the dashboard by the :weight key + def sort_panels!(dashboard) + dashboard[:panel_groups].each do |group| + missing_panels! unless group[:panels].is_a? Array + + group[:panels] = group[:panels].sort_by { |panel| -panel[:weight].to_i } + end + end + end + end + end + end +end diff --git a/lib/gitlab/metrics_dashboard/processor.rb b/lib/gitlab/metrics_dashboard/processor.rb deleted file mode 100644 index 8fa95f1ee2d..00000000000 --- a/lib/gitlab/metrics_dashboard/processor.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module MetricsDashboard - # Responsible for processesing a dashboard hash, inserting - # relevant DB records & sorting for proper rendering in - # the UI. These includes shared metric info, custom metrics - # info, and alerts (only in EE). - class Processor - SEQUENCE = [ - Stages::CommonMetricsInserter, - Stages::ProjectMetricsInserter, - Stages::Sorter - ].freeze - - def initialize(project, environment) - @project = project - @environment = environment - end - - # Returns a new dashboard hash with the results of - # running transforms on the dashboard. - def process(dashboard) - dashboard = dashboard.deep_transform_keys(&:to_sym) - - stage_params = [@project, @environment] - sequence.each { |stage| stage.new(*stage_params).transform!(dashboard) } - - dashboard - end - - private - - def sequence - SEQUENCE - end - end - end -end diff --git a/lib/gitlab/metrics_dashboard/service.rb b/lib/gitlab/metrics_dashboard/service.rb deleted file mode 100644 index 84f174c7d1b..00000000000 --- a/lib/gitlab/metrics_dashboard/service.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -# Fetches the metrics dashboard layout and supplemented the output with DB info. -module Gitlab - module MetricsDashboard - class Service < ::BaseService - SYSTEM_DASHBOARD_NAME = 'common_metrics' - SYSTEM_DASHBOARD_PATH = Rails.root.join('config', 'prometheus', "#{SYSTEM_DASHBOARD_NAME}.yml") - - # Returns a DB-supplemented json representation of a dashboard config file. - def get_dashboard - dashboard_string = Rails.cache.fetch(cache_key) { system_dashboard } - - dashboard = process_dashboard(dashboard_string) - - success(dashboard: dashboard) - rescue Gitlab::MetricsDashboard::Stages::BaseStage::DashboardLayoutError => e - error(e.message, :unprocessable_entity) - end - - private - - # Returns the base metrics shipped with every GitLab service. - def system_dashboard - YAML.load_file(SYSTEM_DASHBOARD_PATH) - end - - def cache_key - "metrics_dashboard_#{SYSTEM_DASHBOARD_NAME}" - end - - # Returns a new dashboard Hash, supplemented with DB info - def process_dashboard(dashboard) - Processor.new(project, params[:environment]).process(dashboard) - end - end - end -end diff --git a/lib/gitlab/metrics_dashboard/stages/base_stage.rb b/lib/gitlab/metrics_dashboard/stages/base_stage.rb deleted file mode 100644 index df49126a07b..00000000000 --- a/lib/gitlab/metrics_dashboard/stages/base_stage.rb +++ /dev/null @@ -1,58 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module MetricsDashboard - module Stages - class BaseStage - DashboardLayoutError = Class.new(StandardError) - - DEFAULT_PANEL_TYPE = 'area-chart' - - attr_reader :project, :environment - - def initialize(project, environment) - @project = project - @environment = environment - end - - # Entry-point to the stage - # @param dashboard [Hash] - # @param project [Project] - # @param environment [Environment] - def transform!(_dashboard) - raise NotImplementedError - end - - protected - - def missing_panel_groups! - raise DashboardLayoutError.new('Top-level key :panel_groups must be an array') - end - - def missing_panels! - raise DashboardLayoutError.new('Each "panel_group" must define an array :panels') - end - - def missing_metrics! - raise DashboardLayoutError.new('Each "panel" must define an array :metrics') - end - - def for_metrics(dashboard) - missing_panel_groups! unless dashboard[:panel_groups].is_a?(Array) - - dashboard[:panel_groups].each do |panel_group| - missing_panels! unless panel_group[:panels].is_a?(Array) - - panel_group[:panels].each do |panel| - missing_metrics! unless panel[:metrics].is_a?(Array) - - panel[:metrics].each do |metric| - yield metric - end - end - end - end - end - end - end -end diff --git a/lib/gitlab/metrics_dashboard/stages/common_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/stages/common_metrics_inserter.rb deleted file mode 100644 index ef70347c6b2..00000000000 --- a/lib/gitlab/metrics_dashboard/stages/common_metrics_inserter.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module MetricsDashboard - module Stages - class CommonMetricsInserter < BaseStage - # For each metric in the dashboard config, attempts to - # find a corresponding database record. If found, - # includes the record's id in the dashboard config. - def transform!(dashboard) - common_metrics = ::PrometheusMetric.common - - for_metrics(dashboard) do |metric| - metric_record = common_metrics.find { |m| m.identifier == metric[:id] } - metric[:metric_id] = metric_record.id if metric_record - end - end - end - end - end -end diff --git a/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb deleted file mode 100644 index ab33ee75cce..00000000000 --- a/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb +++ /dev/null @@ -1,104 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module MetricsDashboard - module Stages - class ProjectMetricsInserter < BaseStage - # Inserts project-specific metrics into the dashboard - # config. If there are no project-specific metrics, - # this will have no effect. - def transform!(dashboard) - project.prometheus_metrics.each do |project_metric| - group = find_or_create_panel_group(dashboard[:panel_groups], project_metric) - panel = find_or_create_panel(group[:panels], project_metric) - find_or_create_metric(panel[:metrics], project_metric) - end - end - - private - - # Looks for a panel_group corresponding to the - # provided metric object. If unavailable, inserts one. - # @param panel_groups [Array] - # @param metric [PrometheusMetric] - def find_or_create_panel_group(panel_groups, metric) - panel_group = find_panel_group(panel_groups, metric) - return panel_group if panel_group - - panel_group = new_panel_group(metric) - panel_groups << panel_group - - panel_group - end - - # Looks for a panel corresponding to the provided - # metric object. If unavailable, inserts one. - # @param panels [Array] - # @param metric [PrometheusMetric] - def find_or_create_panel(panels, metric) - panel = find_panel(panels, metric) - return panel if panel - - panel = new_panel(metric) - panels << panel - - panel - end - - # Looks for a metric corresponding to the provided - # metric object. If unavailable, inserts one. - # @param metrics [Array] - # @param metric [PrometheusMetric] - def find_or_create_metric(metrics, metric) - target_metric = find_metric(metrics, metric) - return target_metric if target_metric - - target_metric = new_metric(metric) - metrics << target_metric - - target_metric - end - - def find_panel_group(panel_groups, metric) - return unless panel_groups - - panel_groups.find { |group| group[:group] == metric.group_title } - end - - def find_panel(panels, metric) - return unless panels - - panel_identifiers = [DEFAULT_PANEL_TYPE, metric.title, metric.y_label] - panels.find { |panel| panel.values_at(:type, :title, :y_label) == panel_identifiers } - end - - def find_metric(metrics, metric) - return unless metrics - - metrics.find { |m| m[:id] == metric.identifier } - end - - def new_panel_group(metric) - { - group: metric.group_title, - priority: metric.priority, - panels: [] - } - end - - def new_panel(metric) - { - type: DEFAULT_PANEL_TYPE, - title: metric.title, - y_label: metric.y_label, - metrics: [] - } - end - - def new_metric(metric) - metric.queries.first.merge(metric_id: metric.id) - end - end - end - end -end diff --git a/lib/gitlab/metrics_dashboard/stages/sorter.rb b/lib/gitlab/metrics_dashboard/stages/sorter.rb deleted file mode 100644 index ca310c9637a..00000000000 --- a/lib/gitlab/metrics_dashboard/stages/sorter.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module MetricsDashboard - module Stages - class Sorter < BaseStage - def transform!(dashboard) - missing_panel_groups! unless dashboard[:panel_groups].is_a? Array - - sort_groups!(dashboard) - sort_panels!(dashboard) - end - - private - - # Sorts the groups in the dashboard by the :priority key - def sort_groups!(dashboard) - dashboard[:panel_groups] = dashboard[:panel_groups].sort_by { |group| -group[:priority].to_i } - end - - # Sorts the panels in the dashboard by the :weight key - def sort_panels!(dashboard) - dashboard[:panel_groups].each do |group| - missing_panels! unless group[:panels].is_a? Array - - group[:panels] = group[:panels].sort_by { |panel| -panel[:weight].to_i } - end - end - end - end - end -end diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml b/spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml new file mode 100644 index 00000000000..c2d3d3d8aca --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml @@ -0,0 +1,36 @@ +dashboard: 'Test Dashboard' +priority: 1 +panel_groups: +- group: Group A + priority: 10 + panels: + - title: "Super Chart A1" + type: "area-chart" + y_label: "y_label" + weight: 1 + metrics: + - id: metric_a1 + query_range: 'query' + unit: unit + label: Legend Label + - title: "Super Chart A2" + type: "area-chart" + y_label: "y_label" + weight: 2 + metrics: + - id: metric_a2 + query_range: 'query' + label: Legend Label + unit: unit +- group: Group B + priority: 1 + panels: + - title: "Super Chart B" + type: "area-chart" + y_label: "y_label" + weight: 1 + metrics: + - id: metric_b + query_range: 'query' + unit: unit + label: Legend Label diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/dashboard.json b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/dashboard.json new file mode 100644 index 00000000000..1ee1205e29a --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/dashboard.json @@ -0,0 +1,13 @@ +{ + "type": "object", + "required": ["dashboard", "priority", "panel_groups"], + "properties": { + "dashboard": { "type": "string" }, + "priority": { "type": "number" }, + "panel_groups": { + "type": "array", + "items": { "$ref": "spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panel_groups.json" } + } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json new file mode 100644 index 00000000000..2d0af57ec2c --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json @@ -0,0 +1,20 @@ +{ + "type": "object", + "required": [ + "unit", + "label" + ], + "oneOf": [ + { "required": ["query"] }, + { "required": ["query_range"] } + ], + "properties": { + "id": { "type": "string" }, + "query_range": { "type": "string" }, + "query": { "type": "string" }, + "unit": { "type": "string" }, + "label": { "type": "string" }, + "track": { "type": "string" } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panel_groups.json b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panel_groups.json new file mode 100644 index 00000000000..d7a390adcdc --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panel_groups.json @@ -0,0 +1,17 @@ +{ + "type": "object", + "required": [ + "group", + "priority", + "panels" + ], + "properties": { + "group": { "type": "string" }, + "priority": { "type": "number" }, + "panels": { + "type": "array", + "items": { "$ref": "panels.json" } + } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panels.json b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panels.json new file mode 100644 index 00000000000..1548daacd64 --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panels.json @@ -0,0 +1,20 @@ +{ + "type": "object", + "required": [ + "title", + "y_label", + "weight", + "metrics" + ], + "properties": { + "title": { "type": "string" }, + "type": { "type": "string" }, + "y_label": { "type": "string" }, + "weight": { "type": "number" }, + "metrics": { + "type": "array", + "items": { "$ref": "metrics.json" } + } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml b/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml deleted file mode 100644 index c2d3d3d8aca..00000000000 --- a/spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml +++ /dev/null @@ -1,36 +0,0 @@ -dashboard: 'Test Dashboard' -priority: 1 -panel_groups: -- group: Group A - priority: 10 - panels: - - title: "Super Chart A1" - type: "area-chart" - y_label: "y_label" - weight: 1 - metrics: - - id: metric_a1 - query_range: 'query' - unit: unit - label: Legend Label - - title: "Super Chart A2" - type: "area-chart" - y_label: "y_label" - weight: 2 - metrics: - - id: metric_a2 - query_range: 'query' - label: Legend Label - unit: unit -- group: Group B - priority: 1 - panels: - - title: "Super Chart B" - type: "area-chart" - y_label: "y_label" - weight: 1 - metrics: - - id: metric_b - query_range: 'query' - unit: unit - label: Legend Label diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json deleted file mode 100644 index 2d7601b64b1..00000000000 --- a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/dashboard.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "type": "object", - "required": [ - "dashboard", - "priority", - "panel_groups" - ], - "properties": { - "dashboard": { "type": "string" }, - "priority": { "type": "number" }, - "panel_groups": { - "type": "array", - "items": { "$ref": "spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panel_groups.json" } - } - }, - "additionalProperties": false -} diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/metrics.json b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/metrics.json deleted file mode 100644 index 2d0af57ec2c..00000000000 --- a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/metrics.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "type": "object", - "required": [ - "unit", - "label" - ], - "oneOf": [ - { "required": ["query"] }, - { "required": ["query_range"] } - ], - "properties": { - "id": { "type": "string" }, - "query_range": { "type": "string" }, - "query": { "type": "string" }, - "unit": { "type": "string" }, - "label": { "type": "string" }, - "track": { "type": "string" } - }, - "additionalProperties": false -} diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panel_groups.json b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panel_groups.json deleted file mode 100644 index d7a390adcdc..00000000000 --- a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panel_groups.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "type": "object", - "required": [ - "group", - "priority", - "panels" - ], - "properties": { - "group": { "type": "string" }, - "priority": { "type": "number" }, - "panels": { - "type": "array", - "items": { "$ref": "panels.json" } - } - }, - "additionalProperties": false -} diff --git a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panels.json b/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panels.json deleted file mode 100644 index 1548daacd64..00000000000 --- a/spec/fixtures/lib/gitlab/metrics_dashboard/schemas/panels.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "type": "object", - "required": [ - "title", - "y_label", - "weight", - "metrics" - ], - "properties": { - "title": { "type": "string" }, - "type": { "type": "string" }, - "y_label": { "type": "string" }, - "weight": { "type": "number" }, - "metrics": { - "type": "array", - "items": { "$ref": "metrics.json" } - } - }, - "additionalProperties": false -} diff --git a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb new file mode 100644 index 00000000000..e27ba2f1f4e --- /dev/null +++ b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Metrics::Dashboard::Processor do + let(:project) { build(:project) } + let(:environment) { build(:environment) } + let(:dashboard_yml) { YAML.load_file('spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml') } + + describe 'process' do + let(:process_params) { [project, environment] } + let(:dashboard) { described_class.new(*process_params).process(dashboard_yml) } + + context 'when dashboard config corresponds to common metrics' do + let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') } + + it 'inserts metric ids into the config' do + target_metric = all_metrics.find { |metric| metric[:id] == 'metric_a1' } + + expect(target_metric).to include(:metric_id) + expect(target_metric[:metric_id]).to eq(common_metric.id) + end + end + + context 'when the project has associated metrics' do + let!(:project_response_metric) { create(:prometheus_metric, project: project, group: :response) } + let!(:project_system_metric) { create(:prometheus_metric, project: project, group: :system) } + let!(:project_business_metric) { create(:prometheus_metric, project: project, group: :business) } + + it 'includes project-specific metrics' do + expect(all_metrics).to include get_metric_details(project_system_metric) + expect(all_metrics).to include get_metric_details(project_response_metric) + expect(all_metrics).to include get_metric_details(project_business_metric) + end + + it 'orders groups by priority and panels by weight' do + expected_metrics_order = [ + 'metric_a2', # group priority 10, panel weight 2 + 'metric_a1', # group priority 10, panel weight 1 + 'metric_b', # group priority 1, panel weight 1 + project_business_metric.id, # group priority 0, panel weight nil (0) + project_response_metric.id, # group priority -5, panel weight nil (0) + project_system_metric.id, # group priority -10, panel weight nil (0) + ] + actual_metrics_order = all_metrics.map { |m| m[:id] || m[:metric_id] } + + expect(actual_metrics_order).to eq expected_metrics_order + end + end + + shared_examples_for 'errors with message' do |expected_message| + it 'raises a DashboardLayoutError' do + error_class = Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardLayoutError + + expect { dashboard }.to raise_error(error_class, expected_message) + end + end + + context 'when the dashboard is missing panel_groups' do + let(:dashboard_yml) { {} } + + it_behaves_like 'errors with message', 'Top-level key :panel_groups must be an array' + end + + context 'when the dashboard contains a panel_group which is missing panels' do + let(:dashboard_yml) { { panel_groups: [{}] } } + + it_behaves_like 'errors with message', 'Each "panel_group" must define an array :panels' + end + + context 'when the dashboard contains a panel which is missing metrics' do + let(:dashboard_yml) { { panel_groups: [{ panels: [{}] }] } } + + it_behaves_like 'errors with message', 'Each "panel" must define an array :metrics' + end + end + + private + + def all_metrics + dashboard[:panel_groups].map do |group| + group[:panels].map { |panel| panel[:metrics] } + end.flatten + end + + def get_metric_details(metric) + { + query_range: metric.query, + unit: metric.unit, + label: metric.legend, + metric_id: metric.id + } + end +end diff --git a/spec/lib/gitlab/metrics/dashboard/service_spec.rb b/spec/lib/gitlab/metrics/dashboard/service_spec.rb new file mode 100644 index 00000000000..3f82fd7ebf8 --- /dev/null +++ b/spec/lib/gitlab/metrics/dashboard/service_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Metrics::Dashboard::Service, :use_clean_rails_memory_store_caching do + let(:project) { build(:project) } + let(:environment) { build(:environment) } + + describe 'get_dashboard' do + let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics/dashboard/schemas/dashboard.json')) } + + it 'returns a json representation of the environment dashboard' do + result = described_class.new(project, environment).get_dashboard + + expect(result.keys).to contain_exactly(:dashboard, :status) + expect(result[:status]).to eq(:success) + + expect(JSON::Validator.fully_validate(dashboard_schema, result[:dashboard])).to be_empty + end + + it 'caches the dashboard for subsequent calls' do + expect(YAML).to receive(:load_file).once.and_call_original + + described_class.new(project, environment).get_dashboard + described_class.new(project, environment).get_dashboard + end + + context 'when the dashboard is configured incorrectly' do + before do + allow(YAML).to receive(:load_file).and_return({}) + end + + it 'returns an appropriate message and status code' do + result = described_class.new(project, environment).get_dashboard + + expect(result.keys).to contain_exactly(:message, :http_status, :status) + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(:unprocessable_entity) + end + end + end +end diff --git a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb b/spec/lib/gitlab/metrics_dashboard/processor_spec.rb deleted file mode 100644 index 9a4897944c6..00000000000 --- a/spec/lib/gitlab/metrics_dashboard/processor_spec.rb +++ /dev/null @@ -1,94 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::MetricsDashboard::Processor do - let(:project) { build(:project) } - let(:environment) { build(:environment) } - let(:dashboard_yml) { YAML.load_file('spec/fixtures/lib/gitlab/metrics_dashboard/sample_dashboard.yml') } - - describe 'process' do - let(:process_params) { [project, environment] } - let(:dashboard) { described_class.new(*process_params).process(dashboard_yml) } - - context 'when dashboard config corresponds to common metrics' do - let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') } - - it 'inserts metric ids into the config' do - target_metric = all_metrics.find { |metric| metric[:id] == 'metric_a1' } - - expect(target_metric).to include(:metric_id) - expect(target_metric[:metric_id]).to eq(common_metric.id) - end - end - - context 'when the project has associated metrics' do - let!(:project_response_metric) { create(:prometheus_metric, project: project, group: :response) } - let!(:project_system_metric) { create(:prometheus_metric, project: project, group: :system) } - let!(:project_business_metric) { create(:prometheus_metric, project: project, group: :business) } - - it 'includes project-specific metrics' do - expect(all_metrics).to include get_metric_details(project_system_metric) - expect(all_metrics).to include get_metric_details(project_response_metric) - expect(all_metrics).to include get_metric_details(project_business_metric) - end - - it 'orders groups by priority and panels by weight' do - expected_metrics_order = [ - 'metric_a2', # group priority 10, panel weight 2 - 'metric_a1', # group priority 10, panel weight 1 - 'metric_b', # group priority 1, panel weight 1 - project_business_metric.id, # group priority 0, panel weight nil (0) - project_response_metric.id, # group priority -5, panel weight nil (0) - project_system_metric.id, # group priority -10, panel weight nil (0) - ] - actual_metrics_order = all_metrics.map { |m| m[:id] || m[:metric_id] } - - expect(actual_metrics_order).to eq expected_metrics_order - end - end - - shared_examples_for 'errors with message' do |expected_message| - it 'raises a DashboardLayoutError' do - error_class = Gitlab::MetricsDashboard::Stages::BaseStage::DashboardLayoutError - - expect { dashboard }.to raise_error(error_class, expected_message) - end - end - - context 'when the dashboard is missing panel_groups' do - let(:dashboard_yml) { {} } - - it_behaves_like 'errors with message', 'Top-level key :panel_groups must be an array' - end - - context 'when the dashboard contains a panel_group which is missing panels' do - let(:dashboard_yml) { { panel_groups: [{}] } } - - it_behaves_like 'errors with message', 'Each "panel_group" must define an array :panels' - end - - context 'when the dashboard contains a panel which is missing metrics' do - let(:dashboard_yml) { { panel_groups: [{ panels: [{}] }] } } - - it_behaves_like 'errors with message', 'Each "panel" must define an array :metrics' - end - end - - private - - def all_metrics - dashboard[:panel_groups].map do |group| - group[:panels].map { |panel| panel[:metrics] } - end.flatten - end - - def get_metric_details(metric) - { - query_range: metric.query, - unit: metric.unit, - label: metric.legend, - metric_id: metric.id - } - end -end diff --git a/spec/lib/gitlab/metrics_dashboard/service_spec.rb b/spec/lib/gitlab/metrics_dashboard/service_spec.rb deleted file mode 100644 index d2b8231fbfb..00000000000 --- a/spec/lib/gitlab/metrics_dashboard/service_spec.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::MetricsDashboard::Service, :use_clean_rails_memory_store_caching do - let(:project) { build(:project) } - let(:environment) { build(:environment) } - - describe 'get_dashboard' do - let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics_dashboard/schemas/dashboard.json')) } - - it 'returns a json representation of the environment dashboard' do - result = described_class.new(project, environment).get_dashboard - - expect(result.keys).to contain_exactly(:dashboard, :status) - expect(result[:status]).to eq(:success) - - expect(JSON::Validator.fully_validate(dashboard_schema, result[:dashboard])).to be_empty - end - - it 'caches the dashboard for subsequent calls' do - expect(YAML).to receive(:load_file).once.and_call_original - - described_class.new(project, environment).get_dashboard - described_class.new(project, environment).get_dashboard - end - - context 'when the dashboard is configured incorrectly' do - before do - allow(YAML).to receive(:load_file).and_return({}) - end - - it 'returns an appropriate message and status code' do - result = described_class.new(project, environment).get_dashboard - - expect(result.keys).to contain_exactly(:message, :http_status, :status) - expect(result[:status]).to eq(:error) - expect(result[:http_status]).to eq(:unprocessable_entity) - end - end - end -end -- cgit v1.2.1 From 8926b37d5b0c48b9ef89e4769e622563a9b11e9f Mon Sep 17 00:00:00 2001 From: syasonik Date: Thu, 25 Apr 2019 14:00:51 +0800 Subject: Prefer safe_load and deep_symbolize_keys --- lib/gitlab/metrics/dashboard/processor.rb | 13 ++++++------- lib/gitlab/metrics/dashboard/service.rb | 4 ++-- spec/controllers/projects/environments_controller_spec.rb | 2 +- spec/lib/gitlab/metrics/dashboard/service_spec.rb | 4 ++-- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/lib/gitlab/metrics/dashboard/processor.rb b/lib/gitlab/metrics/dashboard/processor.rb index 36de1d033f2..46fd2f9440d 100644 --- a/lib/gitlab/metrics/dashboard/processor.rb +++ b/lib/gitlab/metrics/dashboard/processor.rb @@ -21,13 +21,12 @@ module Gitlab # Returns a new dashboard hash with the results of # running transforms on the dashboard. - def process(dashboard) - dashboard = dashboard.deep_symbolize_keys - - stage_params = [@project, @environment] - sequence.each { |stage| stage.new(*stage_params).transform!(dashboard) } - - dashboard + def process(raw_dashboard) + raw_dashboard.deep_symbolize_keys.tap do |dashboard| + sequence.each do |stage| + stage.new(@project, @environment).transform!(dashboard) + end + end end private diff --git a/lib/gitlab/metrics/dashboard/service.rb b/lib/gitlab/metrics/dashboard/service.rb index 966d7279aef..b8f144a7222 100644 --- a/lib/gitlab/metrics/dashboard/service.rb +++ b/lib/gitlab/metrics/dashboard/service.rb @@ -23,7 +23,7 @@ module Gitlab # Returns the base metrics shipped with every GitLab service. def system_dashboard - YAML.load_file(SYSTEM_DASHBOARD_PATH) + YAML.safe_load(File.read(SYSTEM_DASHBOARD_PATH)) end def cache_key @@ -32,7 +32,7 @@ module Gitlab # Returns a new dashboard Hash, supplemented with DB info def process_dashboard(dashboard) - Processor.new(project, params[:environment]).process(dashboard) + Gitlab::Metrics::Dashboard::Processor.new(project, params[:environment]).process(dashboard) end end end diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index b43698a6ef7..c1c4be45168 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -485,7 +485,7 @@ describe Projects::EnvironmentsController do context 'when the dashboard could not be provided' do before do - allow(YAML).to receive(:load_file).and_return({}) + allow(YAML).to receive(:safe_load).and_return({}) end it 'returns an error response' do diff --git a/spec/lib/gitlab/metrics/dashboard/service_spec.rb b/spec/lib/gitlab/metrics/dashboard/service_spec.rb index 3f82fd7ebf8..e66c356bf49 100644 --- a/spec/lib/gitlab/metrics/dashboard/service_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/service_spec.rb @@ -19,7 +19,7 @@ describe Gitlab::Metrics::Dashboard::Service, :use_clean_rails_memory_store_cach end it 'caches the dashboard for subsequent calls' do - expect(YAML).to receive(:load_file).once.and_call_original + expect(YAML).to receive(:safe_load).once.and_call_original described_class.new(project, environment).get_dashboard described_class.new(project, environment).get_dashboard @@ -27,7 +27,7 @@ describe Gitlab::Metrics::Dashboard::Service, :use_clean_rails_memory_store_cach context 'when the dashboard is configured incorrectly' do before do - allow(YAML).to receive(:load_file).and_return({}) + allow(YAML).to receive(:safe_load).and_return({}) end it 'returns an appropriate message and status code' do -- cgit v1.2.1 From 0e093940e1135897b996a5f16239eca62cc6089e Mon Sep 17 00:00:00 2001 From: syasonik Date: Thu, 25 Apr 2019 14:13:43 +0800 Subject: Move dashboard param to initialize method --- lib/gitlab/metrics/dashboard/processor.rb | 9 +++++---- lib/gitlab/metrics/dashboard/service.rb | 2 +- lib/gitlab/metrics/dashboard/stages/base_stage.rb | 10 ++++------ lib/gitlab/metrics/dashboard/stages/common_metrics_inserter.rb | 2 +- .../metrics/dashboard/stages/project_metrics_inserter.rb | 2 +- lib/gitlab/metrics/dashboard/stages/sorter.rb | 10 +++++----- spec/lib/gitlab/metrics/dashboard/processor_spec.rb | 4 ++-- 7 files changed, 19 insertions(+), 20 deletions(-) diff --git a/lib/gitlab/metrics/dashboard/processor.rb b/lib/gitlab/metrics/dashboard/processor.rb index 46fd2f9440d..cc34ac53051 100644 --- a/lib/gitlab/metrics/dashboard/processor.rb +++ b/lib/gitlab/metrics/dashboard/processor.rb @@ -14,17 +14,18 @@ module Gitlab Stages::Sorter ].freeze - def initialize(project, environment) + def initialize(project, environment, dashboard) @project = project @environment = environment + @dashboard = dashboard end # Returns a new dashboard hash with the results of # running transforms on the dashboard. - def process(raw_dashboard) - raw_dashboard.deep_symbolize_keys.tap do |dashboard| + def process + @dashboard.deep_symbolize_keys.tap do |dashboard| sequence.each do |stage| - stage.new(@project, @environment).transform!(dashboard) + stage.new(@project, @environment, dashboard).transform! end end end diff --git a/lib/gitlab/metrics/dashboard/service.rb b/lib/gitlab/metrics/dashboard/service.rb index b8f144a7222..79d563cce4f 100644 --- a/lib/gitlab/metrics/dashboard/service.rb +++ b/lib/gitlab/metrics/dashboard/service.rb @@ -32,7 +32,7 @@ module Gitlab # Returns a new dashboard Hash, supplemented with DB info def process_dashboard(dashboard) - Gitlab::Metrics::Dashboard::Processor.new(project, params[:environment]).process(dashboard) + Gitlab::Metrics::Dashboard::Processor.new(project, params[:environment], dashboard).process end end end diff --git a/lib/gitlab/metrics/dashboard/stages/base_stage.rb b/lib/gitlab/metrics/dashboard/stages/base_stage.rb index daecb2698ae..dd4aae6c115 100644 --- a/lib/gitlab/metrics/dashboard/stages/base_stage.rb +++ b/lib/gitlab/metrics/dashboard/stages/base_stage.rb @@ -9,18 +9,16 @@ module Gitlab DEFAULT_PANEL_TYPE = 'area-chart' - attr_reader :project, :environment + attr_reader :project, :environment, :dashboard - def initialize(project, environment) + def initialize(project, environment, dashboard) @project = project @environment = environment + @dashboard = dashboard end # Entry-point to the stage - # @param dashboard [Hash] - # @param project [Project] - # @param environment [Environment] - def transform!(_dashboard) + def transform! raise NotImplementedError end diff --git a/lib/gitlab/metrics/dashboard/stages/common_metrics_inserter.rb b/lib/gitlab/metrics/dashboard/stages/common_metrics_inserter.rb index 20e6b98e20b..3406021bbea 100644 --- a/lib/gitlab/metrics/dashboard/stages/common_metrics_inserter.rb +++ b/lib/gitlab/metrics/dashboard/stages/common_metrics_inserter.rb @@ -8,7 +8,7 @@ module Gitlab # For each metric in the dashboard config, attempts to # find a corresponding database record. If found, # includes the record's id in the dashboard config. - def transform!(dashboard) + def transform! common_metrics = ::PrometheusMetric.common for_metrics(dashboard) do |metric| diff --git a/lib/gitlab/metrics/dashboard/stages/project_metrics_inserter.rb b/lib/gitlab/metrics/dashboard/stages/project_metrics_inserter.rb index 0dc87f1ec16..221610a14d1 100644 --- a/lib/gitlab/metrics/dashboard/stages/project_metrics_inserter.rb +++ b/lib/gitlab/metrics/dashboard/stages/project_metrics_inserter.rb @@ -8,7 +8,7 @@ module Gitlab # Inserts project-specific metrics into the dashboard # config. If there are no project-specific metrics, # this will have no effect. - def transform!(dashboard) + def transform! project.prometheus_metrics.each do |project_metric| group = find_or_create_panel_group(dashboard[:panel_groups], project_metric) panel = find_or_create_panel(group[:panels], project_metric) diff --git a/lib/gitlab/metrics/dashboard/stages/sorter.rb b/lib/gitlab/metrics/dashboard/stages/sorter.rb index 9c4183fff99..ba5aa78059c 100644 --- a/lib/gitlab/metrics/dashboard/stages/sorter.rb +++ b/lib/gitlab/metrics/dashboard/stages/sorter.rb @@ -5,22 +5,22 @@ module Gitlab module Dashboard module Stages class Sorter < BaseStage - def transform!(dashboard) + def transform! missing_panel_groups! unless dashboard[:panel_groups].is_a? Array - sort_groups!(dashboard) - sort_panels!(dashboard) + sort_groups! + sort_panels! end private # Sorts the groups in the dashboard by the :priority key - def sort_groups!(dashboard) + def sort_groups! dashboard[:panel_groups] = dashboard[:panel_groups].sort_by { |group| -group[:priority].to_i } end # Sorts the panels in the dashboard by the :weight key - def sort_panels!(dashboard) + def sort_panels! dashboard[:panel_groups].each do |group| missing_panels! unless group[:panels].is_a? Array diff --git a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb index e27ba2f1f4e..ee7c93fce8d 100644 --- a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb @@ -8,8 +8,8 @@ describe Gitlab::Metrics::Dashboard::Processor do let(:dashboard_yml) { YAML.load_file('spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml') } describe 'process' do - let(:process_params) { [project, environment] } - let(:dashboard) { described_class.new(*process_params).process(dashboard_yml) } + let(:process_params) { [project, environment, dashboard_yml] } + let(:dashboard) { described_class.new(*process_params).process } context 'when dashboard config corresponds to common metrics' do let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') } -- cgit v1.2.1 From 0d34ada57d97d49387b1f266ad7191b9a2cb8849 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 25 Apr 2019 09:43:21 +0100 Subject: Fix EE differences in home_panel.html.haml Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/11029 --- app/views/projects/_home_panel.html.haml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index 50adc19f524..3ca4abddbb8 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -80,6 +80,8 @@ - deleted_message = s_('ForkedFromProjectPath|Forked from %{project_name} (deleted)') = deleted_message % { project_name: fork_source_name(@project) } + = render_if_exists "projects/home_mirror" + - if @project.badges.present? .project-badges.mb-2 - @project.badges.each do |badge| -- cgit v1.2.1 From e8a79b4309528639a834e5e37279d81fbafdacf1 Mon Sep 17 00:00:00 2001 From: Rajat Jain Date: Thu, 25 Apr 2019 09:54:05 +0530 Subject: Hide ScopedBadge overflow notes --- app/assets/stylesheets/pages/labels.scss | 4 +++- changelogs/unreleased/11254-overflow-ce.yml | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/11254-overflow-ce.yml diff --git a/app/assets/stylesheets/pages/labels.scss b/app/assets/stylesheets/pages/labels.scss index 60a840aac1b..13288d8bad1 100644 --- a/app/assets/stylesheets/pages/labels.scss +++ b/app/assets/stylesheets/pages/labels.scss @@ -456,7 +456,9 @@ // Don't hide the overflow in system messages .system-note-message, -.issuable-detail { +.issuable-detail, +.md-preview-holder, +.note-body { .scoped-label-wrapper { .badge { overflow: initial; diff --git a/changelogs/unreleased/11254-overflow-ce.yml b/changelogs/unreleased/11254-overflow-ce.yml new file mode 100644 index 00000000000..dcac46000ac --- /dev/null +++ b/changelogs/unreleased/11254-overflow-ce.yml @@ -0,0 +1,5 @@ +--- +title: Hide ScopedBadge overflow notes +merge_request: 27651 +author: +type: fixed -- cgit v1.2.1 From e7e9929e0d9c7f208394ccb61239228216fe0be8 Mon Sep 17 00:00:00 2001 From: rpereira2 Date: Thu, 25 Apr 2019 16:06:17 +0530 Subject: Fix a bug in shared_examples definition --- spec/controllers/projects/settings/operations_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index 7df63266f71..0dd73972098 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/spec/controllers/projects/settings/operations_controller_spec.rb @@ -11,7 +11,7 @@ describe Projects::Settings::OperationsController do project.add_maintainer(user) end - shared_context 'PATCHable' do + shared_examples 'PATCHable' do let(:operations_update_service) { instance_double(::Projects::Operations::UpdateService) } let(:operations_url) { project_settings_operations_url(project) } -- cgit v1.2.1 From 56115b649400b47c7d9f530d300c4106a9dacf1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 25 Apr 2019 12:56:02 +0200 Subject: Put a failing spec in quarantine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- spec/features/issuables/markdown_references/jira_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/issuables/markdown_references/jira_spec.rb b/spec/features/issuables/markdown_references/jira_spec.rb index 8eaccfc0949..cc04798248c 100644 --- a/spec/features/issuables/markdown_references/jira_spec.rb +++ b/spec/features/issuables/markdown_references/jira_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -describe "Jira", :js do +describe "Jira", :js, :quarantine do let(:user) { create(:user) } let(:actual_project) { create(:project, :public, :repository) } let(:merge_request) { create(:merge_request, target_project: actual_project, source_project: actual_project) } -- cgit v1.2.1 From ea1f01811455bab47ad687b34d711b00da75719b Mon Sep 17 00:00:00 2001 From: Ahmad Haghighi Date: Thu, 25 Apr 2019 11:40:34 +0000 Subject: Add auto direction to support rtl languages --- app/assets/javascripts/boards/components/issue_card_inner.vue | 2 +- .../javascripts/ide/components/commit_sidebar/message_field.vue | 1 + app/assets/javascripts/issue_show/components/description.vue | 1 + app/assets/javascripts/issue_show/components/fields/description.vue | 1 + app/assets/javascripts/issue_show/components/fields/title.vue | 1 + app/assets/javascripts/issue_show/components/title.vue | 1 + app/assets/javascripts/notes/components/comment_form.vue | 1 + app/assets/javascripts/notes/components/note_body.vue | 1 + app/assets/javascripts/notes/components/note_form.vue | 1 + .../vue_merge_request_widget/components/states/commit_edit.vue | 1 + app/assets/stylesheets/pages/boards.scss | 4 ++++ app/assets/stylesheets/pages/issuable.scss | 1 + app/helpers/broadcast_messages_helper.rb | 2 +- app/views/admin/broadcast_messages/_form.html.haml | 1 + app/views/events/event/_common.html.haml | 3 ++- app/views/events/event/_note.html.haml | 3 ++- app/views/projects/_zen.html.haml | 1 + app/views/projects/issues/_issue.html.haml | 2 +- app/views/shared/issuable/form/_title.html.haml | 2 +- changelogs/unreleased/48479-auto-direction-for-issue-title.yml | 5 +++++ 20 files changed, 29 insertions(+), 6 deletions(-) create mode 100644 changelogs/unreleased/48479-auto-direction-for-issue-title.yml diff --git a/app/assets/javascripts/boards/components/issue_card_inner.vue b/app/assets/javascripts/boards/components/issue_card_inner.vue index 6aa689b4056..17de7b2cf1e 100644 --- a/app/assets/javascripts/boards/components/issue_card_inner.vue +++ b/app/assets/javascripts/boards/components/issue_card_inner.vue @@ -168,7 +168,7 @@ export default {