fix: incorrect pod matcher for compactor in mixin when using ssd mode

[QuentinBisson]

What this PR does / why we need it:

This PR fixes the compactor pod matchers in ssd mode. I was sure I covered them all but I apparently missed one again @cstyan :)

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Checklist

• Reviewed the CONTRIBUTING.md guide (required)
• Documentation added
• Tests updated
• Title matches the required conventional commits format, see here
  • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
• Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
• For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
• If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

[QuentinBisson]

@cstyan could you please take a look at this one ?

[cstyan]

was confused for a minute how this could have made it past the linting check, but at first glance I missed that there was another change to the precompiled jsonnet dashboard

[cstyan]

I think we could have been okay without this change? and just doing the container="compactor" -> compactor_matcher change below?

[QuentinBisson]

Oh definitely, I just thought it was easier to read. If you prefer I will change this tomorrow

[cstyan]

I think then 'pod=~"(compactor|%s-backend.*|loki-single-binary)"' % $._config.ssd.pod_prefix_matcher should cover the case where we were previously used container="compactor" right?

[QuentinBisson]

I think it should yes but I'm not familiar with loki in distributed mode.

Then we could change

local compactor_matcher = if $._config.meta_monitoring.enabled
  then 'pod=~"(compactor|%s-backend.*|loki-single-binary)"' % $._config.ssd.pod_prefix_matcher
  else if $._config.ssd.enabled then 'container="loki", pod=~"%s-backend.*"' % $._config.ssd.pod_prefix_matcher
  else 'container="compactor"'

to just:

local compactor_matcher = 'pod=~"(compactor|%s-backend.*|loki-single-binary)"' % $._config.ssd.pod_prefix_matcher

right?

[cstyan]

I think that's mostly right, but it looks like it wouldn't work for %s-backend % $._config.ssd.pod_prefix_matcher because AFAICT the default value for that variable is (loki.*|enterprise-logs)

@MichelHollands can you confirm whether we really need separate selectors for when meta monitoring is enabled?

[QuentinBisson]

So what would you like me to do here?

[cstyan]

@MichelHollands can you confirm what the matcher should be when meta montioring is enabled? I still think @QuentinBisson suggestion of local compactor_matcher = 'pod=~"(compactor|%s-backend.*|loki-single-binary)"' % $._config.ssd.pod_prefix_matcher covers all cases.

[QuentinBisson]

Hey @MichelHollands would you have time to check this one? :)

[cstyan]

@QuentinBisson lets move forward with the simplified matcher you've suggested and open follow ups with fixes if anyone reports any

[QuentinBisson]

It's pushed :)