diff options
27 files changed, 256 insertions, 91 deletions
diff --git a/app/assets/javascripts/boards/components/issue_card_inner.vue b/app/assets/javascripts/boards/components/issue_card_inner.vue index 40d75d53f75..d37e49bab46 100644 --- a/app/assets/javascripts/boards/components/issue_card_inner.vue +++ b/app/assets/javascripts/boards/components/issue_card_inner.vue @@ -1,5 +1,6 @@ <script> import _ from 'underscore'; +import { mapState } from 'vuex'; import { GlTooltipDirective } from '@gitlab/ui'; import { sprintf, __ } from '~/locale'; import Icon from '~/vue_shared/components/icon.vue'; @@ -63,6 +64,7 @@ export default { }; }, computed: { + ...mapState(['isShowingLabels']), numberOverLimit() { return this.issue.assignees.length - this.limitBeforeCounter; }, @@ -92,7 +94,7 @@ export default { return false; }, showLabelFooter() { - return this.issue.labels.find(l => this.showLabel(l)) !== undefined; + return this.isShowingLabels && this.issue.labels.find(this.showLabel); }, issueReferencePath() { const { referencePath, groupId } = this.issue; diff --git a/app/assets/javascripts/boards/index.js b/app/assets/javascripts/boards/index.js index befca70eeae..e76e2341dfd 100644 --- a/app/assets/javascripts/boards/index.js +++ b/app/assets/javascripts/boards/index.js @@ -13,6 +13,7 @@ import 'ee_else_ce/boards/models/issue'; import 'ee_else_ce/boards/models/list'; import '~/boards/models/milestone'; import '~/boards/models/project'; +import store from '~/boards/stores'; import boardsStore from '~/boards/stores/boards_store'; import ModalStore from '~/boards/stores/modal_store'; import BoardService from 'ee_else_ce/boards/services/board_service'; @@ -29,6 +30,7 @@ import { } from '~/lib/utils/common_utils'; import boardConfigToggle from 'ee_else_ce/boards/config_toggle'; import toggleFocusMode from 'ee_else_ce/boards/toggle_focus'; +import toggleLabels from 'ee_else_ce/boards/toggle_labels'; import { setPromotionState, setWeigthFetchingState, @@ -67,6 +69,7 @@ export default () => { BoardSidebar, BoardAddIssuesModal, }, + store, data: { state: boardsStore.state, loading: true, @@ -314,5 +317,6 @@ export default () => { } toggleFocusMode(ModalStore, boardsStore, $boardApp); + toggleLabels(); mountMultipleBoardsSwitcher(); }; diff --git a/app/assets/javascripts/boards/stores/getters.js b/app/assets/javascripts/boards/stores/getters.js new file mode 100644 index 00000000000..4de1576099d --- /dev/null +++ b/app/assets/javascripts/boards/stores/getters.js @@ -0,0 +1,3 @@ +export default { + getLabelToggleState: state => (state.isShowingLabels ? 'on' : 'off'), +}; diff --git a/app/assets/javascripts/boards/stores/index.js b/app/assets/javascripts/boards/stores/index.js index f70395a3771..471b952a212 100644 --- a/app/assets/javascripts/boards/stores/index.js +++ b/app/assets/javascripts/boards/stores/index.js @@ -1,14 +1,18 @@ import Vue from 'vue'; import Vuex from 'vuex'; import state from 'ee_else_ce/boards/stores/state'; +import getters from 'ee_else_ce/boards/stores/getters'; import actions from 'ee_else_ce/boards/stores/actions'; import mutations from 'ee_else_ce/boards/stores/mutations'; Vue.use(Vuex); -export default () => +export const createStore = () => new Vuex.Store({ state, + getters, actions, mutations, }); + +export default createStore(); diff --git a/app/assets/javascripts/boards/stores/state.js b/app/assets/javascripts/boards/stores/state.js index dd16abb01a5..24f44dc5629 100644 --- a/app/assets/javascripts/boards/stores/state.js +++ b/app/assets/javascripts/boards/stores/state.js @@ -1,3 +1,3 @@ export default () => ({ - // ... + isShowingLabels: true, }); diff --git a/app/assets/javascripts/boards/toggle_labels.js b/app/assets/javascripts/boards/toggle_labels.js new file mode 100644 index 00000000000..2d1ec238274 --- /dev/null +++ b/app/assets/javascripts/boards/toggle_labels.js @@ -0,0 +1 @@ +export default () => {}; diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index 2a7a53d8bd7..45d0579052a 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -545,3 +545,15 @@ .board-issue-path.js-show-tooltip { cursor: help; } + +.board-labels-toggle-wrapper { + /** + * Make the wrapper the same height as a button so it aligns properly when the + * filtered-search-box input element increases in size on Linux smaller breakpoints + */ + height: 34px; + + @include media-breakpoint-down(sm) { + margin-bottom: 10px; + } +} diff --git a/app/services/projects/container_repository/delete_tags_service.rb b/app/services/projects/container_repository/delete_tags_service.rb index 5129e2269a8..6a8b68d8ae3 100644 --- a/app/services/projects/container_repository/delete_tags_service.rb +++ b/app/services/projects/container_repository/delete_tags_service.rb @@ -9,25 +9,11 @@ module Projects tag_names = params[:tags] return error('not tags specified') if tag_names.blank? - if can_use? - smart_delete(container_repository, tag_names) - else - unsafe_delete(container_repository, tag_names) - end + smart_delete(container_repository, tag_names) end private - def unsafe_delete(container_repository, tag_names) - deleted_tags = tag_names.select do |tag_name| - container_repository.tag(tag_name).unsafe_delete - end - - return error('could not delete tags') if deleted_tags.empty? - - success(deleted: deleted_tags) - end - # Replace a tag on the registry with a dummy tag. # This is a hack as the registry doesn't support deleting individual # tags. This code effectively pushes a dummy image and assigns the tag to it. @@ -57,10 +43,6 @@ module Projects error('could not delete tags') end end - - def can_use? - Feature.enabled?(:container_registry_smart_delete, project, default_enabled: true) - end end end end diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index 68b7efc6fb4..6b27db23315 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -94,7 +94,8 @@ - else = f.text_field :name, label: s_('Profiles|Full name'), required: true, title: s_("Profiles|Using emojis in names seems fun, but please try to set a status message instead"), wrapper: { class: 'col-md-9 qa-full-name rspec-full-name' }, help: s_("Profiles|Enter your name, so people you know can recognize you") = f.text_field :id, readonly: true, label: s_('Profiles|User ID'), wrapper: { class: 'col-md-3' } - = f.select :role, ::User.roles.keys.map { |role| [role.titleize, role] }, {}, class: 'input-md' + - if experiment_enabled?(:signup_flow) + = f.select :role, ::User.roles.keys.map { |role| [role.titleize, role] }, { prompt: _('Select your role') }, class: 'input-md' = render_if_exists 'profiles/email_settings', form: f = f.text_field :skype, class: 'input-md', placeholder: s_("Profiles|username") diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index 9d580930fb8..2c465bf782f 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -147,6 +147,7 @@ %button.clear-search.hidden{ type: 'button' } = icon('times') + #js-board-labels-toggle .filter-dropdown-container.d-flex.flex-column.flex-md-row - if type == :boards .js-board-config{ data: { can_admin_list: user_can_admin_list, has_scope: board.scoped? } } diff --git a/changelogs/unreleased/record-sidekiq-queuing-latency.yml b/changelogs/unreleased/record-sidekiq-queuing-latency.yml new file mode 100644 index 00000000000..72bacd90e33 --- /dev/null +++ b/changelogs/unreleased/record-sidekiq-queuing-latency.yml @@ -0,0 +1,5 @@ +--- +title: Adds a Sidekiq queue duration metric +merge_request: 19005 +author: +type: other diff --git a/config/puma.example.development.rb b/config/puma.example.development.rb index f23ccc23c9a..6f686437f88 100644 --- a/config/puma.example.development.rb +++ b/config/puma.example.development.rb @@ -14,9 +14,13 @@ rackup 'config.ru' pidfile '/home/git/gitlab/tmp/pids/puma.pid' state_path '/home/git/gitlab/tmp/pids/puma.state' -stdout_redirect '/home/git/gitlab/log/puma.stdout.log', - '/home/git/gitlab/log/puma.stderr.log', - true +## Uncomment the lines if you would like to write puma stdout & stderr streams +## to a different location than rails logs. +## When using GitLab Development Kit, by default, these logs will be consumed +## by runit and can be accessed using `gdk tail rails-web` +# stdout_redirect '/home/git/gitlab/log/puma.stdout.log', +# '/home/git/gitlab/log/puma.stderr.log', +# true # Configure "min" to be the minimum number of threads to use to answer # requests and "max" the maximum. diff --git a/doc/development/pipelines.md b/doc/development/pipelines.md index cd23ab9226f..8d83cea8c7b 100644 --- a/doc/development/pipelines.md +++ b/doc/development/pipelines.md @@ -15,6 +15,8 @@ as much as possible. The current stages are: +- `sync`: This stage is used to synchronize changes from gitlab-org/gitlab to + gitlab-org/gitlab-foss. - `prepare`: This stage includes jobs that prepare artifacts that are needed by jobs in subsequent stages. - `quick-test`: This stage includes test jobs that should run first and fail the diff --git a/lib/gitlab/instrumentation_helper.rb b/lib/gitlab/instrumentation_helper.rb index e6a5facb2a5..edaa9c645b4 100644 --- a/lib/gitlab/instrumentation_helper.rb +++ b/lib/gitlab/instrumentation_helper.rb @@ -21,5 +21,49 @@ module Gitlab payload[:rugged_duration_ms] = Gitlab::RuggedInstrumentation.query_time_ms end end + + # Returns the queuing duration for a Sidekiq job in seconds, as a float, if the + # `enqueued_at` field or `created_at` field is available. + # + # * If the job doesn't contain sufficient information, returns nil + # * If the job has a start time in the future, returns 0 + # * If the job contains an invalid start time value, returns nil + # @param [Hash] job a Sidekiq job, represented as a hash + def self.queue_duration_for_job(job) + # Old gitlab-shell messages don't provide enqueued_at/created_at attributes + enqueued_at = job['enqueued_at'] || job['created_at'] + return unless enqueued_at + + enqueued_at_time = convert_to_time(enqueued_at) + return unless enqueued_at_time + + # Its possible that if theres clock-skew between two nodes + # this value may be less than zero. In that event, we record the value + # as zero. + [elapsed_by_absolute_time(enqueued_at_time), 0].max + end + + # Calculates the time in seconds, as a float, from + # the provided start time until now + # + # @param [Time] start + def self.elapsed_by_absolute_time(start) + (Time.now - start).to_f.round(6) + end + private_class_method :elapsed_by_absolute_time + + # Convert a representation of a time into a `Time` value + # + # @param time_value String, Float time representation, or nil + def self.convert_to_time(time_value) + return time_value if time_value.is_a?(Time) + return Time.iso8601(time_value) if time_value.is_a?(String) + return Time.at(time_value) if time_value.is_a?(Numeric) && time_value > 0 + rescue ArgumentError + # Swallow invalid dates. Better to loose some observability + # than bring all background processing down because of a date + # formatting bug in a client + end + private_class_method :convert_to_time end end diff --git a/lib/gitlab/sidekiq_logging/structured_logger.rb b/lib/gitlab/sidekiq_logging/structured_logger.rb index 853fb2777c3..ca9e3b8428c 100644 --- a/lib/gitlab/sidekiq_logging/structured_logger.rb +++ b/lib/gitlab/sidekiq_logging/structured_logger.rb @@ -36,11 +36,8 @@ module Gitlab payload['message'] = "#{base_message(payload)}: start" payload['job_status'] = 'start' - # Old gitlab-shell messages don't provide enqueued_at/created_at attributes - enqueued_at = payload['enqueued_at'] || payload['created_at'] - if enqueued_at - payload['scheduling_latency_s'] = elapsed_by_absolute_time(Time.iso8601(enqueued_at)) - end + scheduling_latency_s = ::Gitlab::InstrumentationHelper.queue_duration_for_job(payload) + payload['scheduling_latency_s'] = scheduling_latency_s if scheduling_latency_s payload end @@ -98,10 +95,6 @@ module Gitlab end end - def elapsed_by_absolute_time(start) - (Time.now.utc - start).to_f.round(6) - end - def elapsed(t0) t1 = get_time { diff --git a/lib/gitlab/sidekiq_middleware/metrics.rb b/lib/gitlab/sidekiq_middleware/metrics.rb index d45045ca414..bd819843bd4 100644 --- a/lib/gitlab/sidekiq_middleware/metrics.rb +++ b/lib/gitlab/sidekiq_middleware/metrics.rb @@ -15,6 +15,9 @@ module Gitlab def call(_worker, job, queue) labels = create_labels(queue) + queue_duration = ::Gitlab::InstrumentationHelper.queue_duration_for_job(job) + + @metrics[:sidekiq_jobs_queue_duration_seconds].observe(labels, queue_duration) if queue_duration @metrics[:sidekiq_running_jobs].increment(labels, 1) if job['retry_count'].present? @@ -49,12 +52,13 @@ module Gitlab def init_metrics { - sidekiq_jobs_cpu_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_cpu_seconds, 'Seconds of cpu time to run sidekiq job', {}, SIDEKIQ_LATENCY_BUCKETS), - sidekiq_jobs_completion_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_completion_seconds, 'Seconds to complete sidekiq job', {}, SIDEKIQ_LATENCY_BUCKETS), - sidekiq_jobs_failed_total: ::Gitlab::Metrics.counter(:sidekiq_jobs_failed_total, 'Sidekiq jobs failed'), - sidekiq_jobs_retried_total: ::Gitlab::Metrics.counter(:sidekiq_jobs_retried_total, 'Sidekiq jobs retried'), - sidekiq_running_jobs: ::Gitlab::Metrics.gauge(:sidekiq_running_jobs, 'Number of Sidekiq jobs running', {}, :all), - sidekiq_concurrency: ::Gitlab::Metrics.gauge(:sidekiq_concurrency, 'Maximum number of Sidekiq jobs', {}, :all) + sidekiq_jobs_cpu_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_cpu_seconds, 'Seconds of cpu time to run Sidekiq job', {}, SIDEKIQ_LATENCY_BUCKETS), + sidekiq_jobs_completion_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_completion_seconds, 'Seconds to complete Sidekiq job', {}, SIDEKIQ_LATENCY_BUCKETS), + sidekiq_jobs_queue_duration_seconds: ::Gitlab::Metrics.histogram(:sidekiq_jobs_queue_duration_seconds, 'Duration in seconds that a Sidekiq job was queued before being executed', {}, SIDEKIQ_LATENCY_BUCKETS), + sidekiq_jobs_failed_total: ::Gitlab::Metrics.counter(:sidekiq_jobs_failed_total, 'Sidekiq jobs failed'), + sidekiq_jobs_retried_total: ::Gitlab::Metrics.counter(:sidekiq_jobs_retried_total, 'Sidekiq jobs retried'), + sidekiq_running_jobs: ::Gitlab::Metrics.gauge(:sidekiq_running_jobs, 'Number of Sidekiq jobs running', {}, :all), + sidekiq_concurrency: ::Gitlab::Metrics.gauge(:sidekiq_concurrency, 'Maximum number of Sidekiq jobs', {}, :all) } end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index dec0e1cc8a0..1e47dd679ac 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8656,6 +8656,9 @@ msgstr[1] "" msgid "Hide values" msgstr "" +msgid "Hiding all labels" +msgstr "" + msgid "Highest number of requests per minute for each raw path, default to 300. To disable throttling set to 0." msgstr "" @@ -14813,6 +14816,9 @@ msgstr "" msgid "Select user" msgstr "" +msgid "Select your role" +msgstr "" + msgid "Selected levels cannot be used by non-admin users for groups, projects or snippets. If the public level is restricted, user profiles are only visible to logged in users." msgstr "" @@ -15178,6 +15184,9 @@ msgstr "" msgid "Showing all issues" msgstr "" +msgid "Showing all labels" +msgstr "" + msgid "Showing last %{size} of log -" msgstr "" diff --git a/spec/features/profiles/user_edit_profile_spec.rb b/spec/features/profiles/user_edit_profile_spec.rb index 0905ab0aef8..23f660d111a 100644 --- a/spec/features/profiles/user_edit_profile_spec.rb +++ b/spec/features/profiles/user_edit_profile_spec.rb @@ -66,6 +66,34 @@ describe 'User edit profile' do end end + describe 'when I change my role' do + context 'experiment enabled' do + before do + stub_experiment_for_user(signup_flow: true) + visit(profile_path) + end + + it 'changes my role' do + expect(page).to have_content 'Role' + select 'Data Analyst', from: 'user_role' + submit_settings + user.reload + expect(user.role).to eq 'data_analyst' + end + end + + context 'experiment disabled' do + before do + stub_experiment_for_user(signup_flow: false) + visit(profile_path) + end + + it 'does not show the role picker' do + expect(page).not_to have_content 'Role' + end + end + end + context 'user avatar' do before do attach_file(:user_avatar, Rails.root.join('spec', 'fixtures', 'banana_sample.gif')) diff --git a/spec/frontend/boards/stores/getters_spec.js b/spec/frontend/boards/stores/getters_spec.js new file mode 100644 index 00000000000..38b2333e679 --- /dev/null +++ b/spec/frontend/boards/stores/getters_spec.js @@ -0,0 +1,21 @@ +import getters from '~/boards/stores/getters'; + +describe('Boards - Getters', () => { + describe('getLabelToggleState', () => { + it('should return "on" when isShowingLabels is true', () => { + const state = { + isShowingLabels: true, + }; + + expect(getters.getLabelToggleState(state)).toBe('on'); + }); + + it('should return "off" when isShowingLabels is false', () => { + const state = { + isShowingLabels: false, + }; + + expect(getters.getLabelToggleState(state)).toBe('off'); + }); + }); +}); diff --git a/spec/javascripts/boards/board_card_spec.js b/spec/javascripts/boards/board_card_spec.js index 9f441ca319e..51433a58212 100644 --- a/spec/javascripts/boards/board_card_spec.js +++ b/spec/javascripts/boards/board_card_spec.js @@ -10,6 +10,7 @@ import eventHub from '~/boards/eventhub'; import '~/boards/models/label'; import '~/boards/models/assignee'; import '~/boards/models/list'; +import store from '~/boards/stores'; import boardsStore from '~/boards/stores/boards_store'; import boardCard from '~/boards/components/board_card.vue'; import { listObj, boardsMockInterceptor, mockBoardService } from './mock_data'; @@ -40,6 +41,7 @@ describe('Board card', () => { list.issues[0].labels.push(label1); vm = new BoardCardComp({ + store, propsData: { list, issue: list.issues[0], diff --git a/spec/javascripts/boards/board_list_common_spec.js b/spec/javascripts/boards/board_list_common_spec.js index cb337e4cc83..5cd17323d0d 100644 --- a/spec/javascripts/boards/board_list_common_spec.js +++ b/spec/javascripts/boards/board_list_common_spec.js @@ -10,6 +10,7 @@ import BoardList from '~/boards/components/board_list.vue'; import '~/boards/models/issue'; import '~/boards/models/list'; import { listObj, boardsMockInterceptor, mockBoardService } from './mock_data'; +import store from '~/boards/stores'; import boardsStore from '~/boards/stores/boards_store'; window.Sortable = Sortable; @@ -39,6 +40,7 @@ export default function createComponent({ done, listIssueProps = {}, componentPr const component = new BoardListComp({ el, + store, propsData: { disabled: false, list, diff --git a/spec/javascripts/boards/issue_card_spec.js b/spec/javascripts/boards/issue_card_spec.js index 314e051665e..df2b04cd6df 100644 --- a/spec/javascripts/boards/issue_card_spec.js +++ b/spec/javascripts/boards/issue_card_spec.js @@ -10,6 +10,7 @@ import '~/boards/models/issue'; import '~/boards/models/list'; import IssueCardInner from '~/boards/components/issue_card_inner.vue'; import { listObj } from './mock_data'; +import store from '~/boards/stores'; describe('Issue card component', () => { const user = new ListAssignee({ @@ -50,6 +51,7 @@ describe('Issue card component', () => { component = new Vue({ el: document.querySelector('.test-container'), + store, components: { 'issue-card': IssueCardInner, }, diff --git a/spec/lib/gitlab/instrumentation_helper_spec.rb b/spec/lib/gitlab/instrumentation_helper_spec.rb new file mode 100644 index 00000000000..c2674638743 --- /dev/null +++ b/spec/lib/gitlab/instrumentation_helper_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rspec-parameterized' + +describe Gitlab::InstrumentationHelper do + using RSpec::Parameterized::TableSyntax + + describe '.queue_duration_for_job' do + where(:enqueued_at, :created_at, :time_now, :expected_duration) do + "2019-06-01T00:00:00.000+0000" | nil | "2019-06-01T02:00:00.000+0000" | 2.hours.to_f + "2019-06-01T02:00:00.000+0000" | nil | "2019-06-01T02:00:00.001+0000" | 0.001 + "2019-06-01T02:00:00.000+0000" | "2019-05-01T02:00:00.000+0000" | "2019-06-01T02:00:01.000+0000" | 1 + nil | "2019-06-01T02:00:00.000+0000" | "2019-06-01T02:00:00.001+0000" | 0.001 + nil | nil | "2019-06-01T02:00:00.001+0000" | nil + "2019-06-01T02:00:00.000+0200" | nil | "2019-06-01T02:00:00.000-0200" | 4.hours.to_f + 1571825569.998168 | nil | "2019-10-23T12:13:16.000+0200" | 26.001832 + 1571825569 | nil | "2019-10-23T12:13:16.000+0200" | 27 + "invalid_date" | nil | "2019-10-23T12:13:16.000+0200" | nil + "" | nil | "2019-10-23T12:13:16.000+0200" | nil + 0 | nil | "2019-10-23T12:13:16.000+0200" | nil + -1 | nil | "2019-10-23T12:13:16.000+0200" | nil + "2019-06-01T02:00:00.000+0000" | nil | "2019-06-01T00:00:00.000+0000" | 0 + Time.at(1571999233) | nil | "2019-10-25T12:29:16.000+0200" | 123 + end + + with_them do + let(:job) { { 'enqueued_at' => enqueued_at, 'created_at' => created_at } } + + it "returns the correct duration" do + Timecop.freeze(Time.iso8601(time_now)) do + expect(described_class.queue_duration_for_job(job)).to eq(expected_duration) + end + end + end + end +end diff --git a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb index 46fbc069efb..cb870cc996b 100644 --- a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb +++ b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'spec_helper' +require 'fast_spec_helper' describe Gitlab::SidekiqLogging::StructuredLogger do describe '#call' do diff --git a/spec/lib/gitlab/sidekiq_middleware/metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/metrics_spec.rb index df16b9d073c..0d8cff3a295 100644 --- a/spec/lib/gitlab/sidekiq_middleware/metrics_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/metrics_spec.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true -require 'spec_helper' +require 'fast_spec_helper' describe Gitlab::SidekiqMiddleware::Metrics do let(:middleware) { described_class.new } - let(:concurrency_metric) { double('concurrency metric') } + + let(:queue_duration_seconds) { double('queue duration seconds metric') } let(:completion_seconds_metric) { double('completion seconds metric') } let(:user_execution_seconds_metric) { double('user execution seconds metric') } let(:failed_total_metric) { double('failed total metric') } @@ -13,6 +14,7 @@ describe Gitlab::SidekiqMiddleware::Metrics do let(:running_jobs_metric) { double('running jobs metric') } before do + allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_queue_duration_seconds, anything, anything, anything).and_return(queue_duration_seconds) allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_completion_seconds, anything, anything, anything).and_return(completion_seconds_metric) allow(Gitlab::Metrics).to receive(:histogram).with(:sidekiq_jobs_cpu_seconds, anything, anything, anything).and_return(user_execution_seconds_metric) allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_jobs_failed_total, anything).and_return(failed_total_metric) @@ -20,7 +22,6 @@ describe Gitlab::SidekiqMiddleware::Metrics do allow(Gitlab::Metrics).to receive(:gauge).with(:sidekiq_running_jobs, anything, {}, :all).and_return(running_jobs_metric) allow(Gitlab::Metrics).to receive(:gauge).with(:sidekiq_concurrency, anything, {}, :all).and_return(concurrency_metric) - allow(running_jobs_metric).to receive(:increment) allow(concurrency_metric).to receive(:set) end @@ -32,62 +33,76 @@ describe Gitlab::SidekiqMiddleware::Metrics do end end + it 'ignore user execution when measured 0' do + allow(completion_seconds_metric).to receive(:observe) + + expect(user_execution_seconds_metric).not_to receive(:observe) + end + describe '#call' do let(:worker) { double(:worker) } - it 'yields block' do - allow(completion_seconds_metric).to receive(:observe) - allow(user_execution_seconds_metric).to receive(:observe) + let(:job) { {} } + let(:job_status) { :done } + let(:labels) { { queue: :test } } + let(:labels_with_job_status) { { queue: :test, job_status: job_status } } - expect { |b| middleware.call(worker, {}, :test, &b) }.to yield_control.once - end + let(:thread_cputime_before) { 1 } + let(:thread_cputime_after) { 2 } + let(:thread_cputime_duration) { thread_cputime_after - thread_cputime_before } - it 'sets queue specific metrics' do - labels = { queue: :test } - labels_with_job_status = { queue: :test, job_status: :done } - allow(middleware).to receive(:get_thread_cputime).and_return(1, 3) - allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(2, 3) + let(:monotonic_time_before) { 11 } + let(:monotonic_time_after) { 20 } + let(:monotonic_time_duration) { monotonic_time_after - monotonic_time_before } + + let(:queue_duration_for_job) { 0.01 } + + before do + allow(middleware).to receive(:get_thread_cputime).and_return(thread_cputime_before, thread_cputime_after) + allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(monotonic_time_before, monotonic_time_after) + allow(Gitlab::InstrumentationHelper).to receive(:queue_duration_for_job).with(job).and_return(queue_duration_for_job) expect(running_jobs_metric).to receive(:increment).with(labels, 1) expect(running_jobs_metric).to receive(:increment).with(labels, -1) - expect(user_execution_seconds_metric).to receive(:observe).with(labels_with_job_status, 2) - expect(completion_seconds_metric).to receive(:observe).with(labels_with_job_status, 1) - middleware.call(worker, {}, :test) { nil } + expect(queue_duration_seconds).to receive(:observe).with(labels, queue_duration_for_job) if queue_duration_for_job + expect(user_execution_seconds_metric).to receive(:observe).with(labels_with_job_status, thread_cputime_duration) + expect(completion_seconds_metric).to receive(:observe).with(labels_with_job_status, monotonic_time_duration) end - it 'ignore user execution when measured 0' do - allow(completion_seconds_metric).to receive(:observe) - allow(middleware).to receive(:get_thread_cputime).and_return(0, 0) + it 'yields block' do + expect { |b| middleware.call(worker, job, :test, &b) }.to yield_control.once + end + + it 'sets queue specific metrics' do + middleware.call(worker, job, :test) { nil } + end + + context 'when job_duration is not available' do + let(:queue_duration_for_job) { nil } - expect(user_execution_seconds_metric).not_to receive(:observe) + it 'does not set the queue_duration_seconds histogram' do + middleware.call(worker, job, :test) { nil } + end end context 'when job is retried' do - it 'sets sidekiq_jobs_retried_total metric' do - allow(completion_seconds_metric).to receive(:observe) - expect(user_execution_seconds_metric).to receive(:observe) + let(:job) { { 'retry_count' => 1 } } + it 'sets sidekiq_jobs_retried_total metric' do expect(retried_total_metric).to receive(:increment) - middleware.call(worker, { 'retry_count' => 1 }, :test) { nil } + middleware.call(worker, job, :test) { nil } end end context 'when error is raised' do - it 'sets sidekiq_jobs_failed_total and reraises' do - labels = { queue: :test } - labels_with_job_status = { queue: :test, job_status: :fail } - allow(middleware).to receive(:get_thread_cputime).and_return(1, 4) - allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(2, 6) + let(:job_status) { :fail } - expect(running_jobs_metric).to receive(:increment).with(labels, 1) - expect(running_jobs_metric).to receive(:increment).with(labels, -1) + it 'sets sidekiq_jobs_failed_total and reraises' do expect(failed_total_metric).to receive(:increment).with(labels, 1) - expect(user_execution_seconds_metric).to receive(:observe).with(labels_with_job_status, 3) - expect(completion_seconds_metric).to receive(:observe).with(labels_with_job_status, 4) - expect { middleware.call(worker, {}, :test) { raise StandardError, "Failed" } }.to raise_error(StandardError, "Failed") + expect { middleware.call(worker, job, :test) { raise StandardError, "Failed" } }.to raise_error(StandardError, "Failed") end end end diff --git a/spec/services/projects/container_repository/delete_tags_service_spec.rb b/spec/services/projects/container_repository/delete_tags_service_spec.rb index f296ef3a776..91b668495d8 100644 --- a/spec/services/projects/container_repository/delete_tags_service_spec.rb +++ b/spec/services/projects/container_repository/delete_tags_service_spec.rb @@ -57,21 +57,7 @@ describe Projects::ContainerRepository::DeleteTagsService do end end - context 'with dummy tags disabled' do - let(:tags) { %w[A Ba] } - - before do - stub_feature_flags(container_registry_smart_delete: false) - end - - it 'deletes tags one by one' do - expect_delete_tag('sha256:configA') - expect_delete_tag('sha256:configB') - is_expected.to include(status: :success) - end - end - - context 'with dummy tags enabled' do + context 'with tags to delete' do let(:tags) { %w[A Ba] } it 'deletes the tags using a dummy image' do diff --git a/spec/views/profiles/show.html.haml_spec.rb b/spec/views/profiles/show.html.haml_spec.rb index 592b3a56ba3..14e6feed3ab 100644 --- a/spec/views/profiles/show.html.haml_spec.rb +++ b/spec/views/profiles/show.html.haml_spec.rb @@ -8,6 +8,7 @@ describe 'profiles/show' do before do assign(:user, user) allow(controller).to receive(:current_user).and_return(user) + allow(view).to receive(:experiment_enabled?) end context 'when the profile page is opened' do |