OpenSLO: Resolve inconsistency in labels example

Summary

The label example in the spec uses this, but the definition has it as an array of strings and Oslo defines it as this.

We should resolve the inconsistency by updating the example in the spec.

What is the current bug behavior?

Label example uses:

metadata:
  name: string
  displayName: string # optional
  labels:
    userImpacting: "true"
    team: "identity"

Possible fixes

Based on the definition the example should be:

metadata:
  name: string
  displayName: string # optional
  labels:
    userImpacting:
      - "true"
    team:
      - "identity"

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 16 (4 by maintainers)

Most upvoted comments

I propose that we should consider supporting multivalue tags. While I’m not a fan of intentionally setting up a tagging scheme that relies on it, I have seen some integration scenarios where multivalue support is necessary. For example, as soon as a user tries to combine or import tags/labels from a system that is more flexible, or from multiple different systems with different tagging schemes, that presents an interoperability blocker due to collisions/stomping. Since OpenSLO is an integration point, I think we should aim to maximize interop.

Another example of a common key-value system that technically does support multivalue is HTTP headers.

That said, I think we can stop at multiple values. (I wouldn’t recommend supporting multiple instances of the same value, though some labeling systems support even that. I’ve found that multivalue is generally sufficient.)

Thanks, this makes sense, objection removed.

proffalken on Jun 28, 2022

I propose that we should consider supporting multivalue tags. While I’m not a fan of intentionally setting up a tagging scheme that relies on it, I have seen some integration scenarios where multivalue support is necessary. For example, as soon as a user tries to combine or import tags/labels from a system that is more flexible, or from multiple different systems with different tagging schemes, that presents an interoperability blocker due to collisions/stomping. Since OpenSLO is an integration point, I think we should aim to maximize interop.

Another example of a common key-value system that technically does support multivalue is HTTP headers.

That said, I think we can stop at multiple values. (I wouldn’t recommend supporting multiple instances of the same value, though some labeling systems support even that. I’ve found that multivalue is generally sufficient.)

alexnauda on Jun 28, 2022

What about tags? They are mentioned in one place in spec without further explanation.

https://github.com/OpenSLO/OpenSLO/blob/6f873376b6bec4aed205b62a0968b4e73a05df03/README.md?plain=1#L70-L71

They were introduced in #52 as an optional field. But now I’m wondering. Should we treat it as something special? Or is it just label tags that have multiple values assigned? And having tags is just an example of real-world use when you want to have a “tagging feature” like that.

Isn’t tags in here kinda confusing? It looks like additional keyword that allows to put multiple values but effectively, referring to our spec, it is just label. Maybe we should remove that example from here as it might be confusing because it sounds like we expect some kind of additional tagging mechanism?

fpiwowarczyk on Feb 7, 2023

Personally I think this is a problem for the integrator/vendor platform that integrates OpenSLO to solve not for us to worry about.

That’s true! But if OpenSLO is not flexile enough to handle real-world scenarios, those implementors won’t be able to adopt it. In my experience, labels are often imported from other systems (in our case, probably from multiple instances of metrics data sources) and even within a medium-sized company or operation, there are very likely to be collisions between tagging schemes.

alexnauda on Jun 30, 2022

I’m fine with that.

In creating the structs to match the spec I had err’d towards an array of strings as the spec supported a single string or an array of strings, but when defining the struct I didn’t think that was possible, and went with the broadest option.

If everyone is good with modifying the spec to support a key with a single value only, it works for me

kenfinnigan on Jun 21, 2022