-
Notifications
You must be signed in to change notification settings - Fork 34
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 template testing functionality #77
Conversation
This method will use the given alerts as template context data and attempt to interpolate each of the top-level definition blocks passed in through the template field. A top-level definition is one that isn't used by any others. Default templates and any already saved templates defined in the current AM as associated templates will be available for reference by the ones being tested. The exception to this is the saved template with the same name as the one passed via the name field. This is so that users can real-time test out modifications to existing templates (which is, ultimately, the end-goal of this endpoint).
@@ -246,6 +247,23 @@ func (am *GrafanaAlertmanager) TestReceivers(ctx context.Context, c TestReceiver | |||
return newTestReceiversResult(testAlert, append(invalid, results...), now), nil | |||
} | |||
|
|||
func (am *GrafanaAlertmanager) buildSingleIntegration(r *GrafanaIntegrationConfig, tmpl *templates.Template) (*Integration, error) { |
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 is moved from grafana/grafana
which seems correct since it's used only for receiver testing.
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.
Testing API code needs to be refactored to build by receiver
@@ -246,6 +247,23 @@ func (am *GrafanaAlertmanager) TestReceivers(ctx context.Context, c TestReceiver | |||
return newTestReceiversResult(testAlert, append(invalid, results...), now), nil | |||
} | |||
|
|||
func (am *GrafanaAlertmanager) buildSingleIntegration(r *GrafanaIntegrationConfig, tmpl *templates.Template) (*Integration, error) { |
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.
Testing API code needs to be refactored to build by receiver
I get a recursive stack depth error for the following:
I think we should be able to make this work as per the following example:
|
This patch works for me :)
I'm not sure what you want to do with this code though as it's not required in the design doc for what we're going to deliver before Grafana 10. newTmpl, err := am.TemplateFromPaths(paths, captureTemplate)
if err != nil {
return nil, err
} |
notify/grafana_alertmanager.go
Outdated
@@ -140,18 +152,19 @@ type Configuration interface { | |||
DispatcherLimits() DispatcherLimits | |||
InhibitRules() []InhibitRule | |||
MuteTimeIntervals() []MuteTimeInterval | |||
ReceiverIntegrations() (map[string][]*Integration, error) | |||
BuildReceiverIntegrationsFunc() func(next *GrafanaIntegrationConfig, tmpl *templates.Template) (Notifier, error) | |||
Receivers() map[string]*APIReceiver |
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 think this can be a slice because information about receiver name is in APIReceiver.
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.
By doing this you will be able to remove this confusing method
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.
LGTM. My last comments are nice to have but not blockers
This method will use the given alerts as template context data and attempt to interpolate each of the top-level definition blocks passed in through the template field. A top-level definition is one that isn't used by any others. Default templates and any already saved templates defined in the current AM as associated templates will be available for reference by the ones being tested. The exception to this is the saved template with the same name as the one passed via the name field. This is so that users can real-time test out modifications to existing templates (which is, ultimately, the end-goal of this endpoint).
This adds template testing functionality.
Description
The method will attempt to interpolate each of the top-level definition blocks passed in through the template field with the given alerts as template context data. A top-level definition is one that isn't used by any others.
Default templates and any existing templates defined in the current AM will be available for reference by those being tested. The exception to this are the existing templates coming from the same filename as the one passed via the name field. This is so that users can real-time test out replacements to existing templates.
Notes for reviewer
There's some added complexity in this PR because template testing needs to recreate the existing templates from scratch. It needs to do this for a couple reasons:
text/template
fields.text/template
does not provide a way to remove an already defined template. We need this in order to not mistakenly parse an existing template file that matches the name of the one we're testing as the new template might have removed some definitions. We could overwrite the templates with blanks but this might hide execution_errors.So, in order to allow template testing to recreate the existing template we store the filenames and working directory for re-reading. I chose not to store the template text itself to keep the code as close as possible to existing template parsing which uses
templates.FromGlobs
instead of parsing from string.Previously, the already parsed template was passed into
grafana_alertmanager
duringApplyConfig
. This PR makes it so thatgrafana_alertmanager
is the one that creates the template from the passed in filenames instead.