Commit 8d73134f authored by Sean McGivern's avatar Sean McGivern

Merge branch 'am-add-new-output-support-to-service-ping-report' into 'master'

Add queries and non_queries to ServicePingReport

See merge request gitlab-org/gitlab!80078
parents 74978657 c198a3bb
...@@ -62,11 +62,11 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController ...@@ -62,11 +62,11 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
def usage_data def usage_data
respond_to do |format| respond_to do |format|
format.html do format.html do
usage_data_json = Gitlab::Json.pretty_generate(Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true)) usage_data_json = Gitlab::Json.pretty_generate(Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values, cached: true))
render html: Gitlab::Highlight.highlight('payload.json', usage_data_json, language: 'json') render html: Gitlab::Highlight.highlight('payload.json', usage_data_json, language: 'json')
end end
format.json { render json: Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true).to_json } format.json { render json: Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values, cached: true).to_json }
end end
end end
......
...@@ -16,7 +16,7 @@ class Admin::InstanceReviewController < Admin::ApplicationController ...@@ -16,7 +16,7 @@ class Admin::InstanceReviewController < Admin::ApplicationController
} }
if Gitlab::CurrentSettings.usage_ping_enabled? if Gitlab::CurrentSettings.usage_ping_enabled?
data = Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true) data = Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values, cached: true)
counts = data[:counts] counts = data[:counts]
result[:instance_review].merge!( result[:instance_review].merge!(
......
...@@ -19,7 +19,7 @@ module ServicePing ...@@ -19,7 +19,7 @@ module ServicePing
end end
def raw_payload def raw_payload
@raw_payload ||= ::Gitlab::Usage::ServicePingReport.for(mode: :values) @raw_payload ||= ::Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values)
end end
end end
end end
......
...@@ -33,7 +33,7 @@ module ServicePing ...@@ -33,7 +33,7 @@ module ServicePing
} }
submit_payload({ error: error_payload }, url: error_url) submit_payload({ error: error_payload }, url: error_url)
usage_data = Gitlab::Usage::ServicePingReport.for(mode: :values) usage_data = Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values)
response = submit_usage_data_payload(usage_data) response = submit_usage_data_payload(usage_data)
end end
......
...@@ -1324,7 +1324,7 @@ has more information about Service Ping. ...@@ -1324,7 +1324,7 @@ has more information about Service Ping.
### Generate or get the cached Service Ping ### Generate or get the cached Service Ping
```ruby ```ruby
Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true) Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values, cached: true)
``` ```
### Generate a fresh new Service Ping ### Generate a fresh new Service Ping
...@@ -1332,7 +1332,7 @@ Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true) ...@@ -1332,7 +1332,7 @@ Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true)
This also refreshes the cached Service Ping displayed in the Admin Area This also refreshes the cached Service Ping displayed in the Admin Area
```ruby ```ruby
Gitlab::Usage::ServicePingReport.for(mode: :values) Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values)
``` ```
### Generate and print ### Generate and print
......
...@@ -198,8 +198,8 @@ sequenceDiagram ...@@ -198,8 +198,8 @@ sequenceDiagram
## How Service Ping works ## How Service Ping works
1. The Service Ping [cron job](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/workers/gitlab_service_ping_worker.rb#L24) is set in Sidekiq to run weekly. 1. The Service Ping [cron job](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/workers/gitlab_service_ping_worker.rb#L24) is set in Sidekiq to run weekly.
1. When the cron job runs, it calls [`Gitlab::Usage::ServicePingReport.for(mode: :values)`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/service_ping/submit_service.rb). 1. When the cron job runs, it calls [`Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values)`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/service_ping/submit_service.rb).
1. `Gitlab::Usage::ServicePingReport.for(mode: :values)` [cascades down](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data.rb) to ~400+ other counter method calls. 1. `Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values)` [cascades down](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data.rb) to ~400+ other counter method calls.
1. The response of all methods calls are [merged together](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data.rb#L68) into a single JSON payload. 1. The response of all methods calls are [merged together](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data.rb#L68) into a single JSON payload.
1. The JSON payload is then [posted to the Versions application](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/service_ping/submit_service.rb#L20) 1. The JSON payload is then [posted to the Versions application](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/service_ping/submit_service.rb#L20)
If a firewall exception is needed, the required URL depends on several things. If If a firewall exception is needed, the required URL depends on several things. If
......
...@@ -5,7 +5,7 @@ require 'spec_helper' ...@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe 'Every metric definition' do RSpec.describe 'Every metric definition' do
include UsageDataHelpers include UsageDataHelpers
let(:usage_ping) { Gitlab::Usage::ServicePingReport.for(mode: :values) } let(:usage_ping) { Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values) }
let(:ignored_usage_ping_key_patterns) do let(:ignored_usage_ping_key_patterns) do
%w( %w(
license_add_ons license_add_ons
......
...@@ -18,7 +18,7 @@ module API ...@@ -18,7 +18,7 @@ module API
get 'non_sql_metrics' do get 'non_sql_metrics' do
Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/325534') Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/325534')
data = Gitlab::UsageDataNonSqlMetrics.data data = Gitlab::Usage::ServicePingReport.for(output: :non_sql_metrics_values)
present data present data
end end
......
...@@ -18,7 +18,7 @@ module API ...@@ -18,7 +18,7 @@ module API
get 'queries' do get 'queries' do
Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/325534') Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/325534')
queries = Gitlab::UsageDataQueries.data queries = Gitlab::Usage::ServicePingReport.for(output: :metrics_queries)
present queries present queries
end end
......
...@@ -4,20 +4,32 @@ module Gitlab ...@@ -4,20 +4,32 @@ module Gitlab
module Usage module Usage
class ServicePingReport class ServicePingReport
class << self class << self
def for(mode:, cached: false) def for(output:, cached: false)
case mode.to_sym case output.to_sym
when :values when :all_metrics_values
usage_data(cached) all_metrics_values(cached)
when :metrics_queries
metrics_queries
when :non_sql_metrics_values
non_sql_metrics_values
end end
end end
private private
def usage_data(cached) def all_metrics_values(cached)
Rails.cache.fetch('usage_data', force: !cached, expires_in: 2.weeks) do Rails.cache.fetch('usage_data', force: !cached, expires_in: 2.weeks) do
Gitlab::UsageData.data Gitlab::UsageData.data
end end
end end
def metrics_queries
Gitlab::UsageDataQueries.data
end
def non_sql_metrics_values
Gitlab::UsageDataNonSqlMetrics.data
end
end end
end end
end end
......
...@@ -4,17 +4,17 @@ namespace :gitlab do ...@@ -4,17 +4,17 @@ namespace :gitlab do
namespace :usage_data do namespace :usage_data do
desc 'GitLab | UsageData | Generate raw SQLs for usage ping in YAML' desc 'GitLab | UsageData | Generate raw SQLs for usage ping in YAML'
task dump_sql_in_yaml: :environment do task dump_sql_in_yaml: :environment do
puts Gitlab::UsageDataQueries.data.to_yaml puts Gitlab::Usage::ServicePingReport.for(output: :metrics_queries).to_yaml
end end
desc 'GitLab | UsageData | Generate raw SQLs for usage ping in JSON' desc 'GitLab | UsageData | Generate raw SQLs for usage ping in JSON'
task dump_sql_in_json: :environment do task dump_sql_in_json: :environment do
puts Gitlab::Json.pretty_generate(Gitlab::UsageDataQueries.data) puts Gitlab::Json.pretty_generate(Gitlab::Usage::ServicePingReport.for(output: :metrics_queries))
end end
desc 'GitLab | UsageData | Generate usage ping in JSON' desc 'GitLab | UsageData | Generate usage ping in JSON'
task generate: :environment do task generate: :environment do
puts Gitlab::Json.pretty_generate(Gitlab::Usage::ServicePingReport.for(mode: :values)) puts Gitlab::Json.pretty_generate(Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values))
end end
desc 'GitLab | UsageData | Generate usage ping and send it to Versions Application' desc 'GitLab | UsageData | Generate usage ping and send it to Versions Application'
......
...@@ -23,7 +23,7 @@ RSpec.describe Admin::InstanceReviewController do ...@@ -23,7 +23,7 @@ RSpec.describe Admin::InstanceReviewController do
stub_application_setting(usage_ping_enabled: true) stub_application_setting(usage_ping_enabled: true)
stub_usage_data_connections stub_usage_data_connections
stub_database_flavor_check stub_database_flavor_check
::Gitlab::Usage::ServicePingReport.for(mode: :values) ::Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values)
subject subject
end end
......
...@@ -5,11 +5,27 @@ require 'spec_helper' ...@@ -5,11 +5,27 @@ require 'spec_helper'
RSpec.describe Gitlab::Usage::ServicePingReport, :use_clean_rails_memory_store_caching do RSpec.describe Gitlab::Usage::ServicePingReport, :use_clean_rails_memory_store_caching do
let(:usage_data) { { uuid: "1111" } } let(:usage_data) { { uuid: "1111" } }
context 'for mode: :values' do context 'for output: :all_metrics_values' do
it 'generates the service ping' do it 'generates the service ping' do
expect(Gitlab::UsageData).to receive(:data) expect(Gitlab::UsageData).to receive(:data)
described_class.for(mode: :values) described_class.for(output: :all_metrics_values)
end
end
context 'for output: :metrics_queries' do
it 'generates the service ping' do
expect(Gitlab::UsageDataQueries).to receive(:data)
described_class.for(output: :metrics_queries)
end
end
context 'for output: :non_sql_metrics_values' do
it 'generates the service ping' do
expect(Gitlab::UsageDataNonSqlMetrics).to receive(:data)
described_class.for(output: :non_sql_metrics_values)
end end
end end
...@@ -20,8 +36,8 @@ RSpec.describe Gitlab::Usage::ServicePingReport, :use_clean_rails_memory_store_c ...@@ -20,8 +36,8 @@ RSpec.describe Gitlab::Usage::ServicePingReport, :use_clean_rails_memory_store_c
it 'caches the values' do it 'caches the values' do
allow(Gitlab::UsageData).to receive(:data).and_return(usage_data, new_usage_data) allow(Gitlab::UsageData).to receive(:data).and_return(usage_data, new_usage_data)
expect(described_class.for(mode: :values)).to eq(usage_data) expect(described_class.for(output: :all_metrics_values)).to eq(usage_data)
expect(described_class.for(mode: :values, cached: true)).to eq(usage_data) expect(described_class.for(output: :all_metrics_values, cached: true)).to eq(usage_data)
expect(Rails.cache.fetch('usage_data')).to eq(usage_data) expect(Rails.cache.fetch('usage_data')).to eq(usage_data)
end end
...@@ -29,9 +45,9 @@ RSpec.describe Gitlab::Usage::ServicePingReport, :use_clean_rails_memory_store_c ...@@ -29,9 +45,9 @@ RSpec.describe Gitlab::Usage::ServicePingReport, :use_clean_rails_memory_store_c
it 'writes to cache and returns fresh data' do it 'writes to cache and returns fresh data' do
allow(Gitlab::UsageData).to receive(:data).and_return(usage_data, new_usage_data) allow(Gitlab::UsageData).to receive(:data).and_return(usage_data, new_usage_data)
expect(described_class.for(mode: :values)).to eq(usage_data) expect(described_class.for(output: :all_metrics_values)).to eq(usage_data)
expect(described_class.for(mode: :values)).to eq(new_usage_data) expect(described_class.for(output: :all_metrics_values)).to eq(new_usage_data)
expect(described_class.for(mode: :values, cached: true)).to eq(new_usage_data) expect(described_class.for(output: :all_metrics_values, cached: true)).to eq(new_usage_data)
expect(Rails.cache.fetch('usage_data')).to eq(new_usage_data) expect(Rails.cache.fetch('usage_data')).to eq(new_usage_data)
end end
...@@ -43,8 +59,8 @@ RSpec.describe Gitlab::Usage::ServicePingReport, :use_clean_rails_memory_store_c ...@@ -43,8 +59,8 @@ RSpec.describe Gitlab::Usage::ServicePingReport, :use_clean_rails_memory_store_c
it 'returns fresh data' do it 'returns fresh data' do
allow(Gitlab::UsageData).to receive(:data).and_return(usage_data, new_usage_data) allow(Gitlab::UsageData).to receive(:data).and_return(usage_data, new_usage_data)
expect(described_class.for(mode: :values)).to eq(usage_data) expect(described_class.for(output: :all_metrics_values)).to eq(usage_data)
expect(described_class.for(mode: :values)).to eq(new_usage_data) expect(described_class.for(output: :all_metrics_values)).to eq(new_usage_data)
expect(Rails.cache.fetch('usage_data')).to eq(new_usage_data) expect(Rails.cache.fetch('usage_data')).to eq(new_usage_data)
end end
......
...@@ -118,7 +118,7 @@ RSpec.describe ServicePing::SubmitService do ...@@ -118,7 +118,7 @@ RSpec.describe ServicePing::SubmitService do
it 'generates service ping' do it 'generates service ping' do
stub_response(body: with_dev_ops_score_params) stub_response(body: with_dev_ops_score_params)
expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_call_original expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_call_original
subject.execute subject.execute
end end
...@@ -151,7 +151,7 @@ RSpec.describe ServicePing::SubmitService do ...@@ -151,7 +151,7 @@ RSpec.describe ServicePing::SubmitService do
it 'forces a refresh of usage data statistics before submitting' do it 'forces a refresh of usage data statistics before submitting' do
stub_response(body: with_dev_ops_score_params) stub_response(body: with_dev_ops_score_params)
expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_call_original expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_call_original
subject.execute subject.execute
end end
...@@ -167,7 +167,7 @@ RSpec.describe ServicePing::SubmitService do ...@@ -167,7 +167,7 @@ RSpec.describe ServicePing::SubmitService do
recorded_at = Time.current recorded_at = Time.current
usage_data = { uuid: 'uuid', recorded_at: recorded_at } usage_data = { uuid: 'uuid', recorded_at: recorded_at }
expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data) expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_return(usage_data)
subject.execute subject.execute
...@@ -190,7 +190,7 @@ RSpec.describe ServicePing::SubmitService do ...@@ -190,7 +190,7 @@ RSpec.describe ServicePing::SubmitService do
recorded_at = Time.current recorded_at = Time.current
usage_data = { uuid: 'uuid', recorded_at: recorded_at } usage_data = { uuid: 'uuid', recorded_at: recorded_at }
expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data) expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_return(usage_data)
subject.execute subject.execute
...@@ -235,7 +235,7 @@ RSpec.describe ServicePing::SubmitService do ...@@ -235,7 +235,7 @@ RSpec.describe ServicePing::SubmitService do
recorded_at = Time.current recorded_at = Time.current
usage_data = { uuid: 'uuid', recorded_at: recorded_at } usage_data = { uuid: 'uuid', recorded_at: recorded_at }
expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data) expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_return(usage_data)
subject.execute subject.execute
...@@ -260,7 +260,7 @@ RSpec.describe ServicePing::SubmitService do ...@@ -260,7 +260,7 @@ RSpec.describe ServicePing::SubmitService do
context 'and usage data is empty string' do context 'and usage data is empty string' do
before do before do
allow(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return({}) allow(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_return({})
end end
it_behaves_like 'does not send a blank usage ping payload' it_behaves_like 'does not send a blank usage ping payload'
...@@ -269,7 +269,7 @@ RSpec.describe ServicePing::SubmitService do ...@@ -269,7 +269,7 @@ RSpec.describe ServicePing::SubmitService do
context 'and usage data is nil' do context 'and usage data is nil' do
before do before do
allow(ServicePing::BuildPayloadService).to receive(:execute).and_return(nil) allow(ServicePing::BuildPayloadService).to receive(:execute).and_return(nil)
allow(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(nil) allow(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_return(nil)
end end
it_behaves_like 'does not send a blank usage ping payload' it_behaves_like 'does not send a blank usage ping payload'
...@@ -285,7 +285,7 @@ RSpec.describe ServicePing::SubmitService do ...@@ -285,7 +285,7 @@ RSpec.describe ServicePing::SubmitService do
it 'calls Gitlab::Usage::ServicePingReport .for method' do it 'calls Gitlab::Usage::ServicePingReport .for method' do
usage_data = build_usage_data usage_data = build_usage_data
expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data) expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_return(usage_data)
subject.execute subject.execute
end end
...@@ -329,7 +329,7 @@ RSpec.describe ServicePing::SubmitService do ...@@ -329,7 +329,7 @@ RSpec.describe ServicePing::SubmitService do
it 'calls Gitlab::Usage::ServicePingReport .for method' do it 'calls Gitlab::Usage::ServicePingReport .for method' do
usage_data = build_usage_data usage_data = build_usage_data
expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data) expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(output: :all_metrics_values).and_return(usage_data)
# SubmissionError is raised as a result of 404 in response from HTTP Request # SubmissionError is raised as a result of 404 in response from HTTP Request
expect { subject.execute }.to raise_error(described_class::SubmissionError) expect { subject.execute }.to raise_error(described_class::SubmissionError)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment