Prometheus config fallbacks #115
Conversation
jrauh01
force-pushed
the
prometheus-config
branch
from
a2ae88f to
768e5c9
Compare
pkg/metrics/config.go
Outdated
| } | ||
|
|
||
| stmt, _ := db.BuildUpsertStmt(&schemav1.Config{}) | ||
| if _, err := db.NamedExecContext(ctx, stmt, configPairs); err != nil { |
There was a problem hiding this comment.
Do the INSERTs/UPDATEs in a serializable transaction. Think about why this is a good idea and whether the other side should do that too. See here for how we do it in Icinga DB: https://github.com/Icinga/icingadb/blob/main/pkg/icingadb/ha.go#L289.
As this is a pattern that we are now using a second time, you should consider how this could be done using a function.
lippserd
force-pushed
the
notifications-integration
branch
2 times, most recently
from
e4223a9 to
e9917b7
Compare
jrauh01
force-pushed
the
prometheus-config
branch
from
8435531 to
72fe379
Compare
jrauh01
force-pushed
the
prometheus-config
branch
from
72fe379 to
ccb4b0f
Compare
jrauh01
force-pushed
the
prometheus-config
branch
3 times, most recently
from
5310012 to
308520c
Compare
internal/prometheus.go
Outdated
| } | ||
|
|
||
| if ip == "" { | ||
|
|
There was a problem hiding this comment.
I think you forgot something here.
pkg/metrics/config.go
Outdated
| } | ||
|
|
||
| // Validate checks constraints in the supplied Prometheus configuration and returns an error if they are violated. | ||
| func (c *PrometheusConfig) Validate() error { | ||
| if c.Url == "" && (c.Username != "" || c.Password != "") { | ||
| return errors.New("credentials cannot be provided without a URL") |
There was a problem hiding this comment.
I would relax this. If no URL is specified, the feature is disabled. I don't think we should require all other configurations to be removed as well.
| } | ||
|
|
||
| // Validate checks constraints in the supplied Prometheus configuration and returns an error if they are violated. | ||
| func (c *PrometheusConfig) Validate() error { | ||
| if (c.Username == "") != (c.Password == "") { | ||
| return errors.New("both username and password must be provided") | ||
| } | ||
| return nil |
There was a problem hiding this comment.
Please add a newline before return.
pkg/metrics/config.go
Outdated
| } | ||
|
|
||
| // Validate checks constraints in the supplied Prometheus configuration and returns an error if they are violated. | ||
| func (c *PrometheusConfig) Validate() error { | ||
| if (c.Username == "") != (c.Password == "") { |
There was a problem hiding this comment.
notifications.Config only validates configuration if url is set, i,e. if c.Url != "" .... We could do the same here so that validation works the same for both.
jrauh01
force-pushed
the
prometheus-config
branch
from
81f9484 to
be15d30
Compare
Add more options to set Prometheus config: