Alertmanager's alertmanagerConfigMatcherStrategy issue

[artemlive]

I've tested this PR, and there is another logical error.
When the config of the Alertmanager got generated, since we don't use the Alertmanager CRD, we built it with the defaults:

alloy/internal/component/mimir/alerts/kubernetes/events.go

Lines 178 to 180 in 8c0090c

	// TODO: Add an option to get an Alertmanager CRD through k8s informers.
	cfgBuilder = alertmanager.NewConfigBuilder(slog.New(logging.NewSlogGoKitHandler(c.logger)), *version, store, &monitoringv1.Alertmanager{})
	)

And it breaks the config, since it implicitly adds the

    matchers:
    - namespace="mimir"

to the main route:

...
 routes:
  - receiver: mimir/alertmanager-config/sre-alerts
    group_by:
    - alertname
    - cluster
    matchers:
    - namespace="mimir" #<---- the issue, since we care only about the alerts from the mimir ns
    continue: true
        routes:
          - receiver: mimir/alertmanager-config/other-alerts
            matchers:
            - cluster="staging"
            - project="project1"
            continue: false
            repeat_interval: 12h
...

Because we don't use the Alertmanager CRD, where we can set the alertmanagerConfigMatcherStrategy.

Originally posted by @artemlive in #5010 (comment)

[timonegk]

Do you think it would make more sense to always use NoneConfigMatcherStrategyType or should the setting be exposed to the alloy component?

[artemlive]

Do you think it would make more sense to always use NoneConfigMatcherStrategyType or should the setting be exposed to the alloy component?

Actually, I don't see any reason to expose this to the component config, because we don't watch any Alertmanager manifests, but the option OnNamespaceExceptForAlertmanagerNamespaceConfigMatcherStrategyType requires the Alertmanager manifest to be considered.
Every AlertmanagerConfig "watches" its own namespace by default, or if we use the OnNamespaceExceptForAlertmanagerNamespaceConfigMatcherStrategyType, then we match all alerts if the Alertmanager manifest is in the same namespace as the AlertmanagerConfig.

I'd rather put NoneConfigMatcherStrategyType and define the matchers' logic explicitly in the manifest if needed, or the Alertmanager's watching logic must be implemented to support all the options from here.

[ptodev]

It is possible to expose this as a configuration argument in Alloy. I'd like to align with the Prometheus Operator's default way of merging configs so that we minimise surprises for users. Do you think it'd make sense to have an Alloy argument which defaults to OnNamespace?

I'm not sure whether Alloy will ever really support the Alertmanager CRD since it has too much information in it... Alloy isn't going to manage the Alertmanager and having this CRD in k8s just to set a few values here and there for Alloy to use doesn't look ergonomic.

[timonegk]

Do you think it'd make sense to have an Alloy argument which defaults to OnNamespace?

@ptodev Yes, I think that would make sense. That way, the behavior doesn't change for users who need the additional namespace matchers but it's possible to set it to None if you don't need it. OnNamespaceExceptForAlertmanagerNamespace could also be implemented and refer to the namespace to which alloy is deployed.

I'd rather put NoneConfigMatcherStrategyType and define the matchers' logic explicitly in the manifest if needed

@artemlive I think that the concept of adding the namespace matcher to the config automatically makes sense in multi-team setups where each team has a dedicated namespace and should only be able to receive alerts from that namespace, so Alloy should probably support it as well.

[artemlive]

@timonegk, yes, it absolutely makes sense. If there are no plans to add watching for the Alertmanager manifest, it'd be better to expose this as an option.