Skip to content
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

[BUG] Missing ctx variables for Actions #200

Open
qreshi opened this issue Oct 11, 2021 · 10 comments
Open

[BUG] Missing ctx variables for Actions #200

qreshi opened this issue Oct 11, 2021 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@qreshi
Copy link
Contributor

qreshi commented Oct 11, 2021

Describe the bug
There is a discrepancy between the ctx variables mentioned in the documentation and those actually made available in the plugin.

Expected behavior
The available ctx variables should match what the documentation details.

Additional context
This is not a regression as it seems we never supported the missing ctx variables. However, since I believe that the missing ctx variables would be useful to users, we should add them to the plugin instead of amending the documentation.

@qreshi qreshi added bug Something isn't working Beta untriaged labels Oct 11, 2021
@qreshi
Copy link
Contributor Author

qreshi commented Oct 11, 2021

Some related issues: #57 #99.

After triage, we can include the exact difference in supported ctx variables and those included in the documentation to scope out the changes needed.

@qreshi qreshi removed the Beta label Oct 11, 2021
@qreshi
Copy link
Contributor Author

qreshi commented Nov 16, 2021

Including #50 here as well since the documentation shows ctx.monitor.inputs.search.query which I believe is what that user was looking for but is not available in ctx.

@qreshi
Copy link
Contributor Author

qreshi commented Nov 18, 2021

Here is a breakdown of all the ctx variables and any discrepancies between their inclusion in documentation, current support and intention to support. Making the changes themselves are straightforward, we'll just first need to agree on what we do want to support and what we don't.

Monitor variables

Variable In Documentation Currently Supported Should be supported
ctx.monitor._id No Yes Yes
ctx.monitor._version No Yes Yes
ctx.monitor.name Yes Yes Yes
ctx.monitor.type Yes No No. This is just the scheduled job type which will always be "monitor"
ctx.monitor.monitor_type No No Yes. This could be useful since there are multiple Monitor types now
triggers Yes No No. This seems to be a typo in documentation, the trigger related ctx variables are mentioned separately
ctx.monitor.user Yes No No, user information is not returned in Monitor responses
ctx.monitor.enabled Yes Yes Yes, although if the Action can be sent this should always be true
ctx.monitor.enabled_time Yes No Yes
ctx.monitor.last_update_time Yes No Yes
ctx.monitor.schedule Yes No Yes
ctx.monitor.schedule.period.interval Yes No Yes but the schedule can be a cron instead of a period
ctx.monitor.schedule.period.unit Yes No Yes but same as above, schedule can be a cron instead of a `period
ctx.monitor.schedule.cron.expression No No Yes if schedule.period will be supported
ctx.monitor.schedule.cron.timezone No No Yes if schedule.period will be supported
ctx.monitor.inputs Yes No Yes
ctx.monitor.inputs.search.indices Yes No Yes
ctx.monitor.inputs.search.query Yes No Yes

Trigger variables

Variable In Documentation Currently Supported Should be supported
ctx.trigger.id Yes Yes Yes
ctx.trigger.name Yes Yes Yes
ctx.trigger.severity Yes Yes Yes
ctx.trigger.condition Yes No Yes
ctx.trigger.condition.script.source Yes (but the description is wrong) No Yes
ctx.trigger.condition.script.lang Yes (but the description is wrong) No Yes
ctx.trigger.actions Yes Yes Yes, although if it only contains the one action it should have been called ctx.trigger.action (but since it already exists we can't change it)

Action variables

Variable In Documentation Currently Supported Should be supported
ctx.trigger.actions.id Yes No Yes
ctx.trigger.actions.name Yes Yes Yes
ctx.trigger.actions.destination_id Yes No Yes
ctx.trigger.actions.message_template.source Yes No No
ctx.trigger.actions.message_template.lang Yes No No (this is always mustache for now)
ctx.trigger.actions.throttle_enabled Yes No Yes
ctx.trigger.actions.subject_template.source Yes No No
ctx.trigger.actions.subject_template.lang Yes No No (this is always mustache for now)

Other variables

Variable In Documentation Currently Supported Should be supported
ctx.results Yes Yes Yes
ctx.last_update_time Yes No No, this will be covered by ctx.monitor.last_update_time when it is supported
ctx.periodStart Yes Yes Yes
ctx.periodEnd Yes Yes Yes
ctx.error Yes Yes Yes
ctx.alert Yes Yes Yes
ctx.dedupedAlerts Yes Yes Yes
ctx.newAlerts Yes Yes Yes
ctx.completedAlerts Yes Yes Yes
bucket_keys Yes Yes Yes
parent_bucket_path Yes Yes Yes

Below are also the asTemplateArg() methods of the relevant areas in the code which are used to populate the contents of ctx for reference.

TriggerExecutionContext:

open fun asTemplateArg(): Map<String, Any?> {
return mapOf(
"monitor" to monitor.asTemplateArg(),
"results" to results,
"periodStart" to periodStart,
"periodEnd" to periodEnd,
"error" to error
)
}

QueryLevelTriggerExecutionContext:

override fun asTemplateArg(): Map<String, Any?> {
val tempArg = super.asTemplateArg().toMutableMap()
tempArg["trigger"] = trigger.asTemplateArg()
tempArg["alert"] = alert?.asTemplateArg()
return tempArg
}

BucketLevelTriggerExecutionContext:

override fun asTemplateArg(): Map<String, Any?> {
val tempArg = super.asTemplateArg().toMutableMap()
tempArg["trigger"] = trigger.asTemplateArg()
tempArg["dedupedAlerts"] = dedupedAlerts.map { it.asTemplateArg() }
tempArg["newAlerts"] = newAlerts.map { it.asTemplateArg() }
tempArg["completedAlerts"] = completedAlerts.map { it.asTemplateArg() }
return tempArg
}

Monitor:

fun asTemplateArg(): Map<String, Any> {
return mapOf(_ID to id, _VERSION to version, NAME_FIELD to name, ENABLED_FIELD to enabled)
}

QueryLevelTrigger:

fun asTemplateArg(): Map<String, Any> {
return mapOf(
ID_FIELD to id, NAME_FIELD to name, SEVERITY_FIELD to severity,
ACTIONS_FIELD to actions.map { it.asTemplateArg() }
)
}

BucketLevelTrigger:

fun asTemplateArg(): Map<String, Any> {
return mapOf(
ID_FIELD to id,
NAME_FIELD to name,
SEVERITY_FIELD to severity,
ACTIONS_FIELD to actions.map { it.asTemplateArg() },
PARENT_BUCKET_PATH to getParentBucketPath()
)
}

Action:

fun asTemplateArg(): Map<String, Any> {
return mapOf(NAME_FIELD to name)
}

@qreshi qreshi removed the untriaged label Nov 18, 2021
@lezzago
Copy link
Member

lezzago commented Nov 22, 2021

The ctx.alert's documentation needs to mention this stores the previous state

@qreshi
Copy link
Contributor Author

qreshi commented Nov 26, 2021

After some discussion, the table above now reflects the current state of what is planned to be supported and what isn't.

There will also need to be a separate PR for the documentation for changes needed there, those will refer back to this issue for tracking purposes.

Do note that once the ctx variables are added we will not be removing them haphazardly since it would be a breaking change. However, for any ctx variables that we are choosing not to support at this time, they can easily be added in the future if users express interest in them.

@air3ijai
Copy link

Can someone provide details about above mentioned tables? For example, we are trying to add two variables to the message in Action:

Trigger:  {{ ctx.trigger.condition.script.source }}
Index:    {{ ctx.monitor.inputs.0.search.indices }}

It appear in preview but is empty in final destination - Slack in our case.
It means that it is not supported yet or not fully supported, why we see it in preview but can't see it in the Slack?

Screenshot 2022-03-12 at 15 39 24

Screenshot 2022-03-12 at 15 40 15

partlov pushed a commit to partlov/common-utils that referenced this issue Nov 10, 2022
- Issue opensearch-project/alerting#200

Signed-off-by: Petar Partlov <p.partlov@paysend.com>
partlov pushed a commit to partlov/common-utils that referenced this issue Nov 10, 2022
- Issue opensearch-project/alerting#200

Signed-off-by: Petar Partlov <partlov@gmail.com>
partlov pushed a commit to partlov/common-utils that referenced this issue Nov 10, 2022
- Issue opensearch-project/alerting#200

Signed-off-by: Petar Partlov <partlov@gmail.com>
partlov pushed a commit to partlov/common-utils that referenced this issue Nov 10, 2022
- Issue opensearch-project/alerting#200

Signed-off-by: Petar Partlov <partlov@gmail.com>
partlov pushed a commit to partlov/common-utils that referenced this issue Nov 10, 2022
- Issue opensearch-project/alerting#200

Signed-off-by: Petar Partlov <partlov@gmail.com>
partlov pushed a commit to partlov/common-utils that referenced this issue Nov 10, 2022
- Issue opensearch-project/alerting#200

Signed-off-by: Petar Partlov <partlov@gmail.com>
partlov pushed a commit to partlov/common-utils that referenced this issue Nov 10, 2022
- Issue opensearch-project/alerting#200

Signed-off-by: Petar Partlov <partlov@gmail.com>
partlov pushed a commit to partlov/common-utils that referenced this issue Nov 10, 2022
- Issue opensearch-project/alerting#200

Signed-off-by: Petar Partlov <partlov@gmail.com>
partlov pushed a commit to partlov/common-utils that referenced this issue Nov 15, 2022
- Issue opensearch-project/alerting#200

Signed-off-by: Petar Partlov <partlov@gmail.com>
partlov pushed a commit to partlov/common-utils that referenced this issue Nov 15, 2022
- Issue opensearch-project/alerting#200

Signed-off-by: Petar Partlov <partlov@gmail.com>
partlov pushed a commit to partlov/common-utils that referenced this issue Nov 15, 2022
- Issue opensearch-project/alerting#200

Signed-off-by: Petar Partlov <partlov@gmail.com>
partlov pushed a commit to partlov/common-utils that referenced this issue Nov 15, 2022
- Issue opensearch-project/alerting#200

Signed-off-by: Petar Partlov <partlov@gmail.com>
partlov pushed a commit to partlov/documentation-website that referenced this issue Nov 15, 2022
- Issue opensearch-project/alerting#200
- Make documentation consistent with code

Signed-off-by: Petar Partlov <partlov@gmail.com>
@partlov
Copy link
Contributor

partlov commented Nov 17, 2022

PRs created for code and documentation changes

@qreshi
Copy link
Contributor Author

qreshi commented Jan 6, 2023

Neither the PR (#504) nor the documentation changes have been made for this issue @praveensameneni. The PR will need to be rebased and merged and we need the documentation changes completely (across both ODFE and OpenSearch for consistency) before closing this out.

Update: It appears @partlov's PR is the most up to date since the contents of the ctx definition are in common-utils now. In that case, those would be the PRs to review before closing this out. We'd want to ensure that everything from the original PR has still been carried over during review.

@CarolynRemacle
Copy link

Is this scheduled to be part of an official release? I would really like to be able to include the monitor query and its components in some of my alert messages.

@jowg-amazon
Copy link
Collaborator

jowg-amazon commented Aug 12, 2024

Is this scheduled to be part of an official release? I would really like to be able to include the monitor query and its components in some of my alert messages.

@CarolynRemacle This issue should be fixed with this PR and will be available in the next release: opensearch-project/common-utils#710

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants