Fixing Issue #200 (Adding support for various ctx variable fields) #504

toepkerd-zz · 2022-07-27T00:45:42Z

Signed-off-by: Dennis Toepker toepkerd@amazon.com

Issue #200

Added support for various ctx fields by implementing asTemplateArg() in various classes

CheckList:

Commits are signed per the DCO using --signoff

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.

…#486) * Added 2.1 release notes. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Added 2.1 release notes. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> (cherry picked from commit 9f3c393) Co-authored-by: AWSHurneyt <hurneyt@amazon.com>

* Added 2.1 release notes. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Added 2.1 release notes. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

…in various classes Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

codecov-commenter · 2022-07-27T01:24:02Z

Codecov Report

Merging #504 (6ebfdce) into main (6d3fb50) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #504      +/-   ##
============================================
- Coverage     76.89%   76.86%   -0.03%     
- Complexity      176      178       +2     
============================================
  Files           166      166              
  Lines          8370     8399      +29     
  Branches       1232     1232              
============================================
+ Hits           6436     6456      +20     
- Misses         1329     1340      +11     
+ Partials        605      603       -2

Impacted Files	Coverage Δ
...in/kotlin/org/opensearch/alerting/model/Trigger.kt	`80.00% <ø> (ø)`
.../kotlin/org/opensearch/alerting/util/IndexUtils.kt	`70.83% <ø> (-2.09%)`	⬇️
...kotlin/org/opensearch/alerting/core/model/Input.kt	`75.00% <ø> (ø)`
...selectorext/BucketSelectorExtAggregationBuilder.kt	`56.71% <100.00%> (+2.45%)`	⬆️
...rg/opensearch/alerting/model/BucketLevelTrigger.kt	`89.23% <100.00%> (-1.54%)`	⬇️
.../opensearch/alerting/model/DocumentLevelTrigger.kt	`81.33% <100.00%> (+0.25%)`	⬆️
...in/kotlin/org/opensearch/alerting/model/Monitor.kt	`88.63% <100.00%> (+0.61%)`	⬆️
...org/opensearch/alerting/model/QueryLevelTrigger.kt	`85.91% <100.00%> (+0.62%)`	⬆️
...lin/org/opensearch/alerting/model/action/Action.kt	`86.59% <100.00%> (+0.57%)`	⬆️
...nsearch/alerting/core/model/ClusterMetricsInput.kt	`92.35% <100.00%> (+0.04%)`	⬆️
... and 12 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

qreshi · 2022-07-28T20:56:12Z

...org/opensearch/alerting/aggregation/bucketselectorext/BucketSelectorExtAggregationBuilder.kt

@@ -19,7 +20,7 @@ import org.opensearch.search.aggregations.pipeline.AbstractPipelineAggregationBu
 import org.opensearch.search.aggregations.pipeline.BucketHelpers.GapPolicy
 import org.opensearch.search.aggregations.pipeline.PipelineAggregator
 import java.io.IOException
-import java.util.Objects
+import java.util.*


We typically avoid wildcard imports since they are very generic in the libraries they end up including. If you're using Intellij, it is probably doing this for you automatically. You can refer to this to disable that behavior.

qreshi · 2022-07-28T20:58:56Z

alerting/src/main/kotlin/org/opensearch/alerting/model/DocumentLevelTrigger.kt

@@ -66,11 +70,13 @@ data class DocumentLevelTrigger(
    }

    /** Returns a representation of the trigger suitable for passing into painless and mustache scripts. */
-    fun asTemplateArg(): Map<String, Any> {
+    override fun asTemplateArg(): Map<String, Any?> {
+        log.info("inside docleveltrigger astemplatearg")


Can remove this if it was only used for debugging purposes. Same goes for the other log.infos in asTemplateArg()s.

qreshi · 2022-07-28T21:03:52Z

alerting/src/test/kotlin/org/opensearch/alerting/MonitorTests.kt

+        // WHEN
+        val templateArgs = monitor.asTemplateArg()
+
+        this.logger.info("template args in log: $templateArgs")


This can be removed.

qreshi · 2022-07-28T21:14:05Z

alerting/src/test/kotlin/org/opensearch/alerting/MonitorTests.kt

+        assertEquals(
+            "Template args monitor type does not match\nhere's templateArgs: $templateArgs \n" +
+                "here's monitor: $monitor \n here's toXContent: ${monitor.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)}",
+            templateArgs[Monitor.MONITOR_TYPE_FIELD], monitor.monitorType.toString()
+        )


I believe the assertion method will output the expected and actual arguments when the assertion fails but if you do want to keep the message as-is, using a raw string with variable templating might be a cleaner solution.

Example:

assertEquals( """ Template args monitor type does not match. Template args was: $templateArgs Monitor is: $monitor """.trimIndent(), templateArgs[Monitor.MONITOR_TYPE_FIELD], monitor.monitorType.toString() )

qreshi · 2022-07-28T21:17:53Z

release-notes/opensearch-alerting.release-notes-2.1.0.0.md

@@ -0,0 +1,9 @@
+## Version 2.1.0.0 2022-07-06


This may have been pulled into the PR because your dev branch is old. Could you rebase your dev branch with the current main?

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

…ensearch-project#502) * Version increment automation Signed-off-by: pgodithi <pgodithi@amazon.com> * Version increment automation: task rename updateVersion Signed-off-by: pgodithi <pgodithi@amazon.com> (cherry picked from commit 3b8bfe7) Signed-off-by: prudhvigodithi <pgodithi@amazon.com> Co-authored-by: Prudhvi Godithi <pgodithi@amazon.com>

….1.0.0 zip following deprecation of ODFE. (opensearch-project#510) (opensearch-project#511) Signed-off-by: AWSHurneyt <hurneyt@amazon.com> (cherry picked from commit fea6b4a) Co-authored-by: AWSHurneyt <hurneyt@amazon.com>

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

lezzago · 2023-01-19T18:16:15Z

Closing this PR as there are many conflicts and many of the changes have been merged in this PR: opensearch-project/common-utils#318.

May need to reopen another PR to add support for anything else missing.

opensearch-trigger-bot bot and others added 5 commits July 6, 2022 14:42

Added 2.1 release notes. (opensearch-project#485)

4c0e1b9

* Added 2.1 release notes. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Added 2.1 release notes. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

Added support for various ctx fields by implementing asTemplateArg() …

15b6872

…in various classes Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

updated commit with signoff

f5b25af

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

another attempt commit with signoff

b0cd900

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

toepkerd-zz force-pushed the 2.1 branch from e38b310 to b0cd900 Compare July 27, 2022 01:19

toepkerd-zz marked this pull request as ready for review July 27, 2022 01:38

toepkerd-zz requested a review from a team July 27, 2022 01:38

toepkerd-zz changed the title ~~2.1~~ Fixing Issue #200 (Adding support for various ctx variable fields) Jul 27, 2022

qreshi reviewed Jul 28, 2022

View reviewed changes

toepkerd-zz added 11 commits July 29, 2022 18:07

added integration tests and other small changes

89fcd70

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

deleted another log statement

b63f01d

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

made sure to delete wildcard imports

53e6485

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

adjusted randomMonitor() to pass in correct Inputs

b1446f3

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

deleted some log statements

f00bf8c

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

Merge branch 'main' of https://github.com/opensearch-project/alerting

99ecb03

deleted another log statement

c55ebe3

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

made sure to delete wildcard imports

34abd0b

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

adjusted randomMonitor() to pass in correct Inputs

77f59f7

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

deleted some log statements

19ed791

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

Merge branch '2.1' of github.com:toepkerd/alerting into 2.1

e7b82f3

toepkerd-zz added the backport 2.x label Aug 1, 2022

toepkerd-zz and others added 7 commits August 1, 2022 15:38

found another semi-log statement to delete

539b88f

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

Merge branch 'opensearch-project:2.1' into 2.1

6ebfdce

Merge branch 'main' of https://github.com/opensearch-project/alerting

7d77eb7

deleted another log statement

8c6d990

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

made sure to delete wildcard imports

cb5c760

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

toepkerd-zz added 4 commits August 5, 2022 16:06

adjusted randomMonitor() to pass in correct Inputs

b6d1f6c

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

deleted some log statements

1712fc7

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

found another semi-log statement to delete

e683222

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>

Merge branch '2.1' of github.com:toepkerd/alerting into 2.1

71de011

qreshi mentioned this pull request Jan 6, 2023

[BUG] Missing ctx variables for Actions #200

Open

lezzago closed this Jan 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing Issue #200 (Adding support for various ctx variable fields) #504

Fixing Issue #200 (Adding support for various ctx variable fields) #504

toepkerd-zz commented Jul 27, 2022 •

edited

Loading

codecov-commenter commented Jul 27, 2022 •

edited

Loading

qreshi Jul 28, 2022

qreshi Jul 28, 2022

qreshi Jul 28, 2022

qreshi Jul 28, 2022

qreshi Jul 28, 2022

lezzago commented Jan 19, 2023

Fixing Issue #200 (Adding support for various ctx variable fields) #504

Fixing Issue #200 (Adding support for various ctx variable fields) #504

Conversation

toepkerd-zz commented Jul 27, 2022 • edited Loading

codecov-commenter commented Jul 27, 2022 • edited Loading

Codecov Report

qreshi Jul 28, 2022

Choose a reason for hiding this comment

qreshi Jul 28, 2022

Choose a reason for hiding this comment

qreshi Jul 28, 2022

Choose a reason for hiding this comment

qreshi Jul 28, 2022

Choose a reason for hiding this comment

qreshi Jul 28, 2022

Choose a reason for hiding this comment

lezzago commented Jan 19, 2023

toepkerd-zz commented Jul 27, 2022 •

edited

Loading

codecov-commenter commented Jul 27, 2022 •

edited

Loading