Open Telemetry Uncategorized, 6 Oct 2022

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: 2022-10-06 meeting

Description

Instrumentation: Messaging

A

B

It is Anthony.

A

Hello, how y'all doing that may have been the fastest nomination? Pr merge, I've, seen 10 minutes from open to March.

B

Yeah I know right: I was reminded of I. Think it's like every year. They do this, where it's like all the nominations seem to come in on the last day and I was noticing this morning. I was like well, almost all of them are already there. So I better get my act together.

A

What did I miss a.

B

Self-Nominated for the governance committee.

C

B

Yeah, if uh you know of other people, the I think it's open through the rest of the day. Yeah.

A

I think today is the last day yeah.

B

A

um Not really an issue there's um well there's an issue for the election there's several issues, um but in the community repo in the elections, folder um all the details in there yeah.

A

On the bright side, we have at least enough candidates to fill all the slots, so we don't have to worry about the contingency when I brought up in the maintainers meeting this morning or this this week.

B

Yeah, it's funny when you brought that up. I was just like yeah I, wonder what happens and maybe I don't know.

A

Like it's the sort of thing that never should happen uh yeah, but things that never should happen, have a tendency to happen.

D

Are you trying to quote Murphy's Law or something.

A

I I don't know it's not quite Murphy's laws, but there's there's something with it. Yeah like as soon as you assume that something can never happen. It's going to happen.

B

I think it's called software development, that's what that is.

A

Yeah right, especially operating at scale, you used to have to remind our product managers what things that happened, one in a million times happen several thousand times a day for us.

D

D

Billions of operations SEC, uh you know an hour or a minute, just don't make any sense anymore.

B

Let's see we're at six people uh just over two minutes past me.

B

Sorry I'm rid of a little behind on that uh setup. Today,.

B

B

Awesome, okay, I think we can uh jump in here and get started. Welcome everyone uh thanks for joining. If you haven't already add yourself to the attendees list here and if you have anything else you want to talk about in uh the meeting at the agenda as um we started off, I wanted to talk a little bit about this release, Milestone that we've had open uh for a little bit.

B

Now it's come to the point where it's been a full week and we haven't actually made any progress on this, so I wanted to check in on it. We probably need to also scope this down, because there are issues and uh PR's that have emerged that uh should probably get released. These are some bug fixes so I wanted to get people's opinions on this. There are um two high-level PR's I noticed that deal with bugs.

B

Well, I see one of them. um This is the default view. Instrument does not match any pipeline; it doesn't have enough reviews. Currently, uh Pablo was taken off of the approvers list, uh so it still needs another review. This is um something that handles uh this bug here, that's in the uh Milestone, and so this is again just one of those default views. Essentially, you have to pass the default view currently.

B

Otherwise, it'll drop things if it doesn't match any views that you do pass uh another one um there's another issue that addresses this and um this issue here. This uh duplicated instrument registration has been a few approaches here.

B

This PR I, don't know why it's not in this um Milestone. Oh it is I, just didn't see it um sorry a little out of it I guess, um but yeah. This again also has one approval.

B

It would also uh address another bug. There are two bugs I'm, sorry that are in this Milestone right here. So I think this is another important, oh yeah, it's right down here and then, um let's see, we have the refactor Prometheus exporter as another PR in here.

B

uh This is something that I don't know. Where's the status in this errand.

D

um I've downscoped it to remove the uh I guess we need to put a change log on it, but I've downscoped, the the set or the thing to remove the reader, um the with reader option.

D

The thrust of this was to enable using this specifically without having to import Prometheus. This gives you back a um the Prometheus. The the thing that we create the Prometheus exporter can be used as the HTTP route and whatnot similar to what was done in the um in the open census. Realm I didn't like there's more options that could be added, but those can also be added after the fact, um and there are workarounds for if you need them right away.

D

uh If I could get a review and get this marriage to uh the sooner the better. But.

B

Gotcha um yeah- this is something I could I could probably get reviewed later on today, David I know you've already taken a look at this. Have you taken a look since new changes have been pushed or I haven't taken a look recently: um okay, I need to I'll look again today, perfect, uh okay, cool, then that I think uh is also on the agenda for potentially getting handled here.

B

I think that would also it would bring in another issue which I will try to find after this, which is the not the histogram bucket, uh which is another issue. I should talk about, but um the importance of Prometheus with Prometheus that we've been dealing with so.

D

B

Would resolve another issue- that's not in here, but um okay I'll try to update the Milestone afterwards, so, uh okay and then the last one. Is this fixed slice related function, bug which, after seeing it and seeing the activity, I think this. This is one that probably should get bumped out of the um the current Milestone, there's uh I think still issues with the implementation.

D

There's issues I'd also like to point out that this actually does a lot of performance damage um granted that it is um limited to when you use attributes with slices only, but it's still, um it still ends up having a lot of copying that happens both when you create and when you read those attributes out so I think like if we are. If this is something that's Priority One needs to get out. This might be okay, but we do need to kind of. We will need to readdress this and find something else.

D

Some other solution, uh moving forward, I I, would believe.

B

Yeah so I I saw your comments. I, don't know of a better way to or any other way to do that, though, um if you want to given the fact that slices and arrays are mutable um at least the underlying data structures are mutable, we need to do copying. So I was wondering what was your thoughts on that all right.

D

So the it's not that we, uh the current implementation prior to this, doesn't do any copying and allows the array the underlying arrays to be mutable. It's just how the API was.

D

um But what this ends up doing. Is it copies? uh When you create a slice, it copies it?

D

uh Oh, it might have copied, but it copies it into a an array, and then we have to also copy it back out afterwards and it's a dynamically created array as well. So it.

B

D

Up yeah yeah, um it ends up being quite a considerably uh larger.

D

um Penalty I I have a branch that I can pull up, but it's like four or five hundred times as CPU time and like quadrupled the number of allocations for per operation.

B

So I mean I, think you're right. We used to copy on the creation, but not on the um the return yeah.

B

Yeah, that's a good question, though I don't know how you would not copy on the return, because if.

C

The API to access the value would instead, let you iterate through item by item instead of asking for a slice. It would give you value to time. Then you can avoid the copies. It's pretty awkward, but um you can do that.

B

Yeah, but that would be an addition of an API array, because we still have to maintain this whole API here right.

C

Yes, but a user who's concerned about the cost could avoid it. In that case,.

B

D

Yeah I just wanted to bring that up um like if the there might be some way around it using unsafe to cast from slice to to array and back, but it becomes, it becomes a fun game. There.

C

I, actually, don't think there's a way to do that, otherwise I would have seen it already. um The the I just wanted to throw in since I'm since we're talking about it. I've been working with a collector of P data, a lot lately like the P data API, and they have to deal with exactly all these issues. They force you to copy on the way in and they force you to copy on the way out.

C

But if you don't want to copy on the way in, you can set item at a time, and if you don't want to copy on the layout, you can retrieve item at time.

B

I got you that might just be have to be what we were going to need to do, um but back to Aaron's point about the long term in the short term. Is this something that we would be willing to accept in the short term Aaron and then uh maybe look into Josh's solution of like this um copy value, stuff, I.

D

Honestly, don't even know what the impact on this in the real world is like I yeah. It could be that we we do this and say hey if you use arrays or if you use slices in your attributes, it's costly try not to where you can.

C

D

I mean it was already.

C

Costly in the instrumentation code, path, right, yeah, yeah and and I- don't believe that this use of reflection and arrays changes that substantially it's only the receiver and right now we have this dangerous situation where one exporter could modify the data while it exports, which is exactly what Peta is protecting you from um and I'm not really concerned about that and I do advocate for unsafety, unsafe apis and P data as well. But um I. Don't think this is adding much instrumentation costs. It's just like a little bit of collection, cost I, think foreign.

B

Yeah I I think you're right like it's. It's the output right.

C

However, and there's a sort of side note, like I, think for a trace user, this is no big deal in a metrics called path. You end up constructing an attribute set um which I think copies it again and that's where things start to get expensive um and I um I think I just wanted to mention that I've. You know the lights that metric SDK, one of the optimizations, was to not construct an attribute set on the synchronous code path.

C

It involves horrible complexity, I'll just admit that, but it is potentially possible to not require the attribute set in the instrument code. Packs which might alleviate this detail and I would expect that this detail is not going to go away, especially when people start to talk about logging, because the logs API or the logs spec for the data model in otel basically includes nested attributes of slices and slices of key values and maps.

C

At that point, these problems essentially will stay with us.

A

If, if we have the copying on retrieval of data, does the set construction still need to copy, or could that be eliminated? To avoid that duplication.

B

I think it has to do with the construction of the set in itself. That's where that copying comes from, but I uh like you're, saying Anthony like if we created like a new set function that was essentially volatile like it didn't do any copying. Could we alleviate that? Is that what you're saying.

A

Well, something like Josh was saying that if we add copying on reading about reading of slice, valued attributes, we would end up double copying when creating an attribute set and I'm. Just wondering if adding copying at read. Time enables us to remove copying at set creation time. But if it doesn't, then obviously that's still a problem. We have to account for somehow.

C

I understand the question and I: don't know the answer: I have to go. Look at the code and this issue again: I've forgotten whether there is a copy at the construction of the attributes set or not.

B

Oh, there definitely is in fact, there's I think three methods to help in that copying, where you can allocate uh some sort of like storage mechanism well, I.

C

Know we I know there's an allocation for the attribute set array, but I'm wondering we don't I. Don't oh.

B

C

Answer this question is whether we're copying something for each value in that array, which is a recursively defined array, essentially I think the answer is no I think I misstated that earlier, in which case Anthony's it may not get much worse for the attribute set cost I need to review the code. Sorry.

A

um And then the the other thing you mentioned is the the log data models. Definition of attributes is different from what we currently have implemented um and I know. I know, there's discussion at the spec level about. Can that be reconciled? How do we reconcile that, um but I think that's something we need to think about as well is.

A

If we want to support the log data model, do we build a separate attribute package for it, or do we fold it into our current attribute and somehow prevent trace and metrics from using non-homogeneous, arrays and maps.

C

Yeah this was discussed in the specsig this week. um I'm sure we all have opinions I there. There was an issue and I can look it up right now, asking for what the sdks all think of it and I. My impression is that it's possible for us to if otel was to say all all signal apis can have. You know key value, lists and key value maps in addition to all the stuff that we already have I I, just I mean it's expensive, but it's possible.

C

We can see how to do it in this issue that we were just looking at right. It's you know, create a raise um representing the p-value list and then recursively use the attribute set to define the identity of a recursively defined map.

C

All this works just for expensive and I said so on the issue for the spec I actually do support just letting go and letting all the things happen.

C

um The the key challenge is what to do with like a Jaeger exporter and I think make it Json, and this isn't too hard, but that's a really undecided spec and maybe what Anthony said is going to happen instead restrict metrics and and tracing from using that. But we've already got the problem as we see for slices it's expensive and it's slippery slope I, don't see why we're going to slide all the way down.

A

It's comfy once you hit the bottom.

B

Yeah, uh so let's try not to go all the way to the bottom: I guess: um okay, I, yeah I, don't know the answer to that one either, but I I do want to see uh specifically I think like what the Proto uh decides on there, because I think that would help inform our decision, but all good things to keep in mind. Well,.

A

The the Proto wasn't going to change. Importantly, the the Proto currently supports nested maps and slices and slices can contain any type. um So our attribute is currently not aligned with the Proto, which is fine, because it's aligned with the trace and Metric spec, which is what we currently have implemented. The the question is: can we align the trace and Metric spec with the Proto, or is there always going to be a disconnect there.

B

I think also the the question is around like what does that log development look like like how are we planning to support this long development, because uh you know I think that's also a big question of whether we would add a log, API or not, and I think that in itself is a big discussion, um but yeah I I, don't know I either again, it comes back to like how that how to look in the future.

B

um That being said, so back to the current implementation, though, uh with this attributes, this was created because there's a duplicate instrument um situation due to uh the string slice is not being recognized as the same actual value and I. Think this again comes back to that set where, like the set, becomes a distinct set, because the slice type itself was a different slice, every single time um and they're not comparable. I. Think that's where it comes back down to like that. There's a functional bug here that this is trying to resolve right and I.

B

Think with that in mind like is it worth incurring an increase in the performance of the output of our attributes to resolve this bug? I guess is the question I want to ask the rest of y'all.

C

How did this not crash in the first place, you can't compare, you can't use a slice, the map key, that panics I'm confused. Actually, you can.

B

If it's a pointer, um oh it's a pointer to a slice, yeah and I- think that's something that came into and it makes sense it's like cool yeah. You can save. These pointers are the same, but then what happens when you create the exact same slice attributes a second time like they should compare to be equal, but obviously they're not they're different pointers. So that's where that was coming in.

D

Do we maybe want to add an API to our attributes to do comparisons.

D

um And use that and.

C

I think we already do the light set of SDK. That I mentioned. Does this so-called fingerprinting so I have some special code? They wrote that just like does type switch and iterates and computes a fingerprint and then, if the fingerprint matches I then go check for equality um using there's I think there's a method. It's got to be a method for that and it may not actually work for slices, but I suspect it does.

D

Yeah but yeah the do we want to take that that kind of code path.

D

Well, I I think um either way like right now. The solution to this seems a lot more than get it done by tomorrow.

D

So if we want to like, we either need to delay the.

B

32. I don't know about that, though. Right because, like this PR solves that problem.

B

C

D

B

Types right like a razor, comparable.

D

I think there is still more work to actually get it working in all cases, because it does Panic still and um I. Think we're gonna have to build a test case around the reason why it's panicking.

B

Well, I I do think. That's there as well. I haven't looked into the reason why I was panicking but I'm guessing it has to do with the reflection.

C

Reflection, code's, really gnarly, that's my biggest reservation to this is it's awful. Nobody understands it.

B

um Yeah, okay, well, I! Guess it's not working currently and it just will continue to.

C

Not work I'm surprised to hear it's panicking, I think it can make be made to work, and that's probably the right thing to do. I just nobody loves reflect foreign.

B

A

B

Think you're right, like I, think it can be made to not panic but I like us, I just haven't spent the time to look into this uh I know the author of the pr hasn't either um and so I think it's just. Somebody needs to spend the time. I do think it's a viable solution. I do think, there's a performance incurred here and there may be a better API.

B

But the thing is it's like: if we accepted this now, then we wouldn't have these duplicate metrics and we could still uh build out that API to help further performance in the future. But Aaron does raise a good point that it technically is not mergeable like there's. It's panicking, um plus other issues with PR, but yeah.

B

um So in essence, I I do think that we want to take it out of this uh Milestone. um After sharing my screen.

A

So can we take a look at what what is done in that milestone, um so I think what I would propose is that we make a release and just roll everything. That's not done into the next Milestone. If there's enough here to justify doing a release.

B

Well, that's: okay, yeah! That's where I wanted to get to today, um so we have an update to the default bucket balances just merged. uh This removes the menus resource fields, which was just clean up the pipeline again cleanup not really user facing. This is I. Think the real functional thing that's been resolved so far, flashing pending Telemetry and um of the floor, fluency shutdown methods doesn't require, uh or it does actually work now.

B

That was one of the big things that we identified last week as something that we'd want to get out to your point, though, Anthony I I do think there are honestly I think that there are two PRS that are really close. They already have one review.

B

um Each I think that this default view, if instrument does not match up any pipeline, is really close. I think that the handle the duplicate, aggregators and log instrument conflicts is it's complex is the only thing that I see is the impediment here. There's only one review, it's 500 lines and um there's a fair amount going on here. So I don't think this is just going to be a 20 minute review for people, so that might not be ready to go out the refactor Prometheus.

B

We don't have any reviews on it, yet it did just change so I I do Wonder, but it also didn't look like it was too big of a deal so.

A

B

Question is like: do you think that we could try to get these three PRS reviewed by the end of the day and then have this come out tomorrow?.

A

I have done blocked off this afternoon. I can put towards that. um Okay, no guarantees, but.

B

I mean yeah, the only one I can say I can review is Prometheus in which I can plan on doing um and that's something I plan on doing I do think. I'll just take this out right now. Oh.

B

uh Shoot probably need another Milestone uh put this into um so this next Milestone I'm guessing. We might also want to uh well we'll just we'll. Do it right now.

B

I, don't know if it's gonna be a point release um or a bug, fix, release or patch release, just because I think there are some other things that we're probably going to want it like this May yeah, anyways uh I'll move this really quick.

B

B

Nothing said so: uh I, don't I I'm I've got time off on the calendar I'm out tomorrow. Is there Anthony or Aaron? Would you be willing to release whatever we have complete by the end of today?.

D

um I have a pack day today, like the earliest. Second release would be Monday or uh Tuesday actually right.

A

Okay, I can probably prepare a release tomorrow, but it'll require review. So if everybody else is booked, um it may be difficult for me to do it alone.

B

I can I can probably tell you: I can review things. I'll have my phone, uh but I yeah I, don't know, I definitely won't be in my um office tomorrow.

A

All right, uh so how about this? If we get the targeted viewers merged I, will try to prepare a release tomorrow and we'll see how far we get with it. Otherwise, we'll deal with it Monday or Tuesday.

B

Yeah, okay, yeah I'm out as well Monday, but I can definitely do Tuesday as well. So that sounds like a plan. Let's, let's do that?

B

B

um uh Yes, this is a reference. Anthony's uh put a link to the specification issue that around the support for maps and all these other things for the logs just in case you're interested uh okay, cool Aaron uh updates from The Collector.

D

So uh went to The Collector meeting yesterday um with the sorry uh the takeaway that I have from that is. uh There is a solution for them that they are going to explore. That doesn't require any of the bridging code that we have worked on um to be merged, so uh kind of abusing Prometheus as the bridge for the time being, seeing as that's their their only output right now um with that being said, uh they do want to test the ultimate solution of you know the bridge, the the metric.

D

What is it metric producer? Originally, they want to test that code path as soon as it comes available and we want to have. They want to have some kind of migration path from registering to collector, to collect two Prometheus collectors to each SDK start up their own, uh their own stuff and register the open census as part of open, Telemetry and then open Telemetry provides all of the data with the resources and and everything so.

D

They are unblocked currently but um uh yeah. They are unblocked, uh but ask that we get something out there for the bridge uh as soon as we can and they underst. They also are aware that the bridge is not going to be released until sometime after there is some stabilization in the the spec around it.

D

B

Okay, so David, where does this put us uh for timelines on next steps for the bridge, then.

C

A

I'm not sure I'm, still figuring things out. Unfortunately, I haven't been able to spend much.

B

Time this week on it, so that's a fairy answer: um okay, yeah, especially now that we don't I, think have a really strong thing that we need to get this out last week. um I think taking our time and making sure we get it right is appropriate here. Yep.

D

I think I'll close the uh the.

A

Current Bridge PR, we can always reopen it. Okay,.

B

D

uh If you take that current well, I guess that current Bridge PR is predicated on that there be a bridge API in the SDK.

D

um We probably want to spin out essentially two PRS from that the bridge apis at some point and then uh the whatever you did for the open census to become a bridge to make it a bridge would be a good PR as well.

A

Sorry, what do you mean by that.

D

Whatever there was some code in the open census bridge to.

A

Make it interface.

D

A

That that implemented, the bridge interface.

D

Yeah that I know it's a little tiny bit, but still that that would probably be a worthwhile PR. Once we have the bridge API settled on.

A

Yeah totally um do people think it's worth leaving the bridge implementation around. Even if the interface doesn't exist, or is that not? um Is it not useful to have things that people shouldn't use.

D

My only concern would be that there is some 12 hour um change to what the spec asks for, and that means we have to change that unused API into something else right.

A

I I was I was trying to figure out. If you were asking me to leave the implementation in place without the API existing.

D

I, don't think we need to do that right now. um We can. We can do that later, but keep a bookmark somewhere if we can sure.

B

Okay, um cool thanks for the update on that one Aaron. Next up, you also have the triage session as.

D

I I just wanted to put there's a standing invite for tomorrow. uh Whoever can make it um the goal there is to take the.

D

uh What is the was a milestone, I think uh all the issues that were in the the beta Milestone and just sit down with each one of them and say you know a should they be there and then B can we work on them now and appropriately put them into the project board so that we can.

D

um We can start collaborating on that.

B

uh Yeah, that sounds great I, think I think we're chipping away with our bug fixes, but there's some uh functional things I think we need to address yep. That being said, there's no beta project boards. That would also be created. I.

D

Just created it this morning, so okay, perfect uh I caught I just copied in what was in the Milestone into the not categorized section so.

B

uh Okay, yeah I think I gotcha. Maybe we can open it up here. Really quick I'll take a look.

B

D

uh On the left clicked uh projects right there at the top- oh it's private uh I could not change that, but it should be for anybody who has maintainer roles or not maintainer um view roles in open telemetry we need to I can always okay.

B

uh Okay, cool, it looks yeah, I I know it's kind of frustrating the ideal place that we can keep the surface here, but we also want to make sure we're communicating it to the outside audience. So I want to make sure the people that aren't members can also see it, which is tough to validate with the people that are on the call um right.

D

Yeah, if it's a problem, it's we can get something fixed, but uh anyways you'll notice that there's a bunch in no status. Those are literally copied from the beta Milestone um they're the same tickets. uh The goal is to put them in one of the first three uh categories.

D

Hopefully it's not done and I am I guess if it's done then cool. That's great.

B

Yeah yeah, the only other thing that I might say in that triaging session is uh taking scope down right so like if we find things in here that are something we can do uh after things have stabilized. We might want to also do that. Yeah.

C

B

C

B

Yeah, perfect, okay, cool um yeah, unfortunately, I won't be there tomorrow, um but yeah I I appreciate you setting that up. That's really appreciated.

B

uh Pogson has, as I don't know, podcast on the call, but um he had this PR here uh and he wanted to I think propagate a lot of these changes throughout the uh the rest of our project so I. He wanted me to call this out and also get this released. I think that release would get coincided pretty well with what we're trying to do with the v0 uh three two two.

B

So that's good, but just kind of a heads up uh I think you wanted to call this out.

A

So this would be sorry this will be for every country type integration is what he's talking about.

B

uh Yeah, it looks like I I'm just reading this now um so I'm, not 100 sure but yeah. It looks like there's also Sharma we're not um cashing our Tracer.

A

um Being created new on each request, essentially yeah yeah, so let's actually something I will I can take up and go through the contribute things. If you want to uh give me it to me, I know: I've been here, you never sent you for a while, but I'm hoping to come back.

B

Hey we're happy for the uh contributions you can make so yeah definitely appreciate it.

A

I think that this is generally a good thing to do, but I also worry that I know a while back. We spent some time trying to ensure that it would be possible to obtain a tracer Provider from an existing span and that some instrumentation may need to take advantage of that which would necess take creating a tracer for every request.

A

How does this fit in? With that use case?.

B

I think that's fair. uh This is an example as well. Okay, this is kind of an odd one.

C

B

um I, don't think that that would work. You know I, think you're, right, I think you would need to create a tracer every time, there's a request because it will likely change the Tracer provider right.

B

um Anthony, it may help if you comment that in this issue, if that makes sense,.

A

Yeah I'll try to dig up the discussions we had um I think it was tonis from um build kit Moby who had use cases where they felt that was important because they had multi-tenancy um and and different providers and Export paths.

A

um It did maybe that we want to say you know our default. Instrumentation simply isn't going to account for that and if that's a specific need, you have, you need other instrumentation, or maybe we find a way to make that configurable, but I'll see if I can plan those discussions and bring forward information on that use case.

A

B

Okay, uh yeah I I think you're 100 right I think we have it in our instrumentation as well, but okay, perfect, I'll leave.

A

That yeah, so so right there in the HTTP instrumentation um it was being done. I, don't think other instrumentation is doing this, um which may lead to buggin, saying oh, hey, you're, just using the same Tracer provider and creating a new Tracer every time. Don't do that which yeah absolutely don't do that. um But for something like this, we kind of need to agree. Yep.

B

um Okay, cool all right, so we've got that uh addressed. I, think that that PR could probably get merged um I, don't I'll. Take a look after this The Next Step I wanted to call out. There's uh uh talk on Jesus week's going fast um Tuesday for the specification, I think around the structure of uh readers and their communication of the temporal leading aggregation based on the specification and so uh I know.

B

David has recently opened an issue um around this, and so I wanted to kind of just call it out, because I had this PR associated with it, which is almost ready to be reviewed with this idea that um there are going to be exporters and they need to be able to communicate through the reader with their temporality or what their default aggregators are just based on the fact that they know these things and the user may not, and reading the specification and looking at implementations like Java and python, what they do is they actually have a exploiter interface that works with these push.

B

Exporters, I'm sorry push readers, that's not right periodic readers um and if they read through this temporality and this aggregation based on the instrument kind and so I um have this PR in place. I would essentially like make us more in line with what is going on in Java and in Python, and by reading the specification, it's still not I, think the the clearest but there's also an issue to track. That's at the specification level to help explain that this kind of thing needs to be possible.

B

This interface doesn't actually explicitly get defined in the specification, so it may not be like the best way to do this, but I wanted to kind of show it as a way that I know. Java and python have done this of looking at.net as well, which is harder for me to understand uh to put it lightly, um and so they do something similar, but it's through I actually I'm, not 100 sure, but it's a little bit different in like their their creation passes this through.

B

So you know, one of the other things is that you could have is some sort of like init method here, um but this was, it seems, pretty straightforward from the uh implementation perspective and in line with what we've already done. So what this also means is that the temporality selector and the aggregation selector they stop being reader options. They start being manual reader options because only the manual reader is the thing.

B

That's going to know these, the the push or sorry the periodic reader is going to infer these from an exporter, so you're not going to configure them when you configure a periodic reader.

B

um But that also means then that things uh all of your exporters need to have configuration options for these. So um things like the otlp metrics uh exporters, then also get these um options to configure the exporter.

B

Similarly, like the um standard, automatrix exporter, so just kind of a heads up on that one I I was going to have this ready to review. I haven't, got it ready to review, but I. Don't just wanted to hear people's thoughts on this. If they've been thinking about this as well, foreign.

D

I I will say this is I, don't know if exactly but very similar to one of the very first iterations of the reader API is that they expose that temporality same thing. For the exporter.

B

Well, it still isn't in the reader API like it, but it's a private method. These.

D

B

Are both yeah? These both already exist, and the only thing is that these are public for the exporter here.

D

Yeah um I'm I'm perfectly happy with them um I believe we had something similar that passed through uh in earlier iterations and I I'm, not 100 sure why they were I, don't remember, 100 why they were removed, but I know. We've. We've talked through this at least once.

B

uh And that's mostly due to confusion. I probably might just on my part, I think from the fact that this uh reader definition is defined to have some sort of configuration here which yeah you need to understand in the context of uh this section as well. So we're trying to fix that in the specification.

D

Yeah um I'm, I'm, 100 I think this actually kind of makes things a little simpler, but uh yeah I'm on board. With this.

C

Yeah I think I still disagree and I like the way it stands today, I mean. Could the exporter return reader options to avoid having to create the same options in every year in every exporter? It's my reaction, I mean the way. I wrote the specification. The reader has those options.

C

Precisely so, when you're handing the exporter to the SDK, you put the options at the reader, because that's literally, the point of the reader concept is to be a place to put those options um in my understanding, I think you're, making a lot of trouble for yourself by forcing every exporter to provide this stuff. When it's already a centralized option on the reader.

B

Well, so yeah I I think that it is uh going to be more of a burden for exported authors for sure the.

C

B

I see, though, what's that thing I.

A

Was just going to ask, do you think most exporters are actually going to provide these options? I was I've been sort of under the impression that otlp is a little bit special and that it's going to want to and the rest of them are not going to be interested.

B

In allowing configuration of temporality well I know like a statsd exporter, would right like that. Definitely is something that can like handle temporality.

D

I think he means actually have a stats. The option to change the temporality, not some way to indicate that it is a delta temporality.

C

D

Because when you register.

C

The status exporter, you know it's Delta, and so you, you use a reader that has the Delta temporality set. It's not configurable,.

B

Well, it is that's. The problem is I, think that this is a problem on.

C

What it means to configure something who is responsible for installing the statsd exporter that person, whoever that is, will be constructing a reader to do so? That's where they put their temporality decision. It's.

B

Not the steps he's actually. Is it the responsibility of the author of the stats be exporter, or is it the responsibility of the user of the stats, the exporter to know what temporality it accepts because I think it's the former and the person who installs it is the latter.

C

I, don't expect that the person who installs exporters is a user I think there's a piece of code that gives you an SDK option to construct a stat C exporter. The SDK option is a reader and an exporter combined with all the stuff set and so I wouldn't as.

B

A user that doesn't exist for.

C

A transporter and then go for a reader I.

B

Go for an SDK! It's not designed that way. Rsdk is designed where you pass an exporter to a reader similar to the lightsab SDK.

C

And then you pass the reader to the to the provider to the music yeah. So.

B

If I have a user of that exporter who initializes the exporter, then they need to pass it to a reader. They also need to pass in an option that says that you should always use a default immorality.

C

B

Seeing the user I I'm not really trying to argue this, like I'm.

C

Well, in the in the Google.

B

C

Right now we're talking about this.

B

This morning, like I, don't understand why this is this. Is that contentious like I, really don't want to rehash that argument again, because I do know that, like the specification all agreed that, like our implementation, is wrong, like I I, they Josh, sir specifically called this out and said that, like this.

C

I didn't score it that way.

A

I I didn't hear that it's wrong. It's just it feels like it makes it worse, for us would be my so.

C

So but let's look at what the the uh the effort to construct this sort of launcher-like thing in the go. Con trade repository is doing they're, going to provide an automatic registration mechanism and the the result of interpreting all of your environment, and all of your configuration will be to provide a list of options for setting up an SDK I'm not going to an exporter and then a reader I'm, giving you a finished option, which is your exporter and your reader. That's one binding for the SDK.

C

So the produce like when we go to talk about auto registration of metrics exporters. There will be a module of code that knows about the stats, the exporter and its temporality, and can give you a finished set of options for the SDK, which includes a reader and an export I mean that's. How I see it I.

B

Mean that's that's great, but I'm not talking about that.

C

Situation back.

B

Yeah expert, like that, that's great, like I I, think that there's a possibility that you could do that in the contrib as a separate module, but what I'm saying is, as if you're outside of that you're sitting there in a world where you are setting up an SDK manually by yourself, this exporter needs to communicate the temporality and the aggregation defaults to the reader.

B

Based on the specification, it needs to be able to do that. Thank you.

C

We shouldn't debate that again, I.

B

Mean no I, don't.

C

Think it's associated.

B

I was I was confused, but I am telling you that that's not the the confusion, that's shared by the community and in fact other implementations have solved this in that particular way that we've seen like you, can look at the job implementation in the python implementation and.

C

This is well you make the reader you make, the exporter be a reader except.

B

Yeah, okay, so why wasn't that done in Java and and then that.

C

Is done in Java, the exporter for Prometheus implements the reader interface.

C

That's like earlier.

B

I'll pull it up for you. It's that's not the case.

B

There you go: here's here's, the exporter, it implements the aggregation, temporary selector and the default aggregation selector. It does not Implement a reader. This is.

C

Something what's the reader exporter, what's the reader interface look like because if it's the combination of those three interfaces, it sounds to me the same.

B

Here's a periodic metric export reader, you pass it a metric exporter.

C

C

So, are you proposing to have a tempered Audi selector I am.

B

An aggregation, selector.

C

Because yeah now I understand we're going way back to where things were a long time ago.

B

Yeah and so similar to what this is actually offering similar how at the reader level, you can also get this temporality.

B

The exporter here allows you to have the temporality to find and I honestly I I, don't care if it's this method structure, where you are implementing this uh temporality selector or the default aggregation selector or if it's something where, at the exporter level, there's some sort of initialization uh function, so that this also has like a register similar to what the reader has where it would return back functions, but at some some level this needs to be able to communicate these two things to the reader that it's being registered with.

B

C

I will I'll accept that that's how people are seeing things it doesn't seem to be written in the spec that age.

B

I agree and I think that's why I created that second issue, because this is what the community is all telling us that needs to be done, but I did not interpret the specification that way either. You and I assure that frustration.

C

Isn't this just an implementation detail.

B

I think it is, but it's also a functional one, because you're right, like I I, don't think there would be a problem if you wanted to have a user to find this other thing. But other people do other people say like what happens when an exporter doesn't support a particular temporality. What happens when an exporter needs.

C

One thing you could do is return errors. This well I.

B

I think that's.

C

The exporter and you see a cumulative temporality. That's an error.

B

uh I mean sure, then you get into the question of like where that error is returned. How that error is handled and I think that that's a great discussion for the specification to have had like nine months ago, but they didn't, and they just decided to choose this other way and so like at this point, like my.

C

Hands we have General guidelines on error, handling, there's nothing wrong with returning an error.

B

You're you're jumping ahead like the first step is it would have had to have been defined as returning an error. It isn't, though, that's what.

C

I'm saying I don't understand why that has to be returned to find exporters are allowed to return errors because.

B

Instead, they Define the specification, as an exporter needs to communicate the aggregation and the temporality to a reader I.

C

Don't know how do I find it you're.

B

C

That's that's what everyone thinks it says, even though it doesn't say that.

B

Okay, well here, I I come back out for you again. Okay,.

C

Knows what a reader an exporter is they're documented and you you can.

D

C

Need to have this conversation.

D

C

D

C

Accuse myself, I really don't care. If ever, if you can change the spec to say it, that the exporter configures the temporality, remember that that's how the original prototype worked as well. There was an aggravator selector, an Aggregate and temporality selector.

B

So I'm not I'm going to change the specification. This is what Josh sort of pointed out. The aggregation and temporality properties used by open symmetry metric SDK are determined when registering a metric Tech exporter through their Associated metric reader. This, through.

C

It does not say that the exporter communicates to the reader when you commit when you set up the reader, you provide an exporter and a temporality through the reader when you're constructing it. That's how I read it still. This is an implementation detail. I, don't think we need to change the spec I. Don't think you need to change your implementation either.

B

Told us that we were doing this wrong.

C

I didn't I was listening to that conversation. I was very confused, but I didn't take it away. That way.

B

So I'm gonna I think that's where the connection lied. Then Josh is that's what they were telling us and I. Don't think that you understood it at the time.

A

I think Aaron's believed this is a conversation that needs to happen in the specific and not in this one, though.

B

I I agree, but I'm also like at a loss, because we currently have a beta SDK app that doesn't comply with the specification. According to the rest of the specifications, say I.

C

Can try to convince them now to understand what was being debated there I mean like literally for me, the the reader is the set of options that you could possibly configure an exporter by and if you make invalid options you get, can you can get errors and I?

C

Don't see a problem with returning errors at setup time, I, don't see a problem with returning errors at export time and just honestly I, don't think this is going to be a real problem for anybody ever because the person who gives you the stats, the exporter, will give you the reader as well.

B

B

So I have uh an issue in the specification repository Josh. If you wouldn't mind commenting to try to resolve that I can pull it up for you and I'll, send it to you later. Otherwise, I think that we can probably try to progress this we're at the end of the agenda.

B

um If there's anything else, somebody else wanted to bring up.

B

B

Well, I think with that, then we could probably end it here thanks everyone for your time, appreciate it uh we'll try to see you all virtually or uh next week uh same place same time,.