Support custom lookback delta from request for query api

[oronsh]

• I added CHANGELOG entry for this change.
• Change is not relevant to the end user.

Changes

• Update prometheus version to v0.38.0.
• Add support for lookback delta from request for query and grpc apis.
• Replace engineFactory with lookbackDeltaFactory, getting rid of multiple engines.

As Thanos supports multi tenancy, we would like to have a custom lookback delta for requests per tenant.
As each tenant can have its own scrape interval and requirements.

Prometheus just added support for it recently:
prometheus/prometheus#9946

Verification

[GiedriusS]

Thanks for this! It's probably a good idea to also remove the creation of multiple engines in the same go because with only this change it would still be a bit awkward to change this parameter in multiple places. We could simply move the logic from that function to here. What do you think?

[oronsh]

Yes so if I got it right, you suggest to calculate the "dynamic lookback delta" according to maxSourceResolutionMillis logic and pass the value to QueryOpts or override it in case we get it from request params?

[oronsh]

@GiedriusS how do you feel about replacing engineFactory with something like this:

func LookbackDeltaFactory(
	eo promql.EngineOpts,
	dynamicLookbackDelta bool
) func(int64) time.Duration {
	resolutions := []int64{downsample.ResLevel0}
	if dynamicLookbackDelta {
		resolutions = []int64{downsample.ResLevel0, downsample.ResLevel1, downsample.ResLevel2}
	}
	var (
		lds = make([]time.Duration, len(resolutions))
		ld  = eo.LookbackDelta.Milliseconds()
	)

	lookbackDelta := eo.LookbackDelta
	for i, r := range resolutions {
		if ld < r {
			lookbackDelta = time.Duration(r) * time.Millisecond
		}

		lds[i] = lookbackDelta
	}
	return func(maxSourceResolutionMillis int64) time.Duration {
		for i := len(resolutions) - 1; i >= 1; i-- {
			left := resolutions[i-1]
			if resolutions[i-1] < ld {
				left = ld
			}
			if left < maxSourceResolutionMillis {
				return lds[i]
			}
		}
		return lds[0]
	}
}

so we can pass it to QueryApi

[fpetkovski]

Could we also add the same parameter to the gRPC query API?

[oronsh]

Sure I'll add it

[oronsh]

done

[GiedriusS]

@GiedriusS how do you feel about replacing engineFactory with something like this:

func LookbackDeltaFactory(
	eo promql.EngineOpts,
	dynamicLookbackDelta bool
) func(int64) time.Duration {
	resolutions := []int64{downsample.ResLevel0}
	if dynamicLookbackDelta {
		resolutions = []int64{downsample.ResLevel0, downsample.ResLevel1, downsample.ResLevel2}
	}
	var (
		lds = make([]time.Duration, len(resolutions))
		ld  = eo.LookbackDelta.Milliseconds()
	)

	lookbackDelta := eo.LookbackDelta
	for i, r := range resolutions {
		if ld < r {
			lookbackDelta = time.Duration(r) * time.Millisecond
		}

		lds[i] = lookbackDelta
	}
	return func(maxSourceResolutionMillis int64) time.Duration {
		for i := len(resolutions) - 1; i >= 1; i-- {
			left := resolutions[i-1]
			if resolutions[i-1] < ld {
				left = ld
			}
			if left < maxSourceResolutionMillis {
				return lds[i]
			}
		}
		return lds[0]
	}
}

so we can pass it to QueryApi

Something like that should work 👍 could you please add unit tests for a such function to ensure that it works properly? Finally, we will be able to get rid of multiple engines. This also means that concurrency limits will finally be enforced properly because previously we had three engines so technically the actual limit was 3 times higher.

[oronsh]

Sure @GiedriusS, I'll add the unit tests :)

[oronsh]

Shouldn't it be

lookbackDelta := g.lookbackDeltaCreate(maxResolution * 1000)

since it's lookbackDeltaCreate (and old queryEngine(..)) suppose to receive parameter in millis?

[GiedriusS]

That's probably correct but it's the caller's responsibility to pass the proper data and I think the gRPC is unused at the moment. So, I think we should multiply it by 1000. @fpetkovski maybe you know the answer?

[fpetkovski]

I think I made a mistake to use seconds in the gRPC API when the HTTP one uses millis. Because of this, I also think we should multiply by 1000.

[oronsh]

done @fpetkovski @GiedriusS 🙏

[oronsh]

@GiedriusS @fpetkovski I implemented the requests:

1. Replacing engineFactory with lookbackDeltaFactory.
2. Adding support for custom lookback delta in grpc

[GiedriusS]

Solid work, just one unsolved conversation in the gRPC API. 💪

[GiedriusS]

Please use merge commits in the future to update your branch instead of rebasing since the latter recreates commits with new commit IDs and then the reviewers have to look through everything again.

[GiedriusS]

Awesome work plus good cleanup! 💪 thank you.