Skip to content

Commit ab059a9

Browse files
tgodziktgodzik
committed
improvement: Disable tracing by default
Previously, tracing was enabled by default and user could get stack traces logged by the zipkin trace. Now, we require setting `bloop.tracing.enabled` property. Tracing is not used heavily currently, so this should not be an issue. Fixes scalacenter#2108
1 parent cba9c0b commit ab059a9

5 files changed

Lines changed: 113 additions & 22 deletions

File tree

‎backend/src/main/scala/bloop/tracing/BraveTracer.scala‎

Lines changed: 96 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,100 @@ import brave.propagation.TraceContextOrSamplingFlags
1414
import zipkin2.codec.SpanBytesEncoder.JSON_V1
1515
import zipkin2.codec.SpanBytesEncoder.JSON_V2
1616

17-
final class BraveTracer private (
17+
sealed trait BraveTracer {
18+
def startNewChildTracer(name: String, tags: (String, String)*): BraveTracer
19+
20+
def trace[T](name: String, tags: (String, String)*)(
21+
thunk: BraveTracer => T
22+
): T
23+
24+
def traceVerbose[T](name: String, tags: (String, String)*)(
25+
thunk: BraveTracer => T
26+
): T
27+
28+
def traceTask[T](name: String, tags: (String, String)*)(
29+
thunk: BraveTracer => Task[T]
30+
): Task[T]
31+
32+
def traceTaskVerbose[T](name: String, tags: (String, String)*)(
33+
thunk: BraveTracer => Task[T]
34+
): Task[T]
35+
36+
def terminate(): Unit
37+
38+
def currentSpan: Option[Span]
39+
40+
def toIndependentTracer(
41+
name: String,
42+
traceProperties: TraceProperties,
43+
tags: (String, String)*
44+
): BraveTracer
45+
46+
}
47+
48+
object NoopTracer extends BraveTracer {
49+
50+
override def startNewChildTracer(name: String, tags: (String, String)*): BraveTracer = this
51+
52+
override def trace[T](name: String, tags: (String, String)*)(thunk: BraveTracer => T): T = thunk(
53+
this
54+
)
55+
56+
override def traceVerbose[T](name: String, tags: (String, String)*)(thunk: BraveTracer => T): T =
57+
thunk(this)
58+
59+
override def traceTask[T](name: String, tags: (String, String)*)(
60+
thunk: BraveTracer => Task[T]
61+
): Task[T] = thunk(this)
62+
63+
override def traceTaskVerbose[T](name: String, tags: (String, String)*)(
64+
thunk: BraveTracer => Task[T]
65+
): Task[T] = thunk(this)
66+
67+
override def terminate(): Unit = ()
68+
69+
override def currentSpan: Option[Span] = None
70+
71+
def toIndependentTracer(
72+
name: String,
73+
traceProperties: TraceProperties,
74+
tags: (String, String)*
75+
): BraveTracer = this
76+
77+
}
78+
79+
object BraveTracer {
80+
81+
def apply(name: String, properties: TraceProperties, tags: (String, String)*): BraveTracer = {
82+
BraveTracer(name, properties, None, tags: _*)
83+
}
84+
85+
def apply(
86+
name: String,
87+
properties: TraceProperties,
88+
ctx: Option[TraceContext],
89+
tags: (String, String)*
90+
): BraveTracer = {
91+
if (properties.enabled) {
92+
BraveTracerInternal(name, properties, ctx, tags: _*)
93+
} else {
94+
NoopTracer
95+
}
96+
97+
}
98+
}
99+
100+
final class BraveTracerInternal private (
18101
tracer: Tracer,
19-
val currentSpan: Span,
102+
val _currentSpan: Span,
20103
closeCurrentSpan: () => Unit,
21104
properties: TraceProperties
22-
) {
105+
) extends BraveTracer {
106+
107+
def currentSpan = Some(_currentSpan)
108+
23109
def startNewChildTracer(name: String, tags: (String, String)*): BraveTracer = {
24-
val span = tags.foldLeft(tracer.newChild(currentSpan.context).name(name)) {
110+
val span = tags.foldLeft(tracer.newChild(_currentSpan.context).name(name)) {
25111
case (span, (tagKey, tagValue)) => span.tag(tagKey, tagValue)
26112
}
27113

@@ -32,7 +118,7 @@ final class BraveTracer private (
32118
span.finish()
33119
}
34120

35-
new BraveTracer(tracer, span, closeHandler, properties)
121+
new BraveTracerInternal(tracer, span, closeHandler, properties)
36122
}
37123

38124
def trace[T](name: String, tags: (String, String)*)(
@@ -67,7 +153,7 @@ final class BraveTracer private (
67153
try thunk(newTracer) // Don't catch and report errors in spans
68154
catch {
69155
case NonFatal(t) =>
70-
newTracer.currentSpan.error(t)
156+
newTracer.currentSpan.foreach(_.error(t))
71157
throw t
72158
} finally {
73159
try newTracer.terminate()
@@ -91,7 +177,7 @@ final class BraveTracer private (
91177
case None => Task.eval(newTracer.terminate())
92178
case Some(value) =>
93179
Task.eval {
94-
newTracer.currentSpan.error(value)
180+
newTracer.currentSpan.foreach(_.error(value))
95181
newTracer.terminate()
96182
}
97183
}
@@ -116,10 +202,10 @@ final class BraveTracer private (
116202
traceProperties: TraceProperties,
117203
tags: (String, String)*
118204
): BraveTracer =
119-
BraveTracer(name, traceProperties, Some(currentSpan.context), tags: _*)
205+
BraveTracer(name, traceProperties, Some(_currentSpan.context), tags: _*)
120206
}
121207

122-
object BraveTracer {
208+
object BraveTracerInternal {
123209
import brave._
124210
import zipkin2.reporter.AsyncReporter
125211
import zipkin2.reporter.urlconnection.URLConnectionSender
@@ -134,10 +220,6 @@ object BraveTracer {
134220
reporterCache.computeIfAbsent(url, newReporter)
135221
}
136222

137-
def apply(name: String, properties: TraceProperties, tags: (String, String)*): BraveTracer = {
138-
BraveTracer(name, properties, None, tags: _*)
139-
}
140-
141223
def apply(
142224
name: String,
143225
properties: TraceProperties,
@@ -178,6 +260,6 @@ object BraveTracer {
178260
spanReporter.flush()
179261
}
180262

181-
new BraveTracer(tracer, rootSpan, closeEverything, properties)
263+
new BraveTracerInternal(tracer, rootSpan, closeEverything, properties)
182264
}
183265
}

‎backend/src/main/scala/bloop/tracing/TraceProperties.scala‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@ case class TraceProperties(
88
verbose: Boolean,
99
localServiceName: String,
1010
traceStartAnnotation: Option[String],
11-
traceEndAnnotation: Option[String]
11+
traceEndAnnotation: Option[String],
12+
enabled: Boolean
1213
)
1314

1415
object TraceProperties {
1516
val default: TraceProperties = {
1617
val verbose = Properties.propOrFalse("bloop.tracing.verbose")
18+
val enabled = Properties.propOrFalse("bloop.tracing.enabled")
1719
val debugTracing = Properties.propOrFalse("bloop.tracing.debugTracing")
1820
val localServiceName = Properties.propOrElse("bloop.tracing.localServiceName", "bloop")
1921
val traceStartAnnotation = Properties.propOrNone("bloop.tracing.traceStartAnnotation")
@@ -30,7 +32,8 @@ object TraceProperties {
3032
verbose,
3133
localServiceName,
3234
traceStartAnnotation,
33-
traceEndAnnotation
35+
traceEndAnnotation,
36+
enabled
3437
)
3538
}
3639
}

‎frontend/src/main/scala/bloop/data/TraceSettings.scala‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ case class TraceSettings(
88
verbose: Option[Boolean],
99
localServiceName: Option[String],
1010
traceStartAnnotation: Option[String],
11-
traceEndAnnotation: Option[String]
11+
traceEndAnnotation: Option[String],
12+
enabled: Option[Boolean]
1213
)
1314

1415
object TraceSettings {
@@ -20,7 +21,8 @@ object TraceSettings {
2021
settings.verbose.getOrElse(default.verbose),
2122
settings.localServiceName.getOrElse(default.localServiceName),
2223
settings.traceStartAnnotation.orElse(default.traceStartAnnotation),
23-
settings.traceEndAnnotation.orElse(default.traceEndAnnotation)
24+
settings.traceEndAnnotation.orElse(default.traceEndAnnotation),
25+
settings.enabled.getOrElse(default.enabled)
2426
)
2527
}
2628

@@ -31,7 +33,8 @@ object TraceSettings {
3133
verbose = Some(properties.verbose),
3234
localServiceName = Some(properties.localServiceName),
3335
traceStartAnnotation = properties.traceStartAnnotation,
34-
traceEndAnnotation = properties.traceEndAnnotation
36+
traceEndAnnotation = properties.traceEndAnnotation,
37+
enabled = Some(properties.enabled)
3538
)
3639
}
3740
}

‎frontend/src/test/scala/bloop/BuildLoaderSpec.scala‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,8 @@ object BuildLoaderSpec extends BaseSuite {
302302
verbose = Some(false),
303303
localServiceName = Some("42"),
304304
traceStartAnnotation = Some("start"),
305-
traceEndAnnotation = Some("end")
305+
traceEndAnnotation = Some("end"),
306+
enabled = Some(true)
306307
)
307308
)
308309
)

‎frontend/src/test/scala/bloop/TracerSpec.scala‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ import bloop.tracing.BraveTracer
66
import bloop.tracing.TraceProperties
77

88
object TracerSpec extends BaseSuite {
9+
10+
private val defaultOn = TraceProperties.default.copy(enabled = true)
911
test("forty clients can send zipkin traces concurrently") {
1012
def sendTrace(id: String): Unit = {
11-
val tracer = BraveTracer(s"encode ${id}", TraceProperties.default)
13+
val tracer = BraveTracer(s"encode ${id}", defaultOn)
1214
Thread.sleep(700)
1315
tracer.trace(s"previous child ${id}") { tracer =>
1416
Thread.sleep(500)
@@ -23,7 +25,7 @@ object TracerSpec extends BaseSuite {
2325
Thread.sleep(750)
2426
val tracer2 = tracer.toIndependentTracer(
2527
s"independent encode ${id}",
26-
TraceProperties.default
28+
defaultOn
2729
)
2830
tracer.terminate()
2931
tracer2.trace(s"previous independent child ${id}") { _ =>

0 commit comments

Comments
 (0)