Support global and wildcard overrides in Generic Forwarding

[Blinkuu]

What this PR does:

This PR adds support for global and wildcard overrides. Before, generic forwarding could only be enabled on a per-tenant basis. To make implementation behave in a similar manner to the rest of the code, I have added support for both wildcard and global overrides. From now on, users are not required to explicitly define forwarders on a per-tenant basis, but can also do so using a wildcard character in per-tenant overrides config ("*") or even define forwarders globally in the overrides section of tempo.yaml.

Example:

Let's imagine we have overrides defined in three ways: globally, using wildcard and on a per-tenant basis.

# tempo.yaml
overrides:
  forwarders: ["my-forwarder-1"]
  per_tenant_override_config: /overrides.yaml

# overrides.yaml
overrides:
  "tenant-0":
    forwarders: [ "my-forwarder-2" ]
  "*":
    forwarders: [ "my-forwarder-3" ]

Given above configuration:

• tenant-0 will forward to forwarder named my-forwarder-2
• Any other tenant will forward to forwarder named my-forwarder-3

If we were to remove the wildcard config from the overrides.yaml:

# overrides.yaml
overrides:
  "tenant-0":
    forwarders: [ "my-forwarder-2" ]

• tenant-0 will still forward to forwarder named my-forwarder-2
• Any other tenant will forward to forwarder named my-forwarder-1

Which issue(s) this PR fixes:
This PR is a continuation of #1775.

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[joe-elliott]

it seems like for tenants w/o a configured forwarder this lock will hit repeatedly b/c it will not be found in the list. should we store a nil value in m.tenantToQueueList to prevent this?

[joe-elliott]

It seems like this is the core of the change? By repeatedly asking for the forwarder we can catch the wildcard scenarios? I'm fine with that idea, but I'd like to find a way to "cache" the nil to prevent constant lookups here.

Other than that looks good.

[Blinkuu]

This is not the case. Please notice that I am assigning a new queueList to the tenantToQueueList map in manager.go:98. Suppose no forwarders are defined at all in the distributor config, OR there are no forwarders defined in the overrides config (either globally, wildcard, or per-tenant). In that case, we don't even enter this code. Below is the part of the code from distributor.go:382 that handles this case:

if len(d.cfg.Forwarders) > 0 && len(d.overrides.Forwarders(userID)) > 0 {
    if err := d.forwardersManager.ForTenant(userID).ForwardTraces(ctx, traces); err != nil {
        _ = level.Warn(d.logger).Log("msg", "failed to forward batches for tenant=%s: %w", userID, err)
    }
}

If we wanna be super safe, we could remove the if check from manager.go:87. This way, "empty" queueLists will get assigned to the map, covering this case. I added this check to optimize for memory here. However, with the extra check in the caller function (in distributor.go) this should not matter and space complexity remains the same for the whole functionality (it will be only worse "locally" in the context of the Manager struct itself).

Let me know your thoughts on this.

[joe-elliott]

I see now. I'll admit I don't like the logic to get the queue is split between this function and distributor.go:382. It's difficult to see all the logic related to how we request a forwarder for a tenant. Likely it was put there because it mirrors the metric generator lines above it?

Personally, I think it would be cleaner to move this logic:

	if len(d.cfg.Forwarders) > 0 && len(d.overrides.Forwarders(userID)) > 0 {

into the manager b/c there would be one place to look to see exactly how a forwarder is requested for a tenant. This is not a blocker though. WDYT?

[Blinkuu]

Moved in ce63eeb.

[joe-elliott]

lgtm @mapno did you want to take a look?

clean up the changelog conflict and we'll merge

[mapno]

LGTM

[Blinkuu]

@mapno I've rebased on top of origin/main and fixed CHANGELOG.md conflicts.