Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

fix/alertmanager: downgrade prometheus/common to fix generated config#63790

Merged
bobheadxi merged 1 commit into
mainfrom
fix-prom-common-dep
Jul 11, 2024
Merged

fix/alertmanager: downgrade prometheus/common to fix generated config#63790
bobheadxi merged 1 commit into
mainfrom
fix-prom-common-dep

Conversation

@bobheadxi

@bobheadxi bobheadxi commented Jul 11, 2024

Copy link
Copy Markdown
Member

The OTEL upgrade https://github.com/sourcegraph/sourcegraph/pull/63171 bumps the prometheus/common package too far via transitive deps, causing us to generate configuration for alertmanager that altertmanager doesn't accept, at least until the alertmanager project cuts a new release with a newer version of promethues/common.

For now we forcibly downgrade with a replace. Everything still builds, so we should be good to go.

Test plan

sg start and sg run prometheus. On main, editing observability.alerts will cause Alertmanager to refuse to accept the generated configuration. With this patch, all is well it seems - config changes go through as expected. This is a similar test plan for https://github.com/sourcegraph/sourcegraph/pull/63329

Changelog

  • Fix Prometheus Alertmanager configuration failing to apply observability.alerts from site config

@jdpleiness jdpleiness left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed in pairing

@bobheadxi bobheadxi enabled auto-merge (squash) July 11, 2024 17:32
@bobheadxi bobheadxi merged commit ffa873f into main Jul 11, 2024
@bobheadxi bobheadxi deleted the fix-prom-common-dep branch July 11, 2024 18:03
sourcegraph-release-bot pushed a commit that referenced this pull request Jul 11, 2024
…#63790)

The OTEL upgrade https://github.com/sourcegraph/sourcegraph/pull/63171
bumps the `prometheus/common` package too far via transitive deps,
causing us to generate configuration for alertmanager that altertmanager
doesn't accept, at least until the alertmanager project cuts a new
release with a newer version of `promethues/common`.

For now we forcibly downgrade with a replace. Everything still builds,
so we should be good to go.

## Test plan
`sg start` and `sg run prometheus`. On `main`, editing
`observability.alerts` will cause Alertmanager to refuse to accept the
generated configuration. With this patch, all is well it seems - config
changes go through as expected. This is a similar test plan for
https://github.com/sourcegraph/sourcegraph/pull/63329

## Changelog

- Fix Prometheus Alertmanager configuration failing to apply
`observability.alerts` from site config

(cherry picked from commit ffa873f)
jdpleiness pushed a commit that referenced this pull request Jul 11, 2024
… generated config (#63793)

The OTEL upgrade https://github.com/sourcegraph/sourcegraph/pull/63171
bumps the `prometheus/common` package too far via transitive deps,
causing us to generate configuration for alertmanager that altertmanager
doesn't accept, at least until the alertmanager project cuts a new
release with a newer version of `promethues/common`.

For now we forcibly downgrade with a replace. Everything still builds,
so we should be good to go.

## Test plan
`sg start` and `sg run prometheus`. On `main`, editing
`observability.alerts` will cause Alertmanager to refuse to accept the
generated configuration. With this patch, all is well it seems - config
changes go through as expected. This is a similar test plan for
https://github.com/sourcegraph/sourcegraph/pull/63329

## Changelog

- Fix Prometheus Alertmanager configuration failing to apply
`observability.alerts` from site config <br> Backport
ffa873f from #63790

Co-authored-by: Robert Lin <robert@bobheadxi.dev>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants