Skip to content

feat: Align start time when the query is received - #20967

Merged
DylanGuedes merged 2 commits into
mainfrom
align-predicate
Feb 26, 2026
Merged

feat: Align start time when the query is received#20967
DylanGuedes merged 2 commits into
mainfrom
align-predicate

Conversation

@DylanGuedes

@DylanGuedes DylanGuedes commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:
This fixes the approach of #20904: Since we were mismatching the new query engine results by not aligning the start timestamp, I implemented an alignment in the planner. On this PR I'm changing the approach to instead do the same procedure we do in the classic engine: I'm changing the query handler to wrap the StepAlignMiddleware instead. This should make the results of classic and new engine identical regarding how they handle step alignment.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR
@DylanGuedes
DylanGuedes marked this pull request as ready for review February 25, 2026 20:44
@DylanGuedes
DylanGuedes requested a review from a team as a code owner February 25, 2026 20:44
@DylanGuedes
DylanGuedes marked this pull request as draft February 25, 2026 23:14
@DylanGuedes DylanGuedes changed the title fix: Align start time in query predicates Feb 25, 2026
@github-actions

github-actions Bot commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

💻 Deploy preview deleted (feat: Align start time when the query is received).

@DylanGuedes
DylanGuedes marked this pull request as ready for review February 26, 2026 00:03
@rfratto

rfratto commented Feb 26, 2026

Copy link
Copy Markdown
Member

Can you add a new unit test for this so we don't regress? 🙏

@DylanGuedes

Copy link
Copy Markdown
Contributor Author

Can you add a new unit test for this so we don't regress? 🙏

absolutely!

@rfratto rfratto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! It might be a good idea to double check with @ashwanthgoli that it's correct to remove the unit tests you took out here

@DylanGuedes

Copy link
Copy Markdown
Contributor Author

Thanks! It might be a good idea to double check with @ashwanthgoli that it's correct to remove the unit tests you took out here

thanks! I'm proceeding with the merge then, the unit tests I'm removing were added by me on #20904. On this PR I'm basically reverting doing the alignment at the engine level to do at the handler level instead (which is how we do in classic Loki).

@DylanGuedes
DylanGuedes merged commit 8e18f10 into main Feb 26, 2026
92 checks passed
@DylanGuedes
DylanGuedes deleted the align-predicate branch February 26, 2026 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2 participants