-
Notifications
You must be signed in to change notification settings - Fork 50
TraceParams are not mentioning the buffer limit #147
Comments
In Java we have a fixed buffer size of 32, though this looks like an implementation detail. Buffer size is not configurable and is not part of TraceParams in Java. |
I don't know if adding a default limit for all types makes sense. Why not just stick with individual limits for events, links, attributes, etc.? |
Hey -- I will try my best to clear up the confusion here. @songy23 linked to a static trace export buffer size, which is not what the Go PR is about. As of right now, here is the limit support for attributes, annotations, message events, links: Global limits that can be overridden:
Per-span limits that can be overridden:
The spec was recently updated in #139 and then again in #141. I find the wording in the spec still a bit ambiguous -- if we need global limit overrides AND per-span limit overrides, or just the latter. My updated CL for Go for the new spec (intentionally not pushed yet) implements BOTH global limit overrides and per-span limit overrides. Per-span limits override any global limit regardless if it was the default value or changed. I asked for clarification this morning and held off on pushing the updated CL in hopes to not make the situation any more confusing than it already is :) |
@jgracenin thanks for the clarification. My 2 cents here:
Anyone has an opinion about this? |
There is a requirement to set the default buffer size globally (see census-instrumentation/opencensus-go#824) but this setting is not represented in TraceParams.
In order to have a more predictable API from user's perspective, would it make sense to represent this setting in the TraceConfig?
/cc @Ramonza @bogdandrutu @songy23
The text was updated successfully, but these errors were encountered: