Generalize source key implementation to arbitrary-length keys

[HappyTetrahedron]

Summary

According to the documentation, the implementation of the dimension lookup process should not make any assumptions about the length of the source key (in number of elements), or the specific meaning of each element.

The implementation currently makes such an assumption, and in particular, the source key is restricted to have either 4 or 5 elements.

In this PR, I remove the maximum length restriction for the source key. The behavior for keys of length 4 or 5 stays the same, so this change should be perfectly backwards compatible.

Also for the sake of backwards compatibility, the association of the first 5 key elements with a specific meaning is kept in the code. The code assumes the key to be of the form query:zone:tenant:namespace:class - an assumption that is already broken in practice, since we use zone as cluster and class as sla_level in APPUiO Managed Reporting. However, this association only exists within this codebase - changing the meaning of most of these elements has no implications (with one exception) as long as it's consistent between the queries and products.

The only assumption that has actual implications on the functionality is that the tenant is the 3rd element. Ideally, the tenant (which is used to map to the correct customer in the target system) should be given in a separate label/field in the query, rather than as part of the lookup key - the lookup key should, in theory, be able to contain arbitrary elements. However, I decided to not open that can of worms right now.

Checklist

• Categorize the PR by setting a good title and adding one of the labels:
  bug, enhancement, documentation, change, breaking, dependency
  as they show up in the changelog
• Update tests.

[bastjan]

Very elegant solution for generating the LookupKeys() 😊

I strongly feel the need for either documentation or a breaking change (make field accessor functions) for the SourceKey Struct.

[corvus-ch]

the tenant […] should be given in a separate label/field in the query, rather than as part of the lookup key

Keep in mind, the lookup key is used both for products and discounts. For this reason, the tenant must be part of the lookup, so we can define prices and discounts on a per-tenant basis.

[HappyTetrahedron]

the tenant […] should be given in a separate label/field in the query, rather than as part of the lookup key

Keep in mind, the lookup key is used both for products and discounts. For this reason, the tenant must be part of the lookup, so we can define prices and discounts on a per-tenant basis.

All clear! I don't want to remove the tenant from the key - it makes sense that it is present. I just want to remove the requirement that the tenant must be at position 3. Because it's ugly that way. ^^

While our billing has the potential for discounts per tenant and therefore needs this info in the lookup key, this codebase only needs the tenant for mapping purposes, and for that, it should not come from the lookup key but from a separate field.

AFAIK all queries already have a tenant_id label... It would be an easy change... but there's a residual risk of breaking something which I'm not currently aware of.

[bastjan]

I don't think making the source key struct private is what you want:

package sourcekey_test

import (
	"fmt"
	"testing"

	"github.com/appuio/appuio-cloud-reporting/pkg/sourcekey"
)

func TestPublic(t *testing.T) {
	x, _ := sourcekey.Parse("query:zone:tenant:namespace:class")
	x.Tenant = "foo"
	fmt.Println("yoloaccess", x.Parts[2])
}

I vote for making the struct public and making the members private (with accessors where needed).

Edit: It makes it also very hard to use when needing to pass it around. You can't use it as a function parameter anymore.

func myfunc(x sourcekey.sourceKey)
              ^ sourceKey not exported by package sourcekey compiler(UnexportedName)

[HappyTetrahedron]

Ah, now I see what you mean.
If we make them private, we might as well remove them, since they're not used package-internally.

[bastjan]

Thanks for putting in the work 🙇‍♀️! 😊