Add MaxLogsPerSpan option #47
Conversation
| @@ -117,6 +118,9 @@ type Options struct { | |||
| // variable-length value types (strings, interface{}, etc). | |||
| MaxLogValueLen int `yaml:"max_log_value_len"` | |||
|
|
|||
| // MaxLogsPerSpan limits the number of logs in a single span. | |||
There was a problem hiding this comment.
remind me -- if someone sets this to 0, it would actually inherit the basictracer default, correct?
There was a problem hiding this comment.
No, we are setting options.MaxLogsPerSpan unconditionally after getting the DefaultOptions(). If we want that we can enclsoe that in a if MaxLogsPerSpan != 0
e7c4895 to
0944454
Compare
|
It looks like the CI build is using an old |
Basictracer now supports a `MaxLogPerSpan` option; plumbing this option in Lightstep as well. Fixes lightstep#46.
0944454 to
f9e89e3
Compare
|
Yes, that happened with the previous build as well. I'll take a look. On Fri, Sep 30, 2016 at 8:38 AM RaduBerinde notifications@github.com
|
|
The CircleCI cache needed to be flushed. Fixed! On Fri, Sep 30, 2016 at 8:40 AM spoons spoons@lightstep.com wrote:
|
|
Sorry I forgot about this... On one hand, I'm not crazy about the way the second set of defaults is expressed here. On the other hand, I don't want to make you change that since it would affect BasicTracer as well. So we can merge this and potentially clean up the options stuff separately. |
|
Thanks @bensigelman I can do some cleanup, what do you have in mind? Exporting the default value from basictracer as a constant that we use here as well? |
|
@RaduBerinde there's a lightstep-internal effort to canonicalize the semantics of tracer tuning options across platforms... until that's settled down, it's probably not worth the churn. (Aside from anything else, I'd rather do a "functional options" idiom for the initialization) |
Basictracer now supports a
MaxLogPerSpanoptions; plumbing this option inLightstep as well.
Fixes #46.