diff --git a/app/services/ci/collect_pipeline_analytics_service.rb b/app/services/ci/collect_pipeline_analytics_service.rb
index 3508df9df685b636b4f798a4e66b32c11189153c..bb9d4d2a9abc053884e25cf49b189f5bdc8700b3 100644
--- a/app/services/ci/collect_pipeline_analytics_service.rb
+++ b/app/services/ci/collect_pipeline_analytics_service.rb
@@ -2,8 +2,6 @@
module Ci
class CollectPipelineAnalyticsService
- TIME_BUCKETS_LIMIT = 1.week.in_hours + 1 # +1 to add some error margin
-
STATUS_GROUP_TO_STATUSES = { success: %w[success], failed: %w[failed], other: %w[canceled skipped] }.freeze
STATUS_GROUPS = STATUS_GROUP_TO_STATUSES.keys.freeze
STATUS_TO_STATUS_GROUP = STATUS_GROUP_TO_STATUSES.flat_map { |k, v| v.product([k]) }.to_h
@@ -25,9 +23,8 @@ def execute
return ServiceResponse.error(message: 'Not allowed') unless allowed?
- if (@to_time - @from_time) / 1.hour > TIME_BUCKETS_LIMIT
- return ServiceResponse.error(message: "Maximum of #{TIME_BUCKETS_LIMIT} 1-hour intervals can be requested")
- end
+ error_message = clickhouse_model.validate_time_window(@from_time, @to_time)
+ return ServiceResponse.error(message: error_message) if error_message
ServiceResponse.success(payload: { aggregate: calculate_aggregate })
end
@@ -39,7 +36,11 @@ def allowed?
end
def clickhouse_model
- ::ClickHouse::Models::Ci::FinishedPipelinesHourly
+ if ::ClickHouse::Models::Ci::FinishedPipelinesHourly.time_window_valid?(@from_time, @to_time)
+ return ::ClickHouse::Models::Ci::FinishedPipelinesHourly
+ end
+
+ ::ClickHouse::Models::Ci::FinishedPipelinesDaily
end
def calculate_aggregate
diff --git a/lib/click_house/models/ci/finished_pipelines_base.rb b/lib/click_house/models/ci/finished_pipelines_base.rb
index e457851391c8423b5d483c6fff7e517f1d5242b9..8b1c7930ae5ebfa66e5900f7fde820f53ab36d59 100644
--- a/lib/click_house/models/ci/finished_pipelines_base.rb
+++ b/lib/click_house/models/ci/finished_pipelines_base.rb
@@ -4,6 +4,14 @@ module ClickHouse # rubocop:disable Gitlab/BoundedContexts -- Existing module
module Models
module Ci
class FinishedPipelinesBase < ClickHouse::Models::BaseModel
+ def self.time_window_valid?(from_time, to_time)
+ raise NotImplementedError, "subclasses of #{self.class.name} must implement #{__method__}"
+ end
+
+ def self.validate_time_window(from_time, to_time)
+ raise NotImplementedError, "subclasses of #{self.class.name} must implement #{__method__}"
+ end
+
def self.for_project(project)
new.for_project(project)
end
diff --git a/lib/click_house/models/ci/finished_pipelines_daily.rb b/lib/click_house/models/ci/finished_pipelines_daily.rb
index 0d761be1a365c4d819b00fcf22d908dc8de421e3..1d903e245f19cc50921fe588572786a2821c65dd 100644
--- a/lib/click_house/models/ci/finished_pipelines_daily.rb
+++ b/lib/click_house/models/ci/finished_pipelines_daily.rb
@@ -4,9 +4,21 @@ module ClickHouse # rubocop:disable Gitlab/BoundedContexts -- Existing module
module Models
module Ci
class FinishedPipelinesDaily < FinishedPipelinesBase
+ TIME_BUCKETS_LIMIT = 366
+
def self.table_name
'ci_finished_pipelines_daily'
end
+
+ def self.time_window_valid?(from_time, to_time)
+ (to_time - from_time) / 1.day < TIME_BUCKETS_LIMIT
+ end
+
+ def self.validate_time_window(from_time, to_time)
+ return if time_window_valid?(from_time, to_time)
+
+ "Maximum of #{TIME_BUCKETS_LIMIT} days can be requested"
+ end
end
end
end
diff --git a/lib/click_house/models/ci/finished_pipelines_hourly.rb b/lib/click_house/models/ci/finished_pipelines_hourly.rb
index 16686a6d689eea95ffc6f3a4118c75128bd54c79..88dccbe1e3d4d2007947636488af9f237669749e 100644
--- a/lib/click_house/models/ci/finished_pipelines_hourly.rb
+++ b/lib/click_house/models/ci/finished_pipelines_hourly.rb
@@ -4,9 +4,21 @@ module ClickHouse # rubocop:disable Gitlab/BoundedContexts -- Existing module
module Models
module Ci
class FinishedPipelinesHourly < FinishedPipelinesBase
+ TIME_BUCKETS_LIMIT = 1.week.in_hours.to_i + 1 # +1 to add some error margin
+
def self.table_name
'ci_finished_pipelines_hourly'
end
+
+ def self.time_window_valid?(from_time, to_time)
+ (to_time - from_time) / 1.hour < TIME_BUCKETS_LIMIT
+ end
+
+ def self.validate_time_window(from_time, to_time)
+ return if time_window_valid?(from_time, to_time)
+
+ "Maximum of #{TIME_BUCKETS_LIMIT} hours can be requested"
+ end
end
end
end
diff --git a/spec/lib/click_house/models/ci/finished_pipelines_daily_spec.rb b/spec/lib/click_house/models/ci/finished_pipelines_daily_spec.rb
index acce86099c16c7d89f4e249b7784b12cca6687f9..ce1bcb234b381bf709eb14362ebc084460d7e5f8 100644
--- a/spec/lib/click_house/models/ci/finished_pipelines_daily_spec.rb
+++ b/spec/lib/click_house/models/ci/finished_pipelines_daily_spec.rb
@@ -4,4 +4,40 @@
RSpec.describe ClickHouse::Models::Ci::FinishedPipelinesDaily, feature_category: :fleet_visibility do
it_behaves_like 'a ci_finished_pipelines aggregation model', :ci_finished_pipelines_daily
+
+ describe '.time_window_valid?', :freeze_time do
+ subject(:time_window_valid?) { described_class.time_window_valid?(from_time, to_time) }
+
+ context 'with time window of less than 366 days' do
+ let(:from_time) { 1.second.after(366.days.ago) }
+ let(:to_time) { Time.current }
+
+ it { is_expected.to eq true }
+ end
+
+ context 'with time window of 366 days' do
+ let(:from_time) { 366.days.ago }
+ let(:to_time) { Time.current }
+
+ it { is_expected.to eq false }
+ end
+ end
+
+ describe '.validate_time_window', :freeze_time do
+ subject(:validate_time_window) { described_class.validate_time_window(from_time, to_time) }
+
+ context 'with time window of less than 366 days' do
+ let(:from_time) { 1.second.after(366.days.ago) }
+ let(:to_time) { Time.current }
+
+ it { is_expected.to be_nil }
+ end
+
+ context 'with time window of 366 days' do
+ let(:from_time) { 366.days.ago }
+ let(:to_time) { Time.current }
+
+ it { is_expected.to eq("Maximum of 366 days can be requested") }
+ end
+ end
end
diff --git a/spec/lib/click_house/models/ci/finished_pipelines_hourly_spec.rb b/spec/lib/click_house/models/ci/finished_pipelines_hourly_spec.rb
index 2bb09b7e27a135665a983ca329a9858ee826e2c2..cb7b8ba0b5fa9e14648f75e49e11083e7e912e7c 100644
--- a/spec/lib/click_house/models/ci/finished_pipelines_hourly_spec.rb
+++ b/spec/lib/click_house/models/ci/finished_pipelines_hourly_spec.rb
@@ -4,4 +4,40 @@
RSpec.describe ClickHouse::Models::Ci::FinishedPipelinesHourly, feature_category: :fleet_visibility do
it_behaves_like 'a ci_finished_pipelines aggregation model', :ci_finished_pipelines_hourly
+
+ describe '.time_window_valid?', :freeze_time do
+ subject(:time_window_valid?) { described_class.time_window_valid?(from_time, to_time) }
+
+ context 'with time window of one week and one hour' do
+ let(:from_time) { (1.hour - 1.second).before(1.week.ago) }
+ let(:to_time) { Time.current }
+
+ it { is_expected.to eq true }
+ end
+
+ context 'with time window of one week and 1 hour' do
+ let(:from_time) { 1.hour.before(1.week.ago) }
+ let(:to_time) { Time.current }
+
+ it { is_expected.to eq false }
+ end
+ end
+
+ describe '.validate_time_window', :freeze_time do
+ subject(:validate_time_window) { described_class.validate_time_window(from_time, to_time) }
+
+ context 'with time window of one week and one hour' do
+ let(:from_time) { (1.hour - 1.second).before(1.week.ago) }
+ let(:to_time) { Time.current }
+
+ it { is_expected.to be_nil }
+ end
+
+ context 'with time window of one week and 1 hour' do
+ let(:from_time) { 1.hour.before(1.week.ago) }
+ let(:to_time) { Time.current }
+
+ it { is_expected.to eq("Maximum of 169 hours can be requested") }
+ end
+ end
end
diff --git a/spec/requests/api/graphql/project/project_pipeline_statistics_spec.rb b/spec/requests/api/graphql/project/project_pipeline_statistics_spec.rb
index d08d54e6403dad081cb9c0fd459ae0d5ebbd84c8..987b195b7ec45e78a4a0d91e1f6a79d2f9a02d01 100644
--- a/spec/requests/api/graphql/project/project_pipeline_statistics_spec.rb
+++ b/spec/requests/api/graphql/project/project_pipeline_statistics_spec.rb
@@ -72,7 +72,7 @@
end
end
- describe 'aggregate' do
+ describe 'aggregate', :aggregate_failures do
subject(:aggregate) do
perform_request
@@ -86,6 +86,9 @@
end
it "contains expected data for the last week" do
+ perform_request
+
+ expect_graphql_errors_to_be_empty
expect(aggregate).to eq({
'label' => nil,
'all' => '0',
@@ -97,6 +100,9 @@
context 'when there are pipelines in last week', time_travel_to: Time.utc(2024, 5, 11) do
it "contains expected data for the last week" do
+ perform_request
+
+ expect_graphql_errors_to_be_empty
expect(aggregate).to eq({
'label' => nil,
'all' => '4',
@@ -112,6 +118,9 @@
let(:to_time) { '2024-05-11T00:00:00+00:00' }
it "contains expected data for the period" do
+ perform_request
+
+ expect_graphql_errors_to_be_empty
expect(aggregate).to eq({
'label' => nil,
'all' => '2',
@@ -120,6 +129,35 @@
'other' => '0'
})
end
+
+ context 'with time window spanning less than 1 year' do
+ let(:from_time) { '2023-05-11T00:00:00+00:00' }
+ let(:to_time) { '2024-05-10T23:59:59+00:00' }
+
+ it "contains expected data for the period" do
+ perform_request
+
+ expect_graphql_errors_to_be_empty
+ expect(aggregate).to eq({
+ 'label' => nil,
+ 'all' => '5',
+ 'success' => '1',
+ 'failed' => '2',
+ 'other' => '1'
+ })
+ end
+ end
+
+ context 'with time window spanning 1 year' do
+ let(:from_time) { '2024-01-01T00:00:00+02:00' }
+ let(:to_time) { '2025-01-01T00:00:00+02:00' }
+
+ it "contains expected data for the period" do
+ perform_request
+
+ expect_graphql_errors_to_include("Maximum of 366 days can be requested")
+ end
+ end
end
end
diff --git a/spec/services/ci/collect_pipeline_analytics_service_spec.rb b/spec/services/ci/collect_pipeline_analytics_service_spec.rb
index fbf56774258eeaf2f3ce9352912c8a288d6bcd71..08d2dfbda23874e49fbd468e505cf6f774662cec 100644
--- a/spec/services/ci/collect_pipeline_analytics_service_spec.rb
+++ b/spec/services/ci/collect_pipeline_analytics_service_spec.rb
@@ -105,6 +105,7 @@
context 'when requesting statistics starting one second before beginning of week' do
let(:from_time) { 1.second.before(starting_time) }
+ let(:to_time) { 1.second.before(ending_time) }
it 'does not include job starting 1 second before start of week' do
expect(result.errors).to eq([])
@@ -115,6 +116,7 @@
context 'when requesting statistics starting one hour before beginning of week' do
let(:from_time) { 1.hour.before(starting_time) }
+ let(:to_time) { 1.second.before(ending_time) }
it 'includes job starting 1 second before start of week' do
expect(result.errors).to eq([])
@@ -123,12 +125,11 @@
end
end
- context 'when requesting hourly statistics that span more than one week' do
- let(:from_time) { (1.hour + 1.second).before(starting_time) }
+ context 'when requesting statistics that span more than one year' do
+ let(:from_time) { (366.days + 1.second).before(starting_time) }
it 'returns an error' do
- expect(result.errors).to contain_exactly(
- "Maximum of #{described_class::TIME_BUCKETS_LIMIT} 1-hour intervals can be requested")
+ expect(result.errors).to contain_exactly("Maximum of 366 days can be requested")
expect(result.error?).to eq(true)
end
end