CorrelationAlert model added #631
Conversation
Signed-off-by: Riya Saxena <[email protected]>
riysaxen-amzn
requested review from
lezzago,
qreshi,
bowenlan-amzn,
rishabhmaurya,
getsaurabh02,
eirsep,
sbcd90,
AWSHurneyt,
engechas and
jowg-amazon
as code owners
Signed-off-by: Riya Saxena <[email protected]>
src/main/kotlin/org/opensearch/commons/alerting/model/UnifiedAlert.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/org/opensearch/commons/alerting/model/CorrelationAlert.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/org/opensearch/commons/alerting/model/UnifiedAlert.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/org/opensearch/commons/alerting/CorrelationAlertTests.kt
Show resolved
Hide resolved
Signed-off-by: Riya Saxena <[email protected]>
Signed-off-by: Riya Saxena <[email protected]>
|
|
||
| init { | ||
| if (errorMessage != null) { | ||
| require((state == Alert.State.DELETED) || (state == Alert.State.ERROR) || (state == Alert.State.AUDIT)) { |
There was a problem hiding this comment.
Why would alert state Deleted mandate a non-null error message
There was a problem hiding this comment.
yeah this one's in Alert state class also, so eventually Monitor based Alert (i.e Alert class) will also extend this BaseClass, therefore.
| } | ||
| } | ||
|
|
||
| val unifiedAlert = parse(xcp, version) |
There was a problem hiding this comment.
Which parse() method is called? Recursive?
There was a problem hiding this comment.
yes, so in this case, the parse function of CorrelationAlert is utilizing the parse function of its superclass BaseAlert to handle the parsing of fields common to both types of alerts. Then, it proceeds to parse the additional fields specific to CorrelationAlert
| } | ||
|
|
||
| @Throws(IOException::class) | ||
| constructor(sin: StreamInput) : this( |
There was a problem hiding this comment.
are there writeable tests for these constructors? like this. https://github.com/opensearch-project/common-utils/blob/main/src/test/kotlin/org/opensearch/commons/alerting/model/WriteableTests.kt
There was a problem hiding this comment.
no, but here src/test/kotlin/org/opensearch/commons/alerting/CorrelationAlertTests.kt in test correlation parse function we are validating such scenarios
| @@ -16,7 +17,11 @@ import java.io.ByteArrayOutputStream | |||
| fun getJsonString(xContent: ToXContent): String { | |||
| ByteArrayOutputStream().use { byteArrayOutputStream -> | |||
| val builder = XContentFactory.jsonBuilder(byteArrayOutputStream) | |||
| xContent.toXContent(builder, ToXContent.EMPTY_PARAMS) | |||
| if (xContent is CorrelationAlert) { | |||
There was a problem hiding this comment.
do we need this change? because this is not part of alerting package.
There was a problem hiding this comment.
yeah I'm using getJsonString method to convert correlation alert Json into string on line 87 in src/test/kotlin/org/opensearch/commons/alerting/CorrelationAlertTests.kt therefore changed this to make it work for CorrelationAlert
* CorrelationALert model added Signed-off-by: Riya Saxena <[email protected]> * fix klint errors Signed-off-by: Riya Saxena <[email protected]> * address the comments Signed-off-by: Riya Saxena <[email protected]> * fix klint errors Signed-off-by: Riya Saxena <[email protected]> --------- Signed-off-by: Riya Saxena <[email protected]> (cherry picked from commit e060f5e) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* CorrelationALert model added * fix klint errors * address the comments * fix klint errors --------- (cherry picked from commit e060f5e) Signed-off-by: Riya Saxena <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Issues Resolved
Alerts in Correlations
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.