add option to limit number of log events

[RaduBerinde]

We had an issue where we were doing endless retries and the memory inside basictracer grew indefinitely. It would be useful to add an option to limit the number of events in a span.

If adding this option is ok, I can work on the PR.

[yurishkuro]

Sounds like a good idea. Only please have sensible defaults so that it works for most people without having to configure the limits, e.g. up to 20 tags per span, up to 10 logs per span with up to 5 fields per log.

@bensigelman do you have better defaults?

cc @tschottdorf

[RaduBerinde]

In the interest of backward compatibility (especially when used as part of other tracers, e.g. lightstep), I would say that by default it should behave as it does now (i.e. no limits).

[jmacd]

I would prefer to have limits, by default, with in-band annotations indicating that some data was dropped. E.g., a meta-tag saying "tags.dropped:4" or a log message saying "did not record 25 log events", etc. ᐧ

On Mon, Sep 26, 2016 at 7:41 AM, RaduBerinde notifications@github.com wrote:

In the interest of backward compatibility (especially when used as part of other tracers, e.g. lightstep), I would say that by default it should behave as it does now (i.e. no limits).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/opentracing/basictracer-go/issues/38#issuecomment-249589815, or mute the thread https://github.com/notifications/unsubscribe-auth/ADdiiTDMX-c_YawMmzKcbkVD9GEZIAItks5qt9mTgaJpZM4KGftB .

[bhs]

Yeah, I think sensible/conservative defaults would be sensible and conservative. This is about protecting the application as much as the tracing system.

+1 to the feature. @RaduBerinde are you volunteering to do the work? :)

[RaduBerinde]

I am - for limiting log events at least. Limiting tags and other things can be done separately if it's an actual concern in practice.

[bhs]

@RaduBerinde great! Happy to review a PR.