Commit 7f8a6e6a authored by Marius Bobin's avatar Marius Bobin

Implement reviewer feedback

Implement reviewer feedback
parent 420f42b0
...@@ -11,7 +11,7 @@ module Projects ...@@ -11,7 +11,7 @@ module Projects
def create def create
result = ::Ci::PrometheusMetrics::ObserveHistogramsService.new(project, permitted_params).execute result = ::Ci::PrometheusMetrics::ObserveHistogramsService.new(project, permitted_params).execute
render json: result.body, status: result.status render json: result.payload, status: result.http_status
end end
private private
......
...@@ -3,14 +3,12 @@ ...@@ -3,14 +3,12 @@
module Ci module Ci
module PrometheusMetrics module PrometheusMetrics
class ObserveHistogramsService class ObserveHistogramsService
Result = Struct.new(:status, :body, keyword_init: true)
class << self class << self
def available_histograms def available_histograms
@available_histograms ||= [ @available_histograms ||= [
histogram(:pipeline_graph_link_calculation_duration_seconds, 'Total time spent calculating links, in seconds', {}, [0.05, 0.1, 0.2, 0.3, 0.4, 0.5, 0.8, 1, 2]), histogram(:pipeline_graph_link_calculation_duration_seconds, 'Total time spent calculating links, in seconds', {}, [0.05, 0.1, 0.2, 0.3, 0.4, 0.5, 0.8, 1, 2]),
histogram(:pipeline_graph_links_total, 'Number of links per graph', {}, [1, 5, 10, 25, 50, 100, 200]), histogram(:pipeline_graph_links_total, 'Number of links per graph', {}, [1, 5, 10, 25, 50, 100, 200]),
histogram(:pipeline_graph_link_per_job_ratio, 'Ratio of links to job per graph', {}, [0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1]) histogram(:pipeline_graph_links_per_job_ratio, 'Ratio of links to job per graph', {}, [0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1])
].to_h ].to_h
end end
...@@ -27,13 +25,13 @@ module Ci ...@@ -27,13 +25,13 @@ module Ci
end end
def execute def execute
return Result.new(status: 202, body: {}) unless enabled? return ServiceResponse.success(http_status: :accepted) unless enabled?
params params
.fetch(:histograms, []) .fetch(:histograms, [])
.each(&method(:observe)) .each(&method(:observe))
Result.new(status: 201, body: {}) ServiceResponse.success(http_status: :created)
end end
private private
......
...@@ -121,7 +121,7 @@ The following metrics are available: ...@@ -121,7 +121,7 @@ The following metrics are available:
| `ci_report_parser_duration_seconds` | Histogram | 13.9 | Time to parse CI/CD report artifacts | `parser` | | `ci_report_parser_duration_seconds` | Histogram | 13.9 | Time to parse CI/CD report artifacts | `parser` |
| `pipeline_graph_link_calculation_duration_seconds` | Histogram | 13.9 | Total time spent calculating links, in seconds | `project` | | `pipeline_graph_link_calculation_duration_seconds` | Histogram | 13.9 | Total time spent calculating links, in seconds | `project` |
| `pipeline_graph_links_total` | Histogram | 13.9 | Number of links per graph | `project` | | `pipeline_graph_links_total` | Histogram | 13.9 | Number of links per graph | `project` |
| `pipeline_graph_link_per_job_ratio` | Histogram | 13.9 | Ratio of links to job per graph | `project` | | `pipeline_graph_links_per_job_ratio` | Histogram | 13.9 | Ratio of links to job per graph | `project` |
## Metrics controlled by a feature flag ## Metrics controlled by a feature flag
......
...@@ -24,6 +24,20 @@ RSpec.describe 'Projects::Ci::PrometheusMetrics::HistogramsController' do ...@@ -24,6 +24,20 @@ RSpec.describe 'Projects::Ci::PrometheusMetrics::HistogramsController' do
expect(response).to have_gitlab_http_status(:not_found) expect(response).to have_gitlab_http_status(:not_found)
end end
end end
context 'with the feature flag disabled' do
before do
stub_feature_flags(ci_accept_frontend_prometheus_metrics: false)
end
it 'returns 202 Accepted' do
post histograms_route(histograms: [
{ name: :pipeline_graph_link_calculation_duration_seconds, value: 1 }
])
expect(response).to have_gitlab_http_status(:accepted)
end
end
end end
def histograms_route(params = {}) def histograms_route(params = {})
......
...@@ -14,7 +14,7 @@ RSpec.describe Ci::PrometheusMetrics::ObserveHistogramsService do ...@@ -14,7 +14,7 @@ RSpec.describe Ci::PrometheusMetrics::ObserveHistogramsService do
context 'with empty data' do context 'with empty data' do
it 'does not raise errors' do it 'does not raise errors' do
expect(subject.status).to eq(201) is_expected.to be_success
end end
end end
...@@ -23,7 +23,7 @@ RSpec.describe Ci::PrometheusMetrics::ObserveHistogramsService do ...@@ -23,7 +23,7 @@ RSpec.describe Ci::PrometheusMetrics::ObserveHistogramsService do
{ {
histograms: [ histograms: [
{ name: 'pipeline_graph_link_calculation_duration_seconds', value: '1' }, { name: 'pipeline_graph_link_calculation_duration_seconds', value: '1' },
{ name: 'pipeline_graph_link_per_job_ratio', value: '0.9' } { name: 'pipeline_graph_links_per_job_ratio', value: '0.9' }
] ]
} }
end end
...@@ -33,13 +33,14 @@ RSpec.describe Ci::PrometheusMetrics::ObserveHistogramsService do ...@@ -33,13 +33,14 @@ RSpec.describe Ci::PrometheusMetrics::ObserveHistogramsService do
expect(histogram_data).to match(a_hash_including({ 0.8 => 0.0, 1 => 1.0, 2 => 1.0 })) expect(histogram_data).to match(a_hash_including({ 0.8 => 0.0, 1 => 1.0, 2 => 1.0 }))
expect(histogram_data(:pipeline_graph_link_per_job_ratio)) expect(histogram_data(:pipeline_graph_links_per_job_ratio))
.to match(a_hash_including({ 0.8 => 0.0, 0.9 => 1.0, 1 => 1.0 })) .to match(a_hash_including({ 0.8 => 0.0, 0.9 => 1.0, 1 => 1.0 }))
end end
it 'returns an empty body and status code' do it 'returns an empty body and status code' do
expect(subject.status).to eq(201) is_expected.to be_success
expect(subject.body).to eq({}) expect(subject.http_status).to eq(:created)
expect(subject.payload).to eq({})
end end
end end
...@@ -73,8 +74,9 @@ RSpec.describe Ci::PrometheusMetrics::ObserveHistogramsService do ...@@ -73,8 +74,9 @@ RSpec.describe Ci::PrometheusMetrics::ObserveHistogramsService do
end end
it 'returns an empty body and status code' do it 'returns an empty body and status code' do
expect(subject.status).to eq(202) is_expected.to be_success
expect(subject.body).to eq({}) expect(subject.http_status).to eq(:accepted)
expect(subject.payload).to eq({})
end end
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