From fee498968558c401702ee07ab108e0edfa2d7708 Mon Sep 17 00:00:00 2001 From: Xenia N Date: Wed, 18 Oct 2023 11:57:41 +0600 Subject: [PATCH 1/5] build(go): update go to 1.19 (#938) --- Dockerfile.api | 2 +- Dockerfile.checker | 2 +- Dockerfile.cli | 2 +- Dockerfile.filter | 2 +- Dockerfile.notifier | 2 +- go.mod | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Dockerfile.api b/Dockerfile.api index f34cbc4e8..1a1e228ae 100644 --- a/Dockerfile.api +++ b/Dockerfile.api @@ -1,4 +1,4 @@ -FROM golang:1.18 as builder +FROM golang:1.19 as builder COPY go.mod go.sum /go/src/github.com/moira-alert/moira/ WORKDIR /go/src/github.com/moira-alert/moira diff --git a/Dockerfile.checker b/Dockerfile.checker index b7d1b7b9e..d6628c867 100644 --- a/Dockerfile.checker +++ b/Dockerfile.checker @@ -1,4 +1,4 @@ -FROM golang:1.18 as builder +FROM golang:1.19 as builder COPY go.mod go.sum /go/src/github.com/moira-alert/moira/ WORKDIR /go/src/github.com/moira-alert/moira diff --git a/Dockerfile.cli b/Dockerfile.cli index 0ef5decc9..44bb49c5c 100644 --- a/Dockerfile.cli +++ b/Dockerfile.cli @@ -1,4 +1,4 @@ -FROM golang:1.18 as builder +FROM golang:1.19 as builder COPY go.mod go.sum /go/src/github.com/moira-alert/moira/ WORKDIR /go/src/github.com/moira-alert/moira diff --git a/Dockerfile.filter b/Dockerfile.filter index 1b7c81cce..3b61a26d8 100644 --- a/Dockerfile.filter +++ b/Dockerfile.filter @@ -1,4 +1,4 @@ -FROM golang:1.18 as builder +FROM golang:1.19 as builder COPY go.mod go.sum /go/src/github.com/moira-alert/moira/ WORKDIR /go/src/github.com/moira-alert/moira diff --git a/Dockerfile.notifier b/Dockerfile.notifier index 13431c7af..3c6dd5063 100644 --- a/Dockerfile.notifier +++ b/Dockerfile.notifier @@ -1,4 +1,4 @@ -FROM golang:1.18 as builder +FROM golang:1.19 as builder COPY go.mod go.sum /go/src/github.com/moira-alert/moira/ WORKDIR /go/src/github.com/moira-alert/moira diff --git a/go.mod b/go.mod index dfdd7baf2..7ceaad8d9 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/moira-alert/moira -go 1.18 +go 1.19 require ( github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible From cac64eaecacfd1f9051e9b664d0f06280d4d4c55 Mon Sep 17 00:00:00 2001 From: Tetrergeru Date: Wed, 18 Oct 2023 14:36:24 +0200 Subject: [PATCH 2/5] fix: trigger check panics (#939) --- api/controller/trigger_metrics.go | 2 +- checker/check.go | 32 +++-- checker/check_test.go | 76 +++++++++-- checker/metrics/conversion/fetched_metrics.go | 6 +- checker/metrics/conversion/set_helper.go | 57 ++++++--- checker/metrics/conversion/set_helper_test.go | 68 +++++----- checker/metrics/conversion/trigger_metrics.go | 63 ++++----- .../conversion/trigger_metrics_test.go | 120 +++++++++++------- checker/worker/handler.go | 3 +- helpers.go | 12 +- helpers_test.go | 8 +- metric_source/metric_data.go | 2 + plotting/curve.go | 4 +- plotting/limits.go | 2 +- 14 files changed, 287 insertions(+), 168 deletions(-) diff --git a/api/controller/trigger_metrics.go b/api/controller/trigger_metrics.go index 96bd19d18..bb74c88d9 100644 --- a/api/controller/trigger_metrics.go +++ b/api/controller/trigger_metrics.go @@ -65,7 +65,7 @@ func GetTriggerMetrics(dataBase moira.Database, metricSourceProvider *metricSour for i, l := 0, len(timeSeries.Values); i < l; i++ { timestamp := timeSeries.StartTime + int64(i)*timeSeries.StepTime value := timeSeries.GetTimestampValue(timestamp) - if moira.IsValidFloat64(value) { + if moira.IsFiniteNumber(value) { values = append(values, moira.MetricValue{Value: value, Timestamp: timestamp}) } } diff --git a/checker/check.go b/checker/check.go index 973f00857..b1493a82b 100644 --- a/checker/check.go +++ b/checker/check.go @@ -300,6 +300,9 @@ func (triggerChecker *TriggerChecker) check( ) (moira.CheckData, error) { // Case when trigger have only alone metrics if len(metrics) == 0 { + if len(aloneMetrics) == 0 { + return checkData, nil + } if metrics == nil { metrics = make(map[string]map[string]metricSource.MetricData, 1) } @@ -446,7 +449,7 @@ func (triggerChecker *TriggerChecker) getMetricStepsStates( valueTimestamp := startTime + stepTime*stepsDifference endTimestamp := triggerChecker.until + stepTime for ; valueTimestamp < endTimestamp; valueTimestamp += stepTime { - metricNewState, err := triggerChecker.getMetricDataState(&metrics, &previousState, &valueTimestamp, &checkPoint, logger) + metricNewState, err := triggerChecker.getMetricDataState(metrics, &previousState, &valueTimestamp, &checkPoint, logger) if err != nil { return last, current, err } @@ -459,11 +462,16 @@ func (triggerChecker *TriggerChecker) getMetricStepsStates( return last, current, nil } -func (triggerChecker *TriggerChecker) getMetricDataState(metrics *map[string]metricSource.MetricData, - lastState *moira.MetricState, valueTimestamp, checkPoint *int64, logger moira.Logger) (*moira.MetricState, error) { +func (triggerChecker *TriggerChecker) getMetricDataState( + metrics map[string]metricSource.MetricData, + lastState *moira.MetricState, + valueTimestamp, checkPoint *int64, + logger moira.Logger, +) (*moira.MetricState, error) { if *valueTimestamp <= *checkPoint { return nil, nil } + triggerExpression, values, noEmptyValues := getExpressionValues(metrics, valueTimestamp) if !noEmptyValues { return nil, nil @@ -493,18 +501,24 @@ func (triggerChecker *TriggerChecker) getMetricDataState(metrics *map[string]met ), nil } -func getExpressionValues(metrics *map[string]metricSource.MetricData, valueTimestamp *int64) (*expression.TriggerExpression, map[string]float64, bool) { +func getExpressionValues(metrics map[string]metricSource.MetricData, valueTimestamp *int64) ( + triggerExpression *expression.TriggerExpression, + values map[string]float64, + noEmptyValues bool, +) { expression := &expression.TriggerExpression{ - AdditionalTargetsValues: make(map[string]float64, len(*metrics)-1), + AdditionalTargetsValues: make(map[string]float64, len(metrics)-1), } - values := make(map[string]float64, len(*metrics)) + values = make(map[string]float64, len(metrics)) - for i := 0; i < len(*metrics); i++ { + for i := 0; i < len(metrics); i++ { targetName := fmt.Sprintf("t%d", i+1) - metric := (*metrics)[targetName] + metric := metrics[targetName] + value := metric.GetTimestampValue(*valueTimestamp) values[targetName] = value - if !moira.IsValidFloat64(value) { + + if !moira.IsFiniteNumber(value) { return expression, values, false } if i == 0 { diff --git a/checker/check_test.go b/checker/check_test.go index dc90dcf97..b75e335c9 100644 --- a/checker/check_test.go +++ b/checker/check_test.go @@ -62,7 +62,7 @@ func TestGetMetricDataState(t *testing.T) { var valueTimestamp int64 = 37 var checkPoint int64 = 47 Convey("Checkpoint more than valueTimestamp", t, func() { - metricState, err := triggerChecker.getMetricDataState(&metrics, &metricLastState, &valueTimestamp, &checkPoint, logger) + metricState, err := triggerChecker.getMetricDataState(metrics, &metricLastState, &valueTimestamp, &checkPoint, logger) So(err, ShouldBeNil) So(metricState, ShouldBeNil) }) @@ -71,7 +71,7 @@ func TestGetMetricDataState(t *testing.T) { Convey("Has all value by eventTimestamp step", func() { var valueTimestamp int64 = 42 var checkPoint int64 = 27 - metricState, err := triggerChecker.getMetricDataState(&metrics, &metricLastState, &valueTimestamp, &checkPoint, logger) + metricState, err := triggerChecker.getMetricDataState(metrics, &metricLastState, &valueTimestamp, &checkPoint, logger) So(err, ShouldBeNil) So(metricState, ShouldResemble, &moira.MetricState{ State: moira.StateOK, @@ -86,7 +86,7 @@ func TestGetMetricDataState(t *testing.T) { Convey("No value in main metric data by eventTimestamp step", func() { var valueTimestamp int64 = 66 var checkPoint int64 = 11 - metricState, err := triggerChecker.getMetricDataState(&metrics, &metricLastState, &valueTimestamp, &checkPoint, logger) + metricState, err := triggerChecker.getMetricDataState(metrics, &metricLastState, &valueTimestamp, &checkPoint, logger) So(err, ShouldBeNil) So(metricState, ShouldBeNil) }) @@ -94,7 +94,7 @@ func TestGetMetricDataState(t *testing.T) { Convey("IsAbsent in main metric data by eventTimestamp step", func() { var valueTimestamp int64 = 29 var checkPoint int64 = 11 - metricState, err := triggerChecker.getMetricDataState(&metrics, &metricLastState, &valueTimestamp, &checkPoint, logger) + metricState, err := triggerChecker.getMetricDataState(metrics, &metricLastState, &valueTimestamp, &checkPoint, logger) So(err, ShouldBeNil) So(metricState, ShouldBeNil) }) @@ -102,7 +102,7 @@ func TestGetMetricDataState(t *testing.T) { Convey("No value in additional metric data by eventTimestamp step", func() { var valueTimestamp int64 = 26 var checkPoint int64 = 11 - metricState, err := triggerChecker.getMetricDataState(&metrics, &metricLastState, &valueTimestamp, &checkPoint, logger) + metricState, err := triggerChecker.getMetricDataState(metrics, &metricLastState, &valueTimestamp, &checkPoint, logger) So(err, ShouldBeNil) So(metricState, ShouldBeNil) }) @@ -113,7 +113,7 @@ func TestGetMetricDataState(t *testing.T) { triggerChecker.trigger.ErrorValue = nil var valueTimestamp int64 = 42 var checkPoint int64 = 27 - metricState, err := triggerChecker.getMetricDataState(&metrics, &metricLastState, &valueTimestamp, &checkPoint, logger) + metricState, err := triggerChecker.getMetricDataState(metrics, &metricLastState, &valueTimestamp, &checkPoint, logger) So(err.Error(), ShouldResemble, "error value and warning value can not be empty") So(metricState, ShouldBeNil) }) @@ -974,6 +974,54 @@ func TestCheck(t *testing.T) { }) } +func TestCheckWithNoMetrics(t *testing.T) { + logger, _ := logging.GetLogger("Test") + metricsToCheck := map[string]map[string]metricSource.MetricData{} + + Convey("given triggerChecker.check is called with empty metric map", t, func() { + warnValue := float64(10) + errValue := float64(20) + pattern := "super.puper.pattern" + ttl := int64(600) + + lastCheck := moira.CheckData{ + Metrics: make(map[string]moira.MetricState), + State: moira.StateNODATA, + Timestamp: 66, + } + + triggerChecker := TriggerChecker{ + triggerID: "SuperId", + logger: logger, + config: &Config{}, + from: 3617, + until: 3667, + ttl: ttl, + ttlState: moira.TTLStateNODATA, + trigger: &moira.Trigger{ + ErrorValue: &errValue, + WarnValue: &warnValue, + TriggerType: moira.RisingTrigger, + Targets: []string{pattern}, + Patterns: []string{pattern}, + }, + lastCheck: &lastCheck, + } + aloneMetrics := map[string]metricSource.MetricData{} + checkData := newCheckData(&lastCheck, triggerChecker.until) + newCheckData, err := triggerChecker.check(metricsToCheck, aloneMetrics, checkData, logger) + + So(err, ShouldBeNil) + So(newCheckData, ShouldResemble, moira.CheckData{ + Metrics: map[string]moira.MetricState{}, + MetricsToTargetRelation: map[string]string{}, + Timestamp: triggerChecker.until, + State: moira.StateNODATA, + Score: 0, + }) + }) +} + func TestIgnoreNodataToOk(t *testing.T) { mockCtrl := gomock.NewController(t) logger, _ := logging.GetLogger("Test") @@ -1501,25 +1549,25 @@ func TestGetExpressionValues(t *testing.T) { expectedValues := map[string]float64{"t1": 0} var valueTimestamp int64 = 17 - expression, values, noEmptyValues := getExpressionValues(&metrics, &valueTimestamp) + expression, values, noEmptyValues := getExpressionValues(metrics, &valueTimestamp) So(noEmptyValues, ShouldBeTrue) So(expression, ShouldResemble, expectedExpression) So(values, ShouldResemble, expectedValues) }) Convey("last value is empty", func() { var valueTimestamp int64 = 67 - _, _, noEmptyValues := getExpressionValues(&metrics, &valueTimestamp) + _, _, noEmptyValues := getExpressionValues(metrics, &valueTimestamp) So(noEmptyValues, ShouldBeFalse) }) Convey("value before first value", func() { var valueTimestamp int64 = 11 - _, _, noEmptyValues := getExpressionValues(&metrics, &valueTimestamp) + _, _, noEmptyValues := getExpressionValues(metrics, &valueTimestamp) So(noEmptyValues, ShouldBeFalse) }) Convey("value in the middle is empty ", func() { var valueTimestamp int64 = 44 - _, _, noEmptyValues := getExpressionValues(&metrics, &valueTimestamp) + _, _, noEmptyValues := getExpressionValues(metrics, &valueTimestamp) So(noEmptyValues, ShouldBeFalse) }) @@ -1531,7 +1579,7 @@ func TestGetExpressionValues(t *testing.T) { expectedValues := map[string]float64{"t1": 3} var valueTimestamp int64 = 53 - expression, values, noEmptyValues := getExpressionValues(&metrics, &valueTimestamp) + expression, values, noEmptyValues := getExpressionValues(metrics, &valueTimestamp) So(noEmptyValues, ShouldBeTrue) So(expression, ShouldResemble, expectedExpression) So(values, ShouldResemble, expectedValues) @@ -1560,13 +1608,13 @@ func TestGetExpressionValues(t *testing.T) { Convey("t1 value in the middle is empty ", func() { var valueTimestamp int64 = 29 - _, _, noEmptyValues := getExpressionValues(&metrics, &valueTimestamp) + _, _, noEmptyValues := getExpressionValues(metrics, &valueTimestamp) So(noEmptyValues, ShouldBeFalse) }) Convey("t1 and t2 values in the middle is empty ", func() { var valueTimestamp int64 = 42 - _, _, noEmptyValues := getExpressionValues(&metrics, &valueTimestamp) + _, _, noEmptyValues := getExpressionValues(metrics, &valueTimestamp) So(noEmptyValues, ShouldBeFalse) }) @@ -1574,7 +1622,7 @@ func TestGetExpressionValues(t *testing.T) { expectedValues := map[string]float64{"t1": 0, "t2": 4} var valueTimestamp int64 = 17 - expression, values, noEmptyValues := getExpressionValues(&metrics, &valueTimestamp) + expression, values, noEmptyValues := getExpressionValues(metrics, &valueTimestamp) So(noEmptyValues, ShouldBeTrue) So(expression.MainTargetValue, ShouldBeIn, []float64{0, 4}) So(values, ShouldResemble, expectedValues) diff --git a/checker/metrics/conversion/fetched_metrics.go b/checker/metrics/conversion/fetched_metrics.go index 51cf196c2..3261946c4 100644 --- a/checker/metrics/conversion/fetched_metrics.go +++ b/checker/metrics/conversion/fetched_metrics.go @@ -38,15 +38,15 @@ func (m FetchedTargetMetrics) CleanWildcards() FetchedTargetMetrics { // the same name and returns new FetchedPatternMetrics without duplicates and slice of duplicated metrics names. func (m FetchedTargetMetrics) Deduplicate() (FetchedTargetMetrics, []string) { deduplicated := NewFetchedTargetMetricsWithCapacity(len(m)) - collectedNames := make(setHelper, len(m)) + collectedNames := make(set[string], len(m)) var duplicates []string for _, metric := range m { - if collectedNames[metric.Name] { + if collectedNames.contains(metric.Name) { duplicates = append(duplicates, metric.Name) } else { deduplicated = append(deduplicated, metric) } - collectedNames[metric.Name] = true + collectedNames[metric.Name] = void } return deduplicated, duplicates } diff --git a/checker/metrics/conversion/set_helper.go b/checker/metrics/conversion/set_helper.go index c386500e4..bbe98e5ca 100644 --- a/checker/metrics/conversion/set_helper.go +++ b/checker/metrics/conversion/set_helper.go @@ -1,37 +1,64 @@ package conversion -// setHelper is a map that represents a set of strings with corresponding methods. -type setHelper map[string]bool +var void struct{} = struct{}{} -// newSetHelperFromTriggerTargetMetrics is a constructor function for setHelper. -func newSetHelperFromTriggerTargetMetrics(metrics TriggerTargetMetrics) setHelper { - result := make(setHelper, len(metrics)) +// set[string] is a map that represents a set of strings with corresponding methods. +type set[K comparable] map[K]struct{} + +func (set set[K]) contains(key K) bool { + _, ok := set[key] + return ok +} + +func (set set[K]) insert(key K) { + set[key] = void +} + +func newSet[K comparable](value map[K]bool) set[K] { + res := make(set[K], len(value)) + + for k, v := range value { + if v { + res.insert(k) + } + } + + return res +} + +// newSetFromTriggerTargetMetrics is a constructor function for setHelper. +func newSetFromTriggerTargetMetrics(metrics TriggerTargetMetrics) set[string] { + result := make(set[string], len(metrics)) for metricName := range metrics { - result[metricName] = true + result.insert(metricName) } return result } // diff is a set relative complement operation that returns a new set with elements // that appear only in second set. -func (h setHelper) diff(other setHelper) setHelper { - result := make(setHelper, len(h)) +func (self set[string]) diff(other set[string]) set[string] { + result := make(set[string], len(self)) + for metricName := range other { - if _, ok := h[metricName]; !ok { - result[metricName] = true + if !self.contains(metricName) { + result.insert(metricName) } } + return result } // union is a sets union operation that return a new set with elements from both sets. -func (h setHelper) union(other setHelper) setHelper { - result := make(setHelper, len(h)+len(other)) - for metricName := range h { - result[metricName] = true +func (self set[string]) union(other set[string]) set[string] { + result := make(set[string], len(self)+len(other)) + + for metricName := range self { + result.insert(metricName) } for metricName := range other { - result[metricName] = true + result.insert(metricName) } + return result } diff --git a/checker/metrics/conversion/set_helper_test.go b/checker/metrics/conversion/set_helper_test.go index ab53e26c3..93c477beb 100644 --- a/checker/metrics/conversion/set_helper_test.go +++ b/checker/metrics/conversion/set_helper_test.go @@ -13,14 +13,14 @@ func Test_newSetHelperFromTriggerTargetMetrics(t *testing.T) { tests := []struct { name string args args - want setHelper + want set[string] }{ { name: "is empty", args: args{ metrics: TriggerTargetMetrics{}, }, - want: setHelper{}, + want: set[string]{}, }, { name: "is not empty", @@ -29,14 +29,14 @@ func Test_newSetHelperFromTriggerTargetMetrics(t *testing.T) { "metric.test.1": {Name: "metric.name.1"}, }, }, - want: setHelper{"metric.test.1": true}, + want: set[string]{"metric.test.1": void}, }, } Convey("TriggerPatterMetrics", t, func() { for _, tt := range tests { Convey(tt.name, func() { - actual := newSetHelperFromTriggerTargetMetrics(tt.args.metrics) + actual := newSetFromTriggerTargetMetrics(tt.args.metrics) So(actual, ShouldResemble, tt.want) }) } @@ -45,53 +45,53 @@ func Test_newSetHelperFromTriggerTargetMetrics(t *testing.T) { func Test_setHelper_union(t *testing.T) { type args struct { - other setHelper + other set[string] } tests := []struct { name string - h setHelper + h set[string] args args - want setHelper + want set[string] }{ { name: "Both empty", - h: setHelper{}, + h: set[string]{}, args: args{ - other: setHelper{}, + other: set[string]{}, }, - want: setHelper{}, + want: set[string]{}, }, { name: "Target is empty, other is not empty", - h: setHelper{}, + h: set[string]{}, args: args{ - other: setHelper{"metric.test.1": true}, + other: set[string]{"metric.test.1": void}, }, - want: setHelper{"metric.test.1": true}, + want: set[string]{"metric.test.1": void}, }, { name: "Target is not empty, other is empty", - h: setHelper{"metric.test.1": true}, + h: set[string]{"metric.test.1": void}, args: args{ - other: setHelper{}, + other: set[string]{}, }, - want: setHelper{"metric.test.1": true}, + want: set[string]{"metric.test.1": void}, }, { name: "Both are not empty", - h: setHelper{"metric.test.1": true}, + h: set[string]{"metric.test.1": void}, args: args{ - other: setHelper{"metric.test.2": true}, + other: set[string]{"metric.test.2": void}, }, - want: setHelper{"metric.test.1": true, "metric.test.2": true}, + want: set[string]{"metric.test.1": void, "metric.test.2": void}, }, { name: "Both are not empty and have same names", - h: setHelper{"metric.test.1": true, "metric.test.2": true}, + h: set[string]{"metric.test.1": void, "metric.test.2": void}, args: args{ - other: setHelper{"metric.test.2": true, "metric.test.3": true}, + other: set[string]{"metric.test.2": void, "metric.test.3": void}, }, - want: setHelper{"metric.test.1": true, "metric.test.2": true, "metric.test.3": true}, + want: set[string]{"metric.test.1": void, "metric.test.2": void, "metric.test.3": void}, }, } Convey("union", t, func() { @@ -106,37 +106,37 @@ func Test_setHelper_union(t *testing.T) { func Test_setHelper_diff(t *testing.T) { type args struct { - other setHelper + other set[string] } tests := []struct { name string - h setHelper + h set[string] args args - want setHelper + want set[string] }{ { name: "both have same elements", - h: setHelper{"t1": true, "t2": true}, + h: set[string]{"t1": void, "t2": void}, args: args{ - other: setHelper{"t1": true, "t2": true}, + other: set[string]{"t1": void, "t2": void}, }, - want: setHelper{}, + want: set[string]{}, }, { name: "other have additional values", - h: setHelper{"t1": true, "t2": true}, + h: set[string]{"t1": void, "t2": void}, args: args{ - other: setHelper{"t1": true, "t2": true, "t3": true}, + other: set[string]{"t1": void, "t2": void, "t3": void}, }, - want: setHelper{"t3": true}, + want: set[string]{"t3": void}, }, { name: "origin have additional values", - h: setHelper{"t1": true, "t2": true, "t3": true}, + h: set[string]{"t1": void, "t2": void, "t3": void}, args: args{ - other: setHelper{"t1": true, "t2": true}, + other: set[string]{"t1": void, "t2": void}, }, - want: setHelper{}, + want: set[string]{}, }, } Convey("diff", t, func() { diff --git a/checker/metrics/conversion/trigger_metrics.go b/checker/metrics/conversion/trigger_metrics.go index ea58fabea..deeaef8f9 100644 --- a/checker/metrics/conversion/trigger_metrics.go +++ b/checker/metrics/conversion/trigger_metrics.go @@ -27,7 +27,7 @@ func NewTriggerTargetMetrics(source FetchedTargetMetrics) TriggerTargetMetrics { // Populate is a function that takes the list of metric names that first appeared and // adds metrics with this names and empty values. -func (m TriggerTargetMetrics) Populate(lastMetrics map[string]bool, from, to int64) TriggerTargetMetrics { +func (m TriggerTargetMetrics) Populate(lastMetrics set[string], from, to int64) TriggerTargetMetrics { result := newTriggerTargetMetricsWithCapacity(len(m)) var firstMetric metricSource.MetricData @@ -62,35 +62,37 @@ func NewTriggerMetricsWithCapacity(capacity int) TriggerMetrics { // Populate is a function that takes TriggerMetrics and populate targets // that is missing metrics that appear in another targets except the targets that have // only alone metrics. -func (m TriggerMetrics) Populate(lastMetrics map[string]moira.MetricState, declaredAloneMetrics map[string]bool, from int64, to int64) TriggerMetrics { +func (triggerMetrics TriggerMetrics) Populate(lastMetrics map[string]moira.MetricState, declaredAloneMetrics map[string]bool, from int64, to int64) TriggerMetrics { // This one have all metrics that should be in final TriggerMetrics. // This structure filled with metrics from last check, // current received metrics alone metrics from last check. - allMetrics := make(map[string]map[string]bool, len(m)) + allMetrics := make(map[string]set[string], len(triggerMetrics)) for metricName, metricState := range lastMetrics { for targetName := range metricState.Values { if _, ok := allMetrics[targetName]; !ok { - allMetrics[targetName] = make(map[string]bool) + allMetrics[targetName] = make(set[string]) } - allMetrics[targetName][metricName] = true + + allMetrics[targetName].insert(metricName) } } - for targetName, metrics := range m { + for targetName, metrics := range triggerMetrics { + if _, ok := allMetrics[targetName]; !ok { + allMetrics[targetName] = make(set[string]) + } + for metricName := range metrics { - if _, ok := allMetrics[targetName]; !ok { - allMetrics[targetName] = make(map[string]bool) - } - allMetrics[targetName][metricName] = true + allMetrics[targetName].insert(metricName) } } - diff := m.Diff(declaredAloneMetrics) + diff := triggerMetrics.FindMissingMetrics(newSet(declaredAloneMetrics)) for targetName, metrics := range diff { for metricName := range metrics { - allMetrics[targetName][metricName] = true + allMetrics[targetName].insert(metricName) } } @@ -100,7 +102,7 @@ func (m TriggerMetrics) Populate(lastMetrics map[string]moira.MetricState, decla // if declaredAloneMetrics[targetName] { // continue // } - targetMetrics, ok := m[targetName] + targetMetrics, ok := triggerMetrics[targetName] if !ok { targetMetrics = newTriggerTargetMetricsWithCapacity(len(metrics)) } @@ -138,20 +140,21 @@ func (m TriggerMetrics) Populate(lastMetrics map[string]moira.MetricState, decla // { // "t3": {metrics}, // } -func (m TriggerMetrics) FilterAloneMetrics(declaredAloneMetrics map[string]bool) (TriggerMetrics, AloneMetrics, error) { +func (triggerMetrics TriggerMetrics) FilterAloneMetrics(declaredAloneMetrics map[string]bool) (TriggerMetrics, AloneMetrics, error) { if len(declaredAloneMetrics) == 0 { - return m, NewAloneMetricsWithCapacity(0), nil + return triggerMetrics, NewAloneMetricsWithCapacity(0), nil } - result := NewTriggerMetricsWithCapacity(len(m)) - aloneMetrics := NewAloneMetricsWithCapacity(len(m)) // Just use len of m for optimization + metricCountUpperBound := len(triggerMetrics) + result := NewTriggerMetricsWithCapacity(metricCountUpperBound) + aloneMetrics := NewAloneMetricsWithCapacity(metricCountUpperBound) errorBuilder := newErrUnexpectedAloneMetricBuilder() errorBuilder.setDeclared(declaredAloneMetrics) - for targetName, targetMetrics := range m { + for targetName, targetMetrics := range triggerMetrics { if !declaredAloneMetrics[targetName] { - result[targetName] = m[targetName] + result[targetName] = triggerMetrics[targetName] continue } @@ -171,28 +174,28 @@ func (m TriggerMetrics) FilterAloneMetrics(declaredAloneMetrics map[string]bool) return result, aloneMetrics, nil } -// Diff is a function that returns a map of target names with metric names that are absent in +// FindMissingMetrics is a function that returns a map of target names with metric names that are absent in // current target but appear in another targets. -func (m TriggerMetrics) Diff(declaredAloneMetrics map[string]bool) map[string]map[string]bool { - result := make(map[string]map[string]bool) +func (triggerMetrics TriggerMetrics) FindMissingMetrics(declaredAloneMetrics set[string]) map[string]set[string] { + result := make(map[string]set[string]) - if len(m) == 0 { + if len(triggerMetrics) == 0 { return result } - fullMetrics := make(setHelper) + fullMetrics := make(set[string]) - for targetName, targetMetrics := range m { - if declaredAloneMetrics[targetName] { + for targetName, targetMetrics := range triggerMetrics { + if declaredAloneMetrics.contains(targetName) { continue } - currentMetrics := newSetHelperFromTriggerTargetMetrics(targetMetrics) + currentMetrics := newSetFromTriggerTargetMetrics(targetMetrics) fullMetrics = fullMetrics.union(currentMetrics) } - for targetName, targetMetrics := range m { - metricsSet := newSetHelperFromTriggerTargetMetrics(targetMetrics) - if declaredAloneMetrics[targetName] { + for targetName, targetMetrics := range triggerMetrics { + metricsSet := newSetFromTriggerTargetMetrics(targetMetrics) + if declaredAloneMetrics.contains(targetName) { continue } diff := metricsSet.diff(fullMetrics) diff --git a/checker/metrics/conversion/trigger_metrics_test.go b/checker/metrics/conversion/trigger_metrics_test.go index 94d0fd961..e6baf929d 100644 --- a/checker/metrics/conversion/trigger_metrics_test.go +++ b/checker/metrics/conversion/trigger_metrics_test.go @@ -35,7 +35,7 @@ func TestNewTriggerTargetMetrics(t *testing.T) { func TestTriggerTargetMetrics_Populate(t *testing.T) { type args struct { - lastMetrics map[string]bool + lastMetrics set[string] from int64 to int64 } @@ -52,9 +52,9 @@ func TestTriggerTargetMetrics_Populate(t *testing.T) { "metric.test.2": {Name: "metric.test.2", StartTime: 17, StopTime: 67, StepTime: 60, Values: []float64{0}}, }, args: args{ - lastMetrics: map[string]bool{ - "metric.test.1": true, - "metric.test.2": true, + lastMetrics: set[string]{ + "metric.test.1": void, + "metric.test.2": void, }, from: 17, to: 67, @@ -70,9 +70,9 @@ func TestTriggerTargetMetrics_Populate(t *testing.T) { "metric.test.1": {Name: "metric.test.1", StartTime: 17, StopTime: 67, StepTime: 60, Values: []float64{0}}, }, args: args{ - lastMetrics: map[string]bool{ - "metric.test.1": true, - "metric.test.2": true, + lastMetrics: set[string]{ + "metric.test.1": void, + "metric.test.2": void, }, from: 17, to: 67, @@ -117,14 +117,14 @@ func TestTriggerMetrics_Populate(t *testing.T) { to int64 } tests := []struct { - name string - m TriggerMetrics - args args - want TriggerMetrics + name string + triggerMetrics TriggerMetrics + args args + want TriggerMetrics }{ { name: "origin do not have missing metrics", - m: TriggerMetrics{ + triggerMetrics: TriggerMetrics{ "t1": TriggerTargetMetrics{ "metric.test.1": {Name: "metric.test.1"}, "metric.test.2": {Name: "metric.test.2"}, @@ -148,7 +148,7 @@ func TestTriggerMetrics_Populate(t *testing.T) { }, { name: "origin have missing metrics", - m: TriggerMetrics{ + triggerMetrics: TriggerMetrics{ "t1": TriggerTargetMetrics{ "metric.test.1": {Name: "metric.test.1"}, }, @@ -169,14 +169,46 @@ func TestTriggerMetrics_Populate(t *testing.T) { }, }, }, + { + name: "no trigger metrics for t2, empty last check (used to panic before PR #939)", + triggerMetrics: TriggerMetrics{ + "t1": TriggerTargetMetrics{ + "metric.test.1": {Name: "metric.test.1"}, + "metric.test.2": {Name: "metric.test.2"}, + }, + "t2": TriggerTargetMetrics{}, + }, + args: args{ + lastCheck: map[string]moira.MetricState{}, + declaredAloneMetrics: map[string]bool{}, + from: 17, + to: 67, + }, + want: TriggerMetrics{ + "t1": TriggerTargetMetrics{ + "metric.test.1": {Name: "metric.test.1", StopTime: 0}, + "metric.test.2": {Name: "metric.test.2", StopTime: 0}, + }, + "t2": TriggerTargetMetrics{ + "metric.test.1": {Name: "metric.test.1", StartTime: 17, StopTime: 67, StepTime: 60, Values: []float64{math.NaN()}}, + "metric.test.2": {Name: "metric.test.2", StartTime: 17, StopTime: 67, StepTime: 60, Values: []float64{math.NaN()}}, + }, + }, + }, } Convey("Populate", t, func() { - for _, tt := range tests { - Convey(tt.name, func() { - actual := tt.m.Populate(tt.args.lastCheck, tt.args.declaredAloneMetrics, tt.args.from, tt.args.to) - So(actual, ShouldHaveLength, len(tt.want)) + for _, testCase := range tests { + Convey(testCase.name, func() { + actual := testCase.triggerMetrics.Populate( + testCase.args.lastCheck, + testCase.args.declaredAloneMetrics, + testCase.args.from, + testCase.args.to, + ) + + So(actual, ShouldHaveLength, len(testCase.want)) for targetName, metrics := range actual { - wantMetrics, ok := tt.want[targetName] + wantMetrics, ok := testCase.want[targetName] So(metrics, ShouldHaveLength, len(wantMetrics)) So(ok, ShouldBeTrue) for metricName, actualMetric := range metrics { @@ -305,13 +337,13 @@ func TestTriggerMetrics_FilterAloneMetrics(t *testing.T) { func TestTriggerMetrics_Diff(t *testing.T) { tests := []struct { name string - m TriggerMetrics + triggerMetrics TriggerMetrics declaredAloneMetrics map[string]bool - want map[string]map[string]bool + want map[string]set[string] }{ { name: "all targets have same metrics", - m: TriggerMetrics{ + triggerMetrics: TriggerMetrics{ "t1": TriggerTargetMetrics{ "metric.test.1": {Name: "metric.test.1"}, "metric.test.2": {Name: "metric.test.2"}, @@ -324,11 +356,11 @@ func TestTriggerMetrics_Diff(t *testing.T) { }, }, declaredAloneMetrics: map[string]bool{}, - want: map[string]map[string]bool{}, + want: map[string]set[string]{}, }, { name: "one target have missed metric", - m: TriggerMetrics{ + triggerMetrics: TriggerMetrics{ "t1": TriggerTargetMetrics{ "metric.test.1": {Name: "metric.test.1"}, "metric.test.2": {Name: "metric.test.2"}, @@ -340,11 +372,11 @@ func TestTriggerMetrics_Diff(t *testing.T) { }, }, declaredAloneMetrics: map[string]bool{}, - want: map[string]map[string]bool{"t2": {"metric.test.3": true}}, + want: map[string]set[string]{"t2": {"metric.test.3": void}}, }, { name: "one target is alone metric", - m: TriggerMetrics{ + triggerMetrics: TriggerMetrics{ "t1": TriggerTargetMetrics{ "metric.test.1": {Name: "metric.test.1"}, "metric.test.2": {Name: "metric.test.2"}, @@ -355,11 +387,11 @@ func TestTriggerMetrics_Diff(t *testing.T) { }, }, declaredAloneMetrics: map[string]bool{"t2": true}, - want: map[string]map[string]bool{}, + want: map[string]set[string]{}, }, { name: "another target have missed metric", - m: TriggerMetrics{ + triggerMetrics: TriggerMetrics{ "t1": TriggerTargetMetrics{ "metric.test.1": {Name: "metric.test.1"}, "metric.test.2": {Name: "metric.test.2"}, @@ -373,11 +405,11 @@ func TestTriggerMetrics_Diff(t *testing.T) { }, }, declaredAloneMetrics: map[string]bool{}, - want: map[string]map[string]bool{"t1": {"metric.test.4": true}}, + want: map[string]set[string]{"t1": {"metric.test.4": void}}, }, { name: "one target is empty", - m: TriggerMetrics{ + triggerMetrics: TriggerMetrics{ "t1": TriggerTargetMetrics{}, "t2": TriggerTargetMetrics{ "metric.test.1": {Name: "metric.test.1"}, @@ -387,16 +419,16 @@ func TestTriggerMetrics_Diff(t *testing.T) { }, }, declaredAloneMetrics: map[string]bool{}, - want: map[string]map[string]bool{"t1": { - "metric.test.1": true, - "metric.test.2": true, - "metric.test.3": true, - "metric.test.4": true, + want: map[string]set[string]{"t1": { + "metric.test.1": void, + "metric.test.2": void, + "metric.test.3": void, + "metric.test.4": void, }}, }, { name: "Multiple targets with different metrics", - m: TriggerMetrics{ + triggerMetrics: TriggerMetrics{ "t1": TriggerTargetMetrics{ "metric.test.2": {Name: "metric.test.2"}, "metric.test.3": {Name: "metric.test.3"}, @@ -419,27 +451,27 @@ func TestTriggerMetrics_Diff(t *testing.T) { }, }, declaredAloneMetrics: map[string]bool{}, - want: map[string]map[string]bool{ + want: map[string]set[string]{ "t1": { - "metric.test.1": true, + "metric.test.1": void, }, "t2": { - "metric.test.2": true, + "metric.test.2": void, }, "t3": { - "metric.test.3": true, + "metric.test.3": void, }, "t4": { - "metric.test.4": true, + "metric.test.4": void, }, }, }, } Convey("Diff", t, func() { - for _, tt := range tests { - Convey(tt.name, func() { - actual := tt.m.Diff(tt.declaredAloneMetrics) - So(actual, ShouldResemble, tt.want) + for _, testCase := range tests { + Convey(testCase.name, func() { + actual := testCase.triggerMetrics.FindMissingMetrics(newSet(testCase.declaredAloneMetrics)) + So(actual, ShouldResemble, testCase.want) }) } }) diff --git a/checker/worker/handler.go b/checker/worker/handler.go index 24ef5c947..dddb81c6d 100644 --- a/checker/worker/handler.go +++ b/checker/worker/handler.go @@ -34,8 +34,7 @@ func (check *Checker) startTriggerHandler(triggerIDsToCheck <-chan string, metri } } -func (check *Checker) handleTrigger(triggerID string, metrics *metrics.CheckMetrics) error { - var err error +func (check *Checker) handleTrigger(triggerID string, metrics *metrics.CheckMetrics) (err error) { defer func() { if r := recover(); r != nil { err = fmt.Errorf("panic: '%s' stack: %s", r, debug.Stack()) diff --git a/helpers.go b/helpers.go index 9a999fb7a..1c4a367a5 100644 --- a/helpers.go +++ b/helpers.go @@ -76,15 +76,9 @@ func UseFloat64(f *float64) float64 { return *f } -// IsValidFloat64 checks float64 for Inf and NaN. If it is then float64 is not valid -func IsValidFloat64(val float64) bool { - if math.IsNaN(val) { - return false - } - if math.IsInf(val, 0) { - return false - } - return true +// IsFiniteNumber checks float64 for Inf and NaN. If it is then float64 is not valid +func IsFiniteNumber(val float64) bool { + return !(math.IsNaN(val) || math.IsInf(val, 0)) } // Subset return whether first is a subset of second diff --git a/helpers_test.go b/helpers_test.go index d2800ee79..803376070 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -203,10 +203,10 @@ func TestChunkSlice(t *testing.T) { func TestIsValidFloat64(t *testing.T) { Convey("values +Inf -Inf and NaN is invalid", t, func() { - So(IsValidFloat64(math.NaN()), ShouldBeFalse) - So(IsValidFloat64(math.Inf(-1)), ShouldBeFalse) - So(IsValidFloat64(math.Inf(1)), ShouldBeFalse) - So(IsValidFloat64(3.14), ShouldBeTrue) + So(IsFiniteNumber(math.NaN()), ShouldBeFalse) + So(IsFiniteNumber(math.Inf(-1)), ShouldBeFalse) + So(IsFiniteNumber(math.Inf(1)), ShouldBeFalse) + So(IsFiniteNumber(3.14), ShouldBeTrue) }) } diff --git a/metric_source/metric_data.go b/metric_source/metric_data.go index f3f020535..a70b49804 100644 --- a/metric_source/metric_data.go +++ b/metric_source/metric_data.go @@ -47,7 +47,9 @@ func (metricData *MetricData) GetTimestampValue(valueTimestamp int64) float64 { if valueTimestamp < metricData.StartTime { return math.NaN() } + valueIndex := int((valueTimestamp - metricData.StartTime) / metricData.StepTime) + if len(metricData.Values) <= valueIndex { return math.NaN() } diff --git a/plotting/curve.go b/plotting/curve.go index 186e35eed..c86531708 100644 --- a/plotting/curve.go +++ b/plotting/curve.go @@ -60,7 +60,7 @@ func describePlotCurves(metricData metricSource.MetricData) []plotCurve { for valInd := start; valInd < len(metricData.Values); valInd++ { pointValue := metricData.Values[valInd] - if moira.IsValidFloat64(pointValue) { + if moira.IsFiniteNumber(pointValue) { timeStampValue := moira.Int64ToTime(timeStamp) curves[curvesInd].timeStamps = append(curves[curvesInd].timeStamps, timeStampValue) curves[curvesInd].values = append(curves[curvesInd].values, pointValue) @@ -80,7 +80,7 @@ func resolveFirstPoint(metricData metricSource.MetricData) (int, int64) { start := 0 startTime := metricData.StartTime for _, metricVal := range metricData.Values { - if !moira.IsValidFloat64(metricVal) { + if !moira.IsFiniteNumber(metricVal) { start++ startTime += metricData.StepTime } else { diff --git a/plotting/limits.go b/plotting/limits.go index 5cc91f243..c1f975f25 100644 --- a/plotting/limits.go +++ b/plotting/limits.go @@ -33,7 +33,7 @@ func resolveLimits(metricsData []metricSource.MetricData) plotLimits { allTimes := make([]time.Time, 0) for _, metricData := range metricsData { for _, metricValue := range metricData.Values { - if moira.IsValidFloat64(metricValue) { + if moira.IsFiniteNumber(metricValue) { allValues = append(allValues, metricValue) } } From 27662e7519888065a63eb6b13073b9b2a701eb66 Mon Sep 17 00:00:00 2001 From: Iurii Pliner Date: Wed, 25 Oct 2023 07:59:33 +0100 Subject: [PATCH 3/5] fix: calculate RemoteAllowed based on remote/prom configs (#945) Currently, RemoteAllowed controls if triggers of both "remote" types (remote graphite and prometheus) could be created, but it is calculated only based on remote graphite config. In our case we only use prometheus triggers, so it is a little odd to fill in remote graphite config section to allow to create prometheus triggers. --- cmd/api/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/api/main.go b/cmd/api/main.go index 879a99d55..bbf76fffd 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -137,7 +137,7 @@ func main() { prometheusSource, ) - webConfigContent, err := applicationConfig.Web.getSettings(remoteConfig.Enabled) + webConfigContent, err := applicationConfig.Web.getSettings(remoteConfig.Enabled || prometheusConfig.Enabled) if err != nil { logger.Fatal(). Error(err). From 3faa4fc2fb1f428c37774bfa8b7d9b58862cd82a Mon Sep 17 00:00:00 2001 From: Xenia N Date: Wed, 25 Oct 2023 17:59:08 +0600 Subject: [PATCH 4/5] feat(notifier): add metric to dropped notifications count (#942) --- metrics/notifier.go | 20 +++++++++++++++++-- notifier/notifier.go | 43 ++++++++++++++++++++--------------------- notifier/registrator.go | 2 ++ 3 files changed, 41 insertions(+), 24 deletions(-) diff --git a/metrics/notifier.go b/metrics/notifier.go index a2b5889e9..bcf61daa3 100644 --- a/metrics/notifier.go +++ b/metrics/notifier.go @@ -1,6 +1,6 @@ package metrics -// NotifierMetrics is a collection of metrics used in notifier +// NotifierMetrics is a collection of metrics used in notifier. type NotifierMetrics struct { SubsMalformed Meter EventsReceived Meter @@ -10,11 +10,12 @@ type NotifierMetrics struct { SendingFailed Meter SendersOkMetrics MetersCollection SendersFailedMetrics MetersCollection + SendersDroppedNotifications MetersCollection PlotsBuildDurationMs Histogram PlotsEvaluateTriggerDurationMs Histogram } -// ConfigureNotifierMetrics is notifier metrics configurator +// ConfigureNotifierMetrics is notifier metrics configurator. func ConfigureNotifierMetrics(registry Registry, prefix string) *NotifierMetrics { return &NotifierMetrics{ SubsMalformed: registry.NewMeter("subs", "malformed"), @@ -25,7 +26,22 @@ func ConfigureNotifierMetrics(registry Registry, prefix string) *NotifierMetrics SendingFailed: registry.NewMeter("sending", "failed"), SendersOkMetrics: NewMetersCollection(registry), SendersFailedMetrics: NewMetersCollection(registry), + SendersDroppedNotifications: NewMetersCollection(registry), PlotsBuildDurationMs: registry.NewHistogram("plots", "build", "duration", "ms"), PlotsEvaluateTriggerDurationMs: registry.NewHistogram("plots", "evaluate", "trigger", "duration", "ms"), } } + +// MarkSendersDroppedNotifications marks metrics as 1 by contactType for dropped notifications. +func (metrics *NotifierMetrics) MarkSendersDroppedNotifications(contactType string) { + if metric, found := metrics.SendersDroppedNotifications.GetRegisteredMeter(contactType); found { + metric.Mark(1) + } +} + +// MarkSendersOkMetrics marks metrics as 1 by contactType when notifications were successfully sent. +func (metrics *NotifierMetrics) MarkSendersOkMetrics(contactType string) { + if metric, found := metrics.SendersOkMetrics.GetRegisteredMeter(contactType); found { + metric.Mark(1) + } +} diff --git a/notifier/notifier.go b/notifier/notifier.go index ea945412d..fd0752c6d 100644 --- a/notifier/notifier.go +++ b/notifier/notifier.go @@ -208,30 +208,29 @@ func (notifier *StandardNotifier) runSender(sender moira.Sender, ch chan Notific err = sender.SendEvents(pkg.Events, pkg.Contact, pkg.Trigger, plots, pkg.Throttled) if err == nil { - if metric, found := notifier.metrics.SendersOkMetrics.GetRegisteredMeter(pkg.Contact.Type); found { - metric.Mark(1) - } - } else { - switch e := err.(type) { - case moira.SenderBrokenContactError: + notifier.metrics.MarkSendersOkMetrics(pkg.Contact.Type) + continue + } + switch e := err.(type) { + case moira.SenderBrokenContactError: + log.Warning(). + Error(e). + Msg("Cannot send to broken contact") + notifier.metrics.MarkSendersDroppedNotifications(pkg.Contact.Type) + default: + if pkg.FailCount > notifier.config.MaxFailAttemptToSendAvailable { + log.Error(). + Error(err). + Int("fail_count", pkg.FailCount). + Msg("Cannot send notification") + } else { log.Warning(). - Error(e). - Msg("Cannot send to broken contact") - - default: - if pkg.FailCount > notifier.config.MaxFailAttemptToSendAvailable { - log.Error(). - Error(err). - Int("fail_count", pkg.FailCount). - Msg("Cannot send notification") - } else { - log.Warning(). - Error(err). - Msg("Cannot send notification") - } - - notifier.resend(&pkg, err.Error()) + Error(err). + Msg("Cannot send notification") + notifier.metrics.MarkSendersDroppedNotifications(pkg.Contact.Type) } + + notifier.resend(&pkg, err.Error()) } } } diff --git a/notifier/registrator.go b/notifier/registrator.go index 841e064da..e31f6e7b1 100644 --- a/notifier/registrator.go +++ b/notifier/registrator.go @@ -112,6 +112,7 @@ func (notifier *StandardNotifier) RegisterSender(senderSettings map[string]inter default: senderIdent = senderType } + err := sender.Init(senderSettings, notifier.logger, notifier.config.Location, notifier.config.DateTimeFormat) if err != nil { return fmt.Errorf("failed to initialize sender [%s], err [%s]", senderIdent, err.Error()) @@ -120,6 +121,7 @@ func (notifier *StandardNotifier) RegisterSender(senderSettings map[string]inter notifier.senders[senderIdent] = eventsChannel notifier.metrics.SendersOkMetrics.RegisterMeter(senderIdent, getGraphiteSenderIdent(senderIdent), "sends_ok") notifier.metrics.SendersFailedMetrics.RegisterMeter(senderIdent, getGraphiteSenderIdent(senderIdent), "sends_failed") + notifier.metrics.SendersDroppedNotifications.RegisterMeter(senderIdent, getGraphiteSenderIdent(senderIdent), "notifications_dropped") notifier.runSenders(sender, eventsChannel) notifier.logger.Info(). String("sender_id", senderIdent). From 8a9a727e51eeffc3adc2e3c98b13c20a551020a4 Mon Sep 17 00:00:00 2001 From: almostinf <87192879+almostinf@users.noreply.github.com> Date: Thu, 26 Oct 2023 13:52:53 +0300 Subject: [PATCH 5/5] fix(api): fix remove contact in teams (#940) --- api/handler/contact.go | 4 +- api/handler/contact_test.go | 802 +++++++++++++++++++++++++++++++++ database/redis/reply/metric.go | 4 +- 3 files changed, 806 insertions(+), 4 deletions(-) create mode 100644 api/handler/contact_test.go diff --git a/api/handler/contact.go b/api/handler/contact.go index 9c4f338b5..084c5f3f5 100644 --- a/api/handler/contact.go +++ b/api/handler/contact.go @@ -100,7 +100,7 @@ func createNewContact(writer http.ResponseWriter, request *http.Request) { } userLogin := middleware.GetLogin(request) - if err := controller.CreateContact(database, contact, userLogin, ""); err != nil { + if err := controller.CreateContact(database, contact, userLogin, contact.TeamID); err != nil { render.Render(writer, request, err) //nolint return } @@ -176,7 +176,7 @@ func updateContact(writer http.ResponseWriter, request *http.Request) { // @router /contact/{contactID} [delete] func removeContact(writer http.ResponseWriter, request *http.Request) { contactData := request.Context().Value(contactKey).(moira.ContactData) - err := controller.RemoveContact(database, contactData.ID, contactData.User, "") + err := controller.RemoveContact(database, contactData.ID, contactData.User, contactData.Team) if err != nil { render.Render(writer, request, err) //nolint } diff --git a/api/handler/contact_test.go b/api/handler/contact_test.go new file mode 100644 index 000000000..03aae2e77 --- /dev/null +++ b/api/handler/contact_test.go @@ -0,0 +1,802 @@ +package handler + +import ( + "bytes" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/golang/mock/gomock" + "github.com/moira-alert/moira" + "github.com/moira-alert/moira/api" + "github.com/moira-alert/moira/api/dto" + "github.com/moira-alert/moira/api/middleware" + db "github.com/moira-alert/moira/database" + mock_moira_alert "github.com/moira-alert/moira/mock/moira-alert" + . "github.com/smartystreets/goconvey/convey" +) + +const ( + ContactIDKey = "contactID" + ContactKey = "contact" + LoginKey = "login" + defaultContact = "testContact" + defaultLogin = "testLogin" +) + +func TestGetAllContacts(t *testing.T) { + Convey("Test get all contacts", t, func() { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + responseWriter := httptest.NewRecorder() + mockDb := mock_moira_alert.NewMockDatabase(mockCtrl) + + testErr := errors.New("test error") + + Convey("Correctly returns all contacts", func() { + mockDb.EXPECT().GetAllContacts().Return([]*moira.ContactData{ + { + ID: defaultContact, + Type: "mail", + Value: "moira@skbkontur.ru", + User: "moira", + Team: "", + }, + }, nil).Times(1) + database = mockDb + + expected := &dto.ContactList{ + List: []*moira.ContactData{ + { + ID: defaultContact, + Type: "mail", + Value: "moira@skbkontur.ru", + User: "moira", + Team: "", + }, + }, + } + + testRequest := httptest.NewRequest(http.MethodGet, "/contact", nil) + + getAllContacts(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, _ := io.ReadAll(response.Body) + contents := string(contentBytes) + actual := &dto.ContactList{} + err := json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, expected) + So(response.StatusCode, ShouldEqual, http.StatusOK) + }) + + Convey("Internal server error when trying to get all contacts", func() { + mockDb.EXPECT().GetAllContacts().Return(nil, testErr).Times(1) + database = mockDb + + expected := &api.ErrorResponse{ + StatusText: "Internal Server Error", + ErrorText: testErr.Error(), + } + + testRequest := httptest.NewRequest(http.MethodGet, "/contact", nil) + + getAllContacts(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, _ := io.ReadAll(response.Body) + contents := string(contentBytes) + actual := &api.ErrorResponse{} + err := json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, expected) + So(response.StatusCode, ShouldEqual, http.StatusInternalServerError) + }) + }) +} + +func TestGetContactById(t *testing.T) { + Convey("Test get contact by id", t, func() { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + responseWriter := httptest.NewRecorder() + mockDb := mock_moira_alert.NewMockDatabase(mockCtrl) + + testErr := errors.New("test error") + + Convey("Correctly returns contact by id", func() { + contactID := defaultContact + mockDb.EXPECT().GetContact(contactID).Return(moira.ContactData{ + ID: contactID, + Type: "mail", + Value: "moira@skbkontur.ru", + User: "test", + Team: "", + }, nil).Times(1) + database = mockDb + + expected := &dto.Contact{ + ID: contactID, + Type: "mail", + Value: "moira@skbkontur.ru", + User: "test", + TeamID: "", + } + + testRequest := httptest.NewRequest(http.MethodGet, "/contact/"+contactID, nil) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactIDKey, contactID)) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactKey, moira.ContactData{ID: contactID})) + + getContactById(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &dto.Contact{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, expected) + So(response.StatusCode, ShouldEqual, http.StatusOK) + }) + + Convey("Internal server error when trying to get a contact by id", func() { + contactID := defaultContact + mockDb.EXPECT().GetContact(contactID).Return(moira.ContactData{}, testErr).Times(1) + database = mockDb + + expected := &api.ErrorResponse{ + StatusText: "Internal Server Error", + ErrorText: testErr.Error(), + } + + testRequest := httptest.NewRequest(http.MethodGet, "/contact/"+contactID, nil) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactIDKey, contactID)) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactKey, moira.ContactData{ID: contactID})) + + getContactById(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &api.ErrorResponse{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, expected) + So(response.StatusCode, ShouldEqual, http.StatusInternalServerError) + }) + }) +} + +func TestCreateNewContact(t *testing.T) { + Convey("Test create new contact", t, func() { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + responseWriter := httptest.NewRecorder() + mockDb := mock_moira_alert.NewMockDatabase(mockCtrl) + + login := defaultLogin + testErr := errors.New("test error") + + newContactDto := &dto.Contact{ + ID: defaultContact, + Type: "mail", + Value: "moira@skbkontur.ru", + User: login, + TeamID: "", + } + + Convey("Correctly create new contact with the given id", func() { + jsonContact, err := json.Marshal(newContactDto) + So(err, ShouldBeNil) + + mockDb.EXPECT().GetContact(defaultContact).Return(moira.ContactData{}, db.ErrNil).Times(1) + mockDb.EXPECT().SaveContact(&moira.ContactData{ + ID: newContactDto.ID, + Type: newContactDto.Type, + Value: newContactDto.Value, + User: newContactDto.User, + Team: newContactDto.TeamID, + }).Return(nil).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodPut, "/contact", bytes.NewBuffer(jsonContact)) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), LoginKey, login)) + testRequest.Header.Add("content-type", "application/json") + + createNewContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &dto.Contact{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, newContactDto) + So(response.StatusCode, ShouldEqual, http.StatusOK) + }) + + Convey("Correctly create new contact without given id", func() { + newContactDto.ID = "" + defer func() { + newContactDto.ID = defaultContact + }() + + jsonContact, err := json.Marshal(newContactDto) + So(err, ShouldBeNil) + + mockDb.EXPECT().SaveContact(gomock.Any()).Return(nil).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodPut, "/contact", bytes.NewBuffer(jsonContact)) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), LoginKey, login)) + testRequest.Header.Add("content-type", "application/json") + + createNewContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &dto.Contact{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual.TeamID, ShouldEqual, newContactDto.TeamID) + So(actual.Type, ShouldEqual, newContactDto.Type) + So(actual.User, ShouldEqual, newContactDto.User) + So(actual.Value, ShouldEqual, newContactDto.Value) + So(response.StatusCode, ShouldEqual, http.StatusOK) + }) + + Convey("Trying to create a new contact with the id of an existing contact", func() { + expected := &api.ErrorResponse{ + StatusText: "Invalid request", + ErrorText: "contact with this ID already exists", + } + jsonContact, err := json.Marshal(newContactDto) + So(err, ShouldBeNil) + + mockDb.EXPECT().GetContact(newContactDto.ID).Return(moira.ContactData{ + ID: newContactDto.ID, + Type: newContactDto.Type, + Value: newContactDto.Value, + User: newContactDto.User, + Team: newContactDto.TeamID, + }, nil).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodPut, "/contact", bytes.NewBuffer(jsonContact)) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), LoginKey, login)) + testRequest.Header.Add("content-type", "application/json") + + createNewContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &api.ErrorResponse{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, expected) + So(response.StatusCode, ShouldEqual, http.StatusBadRequest) + }) + + Convey("Internal error when trying to create a new contact with id", func() { + expected := &api.ErrorResponse{ + StatusText: "Internal Server Error", + ErrorText: testErr.Error(), + } + jsonContact, err := json.Marshal(newContactDto) + So(err, ShouldBeNil) + + mockDb.EXPECT().GetContact(newContactDto.ID).Return(moira.ContactData{}, testErr).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodPut, "/contact", bytes.NewBuffer(jsonContact)) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), LoginKey, login)) + testRequest.Header.Add("content-type", "application/json") + + createNewContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &api.ErrorResponse{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, expected) + So(response.StatusCode, ShouldEqual, http.StatusInternalServerError) + }) + + Convey("Trying to create a contact when both userLogin and teamID specified", func() { + newContactDto.TeamID = "test" + defer func() { + newContactDto.TeamID = "" + }() + + expected := &api.ErrorResponse{ + StatusText: "Internal Server Error", + ErrorText: "CreateContact: cannot create contact when both userLogin and teamID specified", + } + jsonContact, err := json.Marshal(newContactDto) + So(err, ShouldBeNil) + + testRequest := httptest.NewRequest(http.MethodPut, "/contact", bytes.NewBuffer(jsonContact)) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), LoginKey, login)) + testRequest.Header.Add("content-type", "application/json") + + createNewContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &api.ErrorResponse{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, expected) + So(response.StatusCode, ShouldEqual, http.StatusInternalServerError) + }) + }) +} + +func TestUpdateContact(t *testing.T) { + Convey("Test update contact", t, func() { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + responseWriter := httptest.NewRecorder() + mockDb := mock_moira_alert.NewMockDatabase(mockCtrl) + + testErr := errors.New("test error") + contactID := defaultContact + updatedContactDto := &dto.Contact{ + ID: contactID, + Type: "mail", + Value: "moira@skbkontur.ru", + User: "test", + TeamID: "", + } + + Convey("Successful contact updated", func() { + jsonContact, err := json.Marshal(updatedContactDto) + So(err, ShouldBeNil) + + mockDb.EXPECT().SaveContact(&moira.ContactData{ + ID: updatedContactDto.ID, + Type: updatedContactDto.Type, + Value: updatedContactDto.Value, + User: updatedContactDto.User, + }).Return(nil).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodPut, "/contact/"+contactID, bytes.NewBuffer(jsonContact)) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactKey, moira.ContactData{ + ID: contactID, + Type: updatedContactDto.Type, + Value: updatedContactDto.Value, + User: updatedContactDto.User, + })) + testRequest.Header.Add("content-type", "application/json") + + updateContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &dto.Contact{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, updatedContactDto) + So(response.StatusCode, ShouldEqual, http.StatusOK) + }) + + Convey("Internal error when trying to update contact", func() { + expected := &api.ErrorResponse{ + StatusText: "Internal Server Error", + ErrorText: testErr.Error(), + } + jsonContact, err := json.Marshal(updatedContactDto) + So(err, ShouldBeNil) + + mockDb.EXPECT().SaveContact(&moira.ContactData{ + ID: updatedContactDto.ID, + Type: updatedContactDto.Type, + Value: updatedContactDto.Value, + User: updatedContactDto.User, + }).Return(testErr).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodPut, "/contact/"+contactID, bytes.NewBuffer(jsonContact)) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactKey, moira.ContactData{ + ID: contactID, + Type: updatedContactDto.Type, + Value: updatedContactDto.Value, + User: updatedContactDto.User, + })) + testRequest.Header.Add("content-type", "application/json") + + updateContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &api.ErrorResponse{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, expected) + So(response.StatusCode, ShouldEqual, http.StatusInternalServerError) + }) + }) +} + +func TestRemoveContact(t *testing.T) { + Convey("Test remove contact", t, func() { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + responseWriter := httptest.NewRecorder() + mockDb := mock_moira_alert.NewMockDatabase(mockCtrl) + + testErr := errors.New("test error") + contactID := defaultContact + + Convey("Successful deletion of a contact without user, team id and subscriptions", func() { + mockDb.EXPECT().GetSubscriptions([]string{}).Return([]*moira.SubscriptionData{}, nil).Times(1) + mockDb.EXPECT().RemoveContact(contactID).Return(nil).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodDelete, "/contact/"+contactID, nil) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactKey, moira.ContactData{ + ID: contactID, + Type: "mail", + Value: "moira@skbkontur.ru", + })) + testRequest.Header.Add("content-type", "application/json") + + removeContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + actual := contentBytes + + So(actual, ShouldResemble, []byte{}) + So(response.StatusCode, ShouldEqual, http.StatusOK) + }) + + Convey("Successful deletion of a contact without team id and subscriptions", func() { + mockDb.EXPECT().GetUserSubscriptionIDs("test").Return([]string{}, nil).Times(1) + mockDb.EXPECT().GetSubscriptions([]string{}).Return([]*moira.SubscriptionData{}, nil).Times(1) + mockDb.EXPECT().RemoveContact(contactID).Return(nil).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodDelete, "/contact/"+contactID, nil) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactKey, moira.ContactData{ + ID: contactID, + Type: "mail", + Value: "moira@skbkontur.ru", + User: "test", + })) + testRequest.Header.Add("content-type", "application/json") + + removeContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + actual := contentBytes + + So(actual, ShouldResemble, []byte{}) + So(response.StatusCode, ShouldEqual, http.StatusOK) + }) + + Convey("Successful deletion of a contact without user id and subscriptions", func() { + mockDb.EXPECT().GetTeamSubscriptionIDs("test").Return([]string{}, nil).Times(1) + mockDb.EXPECT().GetSubscriptions([]string{}).Return([]*moira.SubscriptionData{}, nil).Times(1) + mockDb.EXPECT().RemoveContact(contactID).Return(nil).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodDelete, "/contact/"+contactID, nil) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactKey, moira.ContactData{ + ID: contactID, + Type: "mail", + Value: "moira@skbkontur.ru", + Team: "test", + })) + testRequest.Header.Add("content-type", "application/json") + + removeContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + actual := contentBytes + + So(actual, ShouldResemble, []byte{}) + So(response.StatusCode, ShouldEqual, http.StatusOK) + }) + + Convey("Successful deletion of a contact without subscriptions", func() { + mockDb.EXPECT().GetUserSubscriptionIDs("test").Return([]string{}, nil).Times(1) + mockDb.EXPECT().GetTeamSubscriptionIDs("test").Return([]string{}, nil).Times(1) + mockDb.EXPECT().GetSubscriptions([]string{}).Return([]*moira.SubscriptionData{}, nil).Times(1) + mockDb.EXPECT().RemoveContact(contactID).Return(nil).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodDelete, "/contact/"+contactID, nil) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactKey, moira.ContactData{ + ID: contactID, + Type: "mail", + Value: "moira@skbkontur.ru", + User: "test", + Team: "test", + })) + testRequest.Header.Add("content-type", "application/json") + + removeContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + actual := contentBytes + + So(actual, ShouldResemble, []byte{}) + So(response.StatusCode, ShouldEqual, http.StatusOK) + }) + + Convey("Error when deleting a contact, the user has existing subscriptions", func() { + expected := &api.ErrorResponse{ + StatusText: "Invalid request", + ErrorText: "this contact is being used in following subscriptions: (tags: test)", + } + + mockDb.EXPECT().GetUserSubscriptionIDs("test").Return([]string{"test"}, nil).Times(1) + mockDb.EXPECT().GetSubscriptions([]string{"test"}).Return([]*moira.SubscriptionData{ + { + Contacts: []string{"testContact"}, + Tags: []string{"test"}, + }, + }, nil).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodDelete, "/contact/"+contactID, nil) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactKey, moira.ContactData{ + ID: contactID, + Type: "mail", + Value: "moira@skbkontur.ru", + User: "test", + })) + testRequest.Header.Add("content-type", "application/json") + + removeContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &api.ErrorResponse{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, expected) + So(response.StatusCode, ShouldEqual, http.StatusBadRequest) + }) + + Convey("Error when deleting a contact, the team has existing subscriptions", func() { + expected := &api.ErrorResponse{ + StatusText: "Invalid request", + ErrorText: "this contact is being used in following subscriptions: (tags: test)", + } + + mockDb.EXPECT().GetTeamSubscriptionIDs("test").Return([]string{"test"}, nil).Times(1) + mockDb.EXPECT().GetSubscriptions([]string{"test"}).Return([]*moira.SubscriptionData{ + { + Contacts: []string{"testContact"}, + Tags: []string{"test"}, + }, + }, nil).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodDelete, "/contact/"+contactID, nil) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactKey, moira.ContactData{ + ID: contactID, + Type: "mail", + Value: "moira@skbkontur.ru", + Team: "test", + })) + testRequest.Header.Add("content-type", "application/json") + + removeContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &api.ErrorResponse{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, expected) + So(response.StatusCode, ShouldEqual, http.StatusBadRequest) + }) + + Convey("Error when deleting a contact, the user and team has existing subscriptions", func() { + expected := &api.ErrorResponse{ + StatusText: "Invalid request", + ErrorText: "this contact is being used in following subscriptions: (tags: test1), (tags: test2)", + } + + mockDb.EXPECT().GetUserSubscriptionIDs("test1").Return([]string{"test1"}, nil).Times(1) + mockDb.EXPECT().GetTeamSubscriptionIDs("test2").Return([]string{"test2"}, nil).Times(1) + mockDb.EXPECT().GetSubscriptions([]string{"test1", "test2"}).Return([]*moira.SubscriptionData{ + { + Contacts: []string{"testContact"}, + Tags: []string{"test1"}, + }, + { + Contacts: []string{"testContact"}, + Tags: []string{"test2"}, + }, + }, nil).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodDelete, "/contact/"+contactID, nil) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactKey, moira.ContactData{ + ID: contactID, + Type: "mail", + Value: "moira@skbkontur.ru", + Team: "test2", + User: "test1", + })) + testRequest.Header.Add("content-type", "application/json") + + removeContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &api.ErrorResponse{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, expected) + So(response.StatusCode, ShouldEqual, http.StatusBadRequest) + }) + + Convey("Internal server error when deleting of a contact without user, team id and subscriptions", func() { + expected := &api.ErrorResponse{ + StatusText: "Internal Server Error", + ErrorText: testErr.Error(), + } + + mockDb.EXPECT().GetSubscriptions([]string{}).Return([]*moira.SubscriptionData{}, nil).Times(1) + mockDb.EXPECT().RemoveContact(contactID).Return(testErr).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodDelete, "/contact/"+contactID, nil) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactKey, moira.ContactData{ + ID: contactID, + Type: "mail", + Value: "moira@skbkontur.ru", + })) + testRequest.Header.Add("content-type", "application/json") + + removeContact(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &api.ErrorResponse{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, expected) + So(response.StatusCode, ShouldEqual, http.StatusInternalServerError) + }) + }) +} + +func TestSendTestContactNotification(t *testing.T) { + Convey("Test send test contact notification", t, func() { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + responseWriter := httptest.NewRecorder() + mockDb := mock_moira_alert.NewMockDatabase(mockCtrl) + + testErr := errors.New("test error") + contactID := defaultContact + + Convey("Successful send test contact notification", func() { + mockDb.EXPECT().PushNotificationEvent(gomock.Any(), false).Return(nil).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodPost, "/contact/"+contactID+"/test", nil) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactIDKey, contactID)) + + sendTestContactNotification(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + actual := contentBytes + + So(actual, ShouldResemble, []byte{}) + So(response.StatusCode, ShouldEqual, http.StatusOK) + }) + + Convey("Internal server error when sendin test contact notification", func() { + expected := &api.ErrorResponse{ + StatusText: "Internal Server Error", + ErrorText: testErr.Error(), + } + + mockDb.EXPECT().PushNotificationEvent(gomock.Any(), false).Return(testErr).Times(1) + database = mockDb + + testRequest := httptest.NewRequest(http.MethodPost, "/contact/"+contactID+"/test", nil) + testRequest = testRequest.WithContext(middleware.SetContextValueForTest(testRequest.Context(), ContactIDKey, contactID)) + + sendTestContactNotification(responseWriter, testRequest) + + response := responseWriter.Result() + defer response.Body.Close() + contentBytes, err := io.ReadAll(response.Body) + So(err, ShouldBeNil) + contents := string(contentBytes) + actual := &api.ErrorResponse{} + err = json.Unmarshal([]byte(contents), actual) + So(err, ShouldBeNil) + + So(actual, ShouldResemble, expected) + So(response.StatusCode, ShouldEqual, http.StatusInternalServerError) + }) + }) +} diff --git a/database/redis/reply/metric.go b/database/redis/reply/metric.go index af3a80c49..4a9896864 100644 --- a/database/redis/reply/metric.go +++ b/database/redis/reply/metric.go @@ -18,11 +18,11 @@ func MetricValues(values *redis.ZSliceCmd) ([]*moira.MetricValue, error) { } return nil, fmt.Errorf("failed to read metricValues: %s", err.Error()) } - metricsValues := make([]*moira.MetricValue, 0, len(resultByMetricArr)) //nolint + metricsValues := make([]*moira.MetricValue, 0, len(resultByMetricArr)) for i := 0; i < len(resultByMetricArr); i++ { val := resultByMetricArr[i].Member.(string) valuesArr := strings.Split(val, " ") - if len(valuesArr) != 2 { //nolint + if len(valuesArr) != 2 { return nil, fmt.Errorf("value format is not valid: %s", val) } timestamp, err := strconv.ParseInt(valuesArr[0], 10, 64)