Merge branch '233360-add-usage-data-table-in-postgres-database' into 'master'

Add raw usage data table

See merge request gitlab-org/gitlab!38457

app/models/raw_usage_data.rb

+# frozen_string_literal: true
+
+class RawUsageData < ApplicationRecord
+  validates :payload, presence: true
+  validates :recorded_at, presence: true, uniqueness: true
+
+  def update_sent_at!
+    self.update_column(:sent_at, Time.current) if Feature.enabled?(:save_raw_usage_data)
+  end
+end

app/services/submit_usage_ping_service.rb

@@ -20,23 +20,36 @@ class SubmitUsagePingService
     return unless Gitlab::CurrentSettings.usage_ping_enabled?
     return if User.single_user&.requires_usage_stats_consent?
 
-    payload = Gitlab::UsageData.to_json(force_refresh: true)
-    raise SubmissionError.new('Usage data is blank') if payload.blank?
+    usage_data = Gitlab::UsageData.data(force_refresh: true)
+
+    raise SubmissionError.new('Usage data is blank') if usage_data.blank?
+
+    raw_usage_data = save_raw_usage_data(usage_data)
 
     response = Gitlab::HTTP.post(
       url,
-      body: payload,
+      body: usage_data.to_json,
       allow_local_requests: true,
       headers: { 'Content-type' => 'application/json' }
     )
 
     raise SubmissionError.new("Unsuccessful response code: #{response.code}") unless response.success?
 
+    raw_usage_data.update_sent_at! if raw_usage_data
+
     store_metrics(response)
   end
 
   private
 
+  def save_raw_usage_data(usage_data)
+    return unless Feature.enabled?(:save_raw_usage_data)
+
+    RawUsageData.safe_find_or_create_by(recorded_at: usage_data[:recorded_at]) do |record|
+      record.payload = usage_data
+    end
+  end
+
   def store_metrics(response)
     metrics = response['conv_index'] || response['dev_ops_score']
 

changelogs/unreleased/233360-add-usage-data-table-in-postgres-database.yml

+---
+title: Save usage data in database
+merge_request: 38457
+author:
+type: added

db/migrate/20200803125340_create_raw_usage_data.rb

+# frozen_string_literal: true
+
+class CreateRawUsageData < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    unless table_exists?(:raw_usage_data)
+      create_table :raw_usage_data do |t|
+        t.timestamps_with_timezone
+        t.datetime_with_timezone :recorded_at, null: false
+        t.datetime_with_timezone :sent_at
+        t.jsonb :payload, null: false
+
+        t.index [:recorded_at], unique: true
+      end
+    end
+  end
+
+  def down
+    drop_table :raw_usage_data
+  end
+end

db/schema_migrations/20200803125340

+78a33c142b6182749291ad963397dcda8efe5fe421381612801e5ccd34267326
\ No newline at end of file

db/structure.sql

@@ -14928,6 +14928,24 @@ CREATE SEQUENCE public.push_rules_id_seq
 
 ALTER SEQUENCE public.push_rules_id_seq OWNED BY public.push_rules.id;
 
+CREATE TABLE public.raw_usage_data (
+    id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    recorded_at timestamp with time zone NOT NULL,
+    sent_at timestamp with time zone,
+    payload jsonb NOT NULL
+);
+
+CREATE SEQUENCE public.raw_usage_data_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE public.raw_usage_data_id_seq OWNED BY public.raw_usage_data.id;
+
 CREATE TABLE public.redirect_routes (
     id integer NOT NULL,
     source_id integer NOT NULL,
@@ -17225,6 +17243,8 @@ ALTER TABLE ONLY public.protected_tags ALTER COLUMN id SET DEFAULT nextval('publ
 
 ALTER TABLE ONLY public.push_rules ALTER COLUMN id SET DEFAULT nextval('public.push_rules_id_seq'::regclass);
 
+ALTER TABLE ONLY public.raw_usage_data ALTER COLUMN id SET DEFAULT nextval('public.raw_usage_data_id_seq'::regclass);
+
 ALTER TABLE ONLY public.redirect_routes ALTER COLUMN id SET DEFAULT nextval('public.redirect_routes_id_seq'::regclass);
 
 ALTER TABLE ONLY public.release_links ALTER COLUMN id SET DEFAULT nextval('public.release_links_id_seq'::regclass);
@@ -18450,6 +18470,9 @@ ALTER TABLE ONLY public.protected_tags
 ALTER TABLE ONLY public.push_rules
     ADD CONSTRAINT push_rules_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY public.raw_usage_data
+    ADD CONSTRAINT raw_usage_data_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY public.redirect_routes
     ADD CONSTRAINT redirect_routes_pkey PRIMARY KEY (id);
 
@@ -20525,6 +20548,8 @@ CREATE INDEX index_push_rules_on_is_sample ON public.push_rules USING btree (is_
 
 CREATE INDEX index_push_rules_on_project_id ON public.push_rules USING btree (project_id);
 
+CREATE UNIQUE INDEX index_raw_usage_data_on_recorded_at ON public.raw_usage_data USING btree (recorded_at);
+
 CREATE UNIQUE INDEX index_redirect_routes_on_path ON public.redirect_routes USING btree (path);
 
 CREATE UNIQUE INDEX index_redirect_routes_on_path_unique_text_pattern_ops ON public.redirect_routes USING btree (lower((path)::text) varchar_pattern_ops);

spec/db/schema_spec.rb

@@ -189,6 +189,7 @@ RSpec.describe 'Database schema' do
     "Operations::FeatureFlagScope" => %w[strategies],
     "Operations::FeatureFlags::Strategy" => %w[parameters],
     "Packages::Composer::Metadatum" => %w[composer_json],
+    "RawUsageData" => %w[payload], # Usage data payload changes often, we cannot use one schema
     "Releases::Evidence" => %w[summary]
   }.freeze
 

spec/factories/raw_usage_data.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :raw_usage_data do
+    recorded_at { Time.current }
+    payload { { test: 'test' } }
+  end
+end

spec/models/application_record_spec.rb

@@ -46,6 +46,13 @@ RSpec.describe ApplicationRecord do
         Suggestion.safe_find_or_create_by(attributes, &block)
       end.to yield_with_args(an_object_having_attributes(attributes))
     end
+
+    it 'does not create a record when is not valid' do
+      raw_usage_data = RawUsageData.safe_find_or_create_by({ recorded_at: nil })
+
+      expect(raw_usage_data.id).to be_nil
+      expect(raw_usage_data).not_to be_valid
+    end
   end
 
   describe '.safe_find_or_create_by!' do

spec/models/raw_usage_data_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe RawUsageData do
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:payload) }
+    it { is_expected.to validate_presence_of(:recorded_at) }
+
+    context 'uniqueness validation' do
+      let!(:existing_record) { create(:raw_usage_data) }
+
+      it { is_expected.to validate_uniqueness_of(:recorded_at) }
+    end
+
+    describe '#update_sent_at!' do
+      let(:raw_usage_data) { create(:raw_usage_data) }
+
+      context 'with save_raw_usage_data feature enabled' do
+        before do
+          stub_feature_flags(save_raw_usage_data: true)
+        end
+
+        it 'updates sent_at' do
+          raw_usage_data.update_sent_at!
+
+          expect(raw_usage_data.sent_at).not_to be_nil
+        end
+      end
+
+      context 'with save_raw_usage_data feature disabled' do
+        before do
+          stub_feature_flags(save_raw_usage_data: false)
+        end
+
+        it 'updates sent_at' do
+          raw_usage_data.update_sent_at!
+
+          expect(raw_usage_data.sent_at).to be_nil
+        end
+      end
+    end
+  end
+end

spec/services/submit_usage_ping_service_spec.rb

@@ -52,7 +52,7 @@ RSpec.describe SubmitUsagePingService do
   shared_examples 'does not run' do
     it do
       expect(Gitlab::HTTP).not_to receive(:post)
-      expect(Gitlab::UsageData).not_to receive(:to_json)
+      expect(Gitlab::UsageData).not_to receive(:data)
 
       subject.execute
     end
@@ -113,7 +113,7 @@ RSpec.describe SubmitUsagePingService do
     it 'forces a refresh of usage data statistics before submitting' do
       stub_response(body: without_dev_ops_score_params)
 
-      expect(Gitlab::UsageData).to receive(:to_json).with(force_refresh: true).and_call_original
+      expect(Gitlab::UsageData).to receive(:data).with(force_refresh: true).and_call_original
 
       subject.execute
     end
@@ -134,6 +134,43 @@ RSpec.describe SubmitUsagePingService do
       it_behaves_like 'saves DevOps score data from the response'
     end
 
+    context 'with save_raw_usage_data feature enabled' do
+      before do
+        stub_response(body: with_dev_ops_score_params)
+        stub_feature_flags(save_raw_usage_data: true)
+      end
+
+      it 'creates a raw_usage_data record' do
+        expect { subject.execute }.to change(RawUsageData, :count).by(1)
+      end
+
+      it 'saves the correct payload' do
+        recorded_at = Time.current
+        usage_data = { uuid: 'uuid', recorded_at: recorded_at }
+
+        expect(Gitlab::UsageData).to receive(:data).with(force_refresh: true).and_return(usage_data)
+
+        subject.execute
+
+        raw_usage_data = RawUsageData.find_by(recorded_at: recorded_at)
+
+        expect(raw_usage_data.recorded_at).to be_like_time(recorded_at)
+        expect(raw_usage_data.payload.to_json).to eq(usage_data.to_json)
+      end
+    end
+
+    context 'with save_raw_usage_data feature disabled' do
+      before do
+        stub_response(body: with_dev_ops_score_params)
+      end
+
+      it 'does not create a raw_usage_data record' do
+        stub_feature_flags(save_raw_usage_data: false)
+
+        expect { subject.execute }.to change(RawUsageData, :count).by(0)
+      end
+    end
+
     context 'and usage ping response has unsuccessful status' do
       before do
         stub_response(body: nil, status: 504)
@@ -148,7 +185,7 @@ RSpec.describe SubmitUsagePingService do
 
     context 'and usage data is empty string' do
       before do
-        allow(Gitlab::UsageData).to receive(:to_json).and_return("")
+        allow(Gitlab::UsageData).to receive(:data).and_return({})
       end
 
       it_behaves_like 'does not send a blank usage ping payload'
@@ -156,7 +193,7 @@ RSpec.describe SubmitUsagePingService do
 
     context 'and usage data is nil' do
       before do
-        allow(Gitlab::UsageData).to receive(:to_json).and_return(nil)
+        allow(Gitlab::UsageData).to receive(:data).and_return(nil)
       end
 
       it_behaves_like 'does not send a blank usage ping payload'