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

Alerting: Use URLs in image annotations #66804

Merged
merged 4 commits into from
Apr 26, 2023

Conversation

santihernandezc
Copy link
Contributor

Description

This PR prioritizes URLs over tokens in ImageTokenAnnotation, which will allow us to add support for images in the Grafana Cloud Alertmanager.

There are two possibilities for ImageTokenAnnotation:

  • The first option is to use the image URL.
  • If there's none, the token will be prefixed by token:// and used in the annotation.

Changes

  • The ImageTokenAnnotation is a URI that can be either the image URL or the image token with a prefix.
  • URLs can be used to look for images, so integrations can still query the DB using the value extracted from ImageTokenAnnotation.
  • Instead of adding a *models.Image in state changes, the state manager just adds a URI (string).

Advantages over the previous PR

This PR is meant to replace #66327, and it has a series of advantages over the latter:

  • No modifications to our database schemas.
  • The Token field is still relevant and used to identify images.
  • No duplicate fields (in case of no image URL, we prefixed the Path value and added it to the url column).

Related issue: https://github.com/grafana/alerting-squad/issues/458

@santihernandezc santihernandezc marked this pull request as ready for review April 24, 2023 13:56
@santihernandezc santihernandezc requested a review from a team April 24, 2023 13:56
@santihernandezc santihernandezc added this to the 10.0.0 milestone Apr 24, 2023
pkg/services/ngalert/notifier/images.go Outdated Show resolved Hide resolved
pkg/services/ngalert/notifier/images.go Outdated Show resolved Hide resolved
pkg/services/ngalert/notifier/images.go Outdated Show resolved Hide resolved
pkg/services/ngalert/notifier/images.go Outdated Show resolved Hide resolved
pkg/services/ngalert/state/state.go Outdated Show resolved Hide resolved
Copy link
Contributor

@yuri-tceretian yuri-tceretian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@santihernandezc santihernandezc merged commit b0881da into main Apr 26, 2023
@santihernandezc santihernandezc deleted the santihernandezc/add_url_to_image_annotation branch April 26, 2023 16:06
@zerok zerok modified the milestones: 10.0.0, 10.0.0-preview May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants