Kubernetes SIG Instrumentation, 3 Mar 2022

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: SIG Instrumentation 20220303

Description

SIG Instrumentation Bi-Weekly Meeting March 3rd 2022

A

Hello, everybody and welcome to today's edition of Sig instrumentation. It is Thursday March 3rd 2022. Han. Would you like to kick it off today? Sure.

B

We have uh a couple things on the agenda item. The first thing is the annual report. uh We have this thing in process. uh The action items on me so I will take care of all of the to-do's on here. There's just some links. I need to fill in and some check boxes, uh but yes, I will get this submitted by the end of the week, and I will probably work on it today.

B

uh Next on the agenda is Damien's item and uh Sanchez l yeah.

C

D

Yeah, so it's basically like you know the continuation of what we told during the last tradition. Well, basically, like we had this problem with Lewis, where we wanted to update the stable metric I, basically just had a new label to it, or maybe a bit more than one but other couple label.

D

A

Update stable metrics, that's not what stable means yeah.

D

Yeah I know okay, um but the idea is like how can we achieve that then like, since we cannot update this metric like how can we achieve like just adding a new label to an already stable metric, um and the suggestion that Han proposed last time was, to, like just add, a new metric uh with the new label?

D

um That said, uh the issue that I have with that is that, like there are a couple of ways where we can proceed like we can keep the same name, but then the API would be different, but that's a bit like complex to use. I, don't think you can keep the same name.

A

D

A

A different metric.

D

A

D

A

D

Labels are different, there won't be any conflict.

A

No, no, no, no Prometheus won't allow.

D

That yeah, you can't do.

B

D

B

Sure I'm pretty sure we can pretty sure you can't, because then you would have two sets of metrics with two different sets of labels. Prometheus would not know what to do with that.

D

And I can actually like because, like since the labels are different, like they'll treat it as like two different kind of metrics like they are not, they would have.

A

The same name so no.

B

A

You'd get say: you'd get both so I. Don't think that would work this.

C

Once upon a time where we accidentally, oh.

A

Yes, yes, yeah, the sea advisor was admitting metrics with different labels, depending on whether or not labels were present, and it caused a lot of problems. First, people were scraping them it'll.

C

A

C

Step so metrics were coming and going because it randomly selected one of the two sets.

B

Of labels to keep from the other ones, that's why you use must register, set a register.

C

D

Don't remember: that's.

C

D

This is that yeah. If we have to come in place, we with a new naming for this new metric, then it's problematic like it's hard to use, because it will be harder to discover uh other to comprehend. Like the difference between like the existing metric and the new one um like as a user, it will be out. You really understand like which one is the latest and which one is now recommended to be used.

A

Well, I think it was maybe a mistake to make this a stable metric if there was a desire to later add labels to it, because.

C

A

Can never change it. You can't add labels. You can't do anything like it's stable. It is not going to change that's what stable means not like Yay, it's good. It's graduated, that's not what stable means uh and we do have a proposal to maybe change that naming so people aren't like mistaking it for GA, but stable means the metric will not change.

A

So if you want to change it later, you basically have to like go through the year plus long deprecation process to remove it completely, uh and then you can do whatever you want with it.

D

Yeah but I mentioned that, like um now like there is this discussion about, like keeping all the stable uh everything that is stable, keeping it in kubernetes like not removing it like forever yeah.

B

Yes, that is not.

D

A possibility anymore so.

B

One we can't change the existing stable network, but just we just can't do that because like then, then our stability is like not really stable like and then and then. What is this machinery for? Like um you know, uh creating a new metric is fine. I would create, keep it in Alpha for a while. Let's see, if you like, need to add up New Dimensions to it or whatever, um after it bakes for a really long time, I would consider promoting that to stable and deprecating the old one.

D

Kind of like what Jordan was trying to introduce to not well.

B

The forever thing, the forever thing is contentious for sure, yeah and.

A

I we haven't agreed that we want it to be a forever thing, exactly.

B

So while Jordan is pushing for that, um it is not uncontentious and therefore arguments can be made against it, but I suggest doing it. The right way, which is like introducing a new metric with the New Dimensions that you want baking.

C

Did anyone else just lose yeah.

D

A

D

C

A

Moment to come back.

C

A

On me, you froze for a moment there I.

B

Thought everyone else was Frozen, no.

B

uh Yeah yeah, so what I was saying is like we should do it the right way right, which is uh big, big, the new metric for a really long time, uh and only proposing the swap after we are super certain that we are never going to add labels again.

D

By swap, do you mean like replacing the like the labels in the stable one by the no.

A

We mean like deprecate the stable and.

D

A

Yeah and make the new one stable. Yes,.

D

That's what I mean by.

B

D

So basically, we won't be able to reuse the name of the original.

A

Stability guarantee, yes,.

D

Yeah, which is kind of annoying to me because like to me, the name of the metric is like currently the only way we have to easily.

B

I get I, get I, get that it's annoying, but, like literally there's nothing you can do about this.

D

Potential solution, I.

A

Mean the the solution is: if this was missing, labels it never should have become stable like that is stable means it's not going to change. If there was confusion around what stable meant such that people proposed it for staple and like thought they were going to add like labels to it later like. Maybe we need to update documentation or something to disabuse people of that notion, but, like stable means, it will never change. I.

B

Am not convinced, it's even a great idea to add these labels, because these are really high cardinality labels and.

D

Like we really are trained to bond them,.

B

No, and so like so so, the thing about these labels is like. Oh you well yeah. If you bind them, then I think it's probably fine uh crds yeah, because crds.

D

Obviously will cause there was an issue where it was exploding, yeah.

B

D

Why, like we're, trying to introduce them, but with an actual like uh way to bond them, so I think Luis was working on like an allow list for the label at the webbook level.

C

D

Kind of specify like which resources are critical and which one we want to have dedicated their sales for um so basically adding slos per resource compared to like I, think one, instead of all the web books. um Why.

B

Not why not, why not call one SLO web hook, admission, duration, second or something web operation, SLO yeah.

D

Yeah we we could do that. We could do that. I.

B

Mean that sounds fine, similar.

D

To what we did for GPS Refrigeration.

B

Yeah I mean just an SLO metric I think that's fine! Okay, and if you bind it then the cardinality should be limited, and so you shouldn't even have to worry that much practically speaking about increasing the number of Time series.

B

D

Yeah you can just saying just like I I wanted to see like if you had some way to leverage like the name change or not, but yeah. It seems that we don't.

B

D

Okay, so that's uh Lewis.

A

Do you have any questions for us uh welcome to Sega.

C

Instrumentation.

A

uh We're always this excited around here.

C

Oh well thanks um uh my first time on this call on uh thanks Damian for uh for introducing me and trying to help me work through this um as an observer first time here you know, I I did find it uh I think well a couple comments. First, of course you know your discussion of stable, um I think the and I'm.

C

Currently, if I'm wrong, they mean part of the the questioning of stable is more of you know, other actual changes that are compatible even if you're, stable and uh and I think we, we must have just been incorrect thinking that it was possible to add a label without uh disrupting existing uh use of the metric.

A

Yeah there's no such thing as a compatible API change for labels on metrics, like once a metric becomes stable.

A

It will never change uh so uh because, because there's no way I mean across minor boundaries of releases, maybe we could add labels in terms of like just how API guarantees generally work like that would be consistent, at least with them, but it's totally possible to have components on different versions like what the version SKU within a cluster and therefore that would break so like because you could be scraping two different components with two different versions and then they'd have different sets of labels, and so that's why those stability guarantees are very important. It's.

D

A

Like a compatible API change within the API server, where you know, if the thing doesn't exist, it's never going to get serialized and the apis are always must be ahead of the other component version. So, like everything is okay, uh it's not like that for Prometheus metrics. So that's why we have these rules. It.

B

Helps to think of it in terms of a database table like, if you add a column, to a database table right or if you try to admit like create a row in a database with a non-existent column. Right, that's going to explode. uh But if you try to create a row in the database and omit one of the columns, that's actually a valid SQL statement. Right, like you can insert without specifying all of the columns.

A

Yeah, almost exactly the same way for FCD, though like it's like we have the concept within API review of a compatible API change, we're like we've added a new nullable field right and that's fine between versions, but the fundamental issue with metrics. Why? We cannot add: there's no such thing as a compatible. Api change is because you could be running two components on different miners in the same cluster, both being scraped by the same Prometheus and if they're, on different minor versions, and they have different labels, then, like they're, not the metrics aren't compatible.

A

uh So, like that's, that's the fundamental issues.

B

That's not strictly true, but you can remove a label. Oh.

A

Yeah, you could drop it at the scrape level.

B

But then, even no, even even at the emission level you could, you can drop a label and it will just populate with a no value in Prometheus. It will actually work, but it will break alerts that presume the existence of that.

A

You could admit you could emit a null value for the new thing, but for old versions that have no knowledge of these changes. The previous minor they're not changing anything in terms of how they're emitting that metric.

B

Yeah yeah: it's not.

A

B

You you can't previous can still take a subset of labels.

B

I'm, like pretty certain.

A

What I'm saying is you can't do it and make zero changes to your deployment uh like you'd, have to drop labels somewhere or uh like you'd, have to change how things are deployed? You can't just be like I have this component. It has this metric with this name.

A

It has this set of labels and I have the new version, and it has this set of labels plus one and you throw those into a cluster, and then you scrape both targets like and then you query for the time series in Prometheus without doing additional things, uh they're not going to be preferenceable right. You've got one with like these labels, which are different than these ones, which is the problem that we're trying to avoid the problem that we saw with C advisor randomly emitting different sets of labels and whatnot.

A

This is why I'm saying I, don't think it I, don't think we can ever have a case. It's not like an API uh compatible API change where you can just add a field that happens to be nullable. We can just pretend uh that it didn't exist in a previous version. Yeah.

B

You can't add ever, but you can drop, and that should be fine, except it will break alerts.

C

Okay, um the other thought I had was then um you know what you know. It looks like the way to evolve. Metrics are basically create a new one, with a completely different name and eventually drop the old one. um You know it doesn't seem like there's a facility really within Prometheus to handle it any other way.

C

um Is there anything? Is there anything we can do in the future? To make this more obvious or or for example, like um you know, you know, require a version uh suffix or something on each metric name in the future, so that so that at least the users is very aware of exactly what version he's using and if he sees a V2, for example, he knows I I do I need to move over to V2. What's this over here? Is it offering me more value.

A

I think that's against the Prometheus naming guidelines like I've, never seen, versions embedded in metric names before I think there's a bit of an issue where, like Prometheus I, think as a community hasn't really like considered this sort of scale or because I filed issues against them previously. Asking, for example, like can you add a facility to rename metrics, uh because that would be very useful for.

C

A

uh Like to be able to declare hey I know that this metric said like one thing in this last release, but now we've renamed it and it used to be called this thing. Can you consider them to be the same thing at scrape time uh and I filed an issue requesting that as a feature and was basically told no like? Why would you ever want that? That sounds bad, uh and it's like well like in kubernetes.

A

We have all this like versioning issues with like metrics and uh maybe we're just a very particular special use case, uh and so they consider like that sort of request to be a well. It's just gonna affect kubernetes, it's not going to affect anyone else, I'm, not sure, um but uh yeah. That's uh that is, unfortunately like sort of the response. We've gotten back from them.

A

So, given that there's not really necessarily like people setting the standards in like our dependency Prometheus for like how we maybe should be doing this we're kind of on our own and so we're making it up as we go along, but we're trying to do it in a way that works I think uh it might be good for us to write clearer, Dev documentation if there are any concerns about like what it means to be a stable metric and like warning big flashing letters, uh you know like if you make it stable, you cannot change the labels on it.

A

You cannot change its name. You cannot change the labels like it is stuck like this forever until we remove it. So you.

B

Can always use something in the namespace like hit like the options struct that allows you to construct. The metric definition has several Fields namespace subsystem. Name and subsystem is like the component namespace you could theoretically use as like, V2 or whatever like. This is not like, like just because the Prometheus you know, Community has not said anything like in our case. I feel like it's probably fine.

B

If, if you want to do something like that, um like repurposing the namespace field to add like what happens, is uh subsystem namespace and the name get concatenated together to form the metric yeah. So if, if you wanted to do something like that, I think that would be fine.

C

Again, just you know as an observer, trying to do this this time. I just wondered if there was something I understand that Prometheus itself doesn't have any facility, but if it something to consider for for Cube, if if there was a way to to, as you mentioned, for example, use the namespace field and and for for cube, we we include, like a you, know, a dash V1 Dash or something similar um to give at least Cube more flexibility in the future. Yeah.

A

I, don't think we think we try putting random versions into metrics, because they're not going to be meaningful on a project-wide basis like effectively I think each component owner is going to be coming up with their own versioning I think that'll be very confusing to end users, particularly because we don't currently have documentation of all of the metrics emitted by kubernetes, and so like then, throwing things like effectively. All they've got is like the help text from that come along with the metric. That's the only documentation.

A

We have so like adding versioning stuff that we don't have any docs for. On top of that, I don't think will be a very good user experience.

B

Okay, I mean yeah. Definitely we need to have that stuff documented with all of the component metric stuff I mean that was part of one of our goals. Earlier on and yeah, we should. We should move forward to that in the next cycle. If we, if we can.

A

I had a question for you based on something that you said earlier um so specifically when I was talking about um like API like compatible API changes. Typically, what that means in kubernetes is you have some serializable type and the only compatible changes that you can make to that? Or you can add new fields that are nullable.

A

uh You can't take away Fields, because that would break things, uh and so when I was saying we can't add metrics, because the fields wouldn't exist on the old metric, and you said well, you could maybe drop fields and then just admit that as a null field uh that wouldn't be considered a compatible API change from a kubernetes API point of view, which.

B

A

Wasn't considering that yeah.

B

A

Is it possible to drop labels from yes? Yes,.

B

Ingest, no, no, it's not okay from a process and rules perspective. We don't allow this because we don't allow this not because it will break ingestion. It will not break in gesture. It is in fact API compatible with ingestion to drop a label. Yes,.

A

I wasn't suggesting dropping a label, though I would say adding a label is the.

B

No, adding a label is not API compatible. That's.

A

What I was saying? Yes, absolutely, because we weren't compo, we weren't, uh we weren't, suggesting dropping labels right like in this case they wanted to add a couple of labels and I said well, isn't that API compatible and I'm saying no? It's not API compatible, because if you have two different components and the same cluster that are both emitting metrics one's on the old version, one's on the new version, the new version has an extra label.

A

We now have a mismatch if we had dropped the label IE, you know the old one had whatever and then the new one just emitted a nullable thing. That would be fine, but that wouldn't be considered an A compatible. Api change.

B

No, no, so what I'm saying is, let's say you have a metric with three labels: yeah, yes, and then you remove one of the labels going into version two like another. This episode, no.

A

No I understand that. That's fine yeah that wasn't what I was talking about because that's like definitely not considered a compatible API change from like a kubernetes yeah.

B

But from a Prometheus suggestion perspective, it's okay, yeah.

A

It's fine from prometheus's point of view, but it's not something that we permit per the method of stability policy. Yes and like we can't, uh but the comparison I was making was not can Prometheus do it. It was would, like the you know, equivalent of a compatible API change uh exist uh in the world of metrics like? Is it okay to just go and add a label later?

A

If you decide well, I have the stable thing and I want to like add this thing, because in the world of like kubernetes API types, that's fine uh in the world of Prometheus. That is not fine.

A

Only adding like dropping wouldn't be considered compatible, so like I wasn't even considering that uh it turns out. Yes, that might be fine, but we wouldn't ever do it in the case of a stable metric, because somebody might be alerting on that thing. It'd be like where's, my label, so.

B

Luis, are you on the API Machinery team?

B

You are yes, okay,.

C

So yeah go ahead. Continue! Sorry, so.

B

Yeah I mean like the component owners are supposed to like own. The metrics, uh like personally I was like I, was on board, with promoting the web hook thing to stable and I'm. Also I work on API Machinery stuff, um like personally I, wouldn't have thought that we would have ever wanted to add resource labels to this metric, mostly because of cardinality issues.

B

um So I mean like what would almost make more sense is to create like a separate, SLO metric, if you want to, if you want to measure SLO stuff and like not even include all of the labels that you may have on the original webhook, maybe just make it a completely different one that measures exactly what you need for your SLO mm-hmm.

C

Sure yeah we'll look into that, and the thing is: if you look into this metrics history, the labels have been added and removed in the past. They.

B

Have but they've.

C

B

Stable they've been stable for like at least three or four Cycles before we made it stable. That's why we did it yeah.

C

um But yeah yeah I think that's that that sounds reasonable and we'll we'll go with that.

D

Cool yeah I think it's hard to catch like four components on there like to know whether they would need at some point to add labels on it because, like they might not have like alerting awareness to some certain point like they might not know how to really like create the solos or even like, create good alerting. So they might need a label at some point that they don't have yet um and I, don't think like.

D

Even for us like it's hard to really project in the future like since, for us like, we don't have the knowledge about the component itself. It's also odd to really like try to recommend some changes or whatever yeah.

A

C

A

To interrupt, we have two minutes left, so I just wanted to think. Maybe if there are some action items that we could take away from this discussion, um one thing that I think would be good is to make sure that our documentation and guidelines are clear.

A

Another thing that we might want to consider doing is like adding something to prow that when it sees something happening in the stable metrics folder uh like pops up with a little comment being like you are changing stable metrics like here's the documentation for that, because we don't have anything like that currently. Well,.

B

No, no, we have. We have hack things. So if someone tries to change a stable.

A

No I know it'll prevent them until somebody from physique instrumentation approves it. But I'm saying like for the author, we don't have like a little like comment pop up in the same way that like, if you change something and there's a label applied for API review, it'll be like this. Pr may need API review. Please see this documentation, we don't have anything like that for stable yeah.

B

We can just add this to this shell script so like before you even get to Pro. You will get a failed hack thing. So, like your hack script, if you run it verify stable, metrics yeah, we can just.

A

Add a message that.

B

A

Can't change disabled I think it just might be a little bit more user friendly to do it as a comment on the pr, because otherwise people are only going to see it if they look at the output of the verified job, and it's not going to be clear, to example, like people who aren't expecting verify to fail that, like oh by the way. This is a thing. That's been changed.

B

There's five scripts there's like a hundred.

A

There, no that's what I'm saying: there's like a lot of those scripts so like.

B

You have to check the verify scripts like.

C

I just want to mention.

B

C

Just I just want to mention, there's also a handy script, which just makes it go away so.

B

The generation without.

C

Warning you it just says: okay, you want to update the stable metrics.

B

Yeah yeah, but but no one will approve it uh yeah. So.

A

That's what I'm saying I'm saying as soon as those things exchange I think it might be a little bit more user friendly to maybe see when we have something like that. Come up that we go ahead and like alert the user and have the comment on there so that, like somebody, maybe with top level approver who doesn't know about stable, metrics or whatever, is like oh there's, a stable metric change, I need to talk to instrumentation or whatever so I.

A

Don't know if the I don't know if that folder, for example, uses like no inherit uh like parent owners or anything like that. I.

B

I gotta run to another meeting, but good action items. We should add some stuff to this hack scripts in order to provide more detailed information. Also, the warning about not running the generate stable, metrics thing or the implications of that I think is a good call.

A

Okay thanks everyone for joining today, you gotta go for one minute.

C

Okay, guys thanks bye.