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.
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
feat(query): add promql @ modifier #1281
base: develop
Are you sure you want to change the base?
Conversation
prometheus/src/main/scala/filodb/prometheus/parse/LegacyParser.scala
Outdated
Show resolved
Hide resolved
prometheus/src/test/scala/filodb/prometheus/parse/ParserSpec.scala
Outdated
Show resolved
Hide resolved
prometheus/src/test/scala/filodb/prometheus/parse/ParserSpec.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments. As discussed please add support for start() and end() with "@"
def getTimeParamsWithAt(timeParams: TimeRangeParams, at: Option[Long]): TimeRangeParams = { | ||
at match { | ||
case Some(timestamp) => { | ||
val delta = timestamp - timeParams.end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens when timeParams.end is greater than timestamp?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The negative delta is alright-- the equation I want to satisfy is:
timeParams.end + delta = timestamp
} | ||
|
||
lazy val rangeVectorSelector2: PackratParser[RangeExpression] = | ||
labelSelection ~ simpleLookback ~ offset.? ^^ { | ||
case ls ~ simpleLookback ~ opt => | ||
RangeExpression(None, ls, simpleLookback.duration, opt.map(_.duration)) | ||
RangeExpression(None, ls, simpleLookback.duration, opt.map(_.duration), None) // TODO(a_theimer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change TODO as discussed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
2ddd55c
to
a0444a2
Compare
coordinator/src/main/scala/filodb.coordinator/queryplanner/PlannerHelper.scala
Outdated
Show resolved
Hide resolved
coordinator/src/main/scala/filodb.coordinator/queryplanner/SingleClusterPlanner.scala
Outdated
Show resolved
Hide resolved
coordinator/src/main/scala/filodb.coordinator/queryplanner/SingleClusterPlanner.scala
Outdated
Show resolved
Hide resolved
coordinator/src/main/scala/filodb.coordinator/queryplanner/SingleClusterPlanner.scala
Show resolved
Hide resolved
parser = "shadow" | ||
parser = "antlr" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note!
Pull Request checklist
New behavior :
Adds support for queries such as:
where the
@
modifier simply aligns a query's evaluation time with the@
timestamp.Per the PromQL spec,
@
can be used in conjunction withoffset
, as in:where the relative order of the modifiers does not matter.
Additionally,
start()
andend()
can be used as timestamps, where:start()
andend()
evaluate to the query's start and end times, respectively.start()
andend()
both resolve to the query's evaluation time.Examples:
This modifier is particularly useful for situations such as (from prometheus.io):
See the docs and this blog post for more information.
Additional Notes: