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

Fix sampling when jaeger-baggage header is given #542

Merged
merged 4 commits into from
Aug 30, 2018

Conversation

yurishkuro
Copy link
Member

@yurishkuro yurishkuro commented Aug 30, 2018

Fix for #541

Signed: Yuri the Bug Maker

Yuri Shkuro added 2 commits August 29, 2018 23:16
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
@ghost ghost assigned yurishkuro Aug 30, 2018
@ghost ghost added the review label Aug 30, 2018
@codecov
Copy link

codecov bot commented Aug 30, 2018

Codecov Report

Merging #542 into master will decrease coverage by 0.05%.
The diff coverage is 88.88%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #542      +/-   ##
============================================
- Coverage     88.68%   88.63%   -0.06%     
+ Complexity      516      515       -1     
============================================
  Files            66       66              
  Lines          1891     1891              
  Branches        243      243              
============================================
- Hits           1677     1676       -1     
  Misses          141      141              
- Partials         73       74       +1
Impacted Files Coverage Δ Complexity Δ
...a/io/jaegertracing/internal/JaegerSpanContext.java 96.96% <0%> (-3.04%) 22 <2> (-1)
...n/java/io/jaegertracing/internal/JaegerTracer.java 90.31% <100%> (ø) 24 <0> (ø) ⬇️

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 ced1b3e...d50ae48. Read the comment docs.


import org.junit.Test;

public class PropagationTest {
@Test
public void testDebugCorrelationId() {
Copy link
Member Author

Choose a reason for hiding this comment

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

moved to AdhocHeadersTest

@@ -198,29 +192,4 @@ public void testCustomSpanOnSpanManager() {
// check
assertEquals(activeSpan, tracer.activeSpan());
}

@Test
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to AdhocHeadersTest

Signed-off-by: Yuri Shkuro <ys@uber.com>
jaegerSpanContext = span.context();
assertTrue(jaegerSpanContext.isSampled());
assertTrue(jaegerSpanContext.isDebug());
assertEquals("Coraline", span.getTags().get(Constants.DEBUG_ID_HEADER_KEY));
Copy link
Member Author

Choose a reason for hiding this comment

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

moved to AdhocHeadersTest

JaegerSpan mockSpan = Mockito.mock(JaegerSpan.class);
tracer.scopeManager().activate(mockSpan, true);
assertEquals(mockSpan, tracer.activeSpan());
}
Copy link
Member Author

Choose a reason for hiding this comment

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

moved to ActiveSpanTest


// check
assertEquals(activeSpan, tracer.activeSpan());
}
Copy link
Member Author

Choose a reason for hiding this comment

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

moved to ActiveSpanTest


assertEquals("must have baggage", "v1", span.getBaggageItem("k1"));
assertEquals("must have baggage", "v2", span.getBaggageItem("k2"));
}
Copy link
Member Author

Choose a reason for hiding this comment

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

moved to AdhocHeadersTest

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, Uber Technologies, Inc
* Copyright (c) 2018, The Jaeger Authors.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copyright (c) 2016-2017, Uber Technologies, Inc?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, considering that active span did not exist in 2016 and most of 2017, and it was all implemented by people not from Uber.

Signed-off-by: Yuri Shkuro <ys@uber.com>
@yurishkuro yurishkuro merged commit 8d325b9 into master Aug 30, 2018
@ghost ghost removed the review label Aug 30, 2018
@pavolloffay pavolloffay deleted the fix-jaeger-baggage-sampling branch December 17, 2019 09:56
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