Merge pull request #14876 from grafana/davkal/14795-annotation-step

Prometheus: Fix annotation step calculation

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

@@ -219,8 +219,9 @@ export class PrometheusDatasource {
     };
     };
     const range = Math.ceil(end - start);
     const range = Math.ceil(end - start);
 
 
+    // options.interval is the dynamically calculated interval
     let interval = kbn.interval_to_seconds(options.interval);
     let interval = kbn.interval_to_seconds(options.interval);
-    // Minimum interval ("Min step"), if specified for the query. or same as interval otherwise
+    // Minimum interval ("Min step"), if specified for the query or datasource. or same as interval otherwise
     const minInterval = kbn.interval_to_seconds(
     const minInterval = kbn.interval_to_seconds(
       this.templateSrv.replace(target.interval, options.scopedVars) || options.interval
       this.templateSrv.replace(target.interval, options.scopedVars) || options.interval
     );
     );
@@ -366,12 +367,13 @@ export class PrometheusDatasource {
     const step = annotation.step || '60s';
     const step = annotation.step || '60s';
     const start = this.getPrometheusTime(options.range.from, false);
     const start = this.getPrometheusTime(options.range.from, false);
     const end = this.getPrometheusTime(options.range.to, true);
     const end = this.getPrometheusTime(options.range.to, true);
-    // Unsetting min interval
     const queryOptions = {
     const queryOptions = {
       ...options,
       ...options,
-      interval: '0s',
+      interval: step,
     };
     };
-    const query = this.createQuery({ expr, interval: step }, queryOptions, start, end);
+    // Unsetting min interval for accurate event resolution
+    const minStep = '1s';
+    const query = this.createQuery({ expr, interval: minStep }, queryOptions, start, end);
 
 
     const self = this;
     const self = this;
     return this.performTimeSeriesQuery(query, query.start, query.end).then(results => {
     return this.performTimeSeriesQuery(query, query.start, query.end).then(results => {

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

@@ -577,6 +577,79 @@ describe('PrometheusDatasource', () => {
         expect(results[0].time).toEqual(1);
         expect(results[0].time).toEqual(1);
       });
       });
     });
     });
+
+    describe('step parameter', () => {
+      beforeEach(() => {
+        backendSrv.datasourceRequest = jest.fn(() => Promise.resolve(response));
+        ctx.ds = new PrometheusDatasource(instanceSettings, q, backendSrv as any, templateSrv, timeSrv);
+      });
+
+      it('should use default step for short range if no interval is given', () => {
+        const query = {
+          ...options,
+          range: {
+            from: time({ seconds: 63 }),
+            to: time({ seconds: 123 }),
+          },
+        };
+        ctx.ds.annotationQuery(query);
+        const req = backendSrv.datasourceRequest.mock.calls[0][0];
+        expect(req.url).toContain('step=60');
+      });
+
+      it('should use custom step for short range', () => {
+        const annotation = {
+          ...options.annotation,
+          step: '10s',
+        };
+        const query = {
+          ...options,
+          annotation,
+          range: {
+            from: time({ seconds: 63 }),
+            to: time({ seconds: 123 }),
+          },
+        };
+        ctx.ds.annotationQuery(query);
+        const req = backendSrv.datasourceRequest.mock.calls[0][0];
+        expect(req.url).toContain('step=10');
+      });
+
+      it('should use custom step for short range', () => {
+        const annotation = {
+          ...options.annotation,
+          step: '10s',
+        };
+        const query = {
+          ...options,
+          annotation,
+          range: {
+            from: time({ seconds: 63 }),
+            to: time({ seconds: 123 }),
+          },
+        };
+        ctx.ds.annotationQuery(query);
+        const req = backendSrv.datasourceRequest.mock.calls[0][0];
+        expect(req.url).toContain('step=10');
+      });
+
+      it('should use dynamic step on long ranges if no option was given', () => {
+        const query = {
+          ...options,
+          range: {
+            from: time({ seconds: 63 }),
+            to: time({ hours: 24 * 30, seconds: 63 }),
+          },
+        };
+        ctx.ds.annotationQuery(query);
+        const req = backendSrv.datasourceRequest.mock.calls[0][0];
+        // Range in seconds: (to - from) / 1000
+        // Max_datapints: 11000
+        // Step: range / max_datapints
+        const step = 236;
+        expect(req.url).toContain(`step=${step}`);
+      });
+    });
   });
   });
 
 
   describe('When resultFormat is table and instant = true', () => {
   describe('When resultFormat is table and instant = true', () => {