Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add query structure example #6994
Add query structure example #6994
Changes from 12 commits
7599b89
c1fc3c8
2998d20
07ac98c
e07a229
1fb20ed
7a3ab13
f0805cc
699e246
ba57150
92427b3
d57736c
6a13c15
f210148
1e4528a
527df8f
0e41535
b49cad7
5bc9869
37bc046
4a79182
bcccdf1
4b053e9
fc9321c
08fa092
c27f9c2
5ff7c38
fff652a
1be1f2b
f80b8d6
acd6615
2a7ff8e
62b68ba
b877e85
9323623
b00fb4f
30be2d0
327d65c
dcd7d07
4eb590a
945aa1f
55a64ac
dbec55c
650400a
791aedf
60c2e74
d0c5724
076b88e
9ab7285
6a37a45
89996af
601dea4
0268abb
8e3efd5
66bf288
2f84646
fec48e5
37531cd
d896314
0e7e597
0adb24b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 plugin comes with the default distribution, right? We can mention that so people don't think they need to install it in all cases.
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 would remove this. I would not prescribe what environment to use. What if you want to test it in production? :)
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.
@AWSHurneyt Do we need to be running a certain OpenSearch version?
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.
The alerting plugin has been available since before OpenSearch 1.0; so the alerting plugin version would just need to match the OpenSearch version.
Check failure on line 38 in _observing-your-data/alerting/dashboards-alerting.md
GitHub Actions / vale
[vale] _observing-your-data/alerting/dashboards-alerting.md#L38
Raw output
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.
Should we add a little more information about this requirement? Is there any special setup needed on the client side?
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.
@AWSHurneyt Please provide feedback on the above question. Should we link to the Vizlib GitHub repo? https://github.com/vizlib
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.
@lezzago this question seems related to the feature anywhere visualizations. Could you comment?
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.
@vagimeli these are the normal line charts. Vizlib is mentioned because it is the visualization library behind rendering them. There should be no need to add more information about it. We can even remove the
Vizlib
name since it might add more confusion to the user.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.
On the bottom as opposed to where?
Check failure on line 80 in _observing-your-data/alerting/dashboards-alerting.md
GitHub Actions / vale
[vale] _observing-your-data/alerting/dashboards-alerting.md#L80
Raw output
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.
Would it be better to use parallel verbs? associate/disassociate or link/unlink?
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.
Revised.
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.
The
interval
, andperiod
fields are reversed in this example.This is an example monitor that we use for some of our frontend tests.
https://github.com/opensearch-project/alerting-dashboards-plugin/blob/main/cypress/fixtures/sample_alerts_flyout_query_level_monitor.json#L7-L9
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.
Looks like the example in this PR is also missing the
enabled
field.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.
The
trigger
attribute of the monitor is an array calltriggers
as monitors can be configured with multiple triggers.https://github.com/opensearch-project/alerting-dashboards-plugin/blob/main/cypress/fixtures/sample_alerts_flyout_query_level_monitor.json#L44
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.
The
input
attribute of the monitor is an array calledinputs
. Currently, monitors only support 1 input, but that attribute was implemented as an array to support future enhancements.https://github.com/opensearch-project/alerting-dashboards-plugin/blob/main/cypress/fixtures/sample_alerts_flyout_query_level_monitor.json#L12