Commit ab1e1b55 authored by syasonik's avatar syasonik

Specify time window for additional metrics api

Adds support for start and end parameters in the #additional_metrics
endpoint of the EnvironmentsController. start and end are meant to be
unix timestamps, per the Prometheus API (as the consumer of this
endpoint will eventually be transitioned to a prometheus endpoint).
This functionality is behind the :metrics_time_window feature flag
for development.
parent 47807774
......@@ -10,6 +10,9 @@ class Projects::EnvironmentsController < Projects::ApplicationController
before_action :environment, only: [:show, :edit, :update, :stop, :terminal, :terminal_websocket_authorize, :metrics]
before_action :verify_api_request!, only: :terminal_websocket_authorize
before_action :expire_etag_cache, only: [:index]
before_action only: [:metrics, :additional_metrics] do
push_frontend_feature_flag(:metrics_time_window)
end
def index
@environments = project.environments
......@@ -146,7 +149,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController
def additional_metrics
respond_to do |format|
format.json do
additional_metrics = environment.additional_metrics || {}
additional_metrics = environment.additional_metrics(*metrics_params) || {}
render json: additional_metrics, status: additional_metrics.any? ? :ok : :no_content
end
......@@ -186,6 +189,13 @@ class Projects::EnvironmentsController < Projects::ApplicationController
@environment ||= project.environments.find(params[:id])
end
def metrics_params
return unless Feature.enabled?(:metrics_time_window, project)
return unless params[:start].present? || params[:end].present?
params.require([:start, :end]).values_at(:start, :end)
end
def search_environment_names
return [] unless params[:query]
......
......@@ -51,7 +51,7 @@ module PrometheusAdapter
end
def build_query_args(*args)
args.map(&:id)
args.map { |arg| arg.respond_to?(:id) ? arg.id : arg }
end
end
end
......@@ -170,8 +170,10 @@ class Environment < ApplicationRecord
prometheus_adapter.query(:environment, self) if has_metrics?
end
def additional_metrics
prometheus_adapter.query(:additional_metrics_environment, self) if has_metrics?
def additional_metrics(*args)
return unless has_metrics?
prometheus_adapter.query(:additional_metrics_environment, self, *args.map(&:to_f))
end
# rubocop: disable CodeReuse/ServiceClass
......
......@@ -7,12 +7,16 @@ module Gitlab
include QueryAdditionalMetrics
# rubocop: disable CodeReuse/ActiveRecord
def query(environment_id)
def query(environment_id, timeframe_start = 8.hours.ago, timeframe_end = Time.now)
::Environment.find_by(id: environment_id).try do |environment|
query_metrics(
environment.project,
environment,
common_query_context(environment, timeframe_start: 8.hours.ago.to_f, timeframe_end: Time.now.to_f)
common_query_context(
environment,
timeframe_start: timeframe_start.to_f,
timeframe_end: timeframe_end.to_f
)
)
end
end
......
......@@ -392,7 +392,7 @@ describe Projects::EnvironmentsController do
context 'when requesting metrics as JSON' do
it 'returns a metrics JSON document' do
get :additional_metrics, params: environment_params(format: :json)
additional_metrics
expect(response).to have_gitlab_http_status(204)
expect(json_response).to eq({})
......@@ -412,7 +412,7 @@ describe Projects::EnvironmentsController do
end
it 'returns a metrics JSON document' do
get :additional_metrics, params: environment_params(format: :json)
additional_metrics
expect(response).to be_ok
expect(json_response['success']).to be(true)
......@@ -420,6 +420,32 @@ describe Projects::EnvironmentsController do
expect(json_response['last_update']).to eq(42)
end
end
context 'when only one time param is provided' do
context 'when :metrics_time_window feature flag is disabled' do
before do
stub_feature_flags(metrics_time_window: false)
expect(environment).to receive(:additional_metrics).with(no_args).and_return(nil)
end
it 'returns a time-window agnostic response' do
additional_metrics(start: '1552647300.651094')
expect(response).to have_gitlab_http_status(204)
expect(json_response).to eq({})
end
end
it 'raises an error when start is missing' do
expect { additional_metrics(start: '1552647300.651094') }
.to raise_error(ActionController::ParameterMissing)
end
it 'raises an error when end is missing' do
expect { additional_metrics(start: '1552647300.651094') }
.to raise_error(ActionController::ParameterMissing)
end
end
end
describe 'GET #search' do
......@@ -500,4 +526,8 @@ describe Projects::EnvironmentsController do
project_id: project,
id: environment.id)
end
def additional_metrics(opts = {})
get :additional_metrics, params: environment_params(format: :json, **opts)
end
end
......@@ -9,9 +9,35 @@ describe Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery do
let(:query_params) { [environment.id] }
it 'queries using specific time' do
expect(client).to receive(:query_range).with(anything, start: 8.hours.ago.to_f, stop: Time.now.to_f)
expect(client).to receive(:query_range)
.with(anything, start: 8.hours.ago.to_f, stop: Time.now.to_f)
expect(query_result).not_to be_nil
end
context 'when start and end time parameters are provided' do
let(:query_params) { [environment.id, start_time, end_time] }
context 'as unix timestamps' do
let(:start_time) { 4.hours.ago.to_f }
let(:end_time) { 2.hours.ago.to_f }
it 'queries using the provided times' do
expect(client).to receive(:query_range)
.with(anything, start: start_time, stop: end_time)
expect(query_result).not_to be_nil
end
end
context 'as Date/Time objects' do
let(:start_time) { 4.hours.ago }
let(:end_time) { 2.hours.ago }
it 'queries using the provided times converted to unix' do
expect(client).to receive(:query_range)
.with(anything, start: start_time.to_f, stop: end_time.to_f)
expect(query_result).not_to be_nil
end
end
end
end
end
......@@ -77,6 +77,28 @@ describe PrometheusAdapter, :use_clean_rails_memory_store_caching do
end
end
end
describe 'additional_metrics' do
let(:additional_metrics_environment_query) { Gitlab::Prometheus::Queries::AdditionalMetricsEnvironmentQuery }
let(:environment) { build_stubbed(:environment, slug: 'env-slug') }
let(:time_window) { [1552642245.067, 1552642095.831] }
around do |example|
Timecop.freeze { example.run }
end
context 'with valid data' do
subject { service.query(:additional_metrics_environment, environment, *time_window) }
before do
stub_reactive_cache(service, prometheus_data, additional_metrics_environment_query, environment.id, *time_window)
end
it 'returns reactive data' do
expect(subject).to eq(prometheus_data)
end
end
end
end
describe '#calculate_reactive_cache' do
......@@ -121,4 +143,24 @@ describe PrometheusAdapter, :use_clean_rails_memory_store_caching do
end
end
end
describe '#build_query_args' do
subject { service.build_query_args(*args) }
context 'when active record models are included' do
let(:args) { [double(:environment, id: 12)] }
it 'serializes by id' do
is_expected.to eq [12]
end
end
context 'when args are safe for serialization' do
let(:args) { ['stringy arg', 5, 6.0, :symbolic_arg] }
it 'does nothing' do
is_expected.to eq args
end
end
end
end
......@@ -687,7 +687,8 @@ describe Environment do
describe '#additional_metrics' do
let(:project) { create(:prometheus_project) }
subject { environment.additional_metrics }
let(:metric_params) { [] }
subject { environment.additional_metrics(*metric_params) }
context 'when the environment has additional metrics' do
before do
......@@ -695,12 +696,26 @@ describe Environment do
end
it 'returns the additional metrics from the deployment service' do
expect(environment.prometheus_adapter).to receive(:query)
.with(:additional_metrics_environment, environment)
.and_return(:fake_metrics)
expect(environment.prometheus_adapter)
.to receive(:query)
.with(:additional_metrics_environment, environment)
.and_return(:fake_metrics)
is_expected.to eq(:fake_metrics)
end
context 'when time window arguments are provided' do
let(:metric_params) { [1552642245.067, Time.now] }
it 'queries with the expected parameters' do
expect(environment.prometheus_adapter)
.to receive(:query)
.with(:additional_metrics_environment, environment, *metric_params.map(&:to_f))
.and_return(:fake_metrics)
is_expected.to eq(:fake_metrics)
end
end
end
context 'when the environment does not have metrics' 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