Merge branch 'sy-fix-grafana-embeds' into 'master'

Update Grafana proxy api time range params

See merge request gitlab-org/gitlab!28467

app/controllers/projects/grafana_api_controller.rb

@@ -9,7 +9,7 @@ class Projects::GrafanaApiController < Projects::ApplicationController
       project,
       params[:datasource_id],
       params[:proxy_path],
-      query_params.to_h
+      prometheus_params
     ).execute
 
     return continue_polling_response if result.nil?
@@ -25,6 +25,15 @@ class Projects::GrafanaApiController < Projects::ApplicationController
   end
 
   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

ee/spec/features/markdown/metrics_spec.rb

@@ -26,6 +26,8 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek
 
     import_common_metrics
     stub_any_prometheus_request_with_response
+
+    allow(Prometheus::ProxyService).to receive(:new).and_call_original
   end
 
   after do
@@ -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.y_label)
       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
 
     # Delete when moving to CE
@@ -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[:y_label])
       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
 
     # Delete when moving to CE

spec/controllers/projects/grafana_api_controller_spec.rb

@@ -20,8 +20,8 @@ describe Projects::GrafanaApiController do
         proxy_path: 'api/v1/query_range',
         datasource_id: '1',
         query: 'rate(relevant_metric)',
-        start: '1570441248',
-        end: '1570444848',
+        start_time: '1570441248',
+        end_time: '1570444848',
         step: '900'
       }
     end
@@ -50,7 +50,10 @@ describe Projects::GrafanaApiController do
         expect(Grafana::ProxyService)
           .to have_received(:new)
           .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(json_response).to eq({})

spec/features/markdown/metrics_spec.rb

@@ -34,6 +34,8 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek
     before do
       import_common_metrics
       stub_any_prometheus_request_with_response
+
+      allow(Prometheus::ProxyService).to receive(:new).and_call_original
     end
 
     it 'shows embedded metrics' do
@@ -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_text('Memory 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
 
     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
         expect(page).to have_css('div.prometheus-graph')
         expect(page).to have_text(chart_params[:title])
         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
 
       context 'when two dashboard urls are included' do
@@ -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_2[:title])
           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
@@ -97,6 +117,8 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching, :sidek
       stub_dashboard_request(grafana_base_url)
       stub_datasource_request(grafana_base_url)
       stub_all_grafana_proxy_requests(grafana_base_url)
+
+      allow(Grafana::ProxyService).to receive(:new).and_call_original
     end
 
     it 'shows embedded metrics' do
@@ -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_text('Expired / Evicted')
       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