From 4a6f959ab8f2928225a055d9dc62647c78df5bbe Mon Sep 17 00:00:00 2001 From: Mark Chao Date: Thu, 8 Aug 2019 13:18:57 +0000 Subject: Record usage on snippet usage Generalize wiki page counter for other page types to extend to. --- app/services/create_snippet_service.rb | 1 + app/services/notes/base_service.rb | 4 ++ app/services/notes/create_service.rb | 1 + app/services/update_snippet_service.rb | 4 +- .../63888-snippets-usage-ping-for-create-smau.yml | 5 ++ lib/gitlab/usage_data.rb | 2 + lib/gitlab/usage_data_counters/base_counter.rb | 39 +++++++++++ lib/gitlab/usage_data_counters/note_counter.rb | 39 +++++++++++ lib/gitlab/usage_data_counters/snippet_counter.rb | 8 +++ .../usage_data_counters/wiki_page_counter.rb | 30 +-------- .../usage_data_counters/note_counter_spec.rb | 78 ++++++++++++++++++++++ .../usage_data_counters/snippet_counter_spec.rb | 12 ++++ spec/lib/gitlab/usage_data_spec.rb | 3 + spec/services/create_snippet_service_spec.rb | 16 +++++ spec/services/notes/create_service_spec.rb | 38 +++++++++++ spec/services/update_snippet_service_spec.rb | 17 +++++ 16 files changed, 269 insertions(+), 28 deletions(-) create mode 100644 changelogs/unreleased/63888-snippets-usage-ping-for-create-smau.yml create mode 100644 lib/gitlab/usage_data_counters/base_counter.rb create mode 100644 lib/gitlab/usage_data_counters/note_counter.rb create mode 100644 lib/gitlab/usage_data_counters/snippet_counter.rb create mode 100644 spec/lib/gitlab/usage_data_counters/note_counter_spec.rb create mode 100644 spec/lib/gitlab/usage_data_counters/snippet_counter_spec.rb diff --git a/app/services/create_snippet_service.rb b/app/services/create_snippet_service.rb index 6f1fce4989e..6e5bf823cc7 100644 --- a/app/services/create_snippet_service.rb +++ b/app/services/create_snippet_service.rb @@ -23,6 +23,7 @@ class CreateSnippetService < BaseService if snippet.save UserAgentDetailService.new(snippet, @request).create + Gitlab::UsageDataCounters::SnippetCounter.count(:create) end snippet diff --git a/app/services/notes/base_service.rb b/app/services/notes/base_service.rb index c1260837c12..b4d04c47cc0 100644 --- a/app/services/notes/base_service.rb +++ b/app/services/notes/base_service.rb @@ -9,5 +9,9 @@ module Notes note.noteable.diffs.clear_cache end end + + def increment_usage_counter(note) + Gitlab::UsageDataCounters::NoteCounter.count(:create, note.noteable_type) + end end end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 194c4a43dbc..a09272f0d83 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -41,6 +41,7 @@ module Notes todo_service.new_note(note, current_user) clear_noteable_diffs_cache(note) Suggestions::CreateService.new(note).execute + increment_usage_counter(note) end if quick_actions_service.commands_executed_count.to_i > 0 diff --git a/app/services/update_snippet_service.rb b/app/services/update_snippet_service.rb index 15bc1046a4e..2969c360de5 100644 --- a/app/services/update_snippet_service.rb +++ b/app/services/update_snippet_service.rb @@ -25,6 +25,8 @@ class UpdateSnippetService < BaseService snippet.assign_attributes(params) spam_check(snippet, current_user) - snippet.save + snippet.save.tap do |succeeded| + Gitlab::UsageDataCounters::SnippetCounter.count(:update) if succeeded + end end end diff --git a/changelogs/unreleased/63888-snippets-usage-ping-for-create-smau.yml b/changelogs/unreleased/63888-snippets-usage-ping-for-create-smau.yml new file mode 100644 index 00000000000..1a5a552b120 --- /dev/null +++ b/changelogs/unreleased/63888-snippets-usage-ping-for-create-smau.yml @@ -0,0 +1,5 @@ +--- +title: Count snippet creation, update and comment events +merge_request: 30930 +author: +type: added diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index d5657c474c8..7e3a695e52a 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -140,6 +140,8 @@ module Gitlab [ Gitlab::UsageDataCounters::WikiPageCounter, Gitlab::UsageDataCounters::WebIdeCounter, + Gitlab::UsageDataCounters::NoteCounter, + Gitlab::UsageDataCounters::SnippetCounter, Gitlab::UsageDataCounters::SearchCounter ] end diff --git a/lib/gitlab/usage_data_counters/base_counter.rb b/lib/gitlab/usage_data_counters/base_counter.rb new file mode 100644 index 00000000000..2b52571c3cc --- /dev/null +++ b/lib/gitlab/usage_data_counters/base_counter.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Gitlab::UsageDataCounters + class BaseCounter + extend RedisCounter + + UnknownEvent = Class.new(StandardError) + + class << self + def redis_key(event) + Gitlab::Sentry.track_exception(UnknownEvent, extra: { event: event }) unless known_events.include?(event.to_s) + + "USAGE_#{prefix}_#{event}".upcase + end + + def count(event) + increment(redis_key event) + end + + def read(event) + total_count(redis_key event) + end + + def totals + known_events.map { |e| ["#{prefix}_#{e}".to_sym, read(e)] }.to_h + end + + private + + def known_events + self::KNOWN_EVENTS + end + + def prefix + self::PREFIX + end + end + end +end diff --git a/lib/gitlab/usage_data_counters/note_counter.rb b/lib/gitlab/usage_data_counters/note_counter.rb new file mode 100644 index 00000000000..e93a0bcfa27 --- /dev/null +++ b/lib/gitlab/usage_data_counters/note_counter.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Gitlab::UsageDataCounters + class NoteCounter < BaseCounter + KNOWN_EVENTS = %w[create].freeze + PREFIX = 'note' + COUNTABLE_TYPES = %w[Snippet].freeze + + class << self + def redis_key(event, noteable_type) + "#{super(event)}_#{noteable_type}".upcase + end + + def count(event, noteable_type) + return unless countable?(noteable_type) + + increment(redis_key(event, noteable_type)) + end + + def read(event, noteable_type) + return 0 unless countable?(noteable_type) + + total_count(redis_key(event, noteable_type)) + end + + def totals + { + snippet_comment: read(:create, 'Snippet') + } + end + + private + + def countable?(noteable_type) + COUNTABLE_TYPES.include?(noteable_type.to_s) + end + end + end +end diff --git a/lib/gitlab/usage_data_counters/snippet_counter.rb b/lib/gitlab/usage_data_counters/snippet_counter.rb new file mode 100644 index 00000000000..e4d234ce4d9 --- /dev/null +++ b/lib/gitlab/usage_data_counters/snippet_counter.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module Gitlab::UsageDataCounters + class SnippetCounter < BaseCounter + KNOWN_EVENTS = %w[create update].freeze + PREFIX = 'snippet' + end +end diff --git a/lib/gitlab/usage_data_counters/wiki_page_counter.rb b/lib/gitlab/usage_data_counters/wiki_page_counter.rb index c8b59a3160c..9cfe0be5bab 100644 --- a/lib/gitlab/usage_data_counters/wiki_page_counter.rb +++ b/lib/gitlab/usage_data_counters/wiki_page_counter.rb @@ -1,32 +1,8 @@ # frozen_string_literal: true module Gitlab::UsageDataCounters - class WikiPageCounter - extend RedisCounter - - KNOWN_EVENTS = %w[create update delete].map(&:freeze).freeze - - UnknownEvent = Class.new(StandardError) - - class << self - # Each event gets a unique Redis key - def redis_key(event) - raise UnknownEvent, event unless KNOWN_EVENTS.include?(event.to_s) - - "USAGE_WIKI_PAGES_#{event}".upcase - end - - def count(event) - increment(redis_key event) - end - - def read(event) - total_count(redis_key event) - end - - def totals - KNOWN_EVENTS.map { |e| ["wiki_pages_#{e}".to_sym, read(e)] }.to_h - end - end + class WikiPageCounter < BaseCounter + KNOWN_EVENTS = %w[create update delete].freeze + PREFIX = 'wiki_pages' end end diff --git a/spec/lib/gitlab/usage_data_counters/note_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/note_counter_spec.rb new file mode 100644 index 00000000000..1669a22879f --- /dev/null +++ b/spec/lib/gitlab/usage_data_counters/note_counter_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::UsageDataCounters::NoteCounter, :clean_gitlab_redis_shared_state do + shared_examples 'a note usage counter' do |event, noteable_type| + describe ".count(#{event})" do + it "increments the Note #{event} counter by 1" do + expect do + described_class.count(event, noteable_type) + end.to change { described_class.read(event, noteable_type) }.by 1 + end + end + + describe ".read(#{event})" do + event_count = 5 + + it "returns the total number of #{event} events" do + event_count.times do + described_class.count(event, noteable_type) + end + + expect(described_class.read(event, noteable_type)).to eq(event_count) + end + end + end + + it_behaves_like 'a note usage counter', :create, 'Snippet' + + describe '.totals' do + let(:combinations) do + [ + [:create, 'Snippet', 3] + ] + end + + let(:expected_totals) do + { snippet_comment: 3 } + end + + before do + combinations.each do |event, noteable_type, n| + n.times do + described_class.count(event, noteable_type) + end + end + end + + it 'can report all totals' do + expect(described_class.totals).to include(expected_totals) + end + end + + describe 'unknown events or noteable_type' do + using RSpec::Parameterized::TableSyntax + + let(:unknown_event_error) { Gitlab::UsageDataCounters::BaseCounter::UnknownEvent } + + where(:event, :noteable_type, :expected_count, :should_raise) do + :create | 'Snippet' | 1 | false + :wibble | 'Snippet' | 0 | true + :create | 'Issue' | 0 | false + :wibble | 'Issue' | 0 | false + end + + with_them do + it "handles event" do + if should_raise + expect { described_class.count(event, noteable_type) }.to raise_error(unknown_event_error) + else + described_class.count(event, noteable_type) + + expect(described_class.read(event, noteable_type)).to eq(expected_count) + end + end + end + end +end diff --git a/spec/lib/gitlab/usage_data_counters/snippet_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/snippet_counter_spec.rb new file mode 100644 index 00000000000..65381ed36d1 --- /dev/null +++ b/spec/lib/gitlab/usage_data_counters/snippet_counter_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::UsageDataCounters::SnippetCounter do + it_behaves_like 'a redis usage counter', 'Snippet', :create + it_behaves_like 'a redis usage counter', 'Snippet', :update + + it_behaves_like 'a redis usage counter with totals', :snippet, + create: 3, + update: 2 +end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 297c4f0b683..bf36273251b 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -62,6 +62,9 @@ describe Gitlab::UsageData do )) expect(subject).to include( + snippet_create: a_kind_of(Integer), + snippet_update: a_kind_of(Integer), + snippet_comment: a_kind_of(Integer), wiki_pages_create: a_kind_of(Integer), wiki_pages_update: a_kind_of(Integer), wiki_pages_delete: a_kind_of(Integer), diff --git a/spec/services/create_snippet_service_spec.rb b/spec/services/create_snippet_service_spec.rb index f6b6989b955..9b83f65a17e 100644 --- a/spec/services/create_snippet_service_spec.rb +++ b/spec/services/create_snippet_service_spec.rb @@ -36,6 +36,22 @@ describe CreateSnippetService do end end + describe 'usage counter' do + let(:counter) { Gitlab::UsageDataCounters::SnippetCounter } + + it 'increments count' do + expect do + create_snippet(nil, @admin, @opts) + end.to change { counter.read(:create) }.by 1 + end + + it 'does not increment count if create fails' do + expect do + create_snippet(nil, @admin, {}) + end.not_to change { counter.read(:create) } + end + end + def create_snippet(project, user, opts) CreateSnippetService.new(project, user, opts).execute end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 46abd8f356a..cd4ea9c401d 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -365,5 +365,43 @@ describe Notes::CreateService do .and change { existing_note.updated_at } end end + + describe "usage counter" do + let(:counter) { Gitlab::UsageDataCounters::NoteCounter } + + context 'snippet note' do + let(:snippet) { create(:project_snippet, project: project) } + let(:opts) { { note: 'reply', noteable_type: 'Snippet', noteable_id: snippet.id, project: project } } + + it 'increments usage counter' do + expect do + note = described_class.new(project, user, opts).execute + + expect(note).to be_valid + end.to change { counter.read(:create, opts[:noteable_type]) }.by 1 + end + + it 'does not increment usage counter when creation fails' do + expect do + note = described_class.new(project, user, { note: '' }).execute + + expect(note).to be_invalid + end.not_to change { counter.read(:create, opts[:noteable_type]) } + end + end + + context 'issue note' do + let(:issue) { create(:issue, project: project) } + let(:opts) { { note: 'reply', noteable_type: 'Issue', noteable_id: issue.id, project: project } } + + it 'does not increment usage counter' do + expect do + note = described_class.new(project, user, opts).execute + + expect(note).to be_valid + end.not_to change { counter.read(:create, opts[:noteable_type]) } + end + end + end end end diff --git a/spec/services/update_snippet_service_spec.rb b/spec/services/update_snippet_service_spec.rb index 23ea4e003f8..0678f54c195 100644 --- a/spec/services/update_snippet_service_spec.rb +++ b/spec/services/update_snippet_service_spec.rb @@ -40,6 +40,23 @@ describe UpdateSnippetService do end end + describe 'usage counter' do + let(:counter) { Gitlab::UsageDataCounters::SnippetCounter } + let(:snippet) { create_snippet(nil, @user, @opts) } + + it 'increments count' do + expect do + update_snippet(nil, @admin, snippet, @opts) + end.to change { counter.read(:update) }.by 1 + end + + it 'does not increment count if create fails' do + expect do + update_snippet(nil, @admin, snippet, { title: '' }) + end.not_to change { counter.read(:update) } + end + end + def create_snippet(project, user, opts) CreateSnippetService.new(project, user, opts).execute end -- cgit v1.2.1