Skip to content

Commit

Permalink
Adds ability to receive debug flag from Finagle. If the debug flag is…
Browse files Browse the repository at this point in the history
… set on a span we ensure it is stored. This allows developers to force tracing on a request

Author: @johanoskarsson
Fixes #86
URL: #86
  • Loading branch information
johanoskarsson committed Jul 30, 2012
1 parent a5e1ee3 commit a75e854
Show file tree
Hide file tree
Showing 10 changed files with 88 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import com.twitter.zipkin.Constants
/**
* A span represents one RPC request. A trace is made up of many spans.
*
* A span can contain multiple annotations, some are always incuded such as
* A span can contain multiple annotations, some are always included such as
* Client send -> Server received -> Server send -> Client receive.
*
* Some are created by users, describing application specific information,
Expand All @@ -30,7 +30,7 @@ import com.twitter.zipkin.Constants
object Span {

def apply(span: Span): Span = Span(span.traceId, span.name, span.id,
span.parentId, span.annotations, span.binaryAnnotations)
span.parentId, span.annotations, span.binaryAnnotations, span.debug)
}

/**
Expand All @@ -42,9 +42,10 @@ object Span {
* some fixed ones from the tracing framework
* @param binaryAnnotations binary annotations, can contain more detailed information such as
* serialized objects
* @param debug if this is set we will make sure this span is stored, no matter what the samplers want
*/
case class Span(traceId: Long, name: String, id: Long, parentId: Option[Long],
annotations: List[Annotation], binaryAnnotations: Seq[BinaryAnnotation]) {
annotations: List[Annotation], binaryAnnotations: Seq[BinaryAnnotation], debug: Boolean = false) {
/**
* Order annotations by timestamp.
*/
Expand Down Expand Up @@ -96,7 +97,7 @@ case class Span(traceId: Long, name: String, id: Long, parentId: Option[Long],

new Span(traceId, selectedName, id, parentId,
annotations ++ mergeFrom.annotations,
binaryAnnotations ++ mergeFrom.binaryAnnotations)
binaryAnnotations ++ mergeFrom.binaryAnnotations, debug | mergeFrom.debug)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ class SpanSpec extends Specification {
val ann1 = Annotation(1, "value1", Some(Endpoint(1, 2, "service")))
val ann2 = Annotation(2, "value2", Some(Endpoint(3, 4, "service")))

val span1 = Span(12345, "", 666, None, List(ann1), Nil)
val span2 = Span(12345, "methodcall", 666, None, List(ann2), Nil)
val expectedSpan = Span(12345, "methodcall", 666, None, List(ann1, ann2), Nil)
val span1 = Span(12345, "", 666, None, List(ann1), Nil, true)
val span2 = Span(12345, "methodcall", 666, None, List(ann2), Nil, false)
val expectedSpan = Span(12345, "methodcall", 666, None, List(ann1, ann2), Nil, true)
val actualSpan = span1.mergeSpan(span2)
actualSpan mustEqual expectedSpan
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class ScribeCollectorServiceSpec extends Specification with JMocker with ClassMo

val wrongCatList = List(gen.LogEntry("wrongcat", serializer.toString(ThriftAdapter(validSpan))))

val base64 = "CgABAAAAAAAAAHsLAAMAAAADYm9vCgAEAAAAAAAAAcgPAAYMAAAAAQoAAQAAAAAAAAABCwACAAAAA2JhaAAPAAgMAAAAAAA="
val base64 = "CgABAAAAAAAAAHsLAAMAAAADYm9vCgAEAAAAAAAAAcgPAAYMAAAAAQoAAQAAAAAAAAABCwACAAAAA2JhaAAPAAgMAAAAAAIACQAA"

val queue = mock[WriteQueue[Seq[String]]]
val zkSampleRateConfig = mock[AdjustableRateConfig]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,13 @@ object ThriftAdapter extends Adapter {
case b => b.map { this(_) }
}

new Span(s.traceId, s.name, s.id, s.parentId, annotations, binaryAnnotations)
new Span(s.traceId, s.name, s.id, s.parentId, annotations, binaryAnnotations, s.debug)
}

/* Span to Thrift */
def apply(s: Span): spanType = {
gen.Span(s.traceId, s.name, s.id, s.parentId, s.annotations.map { this(_) },
s.binaryAnnotations.map { this(_) })
s.binaryAnnotations.map { this(_) }, s.debug)
}

/* TraceSummary from Thrift */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,14 @@ class SamplerProcessorFilter(sampler: GlobalSampler) extends ProcessorFilter[Seq
spans.flatMap { span =>
span.serviceNames.foreach { name => Stats.incr("received_" + name) }

if (sampler(span.traceId)) {
/**
* If the span was created with debug mode on we guarantee that it will be
* stored no matter what our sampler tells us
*/
if (span.debug) {
Stats.incr("debugflag")
Some(span)
} else if (sampler(span.traceId)) {
Some(span)
} else {
None
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,20 @@ trait GlobalSampler {
* True: drop trace on the floor
* False: process trace
*/
def apply(traceId: Long) : Boolean = false
def apply(traceId: Long): Boolean = false

}

/**
* None shall pass! Drop all the trace data.
*/
object NullGlobalSampler extends GlobalSampler {
override def apply(traceId: Long) = false
}

/**
* Let everything through.
*/
object EverythingGlobalSampler extends GlobalSampler {
override def apply(traceId: Long) = true
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package com.twitter.zipkin.collector.processor

/*
* Copyright 2012 Twitter Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

import org.specs.Specification
import com.twitter.zipkin.gen
import com.twitter.zipkin.common.{Span, Endpoint, Annotation}
import com.twitter.ostrich.stats.{Histogram, Distribution, Stats}
import com.twitter.zipkin.collector.sampler.{EverythingGlobalSampler, NullGlobalSampler}

class SamplerProcessorFilterSpec extends Specification {

"SamplerProcessorFilter" should {
"let the span pass if debug flag is set" in {
val span = Span(12345, "methodcall", 666, None, List(), Nil, true)
val spans = Seq(span)
val samplerProcessor = new SamplerProcessorFilter(NullGlobalSampler)
samplerProcessor(spans) mustEqual spans
}

"let the span pass if debug flag false and sampler says yes" in {
val span = Span(12345, "methodcall", 666, None, List(), Nil, false)
val spans = Seq(span)
val samplerProcessor = new SamplerProcessorFilter(EverythingGlobalSampler)
samplerProcessor(spans) mustEqual spans
}

"don't let the span pass if debug flag false and sampler says no" in {
val span = Span(12345, "methodcall", 666, None, List(), Nil, false)
val spans = Seq(span)
val samplerProcessor = new SamplerProcessorFilter(NullGlobalSampler)
samplerProcessor(spans) mustEqual Seq()
}
}
}
6 changes: 2 additions & 4 deletions zipkin-test/src/test/resources/TestCollector.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import com.twitter.zipkin.config._
import com.twitter.zipkin.config.sampler.NullAdaptiveSamplerConfig
import com.twitter.zipkin.config.zookeeper.ZooKeeperConfig
import com.twitter.zipkin.collector.sampler.GlobalSampler
import com.twitter.zipkin.collector.sampler.{GlobalSampler, EverythingGlobalSampler}
import com.twitter.conversions.time._
import com.twitter.logging.LoggerFactory
import com.twitter.logging.config._
Expand Down Expand Up @@ -64,9 +64,7 @@ new ScribeZipkinCollectorConfig {
override def adaptiveSamplerConfig = new NullAdaptiveSamplerConfig {}

// sample it all
override def globalSampler: GlobalSampler = new GlobalSampler() {
override def apply(traceId: Long) : Boolean = true
}
override def globalSampler: GlobalSampler = EverythingGlobalSampler

def zkConfig = new ZooKeeperConfig {
servers = List("localhost:2181")
Expand Down
2 changes: 2 additions & 0 deletions zipkin-thrift/src/main/thrift/zipkinCore.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,6 @@ struct Span {
5: optional i64 parent_id, // parent span id
6: list<Annotation> annotations, // list of all annotations/events that occured
8: list<BinaryAnnotation> binary_annotations // any binary annotations
9: optional bool debug = 0 // if true, we DEMAND that this span passes all samplers
}

0 comments on commit a75e854

Please sign in to comment.