Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

Throttle debug traces #264

Closed
wants to merge 1 commit into from
Closed

Throttle debug traces #264

wants to merge 1 commit into from

Conversation

black-adder
Copy link
Contributor

Signed-off-by: Won Jun Jang wjang@uber.com

Signed-off-by: Won Jun Jang <wjang@uber.com>
@@ -239,24 +239,21 @@ func TestDebugCorrelationID(t *testing.T) {
defer closer.Close()

h := http.Header{}
h.Add(JaegerDebugHeader, "value1")
val := "value1"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not related to change, just cleanup

@@ -80,7 +80,7 @@ func (s *Span) SetOperationName(operationName string) opentracing.Span {
// SetTag implements SetTag() of opentracing.Span
func (s *Span) SetTag(key string, value interface{}) opentracing.Span {
s.observer.OnSetTag(key, value)
if key == string(ext.SamplingPriority) && setSamplingPriority(s, value) {
if key == string(ext.SamplingPriority) && !setSamplingPriority(s, 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.

If we decide to debug sample here, we set the "sampling.priority" tag so the agent knows that the root of the debug trace started here.

@codecov
Copy link

codecov bot commented Feb 14, 2018

Codecov Report

Merging #264 into master will increase coverage by 0.27%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #264      +/-   ##
==========================================
+ Coverage    85.7%   85.97%   +0.27%     
==========================================
  Files          54       54              
  Lines        2848     2853       +5     
==========================================
+ Hits         2441     2453      +12     
+ Misses        287      282       -5     
+ Partials      120      118       -2
Impacted Files Coverage Δ
span.go 100% <100%> (+0.99%) ⬆️
tracer.go 89.62% <100%> (+1.05%) ⬆️
zipkin_thrift_span.go 73.11% <0%> (+2.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7f08d5...555dc45. Read the comment docs.

@@ -235,7 +235,7 @@ func (t *Tracer) startSpanWithOptions(
ctx.spanID = SpanID(ctx.traceID.Low)
ctx.parentID = 0
ctx.flags = byte(0)
if hasParent && parent.isDebugIDContainerOnly() {
if hasParent && parent.isDebugIDContainerOnly() && t.isDebugAllowed(operationName) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@isaachier I think I forgot this case, if the debug trace is started via using headers, the tag will be "jaeger-debug-id". I guess we could just add the "sampling.priority" tag here too...

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean an extracted span contains that header or a client locally used headers to start debug tracing somehow. I don't really understand the latter.

@isaachier
Copy link
Contributor

Please close this because it is duplicated in #274.

@black-adder
Copy link
Contributor Author

closed, replaced by #274

@yurishkuro yurishkuro deleted the throttle_debug_traces branch May 5, 2021 21:57
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.

2 participants