Prometheus: fix rate_interval to use min step properly (#27168)

* Prometheus: fix rate_interval to use min step properly

* Update docs/sources/features/datasources/prometheus.md

Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com>

* Simplify `getRateIntervalScopedVariable` (#27174)

Signed-off-by: beorn7 <beorn@grafana.com>

* Modify rate_interval tests to use createQuery

* Change test wording + intervalfactor

Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com>
Co-authored-by: Björn Rabenstein <beorn@grafana.com>

docs/sources/features/datasources/prometheus.md

@@ -112,6 +112,8 @@ Regex:
 ```
 ### Using `$__rate_interval` variable
 
+> **Note:** Available in Grafana 7.2 and above
+
 The `$__rate_interval` variable is meant to be used in the rate function. It is defined as max( `$__interval` + _Scrape interval_, 4 * _Scrape interval_), where _Scrape interval_ is the Min step setting (AKA query_interval, a setting per PromQL query), if any is set, and otherwise the _Scrape interval_ as set in the Prometheus data source (but ignoring any Min interval setting in the panel, because the latter is modified by the resolution setting).
 
 ### Using variables in queries

public/app/plugins/datasource/prometheus/datasource.test.ts

@@ -1619,17 +1619,43 @@ describe('PrometheusDatasource', () => {
   });
 
   describe('The __rate_interval variable', () => {
+    const target = { expr: 'rate(process_cpu_seconds_total[$__rate_interval])', refId: 'A' };
+
+    beforeEach(() => {
+      (templateSrv.replace as any).mockClear();
+    });
+
     it('should be 4 times the scrape interval if interval + scrape interval is lower', () => {
-      const { __rate_interval } = ds.getRateIntervalScopedVariable(23, 23);
-      expect(__rate_interval.value).toBe('60s');
+      ds.createQuery(target, { interval: '15s' } as any, 0, 300);
+      expect((templateSrv.replace as any).mock.calls[1][1]['__rate_interval'].value).toBe('60s');
     });
     it('should be interval + scrape interval if 4 times the scrape interval is lower', () => {
-      const { __rate_interval } = ds.getRateIntervalScopedVariable(56, 56);
-      expect(__rate_interval.value).toBe('71s');
+      ds.createQuery(target, { interval: '5m' } as any, 0, 10080);
+      expect((templateSrv.replace as any).mock.calls[1][1]['__rate_interval'].value).toBe('315s');
     });
-    it('should fall back to 60s if interval is 0', () => {
-      const { __rate_interval } = ds.getRateIntervalScopedVariable(0, 0);
-      expect(__rate_interval.value).toBe('60s');
+    it('should fall back to a scrape interval of 15s if min step is set to 0, resulting in 4*15s = 60s', () => {
+      ds.createQuery({ ...target, interval: '' }, { interval: '15s' } as any, 0, 300);
+      expect((templateSrv.replace as any).mock.calls[1][1]['__rate_interval'].value).toBe('60s');
+    });
+    it('should be 4 times the scrape interval if min step set to 1m and interval is 15s', () => {
+      // For a 5m graph, $__interval is 15s
+      ds.createQuery({ ...target, interval: '1m' }, { interval: '15s' } as any, 0, 300);
+      expect((templateSrv.replace as any).mock.calls[1][1]['__rate_interval'].value).toBe('240s');
+    });
+    it('should be interval + scrape interval if min step set to 1m and interval is 5m', () => {
+      // For a 7d graph, $__interval is 5m
+      ds.createQuery({ ...target, interval: '1m' }, { interval: '5m' } as any, 0, 10080);
+      expect((templateSrv.replace as any).mock.calls[1][1]['__rate_interval'].value).toBe('360s');
+    });
+    it('should be interval + scrape interval if resolution is set to 1/2 and interval is 10m', () => {
+      // For a 7d graph, $__interval is 10m
+      ds.createQuery({ ...target, intervalFactor: 2 }, { interval: '10m' } as any, 0, 10080);
+      expect((templateSrv.replace as any).mock.calls[1][1]['__rate_interval'].value).toBe('1215s');
+    });
+    it('should be 4 times the scrape interval if resolution is set to 1/2 and interval is 15s', () => {
+      // For a 5m graph, $__interval is 15s
+      ds.createQuery({ ...target, intervalFactor: 2 }, { interval: '15s' } as any, 0, 300);
+      expect((templateSrv.replace as any).mock.calls[1][1]['__rate_interval'].value).toBe('60s');
     });
   });
 });

public/app/plugins/datasource/prometheus/datasource.ts

@@ -346,17 +346,19 @@ export class PrometheusDatasource extends DataSourceApi<PromQuery, PromOptions>
 
     // options.interval is the dynamically calculated interval
     let interval: number = kbn.intervalToSeconds(options.interval);
-    // Minimum interval ("Min step"), if specified for the query or datasource. or same as interval otherwise
+    // Minimum interval ("Min step"), if specified for the query, or same as interval otherwise.
     const minInterval = kbn.intervalToSeconds(
       templateSrv.replace(target.interval || options.interval, options.scopedVars)
     );
+    // Scrape interval as specified for the query ("Min step") or otherwise taken from the datasource.
+    const scrapeInterval = kbn.intervalToSeconds(target.interval || this.interval);
     const intervalFactor = target.intervalFactor || 1;
     // Adjust the interval to take into account any specified minimum and interval factor plus Prometheus limits
     const adjustedInterval = this.adjustInterval(interval, minInterval, range, intervalFactor);
     let scopedVars = {
       ...options.scopedVars,
       ...this.getRangeScopedVars(options.range),
-      ...this.getRateIntervalScopedVariable(interval, minInterval),
+      ...this.getRateIntervalScopedVariable(adjustedInterval, scrapeInterval),
     };
     // If the interval was adjusted, make a shallow copy of scopedVars with updated interval vars
     if (interval !== adjustedInterval) {
@@ -364,7 +366,7 @@ export class PrometheusDatasource extends DataSourceApi<PromQuery, PromOptions>
       scopedVars = Object.assign({}, options.scopedVars, {
         __interval: { text: interval + 's', value: interval + 's' },
         __interval_ms: { text: interval * 1000, value: interval * 1000 },
-        ...this.getRateIntervalScopedVariable(interval, minInterval),
+        ...this.getRateIntervalScopedVariable(interval, scrapeInterval),
         ...this.getRangeScopedVars(options.range),
       });
     }
@@ -403,13 +405,12 @@ export class PrometheusDatasource extends DataSourceApi<PromQuery, PromOptions>
     return query;
   }
 
-  getRateIntervalScopedVariable(interval: number, minInterval: number) {
-    let intervalInSeconds = minInterval === interval ? kbn.intervalToSeconds(this.interval) : minInterval;
-    // if intervalInSeconds === 0 then we should fall back to the default 15 seconds
-    if (intervalInSeconds === 0) {
-      intervalInSeconds = 15;
+  getRateIntervalScopedVariable(interval: number, scrapeInterval: number) {
+    // Fall back to the default scrape interval of 15s if scrapeInterval is 0 for some reason.
+    if (scrapeInterval === 0) {
+      scrapeInterval = 15;
     }
-    const rateInterval = Math.max(interval + intervalInSeconds, 4 * intervalInSeconds);
+    const rateInterval = Math.max(interval + scrapeInterval, 4 * scrapeInterval);
     return { __rate_interval: { text: rateInterval + 's', value: rateInterval + 's' } };
   }