Advanced Query Processing Architecture

[zaibkhan]

This PR refactors the query splitting architecture in Grafana’s Loki datasource, standardizing how template variables are interpolated and processed.

Key changes:

• Refactors interpolateVariablesToQueries() to ensure consistent handling of template variables.
• Improves query splitting paths by unifying variable interpolation logic.
• Enhances maintainability and reduces risk of inconsistent query evaluation.

[codoki-pr-intelligence]

Codoki PR Review

Summary: Fix interpolation-filter order, prevent empty expr leaks
What’s good: Unifying variable interpolation via applyTemplateVariables in both time- and shard-splitting paths reduces duplication and brings behavior parity; added tests cover interpolation of $__auto and $step and verify downstream requests.
Review Status: ❌ Requires changes
Overall Priority: High

Issues (Critical & High only)

Severity	Issue	Why it matters
High	Correctness — Filter after interpolation to drop empty expressions	…/loki/querySplitting.ts Filtering by expr before applying template variables can allow empty post-interpolation expressions to slip through (e.g., a variable resolving to ''), leading to invalid downstream requests. Apply interpolation first, then filter on expr to ensure only valid queries are executed.
High	Correctness — Interpolate before filtering to avoid empty expr targets	…/loki/shardQuerySplitting.ts Same risk as in runSplitQuery: filtering by expr before interpolation can allow targets whose expressions resolve to empty strings to proceed, potentially causing request errors. Interpolate first, then filter on expr.

Showing top 2 issues. Critical: 0, High: 2. See inline suggestions for more.

Key Feedback (click to expand)

• Needs improvement: In both runSplitQuery and runShardSplitQuery, filter(expr) occurs before applying template variables; if interpolation resolves to an empty expression, such queries will slip through and be executed, risking backend errors.
• Testing: Consider adding tests where a target expr resolves to an empty string after interpolation (e.g., variable returns ''), asserting that such targets are dropped and no requests are made for them.
• Open questions: Do we expect applyTemplateVariables to be strictly idempotent and never render expr empty? If not, should we standardize the post-interpolation filtering across all query entry points?

Confidence: 3/5 — Needs work before merge (2 high · status: Requires changes)

React with 👍 or 👎 if you found this review useful.

[codoki-pr-intelligence]

⚠️ High: Filtering by expr before applying template variables can allow empty post-interpolation expressions to slip through (e.g., a variable resolving to ''), leading to invalid downstream requests. Apply interpolation first, then filter on expr to ensure only valid queries are executed.

Suggested change

	const queries = request.targets
	```suggestion
	const queries = request.targets
	.filter((query) => !query.hide)
	.map((query) => datasource.applyTemplateVariables(query, request.scopedVars, request.filters))
	.filter((query) => query.expr);

[codoki-pr-intelligence]

⚠️ High: Same risk as in runSplitQuery: filtering by expr before interpolation can allow targets whose expressions resolve to empty strings to proceed, potentially causing request errors. Interpolate first, then filter on expr.

Suggested change

	.map((query) => datasource.applyTemplateVariables(query, request.scopedVars, request.filters));
	```suggestion
	const queries = request.targets
	.filter((query) => !query.hide)
	.map((query) => datasource.applyTemplateVariables(query, request.scopedVars, request.filters))
	.filter((query) => query.expr);