search

census-instrumentation / opencensus-go

A stats collection and distributed tracing framework
http://opencensus.io
Apache License 2.0
2.06k stars 327 forks source link

zpages: add option for enabling / disabling zpages at startup time #772

Open toffaletti opened 6 years ago

toffaletti commented 6 years ago

It is a bit surprising that just importing zpages package (even if you don't register a handler) causes the infinite memory growth described in #597 even if DefaultSampler is NeverSample because of 

func init() {
    internal.LocalSpanStoreEnabled = true
}

and

    if !internal.LocalSpanStoreEnabled && !span.spanContext.IsSampled() {
        return span
    }

This means enabling / disabling has to be done with a build tag right now, but a runtime command line flag would be best.

semistrict commented 6 years ago

Agree this would be better enabled at runtime.

odeke-em commented 6 years ago

Thank you for filing this @toffaletti and thanks @ramonza for working on it!

@ramonza do you think we should remove that init invocation that enables zpages on the zpages.Handler variable? What else could we do here? Thank you.

songy23 commented 6 years ago

FYI in Java ZPages needs to be explicitly initialized in application code: https://github.com/census-instrumentation/opencensus-java/blob/9d533927c764069641ed56dcdcd0a83d05a5a14a/examples/src/main/java/io/opencensus/examples/zpages/ZPagesTester.java#L105

semistrict commented 6 years ago

I think we can go ahead now and remove the Handler var that causes eager init.