Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prometheus: Fix $__rate_interval calculation #77234

Merged
merged 21 commits into from
Nov 13, 2023
Merged

Prometheus: Fix $__rate_interval calculation #77234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
cddd728
Remove unused param
itsmylife Oct 25, 2023
4e3c6a6
simple unit test
itsmylife Oct 25, 2023
c0764a1
rename
itsmylife Oct 25, 2023
a5f6bd2
rename
itsmylife Oct 25, 2023
86719cb
add some comments
itsmylife Oct 25, 2023
1a93423
Update values
itsmylife Oct 25, 2023
6e5386e
refactor
itsmylife Oct 25, 2023
7f25fe6
rename
itsmylife Oct 26, 2023
57d5842
always calculate rate interval
itsmylife Oct 26, 2023
7ca3b93
fix unit tests
itsmylife Oct 26, 2023
b20ca67
Fix indentation
itsmylife Oct 26, 2023
037ffb0
linter fix
itsmylife Oct 26, 2023
044c82b
update test
itsmylife Oct 26, 2023
cea4551
Merge branch 'main' into ismail/fix-prom-rate-interval-calculation
itsmylife Oct 27, 2023
40e6eb5
Merge branch 'main' into ismail/fix-prom-rate-interval-calculation
itsmylife Oct 31, 2023
907c936
Fixing issues with the calculation
itsmylife Oct 31, 2023
ff42354
new test
itsmylife Oct 31, 2023
b3dc735
fix $__interval interpolation
itsmylife Oct 31, 2023
c71cee1
fix test
itsmylife Oct 31, 2023
e7ddfc4
add comment
itsmylife Oct 31, 2023
83163b3
Merge branch 'main' into ismail/fix-prom-rate-interval-calculation
itsmylife Nov 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 46 additions & 22 deletions pkg/tsdb/prometheus/models/query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"time"

"github.com/grafana/grafana-plugin-sdk-go/backend"

"github.com/grafana/grafana/pkg/tsdb/intervalv2"
"github.com/grafana/grafana/pkg/tsdb/prometheus/kinds/dataquery"
)
Expand Down Expand Up @@ -76,21 +77,27 @@ type Query struct {
UtcOffsetSec int64
}

func Parse(query backend.DataQuery, timeInterval string, intervalCalculator intervalv2.Calculator, fromAlert bool) (*Query, error) {
func Parse(query backend.DataQuery, dsScrapeInterval string, intervalCalculator intervalv2.Calculator, fromAlert bool) (*Query, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍 for meaningful names of variables

model := &QueryModel{}
if err := json.Unmarshal(query.JSON, model); err != nil {
return nil, err
}

// Final interval value
interval, err := calculatePrometheusInterval(model.Interval, timeInterval, model.IntervalMs, model.IntervalFactor, query, intervalCalculator)
// Final step value for prometheus
calculatedMinStep, err := calculatePrometheusInterval(model.Interval, dsScrapeInterval, model.IntervalMs, model.IntervalFactor, query, intervalCalculator)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be named step?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but also want to be specific about it. When you read the variable elsewhere it just gives you the hint about itself.

if err != nil {
return nil, err
}

// Interpolate variables in expr
timeRange := query.TimeRange.To.Sub(query.TimeRange.From)
expr := interpolateVariables(model.Expr, model.Interval, interval, timeRange, intervalCalculator, timeInterval)
expr := interpolateVariables(
model.Expr,
query.Interval,
calculatedMinStep,
model.Interval,
timeRange,
)
var rangeQuery, instantQuery bool
if model.Instant == nil {
instantQuery = false
Expand Down Expand Up @@ -118,7 +125,7 @@ func Parse(query backend.DataQuery, timeInterval string, intervalCalculator inte

return &Query{
Expr: expr,
Step: interval,
Step: calculatedMinStep,
LegendFormat: model.LegendFormat,
Start: query.TimeRange.From,
End: query.TimeRange.To,
Expand Down Expand Up @@ -153,7 +160,7 @@ func (query *Query) TimeRange() TimeRange {
}

func calculatePrometheusInterval(
queryInterval, timeInterval string,
queryInterval, dsScrapeInterval string,
intervalMs, intervalFactor int64,
query backend.DataQuery,
intervalCalculator intervalv2.Calculator,
Expand All @@ -167,7 +174,7 @@ func calculatePrometheusInterval(
queryInterval = ""
}

minInterval, err := intervalv2.GetIntervalFrom(timeInterval, queryInterval, intervalMs, 15*time.Second)
minInterval, err := intervalv2.GetIntervalFrom(dsScrapeInterval, queryInterval, intervalMs, 15*time.Second)
if err != nil {
return time.Duration(0), err
}
Expand All @@ -182,7 +189,7 @@ func calculatePrometheusInterval(
// here is where we compare for $__rate_interval or ${__rate_interval}
if originalQueryInterval == varRateInterval || originalQueryInterval == varRateIntervalAlt {
// Rate interval is final and is not affected by resolution
return calculateRateInterval(adjustedInterval, timeInterval, intervalCalculator), nil
return calculateRateInterval(adjustedInterval, dsScrapeInterval), nil
} else {
queryIntervalFactor := intervalFactor
if queryIntervalFactor == 0 {
Expand All @@ -192,12 +199,16 @@ func calculatePrometheusInterval(
}
}

// calculateRateInterval calculates the $__rate_interval value
// queryInterval is the value calculated range / maxDataPoints on the frontend
// queryInterval is shown on the Query Options Panel above the query editor
// requestedMinStep is the data source scrape interval (default 15s)
// requestedMinStep can be changed by setting "Min Step" value in Options panel below the code editor
func calculateRateInterval(
interval time.Duration,
scrapeInterval string,
intervalCalculator intervalv2.Calculator,
queryInterval time.Duration,
requestedMinStep string,
) time.Duration {
scrape := scrapeInterval
scrape := requestedMinStep
if scrape == "" {
scrape = "15s"
}
Expand All @@ -207,34 +218,47 @@ func calculateRateInterval(
return time.Duration(0)
}

rateInterval := time.Duration(int64(math.Max(float64(interval+scrapeIntervalDuration), float64(4)*float64(scrapeIntervalDuration))))
rateInterval := time.Duration(int64(math.Max(float64(queryInterval+scrapeIntervalDuration), float64(4)*float64(scrapeIntervalDuration))))
return rateInterval
}

func interpolateVariables(expr, queryInterval string, interval time.Duration,
// interpolateVariables interpolates built-in variables
// expr PromQL query
// queryInterval Requested interval in milliseconds. This value may be overridden by MinStep in query options
// calculatedMinStep Calculated final step value. It was calculated in calculatePrometheusInterval
// requestedMinStep Requested minimum step value. QueryModel.interval
// timeRange Requested time range for query
func interpolateVariables(
expr string,
queryInterval time.Duration,
calculatedMinStep time.Duration,
requestedMinStep string,
timeRange time.Duration,
intervalCalculator intervalv2.Calculator, timeInterval string) string {
) string {
rangeMs := timeRange.Milliseconds()
rangeSRounded := int64(math.Round(float64(rangeMs) / 1000.0))

var rateInterval time.Duration
if queryInterval == varRateInterval || queryInterval == varRateIntervalAlt {
rateInterval = interval
if requestedMinStep == varRateInterval || requestedMinStep == varRateIntervalAlt {
rateInterval = calculatedMinStep
} else {
rateInterval = calculateRateInterval(interval, timeInterval, intervalCalculator)
if requestedMinStep == varInterval || requestedMinStep == varIntervalAlt {
requestedMinStep = calculatedMinStep.String()
}
rateInterval = calculateRateInterval(queryInterval, requestedMinStep)
}

expr = strings.ReplaceAll(expr, varIntervalMs, strconv.FormatInt(int64(interval/time.Millisecond), 10))
expr = strings.ReplaceAll(expr, varInterval, intervalv2.FormatDuration(interval))
expr = strings.ReplaceAll(expr, varIntervalMs, strconv.FormatInt(int64(queryInterval/time.Millisecond), 10))
expr = strings.ReplaceAll(expr, varInterval, intervalv2.FormatDuration(queryInterval))
expr = strings.ReplaceAll(expr, varRangeMs, strconv.FormatInt(rangeMs, 10))
expr = strings.ReplaceAll(expr, varRangeS, strconv.FormatInt(rangeSRounded, 10))
expr = strings.ReplaceAll(expr, varRange, strconv.FormatInt(rangeSRounded, 10)+"s")
expr = strings.ReplaceAll(expr, varRateIntervalMs, strconv.FormatInt(int64(rateInterval/time.Millisecond), 10))
expr = strings.ReplaceAll(expr, varRateInterval, rateInterval.String())

// Repetitive code, we should have functionality to unify these
expr = strings.ReplaceAll(expr, varIntervalMsAlt, strconv.FormatInt(int64(interval/time.Millisecond), 10))
expr = strings.ReplaceAll(expr, varIntervalAlt, intervalv2.FormatDuration(interval))
expr = strings.ReplaceAll(expr, varIntervalMsAlt, strconv.FormatInt(int64(queryInterval/time.Millisecond), 10))
expr = strings.ReplaceAll(expr, varIntervalAlt, intervalv2.FormatDuration(queryInterval))
expr = strings.ReplaceAll(expr, varRangeMsAlt, strconv.FormatInt(rangeMs, 10))
expr = strings.ReplaceAll(expr, varRangeSAlt, strconv.FormatInt(rangeSRounded, 10))
expr = strings.ReplaceAll(expr, varRangeAlt, strconv.FormatInt(rangeSRounded, 10)+"s")
Expand Down
Loading
Loading