Commit c5d7d316 authored by syasonik's avatar syasonik

Update Proxy api and protect for future changes

The time window params we use in the codebase for time
series charts were recently renamed. The query params
for our GrafanaApiController got missed, so this
updates them to be start_time and end_time.
parent dbdac50a
...@@ -9,7 +9,7 @@ class Projects::GrafanaApiController < Projects::ApplicationController ...@@ -9,7 +9,7 @@ class Projects::GrafanaApiController < Projects::ApplicationController
project, project,
params[:datasource_id], params[:datasource_id],
params[:proxy_path], params[:proxy_path],
query_params.to_h prometheus_params
).execute ).execute
return continue_polling_response if result.nil? return continue_polling_response if result.nil?
...@@ -25,6 +25,15 @@ class Projects::GrafanaApiController < Projects::ApplicationController ...@@ -25,6 +25,15 @@ class Projects::GrafanaApiController < Projects::ApplicationController
end end
def query_params def query_params
params.permit(:query, :start, :end, :step) params.permit(:query, :start_time, :end_time, :step)
end
def prometheus_params
query_params.to_h
.except(:start_time, :end_time)
.merge(
start: query_params[:start_time],
end: query_params[:end_time]
)
end end
end end
...@@ -26,6 +26,8 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek ...@@ -26,6 +26,8 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek
import_common_metrics import_common_metrics
stub_any_prometheus_request_with_response stub_any_prometheus_request_with_response
allow(Prometheus::ProxyService).to receive(:new).and_call_original
end end
after do after do
...@@ -56,6 +58,11 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek ...@@ -56,6 +58,11 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek
expect(page).to have_text(metric.title) expect(page).to have_text(metric.title)
expect(page).to have_text(metric.y_label) expect(page).to have_text(metric.y_label)
expect(page).not_to have_text(metrics_url) expect(page).not_to have_text(metrics_url)
expect(Prometheus::ProxyService)
.to have_received(:new)
.with(alert.environment, 'GET', 'query_range', hash_including('start', 'end', 'step'))
.at_least(:once)
end end
# Delete when moving to CE # Delete when moving to CE
...@@ -93,6 +100,11 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek ...@@ -93,6 +100,11 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek
expect(page).to have_text(query_params[:title]) expect(page).to have_text(query_params[:title])
expect(page).to have_text(query_params[:y_label]) expect(page).to have_text(query_params[:y_label])
expect(page).not_to have_text(metrics_url) expect(page).not_to have_text(metrics_url)
expect(Prometheus::ProxyService)
.to have_received(:new)
.with(cluster, 'GET', 'query_range', hash_including('start', 'end', 'step'))
.at_least(:once)
end end
# Delete when moving to CE # Delete when moving to CE
......
...@@ -20,8 +20,8 @@ describe Projects::GrafanaApiController do ...@@ -20,8 +20,8 @@ describe Projects::GrafanaApiController do
proxy_path: 'api/v1/query_range', proxy_path: 'api/v1/query_range',
datasource_id: '1', datasource_id: '1',
query: 'rate(relevant_metric)', query: 'rate(relevant_metric)',
start: '1570441248', start_time: '1570441248',
end: '1570444848', end_time: '1570444848',
step: '900' step: '900'
} }
end end
...@@ -50,7 +50,10 @@ describe Projects::GrafanaApiController do ...@@ -50,7 +50,10 @@ describe Projects::GrafanaApiController do
expect(Grafana::ProxyService) expect(Grafana::ProxyService)
.to have_received(:new) .to have_received(:new)
.with(project, '1', 'api/v1/query_range', .with(project, '1', 'api/v1/query_range',
params.slice(:query, :start, :end, :step).stringify_keys) { 'query' => params[:query],
'start' => params[:start_time],
'end' => params[:end_time],
'step' => params[:step] })
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to eq({}) expect(json_response).to eq({})
......
...@@ -34,6 +34,8 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek ...@@ -34,6 +34,8 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek
before do before do
import_common_metrics import_common_metrics
stub_any_prometheus_request_with_response stub_any_prometheus_request_with_response
allow(Prometheus::ProxyService).to receive(:new).and_call_original
end end
it 'shows embedded metrics' do it 'shows embedded metrics' do
...@@ -42,6 +44,12 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek ...@@ -42,6 +44,12 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek
expect(page).to have_css('div.prometheus-graph') expect(page).to have_css('div.prometheus-graph')
expect(page).to have_text('Memory Usage (Total)') expect(page).to have_text('Memory Usage (Total)')
expect(page).to have_text('Core Usage (Total)') expect(page).to have_text('Core Usage (Total)')
# Ensure that the FE is calling the BE with expected params
expect(Prometheus::ProxyService)
.to have_received(:new)
.with(environment, 'GET', 'query_range', hash_including('start', 'end', 'step'))
.at_least(:once)
end end
context 'when dashboard params are in included the url' do context 'when dashboard params are in included the url' do
...@@ -61,6 +69,12 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek ...@@ -61,6 +69,12 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek
expect(page).to have_css('div.prometheus-graph') expect(page).to have_css('div.prometheus-graph')
expect(page).to have_text(chart_params[:title]) expect(page).to have_text(chart_params[:title])
expect(page).to have_text(chart_params[:y_label]) expect(page).to have_text(chart_params[:y_label])
# Ensure that the FE is calling the BE with expected params
expect(Prometheus::ProxyService)
.to have_received(:new)
.with(environment, 'GET', 'query_range', hash_including('start', 'end', 'step'))
.at_least(:once)
end end
context 'when two dashboard urls are included' do context 'when two dashboard urls are included' do
...@@ -83,6 +97,12 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek ...@@ -83,6 +97,12 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek
expect(page).to have_text(chart_params[:y_label]) expect(page).to have_text(chart_params[:y_label])
expect(page).to have_text(chart_params_2[:title]) expect(page).to have_text(chart_params_2[:title])
expect(page).to have_text(chart_params_2[:y_label]) expect(page).to have_text(chart_params_2[:y_label])
# Ensure that the FE is calling the BE with expected params
expect(Prometheus::ProxyService)
.to have_received(:new)
.with(environment, 'GET', 'query_range', hash_including('start', 'end', 'step'))
.at_least(:once)
end end
end end
end end
...@@ -97,6 +117,8 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek ...@@ -97,6 +117,8 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek
stub_dashboard_request(grafana_base_url) stub_dashboard_request(grafana_base_url)
stub_datasource_request(grafana_base_url) stub_datasource_request(grafana_base_url)
stub_all_grafana_proxy_requests(grafana_base_url) stub_all_grafana_proxy_requests(grafana_base_url)
allow(Grafana::ProxyService).to receive(:new).and_call_original
end end
it 'shows embedded metrics' do it 'shows embedded metrics' do
...@@ -105,6 +127,12 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek ...@@ -105,6 +127,12 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek
expect(page).to have_css('div.prometheus-graph') expect(page).to have_css('div.prometheus-graph')
expect(page).to have_text('Expired / Evicted') expect(page).to have_text('Expired / Evicted')
expect(page).to have_text('expired - test-attribute-value') expect(page).to have_text('expired - test-attribute-value')
# Ensure that the FE is calling the BE with expected params
expect(Grafana::ProxyService)
.to have_received(:new)
.with(project, anything, anything, hash_including('query', 'start', 'end', 'step'))
.at_least(:once)
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