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

Introduce trace.Config #653

Merged
merged 1 commit into from
Mar 28, 2018
Merged

Conversation

rakyll
Copy link
Contributor

@rakyll rakyll commented Mar 28, 2018

trace.Config allows to set global configuration for tracing
and deprecating the global default sampler setter.

Fixes #652.

@rakyll
Copy link
Contributor Author

rakyll commented Mar 28, 2018

/cc @bogdandrutu @savaki

trace/config.go Outdated
@@ -0,0 +1,36 @@
// Copyright 2017, OpenCensus Authors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2018

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

if !hasParent {
span.spanContext.TraceID = newTraceIDLocked()
}
span.spanContext.SpanID = newSpanIDLocked()
sampler := defaultSampler
sampler := config.DefaultSampler
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this thread safe? Shouldn't you use something like this: https://golang.org/pkg/sync/atomic/#Value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole block is guarded by a mutex right now.

If you are asking about the reference, in Go, things are pass by value. User cannot mutate the config.DefaultSampler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you prefer atomic values for all the global state, I added a TODO for that.

trace.Config allows to set global configuration for tracing
and deprecating the global default sampler setter.

Fixes census-instrumentation#652.
@rakyll rakyll merged commit 7cc1962 into census-instrumentation:master Mar 28, 2018
@rakyll rakyll deleted the traceconfig branch March 28, 2018 22:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants