Merge 37c1b85 into 4112c5d

api/controller/trigger_metrics.go

@@ -24,6 +24,7 @@ func GetTriggerEvaluationResult(dataBase moira.Database, metricSourceProvider *m
 		return nil, &trigger, err
 	}
 
+	from -= from % 60
 	for i, tar := range trigger.Targets {
 		fetchResult, err := metricsSource.Fetch(tar, from, to, fetchRealtimeData)
 		if err != nil {

api/controller/trigger_metrics_test.go

@@ -274,6 +274,7 @@ func TestGetTriggerMetrics(t *testing.T) {
 	metric := "super.puper.metric"
 
 	var from int64 = 17
+	var from2 int64
 	var until int64 = 67
 	var retention int64 = 10
 
@@ -296,7 +297,7 @@ func TestGetTriggerMetrics(t *testing.T) {
 	Convey("Has metrics", t, func() {
 		dataBase.EXPECT().GetTrigger(triggerID).Return(moira.Trigger{ID: triggerID, Targets: []string{pattern}}, nil)
 		localSource.EXPECT().IsConfigured().Return(true, nil)
-		localSource.EXPECT().Fetch(pattern, from, until, false).Return(fetchResult, nil)
+		localSource.EXPECT().Fetch(pattern, from2, until, false).Return(fetchResult, nil)
 		fetchResult.EXPECT().GetMetricsData().Return([]*metricSource.MetricData{metricSource.MakeMetricData(metric, []float64{0, 1, 2, 3, 4}, retention, from)})
 		triggerMetrics, err := GetTriggerMetrics(dataBase, sourceProvider, from, until, triggerID)
 		So(err, ShouldBeNil)
@@ -322,7 +323,7 @@ func TestGetTriggerMetrics(t *testing.T) {
 		expectedError := remote.ErrRemoteTriggerResponse{InternalError: fmt.Errorf("some error"), Target: pattern}
 		dataBase.EXPECT().GetTrigger(triggerID).Return(moira.Trigger{ID: triggerID, Targets: []string{pattern}, IsRemote: true}, nil)
 		remoteSource.EXPECT().IsConfigured().Return(true, nil)
-		remoteSource.EXPECT().Fetch(pattern, from, until, false).Return(nil, expectedError)
+		remoteSource.EXPECT().Fetch(pattern, from2, until, false).Return(nil, expectedError)
 		triggerMetrics, err := GetTriggerMetrics(dataBase, sourceProvider, from, until, triggerID)
 		So(err, ShouldResemble, api.ErrorInternalServer(expectedError))
 		So(triggerMetrics, ShouldBeNil)

api/handler/trigger_render.go

@@ -87,8 +87,7 @@ func buildRenderable(request *http.Request, trigger *moira.Trigger, metricsData
 	if err != nil {
 		return nil, fmt.Errorf("can not initialize plot theme %s", err.Error())
 	}
-	var metricsWhiteList = make([]string, 0)
-	renderable, err := plotTemplate.GetRenderable(trigger, metricsData, metricsWhiteList)
+	renderable, err := plotTemplate.GetRenderable(trigger, metricsData)
 	if err != nil {
 		return nil, err
 	}

metric_source/metric_data.go

@@ -1,6 +1,9 @@
 package metricSource
 
-import "math"
+import (
+	"fmt"
+	"math"
+)
 
 // MetricData is moira implementation of target evaluation result
 type MetricData struct {
@@ -40,13 +43,17 @@ func MakeEmptyMetricData(name string, step, start, stop int64) *MetricData {
 }
 
 // GetTimestampValue gets value of given timestamp index, if value is Nil, then return NaN
-func (timeSeries *MetricData) GetTimestampValue(valueTimestamp int64) float64 {
-	if valueTimestamp < timeSeries.StartTime {
+func (metricData *MetricData) GetTimestampValue(valueTimestamp int64) float64 {
+	if valueTimestamp < metricData.StartTime {
 		return math.NaN()
 	}
-	valueIndex := int((valueTimestamp - timeSeries.StartTime) / timeSeries.StepTime)
-	if len(timeSeries.Values) <= valueIndex {
+	valueIndex := int((valueTimestamp - metricData.StartTime) / metricData.StepTime)
+	if len(metricData.Values) <= valueIndex {
 		return math.NaN()
 	}
-	return timeSeries.Values[valueIndex]
+	return metricData.Values[valueIndex]
+}
+
+func (metricData *MetricData) String() string {
+	return fmt.Sprintf("Metric: %s, StartTime: %v, StopTime: %v, StepTime: %v, Points: %v", metricData.Name, metricData.StartTime, metricData.StopTime, metricData.StepTime, metricData.Values)
 }

metric_source/metric_data_test.go

@@ -1,6 +1,7 @@
 package metricSource
 
 import (
+	"fmt"
 	"math"
 	"testing"
 
@@ -112,3 +113,19 @@ func TestGetTimestampValue(t *testing.T) {
 		So(math.IsNaN(actual), ShouldBeTrue)
 	})
 }
+
+func TestMetricData_String(t *testing.T) {
+	metricData1 := MakeMetricData("123", []float64{1, 2, 3}, 60, 0)
+	metricData2 := MakeEmptyMetricData("123", 10, 50, 100)
+	Convey("MetricData with points", t, func() {
+		So(metricData1.String(), ShouldResemble, "Metric: 123, StartTime: 0, StopTime: 180, StepTime: 60, Points: [1 2 3]")
+	})
+
+	Convey("MetricData with NaN points", t, func() {
+		So(metricData2.String(), ShouldResemble, "Metric: 123, StartTime: 50, StopTime: 100, StepTime: 10, Points: [NaN NaN NaN NaN NaN]")
+	})
+
+	Convey("MetricsData array", t, func() {
+		So(fmt.Sprintf("%v", []*MetricData{metricData1, metricData2}), ShouldResemble, "[Metric: 123, StartTime: 0, StopTime: 180, StepTime: 60, Points: [1 2 3] Metric: 123, StartTime: 50, StopTime: 100, StepTime: 10, Points: [NaN NaN NaN NaN NaN]]")
+	})
+}

notifier/plotting.go

@@ -3,7 +3,6 @@ package notifier
 import (
 	"bytes"
 	"fmt"
-	"strings"
 	"time"
 
 	"github.com/moira-alert/moira/metric_source"
@@ -14,9 +13,7 @@ import (
 	"github.com/moira-alert/moira/plotting"
 )
 
-var (
-	// defaultTimeShift is default time shift to fetch timeseries
-	defaultTimeShift = 1 * time.Minute
+const (
 	// defaultTimeRange is default time range to fetch timeseries
 	defaultTimeRange = 30 * time.Minute
 )
@@ -55,18 +52,9 @@ func (notifier *StandardNotifier) buildNotificationPackagePlot(pkg NotificationP
 	if err != nil {
 		return buff.Bytes(), err
 	}
-	notifier.logger.Debugf("rendering %s timeseries: %s", trigger.ID, strings.Join(metricsToShow, ", "))
-	var md = make([]*metricSource.MetricData, 0, len(metricsData))
-	for _, metricData := range metricsData {
-		md = append(md, &metricSource.MetricData{
-			Name:      metricData.Name,
-			StartTime: metricData.StartTime,
-			StopTime:  metricData.StopTime,
-			StepTime:  metricData.StepTime,
-			Values:    metricData.Values,
-		})
-	}
-	renderable, err := plotTemplate.GetRenderable(trigger, md, metricsToShow)
+	metricsData = getMetricDataToShow(metricsData, metricsToShow)
+	notifier.logger.Debugf("rendering %s metricsData: %s", trigger.ID, metricsData)
+	renderable, err := plotTemplate.GetRenderable(trigger, metricsData)
 	if err != nil {
 		return buff.Bytes(), err
 	}
@@ -100,13 +88,13 @@ func resolveMetricsWindow(logger moira.Logger, trigger moira.TriggerData, pkg No
 		if isWideWindow {
 			return fromTime.Unix(), toTime.Unix()
 		}
-		return toTime.Add(-defaultTimeRange + defaultTimeShift).Unix(), toTime.Add(defaultTimeShift).Unix()
+		return toTime.Add(-defaultTimeRange).Unix(), toTime.Unix()
 	}
 	// resolve local trigger window
 	// window is realtime: use shifted window to fetch actual data from redis
 	// window is not realtime: force realtime window
 	if isRealTimeWindow {
-		return toTime.Add(-defaultTimeRange + defaultTimeShift).Unix(), toTime.Add(defaultTimeShift).Unix()
+		return toTime.Add(-defaultTimeRange).Unix(), toTime.Unix()
 	}
 	return defaultFrom, defaultTo
 }
@@ -134,6 +122,7 @@ func (notifier *StandardNotifier) evaluateTriggerMetrics(from, to int64, trigger
 
 // fetchAvailableSeries calls fetch function with realtime alerting and retries on fail without
 func fetchAvailableSeries(metricsSource metricSource.MetricSource, target string, from, to int64) ([]*metricSource.MetricData, error) {
+	from -= from % 60
 	realtimeFetchResult, realtimeErr := metricsSource.Fetch(target, from, to, true)
 	switch realtimeErr.(type) {
 	case local.ErrEvaluateTargetFailedWithPanic:
@@ -145,3 +134,22 @@ func fetchAvailableSeries(metricsSource metricSource.MetricSource, target string
 	}
 	return realtimeFetchResult.GetMetricsData(), realtimeErr
 }
+
+// getMetricDataToShow returns MetricData limited by whitelist
+func getMetricDataToShow(metricsData []*metricSource.MetricData, metricsWhitelist []string) []*metricSource.MetricData {
+	if len(metricsWhitelist) == 0 {
+		return metricsData
+	}
+	metricsWhitelistHash := make(map[string]struct{}, len(metricsWhitelist))
+	for _, whiteListed := range metricsWhitelist {
+		metricsWhitelistHash[whiteListed] = struct{}{}
+	}
+
+	newMetricsData := make([]*metricSource.MetricData, 0, len(metricsWhitelist))
+	for _, metricData := range metricsData {
+		if _, ok := metricsWhitelistHash[metricData.Name]; ok {
+			newMetricsData = append(newMetricsData, metricData)
+		}
+	}
+	return newMetricsData
+}

notifier/plotting_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/moira-alert/moira/metric_source"
 	"github.com/op/go-logging"
 	. "github.com/smartystreets/goconvey/convey"
 
@@ -35,7 +36,6 @@ func TestResolveMetricsWindow(t *testing.T) {
 	localTrigger := moira.TriggerData{ID: "redisTrigger", IsRemote: false}
 	remoteTrigger := moira.TriggerData{ID: "remoteTrigger", IsRemote: true}
 	timeRange := time.Unix(int64(defaultTimeRange.Seconds()), 0).Unix()
-	timeShift := time.Unix(int64(defaultTimeShift.Seconds()), 0).Unix()
 	var pkg NotificationPackage
 	var pkgs []NotificationPackage
 	var trigger moira.TriggerData
@@ -47,8 +47,8 @@ func TestResolveMetricsWindow(t *testing.T) {
 				_, expectedTo, err := pkg.GetWindow()
 				So(err, ShouldBeNil)
 				from, to := resolveMetricsWindow(logger, trigger, pkg)
-				So(from, ShouldEqual, expectedTo-timeRange+timeShift)
-				So(to, ShouldEqual, expectedTo+timeShift)
+				So(from, ShouldEqual, expectedTo-timeRange)
+				So(to, ShouldEqual, expectedTo)
 			}
 		})
 		Convey("Window is not realtime: force realtime window", func() {
@@ -76,8 +76,8 @@ func TestResolveMetricsWindow(t *testing.T) {
 				_, expectedTo, err := pkg.GetWindow()
 				So(err, ShouldBeNil)
 				from, to := resolveMetricsWindow(logger, trigger, pkg)
-				So(from, ShouldEqual, expectedTo-timeRange+timeShift)
-				So(to, ShouldEqual, expectedTo+timeShift)
+				So(from, ShouldEqual, expectedTo-timeRange)
+				So(to, ShouldEqual, expectedTo)
 			}
 		})
 	})
@@ -95,3 +95,34 @@ func TestResolveMetricsWindow(t *testing.T) {
 		}
 	})
 }
+
+// TestGetMetricDataToShow tests to limited metricsData returns only necessary metricsData
+func TestGetMetricDataToShow(t *testing.T) {
+	givenSeries := []*metricSource.MetricData{
+		metricSource.MakeMetricData("metricPrefix.metricName1", []float64{1}, 1, 1),
+		metricSource.MakeMetricData("metricPrefix.metricName2", []float64{2}, 2, 2),
+		metricSource.MakeMetricData("metricPrefix.metricName3", []float64{3}, 3, 3),
+	}
+	Convey("Limit series by non-empty whitelist", t, func() {
+		Convey("MetricsData has necessary series", func() {
+			metricsWhiteList := []string{"metricPrefix.metricName1", "metricPrefix.metricName2"}
+			metricsData := getMetricDataToShow(givenSeries, metricsWhiteList)
+			So(len(metricsData), ShouldEqual, len(metricsWhiteList))
+			So(metricsData[0].Name, ShouldEqual, metricsWhiteList[0])
+			So(metricsData[1].Name, ShouldEqual, metricsWhiteList[1])
+		})
+		Convey("MetricsData has no necessary series", func() {
+			metricsWhiteList := []string{"metricPrefix.metricName4"}
+			metricsData := getMetricDataToShow(givenSeries, metricsWhiteList)
+			So(len(metricsData), ShouldEqual, 0)
+		})
+	})
+	Convey("Limit series by an empty whitelist", t, func() {
+		metricsWhiteList := make([]string, 0)
+		metricsData := getMetricDataToShow(givenSeries, metricsWhiteList)
+		for metricDataInd := range metricsData {
+			So(metricsData[metricDataInd].Name, ShouldEqual, givenSeries[metricDataInd].Name)
+		}
+		So(len(metricsData), ShouldEqual, len(givenSeries))
+	})
+}

plotting/helpers.go

@@ -8,7 +8,6 @@ import (
 	"unicode/utf8"
 
 	"github.com/dustin/go-humanize"
-	"github.com/moira-alert/moira/metric_source"
 	"github.com/wcharczuk/go-chart"
 )
 
@@ -100,27 +99,3 @@ func floatToHumanizedValueFormatter(v interface{}) string {
 	}
 	return ""
 }
-
-// toLimitedMetricsData returns MetricData limited by whitelist
-func toLimitedMetricsData(metricsData []*metricSource.MetricData, metricsWhitelist []string) []*metricSource.MetricData {
-	if len(metricsWhitelist) == 0 {
-		return metricsData
-	}
-	newMetricsData := make([]*metricSource.MetricData, 0, len(metricsWhitelist))
-	for _, metricData := range metricsData {
-		if isWhiteListedMetric(metricData.Name, metricsWhitelist) {
-			newMetricsData = append(newMetricsData, metricData)
-		}
-	}
-	return newMetricsData
-}
-
-// isWhiteListedMetric returns true if metric is whitelisted
-func isWhiteListedMetric(metricName string, metricsWhitelist []string) bool {
-	for _, whiteListed := range metricsWhitelist {
-		if whiteListed == metricName {
-			return true
-		}
-	}
-	return false
-}

plotting/helpers_test.go

@@ -8,7 +8,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/moira-alert/moira/metric_source"
 	. "github.com/smartystreets/goconvey/convey"
 
 	"github.com/moira-alert/moira"
@@ -207,34 +206,3 @@ func TestGetYAxisValuesFormatter(t *testing.T) {
 		So(formattedValues, ShouldResemble, formattedMetricValues)
 	})
 }
-
-// TestToLimitedMetricsData tests to limited metricsData returns only necessary metricsData
-func TestToLimitedMetricsData(t *testing.T) {
-	givenSeries := []*metricSource.MetricData{
-		metricSource.MakeMetricData("metricPrefix.metricName1", []float64{1}, 1, 1),
-		metricSource.MakeMetricData("metricPrefix.metricName2", []float64{2}, 2, 2),
-		metricSource.MakeMetricData("metricPrefix.metricName3", []float64{3}, 3, 3),
-	}
-	Convey("Limit series by non-empty whitelist", t, func() {
-		Convey("MetricsData has necessary series", func() {
-			metricsWhiteList := []string{"metricPrefix.metricName1", "metricPrefix.metricName2"}
-			metricsData := toLimitedMetricsData(givenSeries, metricsWhiteList)
-			So(len(metricsData), ShouldEqual, len(metricsWhiteList))
-			So(metricsData[0].Name, ShouldEqual, metricsWhiteList[0])
-			So(metricsData[1].Name, ShouldEqual, metricsWhiteList[1])
-		})
-		Convey("MetricsData has no necessary series", func() {
-			metricsWhiteList := []string{"metricPrefix.metricName4"}
-			metricsData := toLimitedMetricsData(givenSeries, metricsWhiteList)
-			So(len(metricsData), ShouldEqual, 0)
-		})
-	})
-	Convey("Limit series by an empty whitelist", t, func() {
-		metricsWhiteList := make([]string, 0)
-		metricsData := toLimitedMetricsData(givenSeries, metricsWhiteList)
-		for metricDataInd := range metricsData {
-			So(metricsData[metricDataInd].Name, ShouldEqual, givenSeries[metricDataInd].Name)
-		}
-		So(len(metricsData), ShouldEqual, len(givenSeries))
-	})
-}

plotting/plot.go

@@ -46,12 +46,11 @@ func GetPlotTemplate(theme string, location *time.Location) (*Plot, error) {
 }
 
 // GetRenderable returns go-chart to render
-func (plot *Plot) GetRenderable(trigger *moira.Trigger, metricsData []*metricSource.MetricData, metricsWhitelist []string) (chart.Chart, error) {
+func (plot *Plot) GetRenderable(trigger *moira.Trigger, metricsData []*metricSource.MetricData) (chart.Chart, error) {
 	var renderable chart.Chart
 
 	plotSeries := make([]chart.Series, 0)
 
-	metricsData = toLimitedMetricsData(metricsData, metricsWhitelist)
 	limits := resolveLimits(metricsData)
 
 	curveSeriesList := getCurveSeriesList(metricsData, plot.theme)