Commit a7e12052 authored by Pawel Chojnacki's avatar Pawel Chojnacki

Use explicit instance of prometheus service and add access methods to it

parent 969b8124
......@@ -23,7 +23,7 @@ class Projects::DeploymentsController < Projects::ApplicationController
end
def additional_metrics
return render_404 unless deployment.has_additional_metrics?
return render_404 unless deployment.prometheus_service.present?
metrics = deployment.additional_metrics
......
......@@ -3,10 +3,10 @@ class Projects::PrometheusController < Projects::ApplicationController
before_action :require_prometheus_metrics!
def active_metrics
matched_metrics = prometheus_service.reactive_query(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself) || {}
respond_to do |format|
format.json do
matched_metrics = prometheus_service.matched_metrics || {}
if matched_metrics.any?
render json: matched_metrics
else
......@@ -18,19 +18,15 @@ class Projects::PrometheusController < Projects::ApplicationController
private
rescue_from(ActionController::UnknownFormat) do |e|
rescue_from(ActionController::UnknownFormat) do
render_404
end
def prometheus_service
project.monitoring_service
end
def has_prometheus_metrics?
prometheus_service&.respond_to?(:reactive_query)
@prometheus_service ||= project.monitoring_services.reorder(nil).find_by(active: true, type: PrometheusService.name)
end
def require_prometheus_metrics!
render_404 unless has_prometheus_metrics?
render_404 unless prometheus_service.present?
end
end
......@@ -103,10 +103,6 @@ class Deployment < ActiveRecord::Base
project.monitoring_service.present?
end
def has_additional_metrics?
has_metrics? && project.monitoring_service&.respond_to?(:reactive_query)
end
def metrics
return {} unless has_metrics?
......@@ -114,11 +110,16 @@ class Deployment < ActiveRecord::Base
end
def additional_metrics
return {} unless has_additional_metrics?
metrics = project.monitoring_service.reactive_query(Gitlab::Prometheus::Queries::AdditionalMetricsDeploymentQuery.name, id, &:itself)
return {} unless prometheus_service.present?
metrics = prometheus_service.additional_deployment_metrics(self)
metrics&.merge(deployment_time: created_at.to_i) || {}
end
def prometheus_service
@prometheus_service ||= project.monitoring_services.reorder(nil).find_by(active: true, type: PrometheusService.name)
end
private
def ref_path
......
......@@ -149,20 +149,24 @@ class Environment < ActiveRecord::Base
project.monitoring_service.present? && available? && last_deployment.present?
end
def has_additional_metrics?
has_metrics? && project.monitoring_service&.respond_to?(:reactive_query)
end
def metrics
project.monitoring_service.environment_metrics(self) if has_metrics?
end
def has_additional_metrics?
prometheus_service.present? && available? && last_deployment.present?
end
def additional_metrics
if has_additional_metrics?
project.monitoring_service.reactive_query(Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery.name, self.id, &:itself)
prometheus_service.additional_environment_metrics(self)
end
end
def prometheus_service
@prometheus_service ||= project.monitoring_services.reorder(nil).find_by(active: true, type: PrometheusService.name)
end
# An environment name is not necessarily suitable for use in URLs, DNS
# or other third-party contexts, so provide a slugified version. A slug has
# the following properties:
......
......@@ -62,8 +62,16 @@ class PrometheusService < MonitoringService
metrics&.merge(deployment_time: created_at.to_i) || {}
end
def reactive_query(query_class, *args, &block)
with_reactive_cache(query_class, *args, &block)
def additional_environment_metrics(environment)
with_reactive_cache(Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery.name, environment.id, &:itself)
end
def additional_deployment_metrics(deployment)
with_reactive_cache(Gitlab::Prometheus::Queries::AdditionalMetricsDeploymentQuery.name, deployment.id, &:itself)
end
def matched_metrics
additional_deployment_metrics(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name, &:itself)
end
# Cache metrics for specific environment
......
......@@ -108,6 +108,68 @@ describe Projects::DeploymentsController do
end
end
describe 'GET #additional_metrics' do
let(:deployment) { create(:deployment, project: project, environment: environment) }
before do
allow(controller).to receive(:deployment).and_return(deployment)
end
context 'when metrics are disabled' do
before do
allow(deployment).to receive(:has_metrics?).and_return false
end
it 'responds with not found' do
get :metrics, deployment_params(id: deployment.id)
expect(response).to be_not_found
end
end
context 'when metrics are enabled' do
let(:prometheus_service) { double('prometheus_service') }
before do
allow(deployment).to receive(:prometheus_service).and_return(prometheus_service)
end
context 'when environment has no metrics' do
before do
expect(deployment).to receive(:additional_metrics).and_return({})
end
it 'returns a empty response 204 response' do
get :additional_metrics, deployment_params(id: deployment.id)
expect(response).to have_http_status(204)
expect(response.body).to eq('')
end
end
context 'when environment has some metrics' do
let(:empty_metrics) do
{
success: true,
metrics: {},
last_update: 42
}
end
before do
expect(deployment).to receive(:additional_metrics).and_return(empty_metrics)
end
it 'returns a metrics JSON document' do
get :additional_metrics, deployment_params(id: deployment.id)
expect(response).to be_ok
expect(json_response['success']).to be(true)
expect(json_response['metrics']).to eq({})
expect(json_response['last_update']).to eq(42)
end
end
end
end
def deployment_params(opts = {})
opts.reverse_merge(namespace_id: project.namespace,
project_id: project,
......
......@@ -8,7 +8,7 @@ describe Projects::PrometheusController do
before do
allow(controller).to receive(:project).and_return(project)
allow(project).to receive(:monitoring_service).and_return(prometheus_service)
allow(controller).to receive(:prometheus_service).and_return(prometheus_service)
project.add_master(user)
sign_in(user)
......@@ -16,11 +16,11 @@ describe Projects::PrometheusController do
describe 'GET #active_metrics' do
context 'when prometheus metrics are enabled' do
before do
allow(prometheus_service).to receive(:reactive_query)
end
context 'when data is not present' do
before do
allow(prometheus_service).to receive(:matched_metrics).and_return({})
end
it 'returns no content response' do
get :active_metrics, project_params(format: :json)
......@@ -32,8 +32,7 @@ describe Projects::PrometheusController do
let(:sample_response) { { some_data: 1 } }
before do
allow(prometheus_service).to receive(:reactive_query).with(Gitlab::Prometheus::Queries::MatchedMetricsQuery.name)
.and_return(sample_response)
allow(prometheus_service).to receive(:matched_metrics).and_return(sample_response)
end
it 'returns no content response' do
......
......@@ -77,6 +77,35 @@ describe Deployment, models: true do
end
end
describe '#additional_metrics' do
let(:deployment) { create(:deployment) }
subject { deployment.additional_metrics }
context 'metrics are disabled' do
it { is_expected.to eq({}) }
end
context 'metrics are enabled' do
let(:simple_metrics) do
{
success: true,
metrics: {},
last_update: 42
}
end
let(:prometheus_service) { double('prometheus_service') }
before do
allow(deployment).to receive(:prometheus_service).and_return(prometheus_service)
allow(prometheus_service).to receive(:additional_deployment_metrics).and_return(simple_metrics)
end
it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) }
end
end
describe '#stop_action' do
let(:build) { create(:ci_build) }
......
......@@ -452,8 +452,8 @@ describe Environment, models: true do
end
it 'returns the additional metrics from the deployment service' do
expect(project.monitoring_service).to receive(:reactive_query)
.with(Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery.name, environment.id)
expect(environment.prometheus_service).to receive(:additional_environment_metrics)
.with(environment)
.and_return(:fake_metrics)
is_expected.to eq(:fake_metrics)
......@@ -470,7 +470,11 @@ describe Environment, models: true do
end
describe '#has_additional_metrics??' do
subject { environment.has_metrics? }
subject { environment.has_additional_metrics? }
before do
end
context 'when the enviroment is available' do
context 'with a deployment service' do
......
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