Configure a max number spans in RunningSpanStoreImpl

[dmichel1]

Hi,

I recently was involved with an incident where a service was not properly closely a span when an error was raised. This caused the JVM to go OOM after a few hours.

Do you have any thoughts or suggestions on how to prevent something similar from happening again? Some configurable limit of how many spans can be stored in the RunningSpanStoreImpl?

[songy23]

Unfortunately for now we don't have an API for configuring the buffer size of in-process running spans. In the implementation running span store is an unbounded list:

opencensus-java/impl_core/src/main/java/io/opencensus/implcore/trace/export/InProcessRunningSpanStoreImpl.java

Line 36 in f0e78ba

runningSpans = new ConcurrentIntrusiveList<RecordEventsSpanImpl>();

We could add an API in TraceConfig to allow users to configure the limit of running spans. Before that I would suggest 1) don't keep too many spans running concurrently and 2) don't keep the lifetime of each span too long.

/cc @bogdandrutu WDYT?

[bogdandrutu]

Couple of things we should do to help here:

1. RunningSpanStoreImpl should be a no-op if no z-pages installed.
2. Add a limit probably when the z-pages are configured that is passed to the RunningSpanStoreImpl.

These things may imply a bit of refactoring of the code, we should start talking about this. I think the first item should be easier to fix.

[bogdandrutu]

@dmichel1 can you please tell me if you enable z-pages https://github.com/census-instrumentation/opencensus-java/tree/master/contrib/zpages or if you use the RunningSpanStore for other reasons?

I was thinking to make that disabled by default and if enabled then will keep a reference to all the Spans.

[dmichel1]

@bogdandrutu we don't have zpages enabled.

Some services might pull in the opencensus-contrib-zpage dependencies for other reasons but only having the dependencies listed won't enable anything unless PageHandlers.startHttpServerAndRegisterAll(8080); is specifically called, right?

I'm also not aware of any RunningSpanStore usage outside of the zpages.

[songy23]

only having the dependencies listed won't enable anything unless PageHandlers.startHttpServerAndRegisterAll(8080); is specifically called, right?

This is correct. After #1875 is released, no running spans will be stored in process if no zPages are explicitly enabled.

[bogdandrutu]

@songy23 for the moment we enable the RunningSpan at the static initialization of the handlers. Probably we need to change that:
https://github.com/census-instrumentation/opencensus-java/blob/master/contrib/zpages/src/main/java/io/opencensus/contrib/zpages/ZPageHandlers.java#L81

We probably need to automatically enable if:
startHttpServerAndRegisterAll or registerAllToHttpServer

If users wants to use the handlers manually via getTracezZPageHandler we can have a enable or just enable when someone calls getTracezZPageHandler()?