From 9f129cf9f16430cc5a399f60aa5684fb72b55bb5 Mon Sep 17 00:00:00 2001
From: Rob Skillington <rob.skillington@gmail.com>
Date: Tue, 17 Nov 2020 17:00:04 -0500
Subject: [PATCH] [query] Add lookback duration from query config (#2913)

---
 .../prometheus/m3coordinator.yml              |  2 ++
 .../prometheus/test.sh                        | 20 ++++++++++++++
 src/cmd/services/m3query/config/config.go     |  2 +-
 src/query/server/query.go                     | 26 +++++++++++++------
 4 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/scripts/docker-integration-tests/prometheus/m3coordinator.yml b/scripts/docker-integration-tests/prometheus/m3coordinator.yml
index cd1a97eb82..79f849a151 100644
--- a/scripts/docker-integration-tests/prometheus/m3coordinator.yml
+++ b/scripts/docker-integration-tests/prometheus/m3coordinator.yml
@@ -23,3 +23,5 @@ query:
         value: hidden
     strip:
     - restricted_metrics_type
+
+lookbackDuration: 10m
diff --git a/scripts/docker-integration-tests/prometheus/test.sh b/scripts/docker-integration-tests/prometheus/test.sh
index 61fe8daf52..3c00c5fb37 100755
--- a/scripts/docker-integration-tests/prometheus/test.sh
+++ b/scripts/docker-integration-tests/prometheus/test.sh
@@ -177,6 +177,25 @@ function test_prometheus_remote_write_map_tags {
     retry_with_backoff prometheus_query_native
 }
 
+function test_query_lookback_applied {
+  # Note: this test depends on the config in m3coordinator.yml for this test
+  # and the following config value "lookbackDuration: 10m".
+  echo "Test lookback config respected"
+  now=$(date +"%s")
+  # Write into past less than the lookback duration.
+  eight_mins_ago=$(( now - 480 ))
+  prometheus_remote_write \
+    "lookback_test" $eight_mins_ago 42 \
+    true "Expected request to succeed" \
+    200 "Expected request to return status code 200" \
+    "unaggregated"
+
+  # Now query and ensure that the latest timestamp is within the last two steps
+  # from now.
+  ATTEMPTS=10 TIMEOUT=2 MAX_TIMEOUT=4 retry_with_backoff  \
+    '[[ $(curl -s "0.0.0.0:7201/api/v1/query_range?query=lookback_test&step=15&start=$(expr $(date "+%s") - 600)&end=$(date "+%s")" | jq -r ".data.result[0].values[-1][0]") -gt $(expr $(date "+%s") - 30) ]]'
+}
+
 function test_query_limits_applied {
   # Test the default series limit applied when directly querying
   # coordinator (limit set to 100 in m3coordinator.yml)
@@ -373,6 +392,7 @@ test_prometheus_remote_write_empty_label_value_returns_400_status_code
 test_prometheus_remote_write_duplicate_label_returns_400_status_code
 test_prometheus_remote_write_too_old_returns_400_status_code
 test_prometheus_remote_write_restrict_metrics_type
+test_query_lookback_applied
 test_query_limits_applied
 test_query_restrict_metrics_type
 test_prometheus_query_native_timeout
diff --git a/src/cmd/services/m3query/config/config.go b/src/cmd/services/m3query/config/config.go
index 093f490fc7..2b74e4e170 100644
--- a/src/cmd/services/m3query/config/config.go
+++ b/src/cmd/services/m3query/config/config.go
@@ -86,7 +86,7 @@ var (
 		ExtendedMetrics: &defaultMetricsExtendedMetricsType,
 	}
 
-	// 5m is the default lookback in Prometheus
+	// 5m is the default lookback in Prometheus.
 	defaultLookbackDuration = 5 * time.Minute
 
 	defaultCarbonIngesterAggregationType = aggregation.Mean
diff --git a/src/query/server/query.go b/src/query/server/query.go
index a6f1aeccd5..8dfdfa407b 100644
--- a/src/query/server/query.go
+++ b/src/query/server/query.go
@@ -479,8 +479,12 @@ func Run(runOpts RunOptions) {
 		graphiteStorageOpts.RenderSeriesAllNaNs = cfg.Carbon.RenderSeriesAllNaNs
 	}
 
-	prometheusEngine := newPromQLEngine(cfg.Query, prometheusEngineRegistry,
+	prometheusEngine, err := newPromQLEngine(cfg, prometheusEngineRegistry,
 		instrumentOptions)
+	if err != nil {
+		logger.Fatal("unable to create PromQL engine", zap.Error(err))
+	}
+
 	handlerOptions, err := options.NewHandlerOptions(downsamplerAndWriter,
 		tagOptions, engine, prometheusEngine, m3dbClusters, clusterClient, cfg,
 		runOpts.DBConfig, fetchOptsBuilder, queryCtxOpts,
@@ -1154,18 +1158,24 @@ func newDownsamplerAndWriter(
 }
 
 func newPromQLEngine(
-	cfg config.QueryConfiguration,
+	cfg config.Configuration,
 	registry *extprom.Registry,
 	instrumentOpts instrument.Options,
-) *prometheuspromql.Engine {
+) (*prometheuspromql.Engine, error) {
+	lookbackDelta, err := cfg.LookbackDurationOrDefault()
+	if err != nil {
+		return nil, err
+	}
+
 	var (
 		kitLogger = kitlogzap.NewZapSugarLogger(instrumentOpts.Logger(), zapcore.InfoLevel)
 		opts      = prometheuspromql.EngineOpts{
-			Logger:     log.With(kitLogger, "component", "prometheus_engine"),
-			Reg:        registry,
-			MaxSamples: cfg.Prometheus.MaxSamplesPerQueryOrDefault(),
-			Timeout:    cfg.TimeoutOrDefault(),
+			Logger:        log.With(kitLogger, "component", "prometheus_engine"),
+			Reg:           registry,
+			MaxSamples:    cfg.Query.Prometheus.MaxSamplesPerQueryOrDefault(),
+			Timeout:       cfg.Query.TimeoutOrDefault(),
+			LookbackDelta: lookbackDelta,
 		}
 	)
-	return prometheuspromql.NewEngine(opts)
+	return prometheuspromql.NewEngine(opts), nil
 }