prometheus / prometheus/alertmanager

Create dedicated types for properties of configuration that have been checked

Open
#4,993 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

component/config
Dominant language
Go
Stars
8.6k
Forks
2.5k
Avg merge
2d 6h
Merged PRs (30d)
61

Description

A structure such as

type WebhookConfig struct {
	NotifierConfig `yaml:",inline" json:",inline"`

	HTTPConfig *commoncfg.HTTPClientConfig `yaml:"http_config,omitempty" json:"http_config,omitempty"`

	// URL to send POST request to.
	URL     SecretTemplateURL `yaml:"url,omitempty" json:"url,omitempty"`
	URLFile string            `yaml:"url_file" json:"url_file"`

	// MaxAlerts is the maximum number of alerts to be sent per webhook message.
	// Alerts exceeding this threshold will be truncated. Setting this to 0
	// allows an unlimited number of alerts.
	MaxAlerts uint64 `yaml:"max_alerts" json:"max_alerts"`

	// Timeout is the maximum time allowed to invoke the webhook. Setting this to 0
	// does not impose a timeout.
	Timeout time.Duration `yaml:"timeout" json:"timeout"`
}

is currently exposed. However, this means things such as

 c.URL == "" && c.URLFile == "" {
		return errors.New("one of url or url_file must be configured")
	}

need to be checked again later in the code, because the fields are validated instead of only creating a single field for later use.

The proposed fix here is to have an effective configuration struct, which contains information consumed by the notifiers.

A more in-depth explanation of this idea can be found here Parse, don't validate

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating WebhookConfig and the validation of URL and URLFile, then trace how notifier configuration is consumed. Define the effective configuration boundary described in the issue so notifiers consume checked values without repeating validation; done means the proposed structure is used consistently by the relevant notifier paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.