Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

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

Open
toffaletti opened this issue Jun 1, 2018 · 4 comments
Open

Comments

@toffaletti
Copy link

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
Copy link
Contributor

Agree this would be better enabled at runtime.

semistrict pushed a commit to semistrict/opencensus-go that referenced this issue Jun 4, 2018
@semistrict semistrict self-assigned this Jun 4, 2018
semistrict pushed a commit to semistrict/opencensus-go that referenced this issue Jun 4, 2018
semistrict pushed a commit to semistrict/opencensus-go that referenced this issue Jun 4, 2018
semistrict pushed a commit to semistrict/opencensus-go that referenced this issue Jun 4, 2018
semistrict pushed a commit to semistrict/opencensus-go that referenced this issue Jun 5, 2018
semistrict pushed a commit that referenced this issue Jun 7, 2018
We still need to actually remove zpage.Handler before we can remove the eager initialization. Will will do this in around a month according to the deprecation policy.

See: #772
@odeke-em
Copy link
Member

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.

@odeke-em odeke-em changed the title Option for enabling / disabling zpages at startup time zpages: add option for enabling / disabling zpages at startup time Aug 20, 2018
@songy23
Copy link
Contributor

songy23 commented Aug 20, 2018

@semistrict
Copy link
Contributor

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants