feature: tune description on monitor and anomaly detection #120

yuye-aws · 2024-01-11T01:12:00Z

Description

This PR tunes description on monitor, anomaly detection, and alert. The description follows the hugging face template: https://huggingface.co/docs/transformers/custom_tools#custom-tools-and-prompts

Issues Resolved

This PR tunes description on monitor and anomaly detection.

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
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.

Signed-off-by: yuye-aws <[email protected]>

xinyual · 2024-01-11T03:12:02Z

Have you checked the performance of new version description?

yuye-aws · 2024-01-11T03:49:04Z

Have you checked the performance of new version description?

Good point! I am planning to check the performance of different descriptions later. As parameter description and returned result is missing now, can we include these information and replace the current description with the new description following hugging face format?

Hailong-am · 2024-01-11T04:06:01Z

integration test depends on k-NN PR to merged opensearch-project/k-NN#1383

ohltyler

Few nits, overall LGTM as a starting point that we can tune later. Can you also include a similar description style for the search alerts tool? That will cover all of the AD and alerting tools.

ohltyler · 2024-01-11T17:22:04Z

src/main/java/org/opensearch/agent/tools/SearchAnomalyDetectorsTool.java

@@ -48,7 +48,8 @@
 @ToolAnnotation(SearchAnomalyDetectorsTool.TYPE)
 public class SearchAnomalyDetectorsTool implements Tool {
    public static final String TYPE = "SearchAnomalyDetectorsTool";
-    private static final String DEFAULT_DESCRIPTION = "Use this tool to search anomaly detectors.";
+    private static final String DEFAULT_DESCRIPTION =
+        "This is a tool that searches anomaly detectors. It takes 12 optional arguments named detectorName which is the explicit name of the monitor (default is null), and detectorNamePattern which is a wildcard query to match detector name (default is null), and indices which defines the index being detected (default is null), and highCardinality which defines whether the anomaly detector is high cardinality (default is null), and lastUpdateTime which defines the latest update time of the anomaly detector (default is null), and sortOrder which defines the order of the results (options are asc or desc, and default is asc), and sortString which defines how to sort the results (default is name.keyword), and size which defines the size of the request to be returned (default is 20), and startIndex which defines the index to start from (default is 0), and running which defines whether the anomaly detector is running (default is null, indicating both), and disabled which defines whether the anomaly detector is disabled (default is null, indicating both), and failed which defines whether the anomaly detector has failed (default is null, indicating both). The tool returns the a list of anomaly detectors.";


nit: Should be "returns the list of anomaly detectors, and the total number of anomaly detectors" for the last sentence

ohltyler · 2024-01-11T17:22:30Z

src/main/java/org/opensearch/agent/tools/SearchAnomalyResultsTool.java

@@ -39,7 +39,8 @@
 @ToolAnnotation(SearchAnomalyResultsTool.TYPE)
 public class SearchAnomalyResultsTool implements Tool {
    public static final String TYPE = "SearchAnomalyResultsTool";
-    private static final String DEFAULT_DESCRIPTION = "Use this tool to search anomaly results.";
+    private static final String DEFAULT_DESCRIPTION =
+        "This is a tool that searches anomaly results. It takes 9 arguments named detectorId which defines the detector ID to filter for (default is null), and realtime which defines whether the anomaly is real time, and anomalyGradeThreshold which defines the threshold for anomaly grade (a number between 0 and 1 that indicates how anomalous a data point is) (default is 0), and dataStartTime which defines the start time of the anomaly query (default is null), and dataEndTime which defines the end time of the anomaly query (default is null), and sortOrder which defines the order of the results (options are asc or desc, and default is desc), and sortString which which defines how to sort the results (default is data_start_time), and size which defines the size of the request to be returned (default is 20), and startIndex which defines the index to start from (default is 0).The tool returns a list of anomaly results.";


Same thing as previous comment - maybe something like "The tool returns a list of anomaly results, and the total number of anomaly results"

ohltyler · 2024-01-11T17:22:52Z

src/main/java/org/opensearch/agent/tools/SearchMonitorsTool.java

@@ -44,7 +44,8 @@
 @ToolAnnotation(SearchMonitorsTool.TYPE)
 public class SearchMonitorsTool implements Tool {
    public static final String TYPE = "SearchMonitorsTool";
-    private static final String DEFAULT_DESCRIPTION = "Use this tool to search alerting monitors.";
+    private static final String DEFAULT_DESCRIPTION =
+        "This is a tool that searches alerting monitors. It takes 10 optional arguments named monitorId which defines the monitor ID to filter for (default is null), and monitorName which defines explicit name of the monitor (default is null), and monitorNamePattern which is a wildcard query to match detector name (default is null), and enabled which defines whether the monitor is enabled (default is null, indicating both), and hasTriggers which defines whether the monitor has triggers enabled (default is null, indicating both), and indices which defines the index being monitored (default is null), and sortOrder which defines the order of the results (options are asc or desc, and default is asc), and sortString which defines how to sort the results (default is name.keyword), and size which defines the size of the request to be returned (default is 20), and startIndex which defines the index to start from (default is 0). The tool returns a list of monitors.";


Same thing as previous comment - maybe something like "The tool returns a list of monitors, and the total number of monitors"

yuye-aws · 2024-01-12T01:15:31Z

Thanks for the review comment. I will

Few nits, overall LGTM as a starting point that we can tune later. Can you also include a similar description style for the search alerts tool? That will cover all of the AD and alerting tools.

I will include the change on search alerts tool in this PR. Please help review, thanks!

yuye-aws · 2024-01-12T01:17:59Z

Thanks for supplementing description on tool return results. I will apply your change.

Signed-off-by: yuye-aws <[email protected]>

codecov · 2024-01-12T01:46:59Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cf20a00) 81.23% compared to head (6c8c705) 80.82%.
Report is 2 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #120      +/-   ##
============================================
- Coverage     81.23%   80.82%   -0.42%     
  Complexity      193      193              
============================================
  Files            13       13              
  Lines           975      975              
  Branches        130      130              
============================================
- Hits            792      788       -4     
- Misses          133      137       +4     
  Partials         50       50

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

yuye-aws · 2024-01-12T01:47:03Z

@ohltyler I have included description on search alert tool and updated description on returned results from tools. This PR is ready for review now.

ohltyler

LGTM! Thanks again for helping out on this!

Signed-off-by: yuye-aws <[email protected]> (cherry picked from commit c8b6898) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

(cherry picked from commit c8b6898) Signed-off-by: yuye-aws <[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>

…h-project#120) (opensearch-project#126) (cherry picked from commit c8b6898) Signed-off-by: yuye-aws <[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> Signed-off-by: yuye-aws <[email protected]>

tune description on monitor and anomaly detection

0d7f939

Signed-off-by: yuye-aws <[email protected]>

yuye-aws requested review from b4sjoo, dhrubo-os, jngz-es, model-collapse, rbhavna, ylwu-amzn, zane-neo, Zhangxunmt, xinyual, amitgalitz, jackiehanyang, owaiskazi19, ohltyler, joshpalis, dbwiddis, kaituo, lezzago, eirsep, sbcd90, zhichao-aws and joshuali925 as code owners January 11, 2024 01:12

opensearch-trigger-bot bot added the backport 2.x label Jan 11, 2024

spotless apply

36fef5e

Signed-off-by: yuye-aws <[email protected]>

yuye-aws changed the title ~~featu: tune description on monitor and anomaly detection~~ feature: tune description on monitor and anomaly detection Jan 11, 2024

ohltyler reviewed Jan 11, 2024

View reviewed changes

yuye-aws added 3 commits January 12, 2024 09:42

add descripton on search alerts tool

4f27dbb

Signed-off-by: yuye-aws <[email protected]>

update description on tool returned results

b8cc807

Signed-off-by: yuye-aws <[email protected]>

update returned result description on search alert tool

6c8c705

Signed-off-by: yuye-aws <[email protected]>

yuye-aws requested a review from ohltyler January 12, 2024 05:48

ohltyler approved these changes Jan 12, 2024

View reviewed changes

ohltyler merged commit c8b6898 into opensearch-project:main Jan 12, 2024
13 checks passed

opensearch-trigger-bot bot mentioned this pull request Jan 12, 2024

[Backport 2.x] feature: tune description on monitor and anomaly detection #126

Merged

yuye-aws deleted the feature/descriptionOnMonitorAndAD branch November 21, 2024 03:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: tune description on monitor and anomaly detection #120

feature: tune description on monitor and anomaly detection #120

yuye-aws commented Jan 11, 2024 •

edited

Loading

xinyual commented Jan 11, 2024

yuye-aws commented Jan 11, 2024

Hailong-am commented Jan 11, 2024

ohltyler left a comment

ohltyler Jan 11, 2024

ohltyler Jan 11, 2024

ohltyler Jan 11, 2024

yuye-aws commented Jan 12, 2024

yuye-aws commented Jan 12, 2024

codecov bot commented Jan 12, 2024 •

edited

Loading

yuye-aws commented Jan 12, 2024

ohltyler left a comment

feature: tune description on monitor and anomaly detection #120

feature: tune description on monitor and anomaly detection #120

Conversation

yuye-aws commented Jan 11, 2024 • edited Loading

Description

Issues Resolved

Check List

xinyual commented Jan 11, 2024

yuye-aws commented Jan 11, 2024

Hailong-am commented Jan 11, 2024

ohltyler left a comment

Choose a reason for hiding this comment

ohltyler Jan 11, 2024

Choose a reason for hiding this comment

ohltyler Jan 11, 2024

Choose a reason for hiding this comment

ohltyler Jan 11, 2024

Choose a reason for hiding this comment

yuye-aws commented Jan 12, 2024

yuye-aws commented Jan 12, 2024

codecov bot commented Jan 12, 2024 • edited Loading

Codecov Report

yuye-aws commented Jan 12, 2024

ohltyler left a comment

Choose a reason for hiding this comment

yuye-aws commented Jan 11, 2024 •

edited

Loading

codecov bot commented Jan 12, 2024 •

edited

Loading