Skip to content

Commit

Permalink
eval: add test cases for events validation (#1636)
Browse files Browse the repository at this point in the history
  • Loading branch information
brharrington authored Mar 29, 2024
1 parent 22c81b0 commit cd48614
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ import org.apache.pekko.stream.scaladsl.StreamConverters
import org.apache.pekko.util.ByteString
import com.netflix.atlas.core.model.DataExpr
import com.netflix.atlas.core.model.Query
import com.netflix.atlas.core.model.StyleExpr
import com.netflix.atlas.core.util.Streams
import com.netflix.atlas.eval.model.ExprType
import com.netflix.atlas.eval.stream.Evaluator.DataSource
import com.netflix.atlas.eval.stream.Evaluator.DataSources
import com.netflix.atlas.json.JsonSupport
Expand Down Expand Up @@ -163,14 +165,11 @@ private[stream] class StreamContext(

// Check that expression is parseable and perform basic static analysis of DataExprs to
// weed out expensive queries up front
val results = interpreter.eval(uri).exprs
results.foreach(_.expr.dataExprs.foreach(validateDataExpr))

// For hi-res streams, require more precise scoping that allows us to more efficiently
// match the data and run it only where needed. This would ideally be applied everywhere,
// but for backwards compatiblity the 1m step is opted out for now.
if (ds.step.toMillis < 60_000) {
results.foreach(_.expr.dataExprs.foreach(expr => restrictsNameAndApp(expr.query)))
val (exprType, exprs) = interpreter.parseQuery(uri)
if (exprType == ExprType.TIME_SERIES) {
exprs.foreach {
case e: StyleExpr => validateStyleExpr(e, ds)
}
}

// Check that there is a backend available for it
Expand All @@ -181,6 +180,17 @@ private[stream] class StreamContext(
}
}

private def validateStyleExpr(styleExpr: StyleExpr, ds: DataSource): Unit = {
styleExpr.expr.dataExprs.foreach(validateDataExpr)

// For hi-res streams, require more precise scoping that allows us to more efficiently
// match the data and run it only where needed. This would ideally be applied everywhere,
// but for backwards compatiblity the 1m step is opted out for now.
if (ds.step.toMillis < 60_000) {
styleExpr.expr.dataExprs.foreach(expr => restrictsNameAndApp(expr.query))
}
}

private def validateDataExpr(expr: DataExpr): Unit = {
Query
.dnfList(expr.query)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -463,11 +463,11 @@ class EvaluatorSuite extends FunSuite {
assertEquals(ds.step, Duration.ofMinutes(1))
}

private def validateOk(params: String): Unit = {
private def validateOk(params: String, path: String = "graph"): Unit = {
val evaluator = new Evaluator(config, registry, system)
val ds = new Evaluator.DataSource(
"test",
s"resource:///gc-pause.dat?$params"
s"synthetic://test/$path?$params"
)
evaluator.validate(ds)
}
Expand Down Expand Up @@ -585,6 +585,25 @@ class EvaluatorSuite extends FunSuite {
)
}

test("validate: events raw") {
validateOk("q=name,foo,:eq,nf.cluster,www-dev,:eq,:and", path = "events")
}

test("validate: events table") {
validateOk("q=name,foo,:eq,nf.cluster,www-dev,:eq,:and,(,value,),:table", path = "events")
}

test("validate: traces") {
validateOk("q=nf.app,www,:eq,nf.app,db,:eq,:child", path = "traces")
}

test("validate: trace time series") {
validateOk(
"q=app,www,:eq,app,db,:eq,:child,app,db,:eq,:sum,:span-time-series",
path = "traces/graph"
)
}

private def invalidHiResQuery(expr: String): Unit = {
val evaluator = new Evaluator(config, registry, system)
val ds = new Evaluator.DataSource(
Expand Down

0 comments on commit cd48614

Please sign in to comment.