fix(query): Use seconds with isRequestStepAligned and alignWindow

Caught two bugs due to mismatch in seconds and milliseconds
when calculating the updated values for aligning the window.

`isRequestStepAligned` will now convert the step to seconds and then check
if the modulo of the start and end time(represented as unix time stamps)
is zero. If either of the values is not aligned, return false. Otherwise
will return true.

`alignWindow` will be called if the result of `isRequestStepAligned` is
false. This method accepts the start, end and step times. It will
convert each number to seconds and then truncate the amount time
proportional to the step function. The current implementation will round
always round down. IE, if the step is an hour then the minutes and
seconds will be removed. See the unit tests for some examples of how the
times will be aligned.

Adds basic unit tests for both methods.

Signed-off-by: pokom <mark.poko@grafana.com>

pkg/prom/query.go

@@ -260,7 +260,10 @@ func (ctx *Context) query(query string, t time.Time) (interface{}, v1.Warnings,
 
 // isRequestStepAligned will check if the start and end times are aligned with the step
 func (ctx *Context) isRequestStepAligned(start, end time.Time, step time.Duration) bool {
-	return start.Unix()%step.Milliseconds() == 0 && end.Unix()%step.Milliseconds() == 0
+	startInUnix := start.Unix()
+	endInUnix := end.Unix()
+	stepInSeconds := step.Milliseconds() / 1e3
+	return startInUnix%stepInSeconds == 0 && endInUnix%stepInSeconds == 0
 }
 
 func (ctx *Context) QueryRange(query string, start, end time.Time, step time.Duration) QueryResultsChan {
@@ -392,10 +395,14 @@ func (ctx *Context) queryRange(query string, start, end time.Time, step time.Dur
 	return toReturn, warnings, nil
 }
 
+// alignWindow will update the start and end times to be aligned with the step duration.
+// Current implementation will always floor the start/end times
 func (ctx *Context) alignWindow(start time.Time, end time.Time, step time.Duration) (time.Time, time.Time) {
-	alignedStart := (start.Unix() / step.Milliseconds()) * start.Unix()
-	alignedEnd := (end.Unix() / step.Milliseconds()) * end.Unix()
-	return time.UnixMilli(alignedStart), time.UnixMilli(alignedEnd)
+	// Convert the step duration from Milliseconds to Seconds to match the Unix timestamp, which is in seconds
+	stepInSeconds := step.Milliseconds() / 1e3
+	alignedStart := (start.Unix() / stepInSeconds) * stepInSeconds
+	alignedEnd := (end.Unix() / stepInSeconds) * stepInSeconds
+	return time.Unix(alignedStart, 0).UTC(), time.Unix(alignedEnd, 0).UTC()
 }
 
 // Extracts the warnings from the resulting json if they exist (part of the prometheus response api).

pkg/prom/query_test.go

@@ -1,6 +1,11 @@
 package prom
 
-import "testing"
+import (
+	"github.com/prometheus/client_golang/api"
+	"reflect"
+	"testing"
+	"time"
+)
 
 func TestWarningsFrom(t *testing.T) {
 	var results interface{}
@@ -25,3 +30,156 @@ func TestWarningsFrom(t *testing.T) {
 		t.Errorf("Unexpected second warning: %s", warnings[1])
 	}
 }
+
+func TestContext_isRequestStepAligned(t *testing.T) {
+	type fields struct {
+		Client         api.Client
+		name           string
+		errorCollector *QueryErrorCollector
+	}
+	type args struct {
+		start time.Time
+		end   time.Time
+		step  time.Duration
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		args   args
+		want   bool
+	}{
+		{
+			name:   "Test with times that are not step aligned to the hour",
+			fields: fields{},
+			args: args{
+				start: time.Date(2022, 11, 7, 4, 59, 30, 0, time.UTC),
+				end:   time.Date(2022, 11, 8, 4, 59, 30, 0, time.UTC),
+				step:  time.Hour,
+			},
+			want: false,
+		},
+		{
+			name:   "Test with times that are step aligned to the hour",
+			fields: fields{},
+			args: args{
+				start: time.Date(2022, 11, 7, 4, 0, 0, 0, time.UTC),
+				end:   time.Date(2022, 11, 8, 4, 0, 0, 0, time.UTC),
+				step:  time.Hour,
+			},
+			want: true,
+		},
+		{
+			name:   "Test with times where start is aligned to the hour but end is not",
+			fields: fields{},
+			args: args{
+				start: time.Date(2022, 11, 7, 4, 0, 0, 0, time.UTC),
+				end:   time.Date(2022, 11, 8, 4, 59, 0, 0, time.UTC),
+				step:  time.Hour,
+			},
+			want: false,
+		},
+		{
+			name:   "Test with times where end is aligned to the hour but start is not",
+			fields: fields{},
+			args: args{
+				start: time.Date(2022, 11, 7, 4, 59, 0, 0, time.UTC),
+				end:   time.Date(2022, 11, 8, 4, 0, 0, 0, time.UTC),
+				step:  time.Hour,
+			},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := &Context{
+				Client:         tt.fields.Client,
+				name:           tt.fields.name,
+				errorCollector: tt.fields.errorCollector,
+			}
+			if got := ctx.isRequestStepAligned(tt.args.start, tt.args.end, tt.args.step); got != tt.want {
+				t.Errorf("isRequestStepAligned() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestContext_alignWindow(t *testing.T) {
+	type fields struct {
+		Client         api.Client
+		name           string
+		errorCollector *QueryErrorCollector
+	}
+	type args struct {
+		start time.Time
+		end   time.Time
+		step  time.Duration
+	}
+	tests := []struct {
+		name      string
+		fields    fields
+		args      args
+		wantStart time.Time
+		wantEnd   time.Time
+	}{
+		{
+			name:   "Do not update the start and end when step-aligned",
+			fields: fields{},
+			args: args{
+				start: time.Date(2022, 11, 7, 4, 0, 0, 0, time.UTC),
+				end:   time.Date(2022, 11, 8, 4, 0, 0, 0, time.UTC),
+				step:  time.Hour,
+			},
+			wantStart: time.Date(2022, 11, 7, 4, 0, 0, 0, time.UTC),
+			wantEnd:   time.Date(2022, 11, 8, 4, 0, 0, 0, time.UTC),
+		},
+		{
+			name:   "Update start to be step-aligned and leave end the same",
+			fields: fields{},
+			args: args{
+				start: time.Date(2022, 11, 7, 4, 59, 0, 0, time.UTC),
+				end:   time.Date(2022, 11, 8, 4, 0, 0, 0, time.UTC),
+				step:  time.Hour,
+			},
+			wantStart: time.Date(2022, 11, 7, 4, 0, 0, 0, time.UTC),
+			wantEnd:   time.Date(2022, 11, 8, 4, 0, 0, 0, time.UTC),
+		},
+		{
+			name:   "Update end to be step-aligned and leave start the same",
+			fields: fields{},
+			args: args{
+				start: time.Date(2022, 11, 7, 4, 0, 0, 0, time.UTC),
+				end:   time.Date(2022, 11, 8, 4, 59, 0, 0, time.UTC),
+				step:  time.Hour,
+			},
+			wantStart: time.Date(2022, 11, 7, 4, 0, 0, 0, time.UTC),
+			wantEnd:   time.Date(2022, 11, 8, 4, 0, 0, 0, time.UTC),
+		},
+		{
+			name:   "Update start and end to be step-aligned",
+			fields: fields{},
+			args: args{
+				start: time.Date(2022, 11, 7, 4, 59, 0, 0, time.UTC),
+				end:   time.Date(2022, 11, 8, 4, 59, 0, 0, time.UTC),
+				step:  time.Hour,
+			},
+			wantStart: time.Date(2022, 11, 7, 4, 0, 0, 0, time.UTC),
+			wantEnd:   time.Date(2022, 11, 8, 4, 0, 0, 0, time.UTC),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := &Context{
+				Client:         tt.fields.Client,
+				name:           tt.fields.name,
+				errorCollector: tt.fields.errorCollector,
+			}
+			got, got1 := ctx.alignWindow(tt.args.start, tt.args.end, tt.args.step)
+			if !reflect.DeepEqual(got, tt.wantStart) {
+				t.Errorf("alignWindow() got = %v, want %v", got, tt.wantStart)
+			}
+			if !reflect.DeepEqual(got1, tt.wantEnd) {
+				t.Errorf("alignWindow() got1 = %v, want %v", got1, tt.wantEnd)
+			}
+		})
+	}
+}