-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Heartbeat] Support JSON expressions / validation of JSON arrays #28073
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
💚 Flaky test reportTests succeeded. 🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
@lowryxiao I noticed you spotted a bugfix earlier, if you have any feedback here it's appreciated as well! |
@@ -258,12 +258,6 @@ This monitor examines match successfully only when 'foo' or 'Foo' in body AND no | |||
name: Demo Service | |||
schedule: '@every 5s' | |||
urls: ["http://localhost:8080/demo/add"] | |||
check.request: |
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.
This was redundant for this section, we're focusing on JSON here
@@ -219,53 +211,3 @@ func checkBody(positiveMatch, negativeMatch []match.Matcher) bodyValidator { | |||
return nil | |||
} | |||
} | |||
|
|||
func checkJSON(checks []*jsonResponseCheck) (bodyValidator, error) { |
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.
moved to checkjson.go
@@ -196,142 +194,6 @@ func TestCheckBody(t *testing.T) { | |||
} | |||
} | |||
|
|||
func TestCheckJson(t *testing.T) { |
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.
moved to checkjson_test.go
source: check.Expression, | ||
}) | ||
} else if check.Condition != nil { | ||
cfgwarn.Deprecate("8.0.0", "JSON conditions are deprecated, use 'expression' instead.") |
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.
I can do a follow-up PR to actually delete this in master
if we agree we should just move to expression
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.
I don't have a strong opinion here, Do you see people using conditions for simpler use case?
Pinging @elastic/uptime (Team:Uptime) |
This pull request doesn't have a |
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.
Overall looks good to me, Added small comments.
source: check.Expression, | ||
}) | ||
} else if check.Condition != nil { | ||
cfgwarn.Deprecate("8.0.0", "JSON conditions are deprecated, use 'expression' instead.") |
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.
I don't have a strong opinion here, Do you see people using conditions for simpler use case?
condition common.MapStr | ||
expectedErrMsg string | ||
expectedContentType string | ||
} | ||
|
||
testCases := []testCase{ | ||
{ | ||
"simple match", | ||
"expression simple match", |
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.
do we want to add one more case when both condition and expression exists?
@vigneshshanmugam I've incorporated your PR feedback into 0e6a11b , I think it's a bit cleaner, esp. with the new config validator |
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.
LGTM 👍🏽
) This PR greatly improves validation of JSON HTTP response bodies in heartbeat in two different ways: Introduces a new notion of JSON expressions, using gval as an expression engine. This allows validation of complex data, including the use of JSON Path expressions within. As an example you can write an expression like: $.nodes[?(@name == "foo"][2].message == "hello" to traverse a JSON object containing a nodes key with nested objects, find all elements named "foo", select the third one, then see if its' message attribute equals "hello". Supports handling APIs that return arrays of JSON objects without a top level object root. Previously, we only parsed JSON response that looked like an object, e.g. {"an": "object"}, we now support: [{"one": 1},{"two": 2}]. Deprecates the condition configuration in favor of this new expression syntax. The new syntax is more consistent and powerful, and should be less effort to maintain. It also splits out the JSON checking code into separate files to cut down on the extreme length of the originals. Fixes #27283 (cherry picked from commit 5ae6d63) # Conflicts: # go.mod
…ion of JSON arrays (#28137) * [Heartbeat] Support JSON expressions / validation of JSON arrays (#28073) This PR greatly improves validation of JSON HTTP response bodies in heartbeat in two different ways: Introduces a new notion of JSON expressions, using gval as an expression engine. This allows validation of complex data, including the use of JSON Path expressions within. As an example you can write an expression like: $.nodes[?(@name == "foo"][2].message == "hello" to traverse a JSON object containing a nodes key with nested objects, find all elements named "foo", select the third one, then see if its' message attribute equals "hello". Supports handling APIs that return arrays of JSON objects without a top level object root. Previously, we only parsed JSON response that looked like an object, e.g. {"an": "object"}, we now support: [{"one": 1},{"two": 2}]. Deprecates the condition configuration in favor of this new expression syntax. The new syntax is more consistent and powerful, and should be less effort to maintain. It also splits out the JSON checking code into separate files to cut down on the extreme length of the originals. Fixes #27283 (cherry picked from commit 5ae6d63) # Conflicts: # go.mod * Fix merge conflict Co-authored-by: Andrew Cholakian <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
…stic#28073) This PR greatly improves validation of JSON HTTP response bodies in heartbeat in two different ways: Introduces a new notion of JSON expressions, using gval as an expression engine. This allows validation of complex data, including the use of JSON Path expressions within. As an example you can write an expression like: $.nodes[?(@name == "foo"][2].message == "hello" to traverse a JSON object containing a nodes key with nested objects, find all elements named "foo", select the third one, then see if its' message attribute equals "hello". Supports handling APIs that return arrays of JSON objects without a top level object root. Previously, we only parsed JSON response that looked like an object, e.g. {"an": "object"}, we now support: [{"one": 1},{"two": 2}]. Deprecates the condition configuration in favor of this new expression syntax. The new syntax is more consistent and powerful, and should be less effort to maintain. It also splits out the JSON checking code into separate files to cut down on the extreme length of the originals. Fixes elastic#27283
This PR greatly improves validation of JSON HTTP response bodies in heartbeat in two different ways:
expressions
, using gval as an expression engine. This allows validation of complex data, including the use of JSON Path expressions within. As an example you can write an expression like:$.nodes[?(@name == "foo"][2].message == "hello"
to traverse a JSON object containing a nodes key with nested objects, find all elements named "foo", select the third one, then see if its'message
attribute equals "hello".{"an": "object"}
, we now support:[{"one": 1},{"two": 2}]
.condition
configuration in favor of this newexpression
syntax. The new syntax is more consistent and powerful, and should be less effort to maintain.It also splits out the JSON checking code into separate files to cut down on the extreme length of the originals.
Fixes #27283
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.How to test this PR locally
Try the docs examples