Merge branch 'sy-grafana-default-times' into 'master'

Default to sensible time range for grafana embeds

See merge request gitlab-org/gitlab!21884

changelogs/unreleased/sy-grafana-default-times.yml

+---
+title: Allow default time window on grafana embeds
+merge_request: 21884
+author:
+type: changed

doc/user/project/integrations/prometheus.md

@@ -769,7 +769,7 @@ Prerequisites for embedding from a Grafana instance:
 1. In the upper-left corner of the page, select a specific value for each variable required for the queries in the chart.
    ![Select Query Variables](img/select_query_variables_v12_5.png)
 1. In Grafana, click on a panel's title, then click **Share** to open the panel's sharing dialog to the **Link** tab.
-1. If your Prometheus queries use Grafana's custom template variables, ensure that "Template variables" and "Current time range" options are toggled to **On**. Of Grafana global template variables, only `$__interval`, `$__from`, and `$__to` are currently supported.
+1. If your Prometheus queries use Grafana's custom template variables, ensure that "Template variables" option is toggled to **On**. Of Grafana global template variables, only `$__interval`, `$__from`, and `$__to` are currently supported. Toggle **On** the "Current time range" option to specify the time range of the chart. Otherwise, the default range will be the last 8 hours.
    ![Grafana Sharing Dialog](img/grafana_sharing_dialog_v12_5.png)
 1. Click **Copy** to copy the URL to the clipboard.
 1. In GitLab, paste the URL into a Markdown field and save. The chart will take a few moments to render.

lib/banzai/filter/inline_grafana_metrics_filter.rb

@@ -13,13 +13,16 @@ module Banzai
         super
       end
 
+      # @return [Hash<Symbol, String>] with keys :grafana_url, :start, and :end
       def embed_params(node)
         query_params = Gitlab::Metrics::Dashboard::Url.parse_query(node['href'])
-        return unless [:panelId, :from, :to].all? do |param|
-          query_params.include?(param)
-        end
 
-        { url: node['href'], start: query_params[:from], end: query_params[:to] }
+        return unless query_params.include?(:panelId)
+
+        time_window = Grafana::TimeWindow.new(query_params[:from], query_params[:to])
+        url = url_with_window(node['href'], query_params, time_window.in_milliseconds)
+
+        { grafana_url: url }.merge(time_window.formatted)
       end
 
       # Selects any links with an href contains the configured
@@ -44,18 +47,24 @@ module Banzai
         Gitlab::Routing.url_helpers.project_grafana_api_metrics_dashboard_url(
           project,
           embedded: true,
-          grafana_url: params[:url],
-          start: format_time(params[:start]),
-          end: format_time(params[:end])
+          **params
         )
       end
 
-      # Formats a timestamp from Grafana for compatibility with
-      # parsing in JS via `new Date(timestamp)`
+      # If the provided url is missing time window parameters,
+      # this inserts the default window into the url, allowing
+      # the embed service to correctly format prometheus
+      # queries during embed processing.
       #
-      # @param time [String] Represents miliseconds since epoch
-      def format_time(time)
-        Time.at(time.to_i / 1000).utc.strftime('%FT%TZ')
+      # @param url [String]
+      # @param query_params [Hash<Symbol, String>]
+      # @param time_window_params [Hash<Symbol, Integer>]
+      # @return [String]
+      def url_with_window(url, query_params, time_window_params)
+        uri = URI(url)
+        uri.query = time_window_params.merge(query_params).to_query
+
+        uri.to_s
       end
 
       # Fetches a dashboard and caches the result for the

lib/grafana/time_window.rb

+# frozen_string_literal: true
+
+module Grafana
+  # Allows for easy formatting and manipulations of timestamps
+  # coming from a Grafana url
+  class TimeWindow
+    include ::Gitlab::Utils::StrongMemoize
+
+    def initialize(from, to)
+      @from = from
+      @to = to
+    end
+
+    def formatted
+      {
+        start: window[:from].formatted,
+        end: window[:to].formatted
+      }
+    end
+
+    def in_milliseconds
+      window.transform_values(&:to_ms)
+    end
+
+    private
+
+    def window
+      strong_memoize(:window) do
+        specified_window
+      rescue Timestamp::Error
+        default_window
+      end
+    end
+
+    def specified_window
+      RangeWithDefaults.new(
+        from: Timestamp.from_ms_since_epoch(@from),
+        to: Timestamp.from_ms_since_epoch(@to)
+      ).to_hash
+    end
+
+    def default_window
+      RangeWithDefaults.new.to_hash
+    end
+  end
+
+  # For incomplete time ranges, adds default parameters to
+  # achieve a complete range. If both full range is provided,
+  # range will be returned.
+  class RangeWithDefaults
+    DEFAULT_RANGE = 8.hours
+
+    # @param from [Grafana::Timestamp, nil] Start of the expected range
+    # @param to [Grafana::Timestamp, nil] End of the expected range
+    def initialize(from: nil, to: nil)
+      @from = from
+      @to = to
+
+      apply_defaults!
+    end
+
+    def to_hash
+      { from: @from, to: @to }.compact
+    end
+
+    private
+
+    def apply_defaults!
+      @to ||= @from ? relative_end : Timestamp.new(Time.now)
+      @from ||= relative_start
+    end
+
+    def relative_start
+      Timestamp.new(DEFAULT_RANGE.before(@to.time))
+    end
+
+    def relative_end
+      Timestamp.new(DEFAULT_RANGE.since(@from.time))
+    end
+  end
+
+  # Offers a consistent API for timestamps originating from
+  # Grafana or other sources, allowing for formatting of timestamps
+  # as consumed by Grafana-related utilities
+  class Timestamp
+    Error = Class.new(StandardError)
+
+    attr_accessor :time
+
+    # @param timestamp [Time]
+    def initialize(time)
+      @time = time
+    end
+
+    # Formats a timestamp from Grafana for compatibility with
+    # parsing in JS via `new Date(timestamp)`
+    def formatted
+      time.utc.strftime('%FT%TZ')
+    end
+
+    # Converts to milliseconds since epoch
+    def to_ms
+      time.to_i * 1000
+    end
+
+    class << self
+      # @param time [String] Representing milliseconds since epoch.
+      #                      This is what JS "decided" unix is.
+      def from_ms_since_epoch(time)
+        return if time.nil?
+
+        raise Error.new('Expected milliseconds since epoch') unless ms_since_epoch?(time)
+
+        new(cast_ms_to_time(time))
+      end
+
+      private
+
+      def cast_ms_to_time(time)
+        Time.at(time.to_i / 1000.0)
+      end
+
+      def ms_since_epoch?(time)
+        ms = time.to_i
+
+        ms.to_s == time && ms.bit_length < 64
+      end
+    end
+  end
+end

spec/lib/banzai/filter/inline_grafana_metrics_filter_spec.rb

@@ -10,18 +10,20 @@ describe Banzai::Filter::InlineGrafanaMetricsFilter do
 
   let(:input) { %(<a href="#{trigger_url}">example</a>) }
   let(:doc) { filter(input) }
+  let(:embed_url) { doc.at_css('.js-render-metrics')['data-dashboard-url'] }
+
   let(:dashboard_path) do
     '/d/XDaNK6amz/gitlab-omnibus-redis' \
-    '?from=1570397739557&to=1570484139557' \
-    '&var-instance=All&panelId=14'
+    '?from=1570397739557&panelId=14' \
+    '&to=1570484139557&var-instance=All'
   end
 
   let(:trigger_url) { grafana_integration.grafana_url + dashboard_path }
   let(:dashboard_url) do
     urls.project_grafana_api_metrics_dashboard_url(
       project,
-      embedded: true,
       grafana_url: trigger_url,
+      embedded: true,
       start: "2019-10-06T21:35:39Z",
       end: "2019-10-07T21:35:39Z"
     )
@@ -29,6 +31,10 @@ describe Banzai::Filter::InlineGrafanaMetricsFilter do
 
   it_behaves_like 'a metrics embed filter'
 
+  around do |example|
+    Timecop.freeze(Time.utc(2019, 3, 17, 13, 10)) { example.run }
+  end
+
   context 'when grafana is not configured' do
     before do
       allow(project).to receive(:grafana_integration).and_return(nil)
@@ -39,7 +45,7 @@ describe Banzai::Filter::InlineGrafanaMetricsFilter do
     end
   end
 
-  context 'when parameters are missing' do
+  context 'when "panelId" parameter is missing' do
     let(:dashboard_path) { '/d/XDaNK6amz/gitlab-omnibus-redis' }
 
     it 'leaves the markdown unchanged' do
@@ -47,6 +53,39 @@ describe Banzai::Filter::InlineGrafanaMetricsFilter do
     end
   end
 
+  context 'when time window parameters are missing' do
+    let(:dashboard_path) { '/d/XDaNK6amz/gitlab-omnibus-redis?panelId=16' }
+
+    it 'sets the window to the last 8 hrs' do
+      expect(embed_url).to include(
+        'from%3D1552799400000', 'to%3D1552828200000',
+        'start=2019-03-17T05%3A10%3A00Z', 'end=2019-03-17T13%3A10%3A00Z'
+      )
+    end
+  end
+
+  context 'when "to" parameter is missing' do
+    let(:dashboard_path) { '/d/XDaNK6amz/gitlab-omnibus-redis?panelId=16&from=1570397739557' }
+
+    it 'sets "to" to 8 hrs after "from"' do
+      expect(embed_url).to include(
+        'from%3D1570397739557', 'to%3D1570426539000',
+        'start=2019-10-06T21%3A35%3A39Z', 'end=2019-10-07T05%3A35%3A39Z'
+      )
+    end
+  end
+
+  context 'when "from" parameter is missing' do
+    let(:dashboard_path) { '/d/XDaNK6amz/gitlab-omnibus-redis?panelId=16&to=1570484139557' }
+
+    it 'sets "from" to 8 hrs before "to"' do
+      expect(embed_url).to include(
+        'from%3D1570455339000', 'to%3D1570484139557',
+        'start=2019-10-07T13%3A35%3A39Z', 'end=2019-10-07T21%3A35%3A39Z'
+      )
+    end
+  end
+
   private
 
   # Nokogiri escapes the URLs, but we don't care about that

spec/lib/grafana/time_window_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Grafana::TimeWindow do
+  let(:from) { '1552799400000' }
+  let(:to) { '1552828200000' }
+
+  around do |example|
+    Timecop.freeze(Time.utc(2019, 3, 17, 13, 10)) { example.run }
+  end
+
+  describe '#formatted' do
+    subject { described_class.new(from, to).formatted }
+
+    it { is_expected.to eq(start: "2019-03-17T05:10:00Z", end: "2019-03-17T13:10:00Z") }
+  end
+
+  describe '#in_milliseconds' do
+    subject { described_class.new(from, to).in_milliseconds }
+
+    it { is_expected.to eq(from: 1552799400000, to: 1552828200000) }
+
+    context 'when non-unix parameters are provided' do
+      let(:to) { Time.now.to_s }
+
+      let(:default_from) { 8.hours.ago.to_i * 1000 }
+      let(:default_to) { Time.now.to_i * 1000 }
+
+      it { is_expected.to eq(from: default_from, to: default_to) }
+    end
+  end
+end
+
+describe Grafana::RangeWithDefaults do
+  let(:from) { Grafana::Timestamp.from_ms_since_epoch('1552799400000') }
+  let(:to) { Grafana::Timestamp.from_ms_since_epoch('1552828200000') }
+
+  around do |example|
+    Timecop.freeze(Time.utc(2019, 3, 17, 13, 10)) { example.run }
+  end
+
+  describe '#to_hash' do
+    subject { described_class.new(from: from, to: to).to_hash }
+
+    it { is_expected.to eq(from: from, to: to) }
+
+    context 'when only "to" is provided' do
+      let(:from) { nil }
+
+      it 'has the expected properties' do
+        expect(subject[:to]).to eq(to)
+        expect(subject[:from].time).to eq(to.time - 8.hours)
+      end
+    end
+
+    context 'when only "from" is provided' do
+      let(:to) { nil }
+
+      it 'has the expected properties' do
+        expect(subject[:to].time).to eq(from.time + 8.hours)
+        expect(subject[:from]).to eq(from)
+      end
+    end
+
+    context 'when no parameters are provided' do
+      let(:to) { nil }
+      let(:from) { nil }
+
+      let(:default_from) { 8.hours.ago }
+      let(:default_to) { Time.now }
+
+      it 'has the expected properties' do
+        expect(subject[:to].time).to eq(default_to)
+        expect(subject[:from].time).to eq(default_from)
+      end
+    end
+  end
+end
+
+describe Grafana::Timestamp do
+  let(:timestamp) { Time.at(1552799400) }
+
+  around do |example|
+    Timecop.freeze(Time.utc(2019, 3, 17, 13, 10)) { example.run }
+  end
+
+  describe '#formatted' do
+    subject { described_class.new(timestamp).formatted }
+
+    it { is_expected.to eq "2019-03-17T05:10:00Z" }
+  end
+
+  describe '#to_ms' do
+    subject { described_class.new(timestamp).to_ms }
+
+    it { is_expected.to eq 1552799400000 }
+  end
+
+  describe '.from_ms_since_epoch' do
+    let(:timestamp) { '1552799400000' }
+
+    subject { described_class.from_ms_since_epoch(timestamp) }
+
+    it { is_expected.to be_a described_class }
+
+    context 'when the input is not a unix-ish timestamp' do
+      let(:timestamp) { Time.now.to_s }
+
+      it 'raises an error' do
+        expect { subject }.to raise_error(Grafana::Timestamp::Error)
+      end
+    end
+  end
+end