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

[grafana] set unified_alerting.screenshots.capture to true when imageRenderer is enabled #3537

Closed
wants to merge 2 commits into from

Conversation

kefi550
Copy link

@kefi550 kefi550 commented Jan 22, 2025

Set environment variable GF_UNIFIED_ALERTING_SCREENSHOTS_CAPTURE to true when imageRenderer.enabled is true.
I changed values.yaml so that unified_alerting.screenshots.capture is true in grafana.ini when imageRenderer is enabled.
This will cause screenshots to be attached to alerts when imageRenderer.enabled is set to true.

As mentioned in #1556, currently screenshot is not attached to alerts even though imageRenderer.enabled is set to true.
It would be natural for alerts to have a screenshot attached when the imageRenderer is enabled.

@CLAassistant
Copy link

CLAassistant commented Jan 22, 2025

CLA assistant check
All committers have signed the CLA.

@jkroepke
Copy link
Collaborator

I feel this should be configurable. and please bump the chart version.

@kefi550 kefi550 force-pushed the set_env_screenshot branch from e0f61f0 to 2ae126d Compare January 22, 2025 13:20
@kefi550
Copy link
Author

kefi550 commented Jan 22, 2025

As you wrote, it's not good that this env variable is not configurable, so I've made this env variable configurable.
And bumped version.

Thank you for reviewing.

@jkroepke
Copy link
Collaborator

I read your message, but I think is should be configurable with default true. Sometimes, there are reasons to disable "cool" feature, for example for cost reasons.

@kefi550 kefi550 force-pushed the set_env_screenshot branch from 2ae126d to c903715 Compare January 25, 2025 14:58
@kefi550 kefi550 changed the title [grafana] set GF_UNIFIED_ALERTING_SCREENSHOTS_CAPTURE to true when imageRenderer is enabled [grafana] set unified_alerting.screenshots.capture to true when imageRenderer is enabled Jan 25, 2025
@kefi550
Copy link
Author

kefi550 commented Jan 25, 2025

I understood that setting in environment variables is higher priority than grafana.ini and is not truly configurable.

I changed values.yaml so that unified_alerting.screenshots.capture is true in grafana.ini when imageRenderer is enabled.
This allows configuring the behavior of unified_alerting.screenshots.capture by changing grafana.ini or env in values.yaml.

(The title and description of the PR have been changed accordingly)

@jkroepke
Copy link
Collaborator

Hi @kefi550

I guess your are using GenAI, I used GenAi as well:

Q: Should unified_alerting.screenshots.capture always set to true if the image renderer is enabled?

A: Based on the knowledge sources provided, I can answer your question about the unified_alerting.screenshots.capture setting in Grafana.  The unified_alerting.screenshots.capture setting should not always be set to true just because the image renderer is enabled. This setting specifically controls whether screenshots are captured for alert notifications, which is a separate feature from the general image rendering capabilities. According to the Grafana documentation on image notifications, you need to explicitly enable screenshot capture for alert notifications by setting capture to true in the [unified_alerting.screenshots] section of the configuration:  

Enable screenshots in notifications. You must have either installed the Grafana image rendering # plugin, or set up Grafana to use a remote rendering service. # For more information on configuration options, refer to [rendering]. capture = false

 The default value is false, which means screenshots are not captured for alert notifications even if the image renderer is enabled.  It's important to note that enabling this feature has some requirements and considerations:  1. You must have the image rendering capability set up, either through the Grafana Image Renderer plugin or a remote rendering service. 2. Capturing screenshots for alerts can be resource-intensive, especially in terms of memory usage. 3. You need to consider where the screenshots will be stored or uploaded, which may require additional configuration.  Therefore, you should only set unified_alerting.screenshots.capture to true if you specifically want to include screenshots in alert notifications and have ensured that your system meets the necessary requirements to support this feature.  For more detailed information on configuring and using images in notifications, you can refer to the Grafana documentation on using images in notifications.

Based on that feedback, the value should not be forced to yes.

@jkroepke jkroepke closed this Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants