fix: Delay calls to Dynatrace APIs such that required data is availab…

…le (#723)

* Simplify event processing logic

Signed-off-by: Arthur Pitman <[email protected]>

* Reorder functions

Signed-off-by: Arthur Pitman <[email protected]>

* Refactor getSLIResults

Signed-off-by: Arthur Pitman <[email protected]>

* Remove commented out imports

Signed-off-by: Arthur Pitman <[email protected]>

* Use ParseTimestamp from keptn/go-utils

Signed-off-by: Arthur Pitman <[email protected]>

* Introduce Timeframe

Signed-off-by: Arthur Pitman <[email protected]>

* Use Timeframe

Signed-off-by: Arthur Pitman <[email protected]>

* Introduce TimeframeDelay

Signed-off-by: Arthur Pitman <[email protected]>

* Use TimeframeDelay in Dynatrace API clients

Signed-off-by: Arthur Pitman <[email protected]>

* Update test

Signed-off-by: Arthur Pitman <[email protected]>

* Update comments for exported functions and structs

Signed-off-by: Arthur Pitman <[email protected]>

internal/common/common.go

@@ -101,23 +101,8 @@ func ReplaceKeptnPlaceholders(input string, keptnEvent adapter.EventContentAdapt
 	return result
 }
 
-// ParseUnixTimestamp parses a timestamp into Unix format
-func ParseUnixTimestamp(timestamp string) (time.Time, error) {
-	parsedTime, err := time.Parse(time.RFC3339, timestamp)
-	if err == nil {
-		return parsedTime, nil
-	}
-
-	timestampInt, err := strconv.ParseInt(timestamp, 10, 64)
-	if err != nil {
-		return time.Now(), err
-	}
-	unix := time.Unix(timestampInt, 0)
-	return unix, nil
-}
-
-// TimestampToString converts time stamp into string
-func TimestampToString(time time.Time) string {
+// TimestampToUnixMillisecondsString converts timestamp into a Unix milliseconds string.
+func TimestampToUnixMillisecondsString(time time.Time) string {
 	return strconv.FormatInt(time.Unix()*1000, 10)
 }
 

internal/common/common_test.go

@@ -1,37 +1,22 @@
 package common
 
 import (
-	keptnapi "github.com/keptn/go-utils/pkg/lib"
-	"github.com/stretchr/testify/assert"
 	"testing"
 	"time"
+
+	keptnapi "github.com/keptn/go-utils/pkg/lib"
+	"github.com/stretchr/testify/assert"
 )
 
-func TestTimestampToString(t *testing.T) {
+func TestTimestampToUnixMillisecondsString(t *testing.T) {
 	dt := time.Date(1970, 1, 1, 0, 1, 23, 456, time.UTC)
 	expected := "83000" // = (1*60 + 23) * 1000 ms
 
-	got := TimestampToString(dt)
+	got := TimestampToUnixMillisecondsString(dt)
 
 	assert.EqualValues(t, expected, got)
 }
 
-// tests the parseUnixTimestamp with invalid params
-func TestParseInvalidUnixTimestamp(t *testing.T) {
-	_, err := ParseUnixTimestamp("")
-
-	assert.NotNil(t, err)
-}
-
-// tests the parseUnixTimestamp with valid params
-func TestParseValidUnixTimestamp(t *testing.T) {
-	expectedTime := time.Date(2019, 10, 24, 15, 44, 27, 152330783, time.UTC)
-
-	got, _ := ParseUnixTimestamp("2019-10-24T15:44:27.152330783Z")
-
-	assert.EqualValues(t, expectedTime, got)
-}
-
 func TestParsePassAndWarningFromString(t *testing.T) {
 	type args struct {
 		customName string

internal/common/timeframe.go

@@ -0,0 +1,35 @@
+package common
+
+import (
+	"errors"
+	"time"
+)
+
+// Timeframe represents a timeframe with a start and end time.
+type Timeframe struct {
+	start time.Time
+	end   time.Time
+}
+
+// NewTimeframe creates a new timeframe from start and end times.
+func NewTimeframe(start time.Time, end time.Time) (*Timeframe, error) {
+	// ensure start time is before end time
+	if end.Sub(start).Seconds() < 0 {
+		return nil, errors.New("error validating timeframe: start needs to be before end")
+	}
+
+	return &Timeframe{
+		start: start,
+		end:   end,
+	}, nil
+}
+
+// Start gets the start of the timeframe.
+func (t Timeframe) Start() time.Time {
+	return t.start
+}
+
+// End gets the end of the timeframe.
+func (t Timeframe) End() time.Time {
+	return t.end
+}

internal/common/timeframe_parser.go

@@ -0,0 +1,36 @@
+package common
+
+import (
+	"fmt"
+
+	"github.com/keptn/go-utils/pkg/common/timeutils"
+)
+
+// TimeframeParser represents a timeframe ready to be parsed.
+type TimeframeParser struct {
+	start string
+	end   string
+}
+
+// NewTimeframeParser creates a new TimeframeParser ready to parse the specified start and end strings.
+func NewTimeframeParser(start string, end string) TimeframeParser {
+	return TimeframeParser{
+		start: start,
+		end:   end,
+	}
+}
+
+// Parse parses the start and end strings to create a Timeframe.
+func (p TimeframeParser) Parse() (*Timeframe, error) {
+	start, err := timeutils.ParseTimestamp(p.start)
+	if err != nil {
+		return nil, fmt.Errorf("error parsing timeframe start: %w", err)
+	}
+
+	end, err := timeutils.ParseTimestamp(p.end)
+	if err != nil {
+		return nil, fmt.Errorf("error parsing timeframe end: %w", err)
+	}
+
+	return NewTimeframe(*start, *end)
+}

internal/common/timeframe_parser_test.go

@@ -0,0 +1,33 @@
+package common
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestTimeframeParser_ValidArgs(t *testing.T) {
+	expectedTimeframe, err := NewTimeframe(time.Date(2022, 2, 1, 10, 0, 40, 0, time.UTC), time.Date(2022, 2, 1, 10, 5, 40, 0, time.UTC))
+	assert.NoError(t, err)
+
+	timeframe, err := NewTimeframeParser("2022-02-01T10:00:40Z", "2022-02-01T10:05:40Z").Parse()
+	assert.NoError(t, err)
+
+	assert.EqualValues(t, expectedTimeframe.Start(), timeframe.Start())
+	assert.EqualValues(t, expectedTimeframe.End(), timeframe.End())
+}
+
+func TestTimeframeParser_InvalidStart(t *testing.T) {
+	timeframe, err := NewTimeframeParser("", "2022-02-01T10:05:40Z").Parse()
+	assert.Error(t, err)
+	assert.Nil(t, timeframe)
+	assert.Contains(t, err.Error(), "error parsing timeframe start")
+}
+
+func TestTimeframeParser_InvalidEnd(t *testing.T) {
+	timeframe, err := NewTimeframeParser("2022-02-01T10:00:40Z", "").Parse()
+	assert.Error(t, err)
+	assert.Nil(t, timeframe)
+	assert.Contains(t, err.Error(), "error parsing timeframe end")
+}

internal/common/timeframe_test.go

@@ -0,0 +1,28 @@
+package common
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNewTimeframe_ValidArgs(t *testing.T) {
+	start := time.Date(2022, 2, 1, 10, 0, 40, 0, time.UTC)
+	end := time.Date(2022, 2, 1, 10, 5, 40, 0, time.UTC)
+
+	timeframe, err := NewTimeframe(start, end)
+	assert.NoError(t, err)
+	assert.EqualValues(t, start, timeframe.Start())
+	assert.EqualValues(t, end, timeframe.End())
+}
+
+func TestNewTimeframe_InvalidEndBeforeStart(t *testing.T) {
+	start := time.Date(2022, 2, 1, 10, 5, 40, 0, time.UTC)
+	end := time.Date(2022, 2, 1, 10, 0, 40, 0, time.UTC)
+
+	timeframe, err := NewTimeframe(start, end)
+	assert.Error(t, err)
+	assert.Nil(t, timeframe)
+	assert.Contains(t, err.Error(), "error validating timeframe")
+}

internal/dynatrace/metrics_client.go

@@ -1,6 +1,7 @@
 package dynatrace
 
 import (
+	"context"
 	"encoding/json"
 	"errors"
 	"time"
@@ -15,6 +16,12 @@ const MetricsPath = "/api/v2/metrics"
 // MetricsQueryPath is the query endpoint for Metrics API v2
 const MetricsQueryPath = MetricsPath + "/query"
 
+// MetricsRequiredDelay is delay required between the end of a timeframe and an Metric V2 API request using it.
+const MetricsRequiredDelay = 2 * time.Minute
+
+// MetricsMaximumWait is maximum acceptable wait time between the end of a timeframe and an Metrics V2 API request using it.
+const MetricsMaximumWait = 4 * time.Minute
+
 const (
 	fromKey           = "from"
 	toKey             = "to"
@@ -25,26 +32,24 @@ const (
 
 // MetricsClientQueryParameters encapsulates the query parameters for the MetricsClient's GetByQuery method.
 type MetricsClientQueryParameters struct {
-	query metrics.Query
-	from  time.Time
-	to    time.Time
+	query     metrics.Query
+	timeframe common.Timeframe
 }
 
 // NewMetricsClientQueryParameters creates new MetricsClientQueryParameters.
-func NewMetricsClientQueryParameters(query metrics.Query, from time.Time, to time.Time) MetricsClientQueryParameters {
+func NewMetricsClientQueryParameters(query metrics.Query, timeframe common.Timeframe) MetricsClientQueryParameters {
 	return MetricsClientQueryParameters{
-		query: query,
-		from:  from,
-		to:    to,
+		query:     query,
+		timeframe: timeframe,
 	}
 }
 
 // encode encodes MetricsClientQueryParameters into a URL-encoded string.
 func (q *MetricsClientQueryParameters) encode() string {
 	queryParameters := newQueryParameters()
 	queryParameters.add(metricSelectorKey, q.query.GetMetricSelector())
-	queryParameters.add(fromKey, common.TimestampToString(q.from))
-	queryParameters.add(toKey, common.TimestampToString(q.to))
+	queryParameters.add(fromKey, common.TimestampToUnixMillisecondsString(q.timeframe.Start()))
+	queryParameters.add(toKey, common.TimestampToUnixMillisecondsString(q.timeframe.End()))
 	queryParameters.add(resolutionKey, "Inf")
 	if q.query.GetEntitySelector() != "" {
 		queryParameters.add(entitySelectorKey, q.query.GetEntitySelector())
@@ -122,6 +127,11 @@ func (mc *MetricsClient) GetByID(metricID string) (*MetricDefinition, error) {
 
 // GetByQuery executes the passed Metrics API Call, validates that the call returns data and returns the data set
 func (mc *MetricsClient) GetByQuery(parameters MetricsClientQueryParameters) (*MetricsQueryResult, error) {
+	err := NewTimeframeDelay(parameters.timeframe, MetricsRequiredDelay, MetricsMaximumWait).Wait(context.TODO())
+	if err != nil {
+		return nil, err
+	}
+
 	body, err := mc.client.Get(MetricsQueryPath + "?" + parameters.encode())
 	if err != nil {
 		return nil, err

internal/dynatrace/problems_v2_client.go

@@ -1,6 +1,7 @@
 package dynatrace
 
 import (
+	"context"
 	"encoding/json"
 	"time"
 
@@ -14,23 +15,27 @@ const ProblemStatusOpen = "OPEN"
 // ProblemsV2Path is the base endpoint for Problems API v2
 const ProblemsV2Path = "/api/v2/problems"
 
+// ProblemsV2RequiredDelay is delay required between the end of a timeframe and an PV2 API request using it.
+const ProblemsV2RequiredDelay = 2 * time.Minute
+
+// ProblemsV2MaximumWait is maximum acceptable wait time between the end of a timeframe and an PV2 API request using it.
+const ProblemsV2MaximumWait = 4 * time.Minute
+
 const (
 	problemSelectorKey = "problemSelector"
 )
 
 // ProblemsV2ClientQueryParameters encapsulates the query parameters for the ProblemsV2Client's GetTotalCountByQuery method.
 type ProblemsV2ClientQueryParameters struct {
-	query problems.Query
-	from  time.Time
-	to    time.Time
+	query     problems.Query
+	timeframe common.Timeframe
 }
 
 // NewProblemsV2ClientQueryParameters creates new ProblemsV2ClientQueryParameters.
-func NewProblemsV2ClientQueryParameters(query problems.Query, from time.Time, to time.Time) ProblemsV2ClientQueryParameters {
+func NewProblemsV2ClientQueryParameters(query problems.Query, timeframe common.Timeframe) ProblemsV2ClientQueryParameters {
 	return ProblemsV2ClientQueryParameters{
-		query: query,
-		from:  from,
-		to:    to,
+		query:     query,
+		timeframe: timeframe,
 	}
 }
 
@@ -44,8 +49,8 @@ func (q *ProblemsV2ClientQueryParameters) encode() string {
 		queryParameters.add(entitySelectorKey, q.query.GetEntitySelector())
 	}
 
-	queryParameters.add(fromKey, common.TimestampToString(q.from))
-	queryParameters.add(toKey, common.TimestampToString(q.to))
+	queryParameters.add(fromKey, common.TimestampToUnixMillisecondsString(q.timeframe.Start()))
+	queryParameters.add(toKey, common.TimestampToUnixMillisecondsString(q.timeframe.End()))
 	return queryParameters.encode()
 }
 
@@ -75,6 +80,11 @@ func NewProblemsV2Client(client ClientInterface) *ProblemsV2Client {
 
 // GetTotalCountByQuery calls the Dynatrace V2 API to retrieve the total count of problems for a given query and timeframe
 func (pc *ProblemsV2Client) GetTotalCountByQuery(parameters ProblemsV2ClientQueryParameters) (int, error) {
+	err := NewTimeframeDelay(parameters.timeframe, ProblemsV2RequiredDelay, ProblemsV2MaximumWait).Wait(context.TODO())
+	if err != nil {
+		return 0, err
+	}
+
 	body, err := pc.client.Get(ProblemsV2Path + "?" + parameters.encode())
 	if err != nil {
 		return 0, err

internal/dynatrace/problems_v2_client_test.go

@@ -2,8 +2,8 @@ package dynatrace
 
 import (
 	"testing"
-	"time"
 
+	"github.com/keptn-contrib/dynatrace-service/internal/common"
 	"github.com/keptn-contrib/dynatrace-service/internal/sli/problems"
 	"github.com/keptn-contrib/dynatrace-service/internal/test"
 	"github.com/stretchr/testify/assert"
@@ -16,10 +16,11 @@ func TestProblemsV2Client_GetTotalCountByQuery(t *testing.T) {
 	dtClient, _, teardown := createDynatraceClient(t, handler)
 	defer teardown()
 
-	startTime := time.Unix(1571649084, 0).UTC()
-	endTime := time.Unix(1571649085, 0).UTC()
+	timeframe, err := common.NewTimeframeParser("2019-10-21T09:11:24Z", "2019-10-21T09:11:25Z").Parse()
+	assert.NoError(t, err)
+
 	problemQuery := problems.NewQuery("status(\"open\")", "")
-	totalProblemCount, err := NewProblemsV2Client(dtClient).GetTotalCountByQuery(NewProblemsV2ClientQueryParameters(problemQuery, startTime, endTime))
+	totalProblemCount, err := NewProblemsV2Client(dtClient).GetTotalCountByQuery(NewProblemsV2ClientQueryParameters(problemQuery, *timeframe))
 
 	assert.NoError(t, err)
 	assert.EqualValues(t, 1, totalProblemCount)