Skip to content

G
gitlab-ce
Commit ea142f1a authored by Alina Mihaila's avatar Alina Mihaila Committed by Mikołaj Wawrzyniak
Browse files
Options

Remove instrumentation class ff

parent c4eec205
Hide whitespace changes
Inline Side-by-side
Showing with 45 additions and 344 deletions
config/feature_flags/development/usage_data_instrumentation.yml deleted 100644 → 0
View file @ c4eec205
---
name: usage_data_instrumentation
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68808
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/345252
milestone: '14.5'
type: development
group: group::product intelligence
default_enabled: false
ee/spec/config/metrics/every_metric_definition_spec.rb
View file @ ea142f1a
......@@ -87,37 +87,10 @@ RSpec.describe 'Every metric definition' do
stub_usage_data_connections
end
context 'with usage_data_instrumentation feature flag' do
let(:msg) { 'see https://docs.gitlab.com/ee/development/service_ping/metrics_dictionary.html#metrics-added-dynamic-to-service-ping-payload' }
context 'when enabled' do
let(:instrumentation_class_keys) do
Gitlab::Usage::MetricDefinition.with_instrumentation_class.map(&:key)
.reject { |v| v =~ Regexp.union(ignored_usage_ping_key_patterns) }
end
let(:files_key_paths) do
(metric_files_key_paths + instrumentation_class_keys).to_set
end
before do
stub_feature_flags(usage_data_instrumentation: true)
end
it 'is included in the Usage Ping hash structure', :aggregate_failures do
expect(files_key_paths).to eql(usage_ping_key_paths.to_set), msg
end
end
context 'when disabled' do
before do
stub_feature_flags(usage_data_instrumentation: false)
end
it 'is included in the Usage Ping hash structure', :aggregate_failures do
expect(metric_files_key_paths).to match_array(usage_ping_key_paths), msg
end
end
it 'is included in the Usage Ping hash structure', :aggregate_failures do
msg = "see https://docs.gitlab.com/ee/development/service_ping/metrics_dictionary.html#metrics-added-dynamic-to-service-ping-payload"
expect(metric_files_key_paths).to match_array(usage_ping_key_paths)
expect(metric_files_key_paths).to match_array(usage_ping_key_paths), msg
end
context 'with value json schema' do
......
ee/spec/lib/ee/gitlab/usage_data_non_sql_metrics_spec.rb
View file @ ea142f1a
......@@ -15,7 +15,7 @@ RSpec.describe Gitlab::UsageDataNonSqlMetrics do
described_class.uncached_data
end
expect(recorder.count).to eq(61)
expect(recorder.count).to eq(59)
end
end
end
ee/spec/lib/ee/gitlab/usage_data_spec.rb
View file @ ea142f1a
......@@ -62,7 +62,7 @@ RSpec.describe Gitlab::UsageData do
subject { described_class.data }
it 'gathers usage data' do
expect(subject.keys).to include(*%w(
expect(subject.keys).to include(*%i(
historical_max_users
license_add_ons
license_plan
......@@ -85,7 +85,7 @@ RSpec.describe Gitlab::UsageData do
expect(count_data[:boards]).to eq(1)
expect(count_data[:projects]).to eq(3)
expect(count_data.keys).to include(*%w(
expect(count_data.keys).to include(*%i(
confidential_epics
container_scanning_jobs
coverage_fuzzing_jobs
......@@ -154,10 +154,6 @@ RSpec.describe Gitlab::UsageData do
describe '.features_usage_data_ee' do
subject { described_class.features_usage_data_ee }
before do
stub_feature_flags(usage_data_instrumentation: false)
end
it 'gathers feature usage data of EE' do
expect(subject[:elasticsearch_enabled]).to eq(Gitlab::CurrentSettings.elasticsearch_search?)
expect(subject[:geo_enabled]).to eq(Gitlab::Geo.enabled?)
......@@ -179,39 +175,9 @@ RSpec.describe Gitlab::UsageData do
expect(subject[:license_trial]).to eq(license.trial?)
expect(subject[:license_subscription_id]).to eq(license.subscription_id)
expect(subject[:license_billable_users]).to eq(license.daily_billable_users_count)
end
context 'with usage_data_instrumentation feature flag' do
let(:license) { ::License.current }
context 'when enabled' do
before do
stub_feature_flags(usage_data_instrumentation: true)
end
it 'returns fallback value to be overriden' do
expect(subject[:licensee]).to eq(Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
expect(subject[:license_md5]).to eq(Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
expect(subject[:historical_max_users]).to eq(Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
uncached_data = described_class.uncached_data
expect(uncached_data[:licensee]).to eq(license.licensee)
expect(uncached_data[:license_md5]).to eq(Digest::MD5.hexdigest(license.data))
expect(uncached_data[:historical_max_users]).to eq(license.historical_max)
end
end
context 'when disabled' do
before do
stub_feature_flags(usage_data_instrumentation: false)
end
it 'computes historical_max_users, licensee and license_md5 values' do
expect(subject[:licensee]).to eq(license.licensee)
expect(subject[:license_md5]).to eq(Digest::MD5.hexdigest(license.data))
expect(subject[:historical_max_users]).to eq(license.historical_max)
end
end
expect(subject[:licensee]).to eq(license.licensee)
expect(subject[:license_md5]).to eq(Digest::MD5.hexdigest(license.data))
expect(subject[:historical_max_users]).to eq(license.historical_max)
end
end
......@@ -557,36 +523,8 @@ RSpec.describe Gitlab::UsageData do
it 'includes accurate usage_activity_by_stage data' do
expect(described_class.usage_activity_by_stage_release({})).to include(projects_mirrored_with_pipelines_enabled: 2)
expect(described_class.usage_activity_by_stage_release(described_class.monthly_time_range_db_params)).to include(projects_mirrored_with_pipelines_enabled: 1)
end
context 'with usage_data_instrumentation feature flag' do
let(:license) { ::License.current }
context 'when enabled' do
before do
stub_feature_flags(usage_data_instrumentation: true)
end
it 'returns fallback value to be overriden' do
expect(described_class.usage_activity_by_stage_release({})).to include(releases_with_group_milestones: Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
expect(described_class.usage_activity_by_stage_release(described_class.monthly_time_range_db_params)).to include(releases_with_group_milestones: Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
uncached_data = described_class.uncached_data
expect(uncached_data[:usage_activity_by_stage][:release]).to include(releases_with_milestones: 2)
expect(uncached_data[:usage_activity_by_stage_monthly][:release]).to include(releases_with_milestones: 1)
end
end
context 'when disabled' do
before do
stub_feature_flags(usage_data_instrumentation: false)
end
it 'computes releases_with_group_milestones values' do
expect(described_class.usage_activity_by_stage_release({})).to include(releases_with_group_milestones: 2)
expect(described_class.usage_activity_by_stage_release(described_class.monthly_time_range_db_params)).to include(releases_with_group_milestones: 1)
end
end
expect(described_class.usage_activity_by_stage_release({})).to include(releases_with_group_milestones: 2)
expect(described_class.usage_activity_by_stage_release(described_class.monthly_time_range_db_params)).to include(releases_with_group_milestones: 1)
end
end
......
lib/gitlab/usage/metrics/names_suggestions/generator.rb
View file @ ea142f1a
......@@ -18,10 +18,6 @@ module Gitlab
private
def instrumentation_metrics
::Gitlab::UsageDataMetrics.suggested_names
end
def count(relation, column = nil, batch: true, batch_size: nil, start: nil, finish: nil)
Gitlab::Usage::Metrics::NameSuggestion.for(:count, column: column, relation: relation)
end
......
lib/gitlab/usage_data.rb
View file @ ea142f1a
......@@ -51,10 +51,7 @@ module Gitlab
clear_memoized
with_finished_at(:recording_ce_finished_at) do
usage_data = usage_data_metrics
usage_data = usage_data.with_indifferent_access.deep_merge(instrumentation_metrics.with_indifferent_access) if Feature.enabled?(:usage_data_instrumentation)
usage_data
usage_data_metrics
end
end
......@@ -757,10 +754,6 @@ module Gitlab
.deep_merge(aggregated_metrics_data)
end
def instrumentation_metrics
Gitlab::UsageDataMetrics.uncached_data # rubocop:disable UsageData/LargeTable
end
def metric_time_period(time_period)
time_period.present? ? '28d' : 'none'
end
......
lib/gitlab/usage_data_non_sql_metrics.rb
View file @ ea142f1a
......@@ -5,13 +5,6 @@ module Gitlab
SQL_METRIC_DEFAULT = -3
class << self
def uncached_data
# instrumentation_metrics is already included with feature flag enabled
return super if Feature.enabled?(:usage_data_instrumentation)
super.with_indifferent_access.deep_merge(instrumentation_metrics.with_indifferent_access)
end
def add_metric(metric, time_frame: 'none', options: {})
metric_class = "Gitlab::Usage::Metrics::Instrumentations::#{metric}".constantize
......@@ -50,12 +43,6 @@ module Gitlab
projects_jira_cloud_active: 0
}
end
private
def instrumentation_metrics
::Gitlab::Usage::Metric.all.map(&:with_instrumentation).reduce({}, :deep_merge)
end
end
end
end
lib/gitlab/usage_data_queries.rb
View file @ ea142f1a
......@@ -5,13 +5,6 @@ module Gitlab
# See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41091
class UsageDataQueries < UsageData
class << self
def uncached_data
# instrumentation_metrics is already included with feature flag enabled
return super if Feature.enabled?(:usage_data_instrumentation)
super.with_indifferent_access.deep_merge(instrumentation_metrics.with_indifferent_access)
end
def add_metric(metric, time_frame: 'none', options: {})
metric_class = "Gitlab::Usage::Metrics::Instrumentations::#{metric}".constantize
......@@ -71,12 +64,6 @@ module Gitlab
def epics_deepest_relationship_level
{ epics_deepest_relationship_level: 0 }
end
private
def instrumentation_metrics
::Gitlab::Usage::Metric.all.map(&:with_instrumentation).reduce({}, :deep_merge)
end
end
end
end
lib/gitlab/utils/usage_data.rb
View file @ ea142f1a
......@@ -43,13 +43,8 @@ module Gitlab
HISTOGRAM_FALLBACK = { '-1' => -1 }.freeze
DISTRIBUTED_HLL_FALLBACK = -2
MAX_BUCKET_SIZE = 100
INSTRUMENTATION_CLASS_FALLBACK = -100
def add_metric(metric, time_frame: 'none', options: {})
# Results of this method should be overwritten by instrumentation class values
# -100 indicates the metric was not properly merged.
return INSTRUMENTATION_CLASS_FALLBACK if Feature.enabled?(:usage_data_instrumentation)
metric_class = "Gitlab::Usage::Metrics::Instrumentations::#{metric}".constantize
metric_class.new(time_frame: time_frame, options: options).value
......
spec/lib/gitlab/usage/metrics/names_suggestions/generator_spec.rb
View file @ ea142f1a
......@@ -25,30 +25,10 @@ RSpec.describe Gitlab::Usage::Metrics::NamesSuggestions::Generator do
end
context 'for count with default column metrics' do
context 'with usage_data_instrumentation feature flag' do
context 'when enabled' do
before do
stub_feature_flags(usage_data_instrumentation: true)
end
it_behaves_like 'name suggestion' do
# corresponding metric is collected with ::Gitlab::UsageDataMetrics.suggested_names
let(:key_path) { 'counts.boards' }
let(:name_suggestion) { /count_boards/ }
end
end
context 'when disabled' do
before do
stub_feature_flags(usage_data_instrumentation: false)
end
it_behaves_like 'name suggestion' do
# corresponding metric is collected with count(Board)
let(:key_path) { 'counts.boards' }
let(:name_suggestion) { /count_boards/ }
end
end
it_behaves_like 'name suggestion' do
# corresponding metric is collected with count(Board)
let(:key_path) { 'counts.boards' }
let(:name_suggestion) { /count_boards/ }
end
end
......
spec/lib/gitlab/usage_data_spec.rb
View file @ ea142f1a
......@@ -80,12 +80,6 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
end
end
end
it 'allows indifferent access' do
allow(::Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:unique_events).and_return(1)
expect(subject[:search_unique_visits][:search_unique_visits_for_any_target_monthly]).to eq(1)
expect(subject[:search_unique_visits]['search_unique_visits_for_any_target_monthly']).to eq(1)
end
end
describe 'usage_activity_by_stage_package' do
......@@ -457,42 +451,16 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
)
end
context 'with usage_data_instrumentation feature flag' do
context 'when enabled' do
it 'merges the data from instrumentation classes' do
stub_feature_flags(usage_data_instrumentation: true)
for_defined_days_back do
user = create(:user)
project = create(:project, creator: user)
create(:issue, project: project, author: user)
create(:issue, project: project, author: User.support_bot)
end
expect(described_class.usage_activity_by_stage_plan({})).to include(issues: Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
expect(described_class.usage_activity_by_stage_plan(described_class.monthly_time_range_db_params)).to include(issues: Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
uncached_data = described_class.uncached_data
expect(uncached_data[:usage_activity_by_stage][:plan]).to include(issues: 3)
expect(uncached_data[:usage_activity_by_stage_monthly][:plan]).to include(issues: 2)
end
it 'does not merge the data from instrumentation classes' do
for_defined_days_back do
user = create(:user)
project = create(:project, creator: user)
create(:issue, project: project, author: user)
create(:issue, project: project, author: User.support_bot)
end
context 'when disabled' do
it 'does not merge the data from instrumentation classes' do
stub_feature_flags(usage_data_instrumentation: false)
for_defined_days_back do
user = create(:user)
project = create(:project, creator: user)
create(:issue, project: project, author: user)
create(:issue, project: project, author: User.support_bot)
end
expect(described_class.usage_activity_by_stage_plan({})).to include(issues: 3)
expect(described_class.usage_activity_by_stage_plan(described_class.monthly_time_range_db_params)).to include(issues: 2)
end
end
expect(described_class.usage_activity_by_stage_plan({})).to include(issues: 3)
expect(described_class.usage_activity_by_stage_plan(described_class.monthly_time_range_db_params)).to include(issues: 2)
end
end
......@@ -510,53 +478,17 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
deployments: 2,
failed_deployments: 2,
releases: 2,
successful_deployments: 2
successful_deployments: 2,
releases_with_milestones: 2
)
expect(described_class.usage_activity_by_stage_release(described_class.monthly_time_range_db_params)).to include(
deployments: 1,
failed_deployments: 1,
releases: 1,
successful_deployments: 1
successful_deployments: 1,
releases_with_milestones: 1
)
end
context 'with usage_data_instrumentation feature flag' do
before do
for_defined_days_back do
user = create(:user)
create(:deployment, :failed, user: user)
release = create(:release, author: user)
create(:milestone, project: release.project, releases: [release])
create(:deployment, :success, user: user)
end
end
context 'when enabled' do
before do
stub_feature_flags(usage_data_instrumentation: true)
end
it 'merges data from instrumentation classes' do
expect(described_class.usage_activity_by_stage_release({})).to include(releases_with_milestones: Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
expect(described_class.usage_activity_by_stage_release(described_class.monthly_time_range_db_params)).to include(releases_with_milestones: Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
uncached_data = described_class.uncached_data
expect(uncached_data[:usage_activity_by_stage][:release]).to include(releases_with_milestones: 2)
expect(uncached_data[:usage_activity_by_stage_monthly][:release]).to include(releases_with_milestones: 1)
end
end
context 'when disabled' do
before do
stub_feature_flags(usage_data_instrumentation: false)
end
it 'does not merge data from instrumentation classes' do
expect(described_class.usage_activity_by_stage_release({})).to include(releases_with_milestones: 2)
expect(described_class.usage_activity_by_stage_release(described_class.monthly_time_range_db_params)).to include(releases_with_milestones: 1)
end
end
end
end
describe 'usage_activity_by_stage_verify' do
......@@ -605,16 +537,15 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
subject { described_class.data }
it 'gathers usage data' do
expect(subject.keys).to include(*UsageDataHelpers::USAGE_DATA_KEYS.map(&:to_s))
expect(subject.keys).to include(*UsageDataHelpers::USAGE_DATA_KEYS)
end
it 'gathers usage counts', :aggregate_failures do
count_data = subject[:counts]
expect(count_data[:boards]).to eq(1)
expect(count_data[:projects]).to eq(4)
count_keys = UsageDataHelpers::COUNTS_KEYS.map(&:to_s)
expect(count_data.keys).to include(*count_keys)
expect(count_keys - count_data.keys).to be_empty
expect(count_data.keys).to include(*UsageDataHelpers::COUNTS_KEYS)
expect(UsageDataHelpers::COUNTS_KEYS - count_data.keys).to be_empty
expect(count_data.values).to all(be_a_kind_of(Integer))
end
......@@ -699,7 +630,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
external_diffs: { enabled: false },
lfs: { enabled: true, object_store: { enabled: false, direct_upload: true, background_upload: false, provider: "AWS" } },
uploads: { enabled: nil, object_store: { enabled: false, direct_upload: true, background_upload: false, provider: "AWS" } },
packages: { enabled: true, object_store: { enabled: false, direct_upload: false, background_upload: true, provider: "AWS" } } }.with_indifferent_access
packages: { enabled: true, object_store: { enabled: false, direct_upload: false, background_upload: true, provider: "AWS" } } }
)
end
......@@ -873,37 +804,12 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
subject { described_class.license_usage_data }
it 'gathers license data' do
expect(subject[:uuid]).to eq(Gitlab::CurrentSettings.uuid)
expect(subject[:version]).to eq(Gitlab::VERSION)
expect(subject[:installation_type]).to eq('gitlab-development-kit')
expect(subject[:active_user_count]).to eq(User.active.size)
expect(subject[:recorded_at]).to be_a(Time)
end
context 'with usage_data_instrumentation feature flag' do
context 'when enabled' do
it 'merges uuid and hostname data from instrumentation classes' do
stub_feature_flags(usage_data_instrumentation: true)
expect(subject[:uuid]).to eq(Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
expect(subject[:hostname]).to eq(Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
expect(subject[:active_user_count]).to eq(Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
uncached_data = described_class.data
expect(uncached_data[:uuid]).to eq(Gitlab::CurrentSettings.uuid)
expect(uncached_data[:hostname]).to eq(Gitlab.config.gitlab.host)
expect(uncached_data[:active_user_count]).to eq(User.active.size)
end
end
context 'when disabled' do
it 'does not merge uuid and hostname data from instrumentation classes' do
stub_feature_flags(usage_data_instrumentation: false)
expect(subject[:uuid]).to eq(Gitlab::CurrentSettings.uuid)
expect(subject[:hostname]).to eq(Gitlab.config.gitlab.host)
expect(subject[:active_user_count]).to eq(User.active.size)
end
end
end
end
context 'when not relying on database records' do
......@@ -1166,46 +1072,18 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
expect(subject[:settings][:gitaly_apdex]).to be_within(0.001).of(0.95)
end
context 'with usage_data_instrumentation feature flag' do
context 'when enabled' do
before do
stub_feature_flags(usage_data_instrumentation: true)
end
it 'reports collected data categories' do
expected_value = %w[standard subscription operational optional]
allow_next_instance_of(ServicePing::PermitDataCategoriesService) do |instance|
expect(instance).to receive(:execute).and_return(expected_value)
end
expect(described_class.data[:settings][:collected_data_categories]).to eq(expected_value)
end
it 'reports collected data categories' do
expected_value = %w[standard subscription operational optional]
it 'gathers service_ping_features_enabled' do
expect(described_class.data[:settings][:service_ping_features_enabled]).to eq(Gitlab::CurrentSettings.usage_ping_features_enabled)
end
allow_next_instance_of(ServicePing::PermitDataCategoriesService) do |instance|
expect(instance).to receive(:execute).and_return(expected_value)
end
context 'when disabled' do
before do
stub_feature_flags(usage_data_instrumentation: false)
end
it 'reports collected data categories' do
expected_value = %w[standard subscription operational optional]
allow_next_instance_of(ServicePing::PermitDataCategoriesService) do |instance|
expect(instance).to receive(:execute).and_return(expected_value)
end
expect(subject[:settings][:collected_data_categories]).to eq(expected_value)
end
expect(subject[:settings][:collected_data_categories]).to eq(expected_value)
end
it 'gathers service_ping_features_enabled' do
expect(subject[:settings][:service_ping_features_enabled]).to eq(Gitlab::CurrentSettings.usage_ping_features_enabled)
end
end
it 'gathers service_ping_features_enabled' do
expect(subject[:settings][:service_ping_features_enabled]).to eq(Gitlab::CurrentSettings.usage_ping_features_enabled)
end
it 'gathers user_cap_feature_enabled' do
......
spec/lib/gitlab/utils/usage_data_spec.rb
View file @ ea142f1a
......@@ -8,26 +8,8 @@ RSpec.describe Gitlab::Utils::UsageData do
describe '#add_metric' do
let(:metric) { 'UuidMetric'}
context 'with usage_data_instrumentation feature flag' do
context 'when enabled' do
before do
stub_feature_flags(usage_data_instrumentation: true)
end
it 'returns -100 value to be overriden' do
expect(described_class.add_metric(metric)).to eq(-100)
end
end
context 'when disabled' do
before do
stub_feature_flags(usage_data_instrumentation: false)
end
it 'computes the metric value for given metric' do
expect(described_class.add_metric(metric)).to eq(Gitlab::CurrentSettings.uuid)
end
end
it 'computes the metric value for given metric' do
expect(described_class.add_metric(metric)).to eq(Gitlab::CurrentSettings.uuid)
end
end
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7