From ea6958896912ad5875e7ed960514439464ab1b18 Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Mon, 24 Jul 2017 11:50:23 -0400 Subject: [PATCH 01/14] Add BaggageRestrictionManager --- .../src/main/java/com/uber/jaeger/Span.java | 23 ++++-- .../src/main/java/com/uber/jaeger/Tracer.java | 20 +++++- .../baggage/BaggageRestrictionManager.java | 36 ++++++++++ .../DefaultBaggageRestrictionManager.java | 41 +++++++++++ .../uber/jaeger/baggage/SanitizedBaggage.java | 32 +++++++++ .../java/com/uber/jaeger/metrics/Metrics.java | 18 +++++ .../test/java/com/uber/jaeger/SpanTest.java | 71 ++++++++++++++++--- .../BaggageRestrictionManagerTest.java | 52 ++++++++++++++ 8 files changed, 277 insertions(+), 16 deletions(-) create mode 100644 jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java create mode 100644 jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java create mode 100644 jaeger-core/src/main/java/com/uber/jaeger/baggage/SanitizedBaggage.java create mode 100644 jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Span.java b/jaeger-core/src/main/java/com/uber/jaeger/Span.java index 6f7f77c52..3ec452824 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/Span.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/Span.java @@ -22,6 +22,7 @@ package com.uber.jaeger; +import com.uber.jaeger.baggage.SanitizedBaggage; import io.opentracing.tag.Tags; import java.util.ArrayList; import java.util.Collections; @@ -117,21 +118,35 @@ public List getLogs() { @Override public Span setBaggageItem(String key, String value) { synchronized (this) { - // TODO emit a metric whenever baggage is updated + if (key == null || value == null) { + return this; + } + SanitizedBaggage baggage = this.tracer.getBaggageRestrictionManager().sanitizeBaggage(key, value); + if (!baggage.isValid()) { + this.tracer.getMetrics().baggageUpdateFailure.inc(1); + return this; + } + if (baggage.isTruncated()) { + this.tracer.getMetrics().baggageTruncate.inc(1); + } String prevItem = this.getBaggageItem(key); - this.context = this.context.withBaggageItem(key, value); + this.context = this.context.withBaggageItem(key, baggage.getSanitizedValue()); + this.tracer.getMetrics().baggageUpdateSuccess.inc(1); if (context.isSampled()) { Map fields = new HashMap(); fields.put("event", "baggage"); fields.put("key", key); - fields.put("value", value); + fields.put("value", baggage.getSanitizedValue()); if (prevItem != null) { fields.put("override", "true"); } + if (baggage.isTruncated()) { + fields.put("truncated", "true"); + } return this.log(fields); } + return this; } - return this; } @Override diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java b/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java index eedde5994..0852d234f 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java @@ -22,7 +22,8 @@ package com.uber.jaeger; -import com.uber.jaeger.Constants; +import com.uber.jaeger.baggage.BaggageRestrictionManager; +import com.uber.jaeger.baggage.DefaultBaggageRestrictionManager; import com.uber.jaeger.exceptions.UnsupportedFormatException; import com.uber.jaeger.metrics.Metrics; import com.uber.jaeger.metrics.NullStatsReporter; @@ -74,6 +75,7 @@ public class Tracer implements io.opentracing.Tracer { private final Map tags; private final boolean zipkinSharedRpcSpan; private final ActiveSpanSource activeSpanSource; + private final BaggageRestrictionManager baggageRestrictionManager; private Tracer( String serviceName, @@ -84,7 +86,8 @@ private Tracer( Metrics metrics, Map tags, boolean zipkinSharedRpcSpan, - ActiveSpanSource activeSpanSource) { + ActiveSpanSource activeSpanSource, + BaggageRestrictionManager baggageRestrictionManager) { this.serviceName = serviceName; this.reporter = reporter; this.sampler = sampler; @@ -93,6 +96,7 @@ private Tracer( this.metrics = metrics; this.zipkinSharedRpcSpan = zipkinSharedRpcSpan; this.activeSpanSource = activeSpanSource; + this.baggageRestrictionManager = baggageRestrictionManager; this.version = loadVersion(); @@ -140,6 +144,10 @@ Reporter getReporter() { return reporter; } + BaggageRestrictionManager getBaggageRestrictionManager() { + return baggageRestrictionManager; + } + void reportSpan(Span span) { reporter.report(span); metrics.spansFinished.inc(1); @@ -448,6 +456,7 @@ public static final class Builder { private Map tags = new HashMap(); private boolean zipkinSharedRpcSpan; private ActiveSpanSource activeSpanSource = new ThreadLocalActiveSpanSource(); + private BaggageRestrictionManager baggageRestrictionManager = new DefaultBaggageRestrictionManager(); public Builder(String serviceName, Reporter reporter, Sampler sampler) { if (serviceName == null || serviceName.trim().length() == 0) { @@ -524,9 +533,14 @@ public Builder withTags(Map tags) { return this; } + public Builder withBaggageRestrictionManager(BaggageRestrictionManager baggageRestrictionManager) { + this.baggageRestrictionManager = baggageRestrictionManager; + return this; + } + public Tracer build() { return new Tracer(this.serviceName, reporter, sampler, registry, clock, metrics, tags, - zipkinSharedRpcSpan, activeSpanSource); + zipkinSharedRpcSpan, activeSpanSource, baggageRestrictionManager); } } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java new file mode 100644 index 000000000..4bc48f713 --- /dev/null +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2017, Uber Technologies, Inc + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package com.uber.jaeger.baggage; + +public abstract class BaggageRestrictionManager { + static final int DEFAULT_MAX_VALUE_LENGTH = 2048; + + public abstract SanitizedBaggage sanitizeBaggage(String key, String value); + + SanitizedBaggage truncateBaggage(String value, int maxValueLength) { + if (value.length() > maxValueLength) { + return SanitizedBaggage.of(true, value.substring(0, maxValueLength), true); + } + return SanitizedBaggage.of(true, value, false); + } +} diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java new file mode 100644 index 000000000..fbd09d61f --- /dev/null +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2017, Uber Technologies, Inc + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package com.uber.jaeger.baggage; + +public class DefaultBaggageRestrictionManager extends BaggageRestrictionManager { + + private final int maxValueLength; + + public DefaultBaggageRestrictionManager() { + this(DEFAULT_MAX_VALUE_LENGTH); + } + + public DefaultBaggageRestrictionManager(int maxValueLength) { + this.maxValueLength = maxValueLength; + } + + @Override + public SanitizedBaggage sanitizeBaggage(String key, String value) { + return truncateBaggage(value, maxValueLength); + } +} diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/SanitizedBaggage.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/SanitizedBaggage.java new file mode 100644 index 000000000..31991b70a --- /dev/null +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/SanitizedBaggage.java @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2017, Uber Technologies, Inc + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package com.uber.jaeger.baggage; + +import lombok.Value; + +@Value(staticConstructor = "of") +public class SanitizedBaggage { + final boolean valid; + final String sanitizedValue; + final boolean truncated; +} diff --git a/jaeger-core/src/main/java/com/uber/jaeger/metrics/Metrics.java b/jaeger-core/src/main/java/com/uber/jaeger/metrics/Metrics.java index a279dfcef..44542656d 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/metrics/Metrics.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/metrics/Metrics.java @@ -211,4 +211,22 @@ public static Metrics fromStatsReporter(StatsReporter reporter) { ) // Number of times the Sampler failed to parse retrieved sampling strategy public Counter samplerParsingFailure; + + @Metric( + name = "baggage-update", + tags = {@Tag(key = "result", value = "ok")} + ) + // Number of times baggage was successfully written or updated on spans + public Counter baggageUpdateSuccess; + + @Metric( + name = "baggage-update", + tags = {@Tag(key = "result", value = "err")} + ) + // Number of times baggage failed to write or update on spans + public Counter baggageUpdateFailure; + + @Metric(name = "baggage-truncate") + // Number of times baggage was truncated as per baggage restrictions + public Counter baggageTruncate; } diff --git a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java index 9f8edfb97..9dae33557 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java @@ -28,6 +28,9 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import com.uber.jaeger.baggage.BaggageRestrictionManager; +import com.uber.jaeger.baggage.DefaultBaggageRestrictionManager; +import com.uber.jaeger.baggage.SanitizedBaggage; import com.uber.jaeger.metrics.InMemoryStatsReporter; import com.uber.jaeger.reporters.InMemoryReporter; import com.uber.jaeger.samplers.ConstSampler; @@ -58,6 +61,7 @@ public void setUp() throws Exception { new Tracer.Builder("SamplerTest", reporter, new ConstSampler(true)) .withStatsReporter(metricsReporter) .withClock(clock) + .withBaggageRestrictionManager(new DefaultBaggageRestrictionManager(15)) .build(); span = (Span) tracer.buildSpan("some-operation").startManual(); } @@ -80,7 +84,48 @@ public void testSetAndGetBaggageItem() { assertEquals(expected, span.getBaggageItem(key)); // Ensure the baggage was logged - this.assertBaggageLogs(span, key, expected, false); + this.assertBaggageLogs(span, key, expected, false, false); + + assertEquals( + 1L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue()); + } + + @Test + public void testInvalidBaggageKey() { + String key = "some.BAGGAGE"; + String value = "luggage"; + BaggageRestrictionManager mgr = mock(BaggageRestrictionManager.class); + + tracer = + new Tracer.Builder("SamplerTest", reporter, new ConstSampler(true)) + .withStatsReporter(metricsReporter) + .withClock(clock) + .withBaggageRestrictionManager(mgr) + .build(); + span = (Span) tracer.buildSpan("some-operation").startManual(); + + when(mgr.sanitizeBaggage(key, value)).thenReturn(SanitizedBaggage.of(false, null, false)); + + span.setBaggageItem(key, value); + assertNull(span.getBaggageItem(key)); + + assertEquals( + 1L, metricsReporter.counters.get("jaeger.baggage-update.result=err").longValue()); + } + + @Test + public void testTruncateBaggage() { + String value = "01234567890123456789"; + String expected = "012345678901234"; + String key = "some.BAGGAGE"; + span.setBaggageItem(key, value); + assertEquals(expected, span.getBaggageItem(key)); + + // Ensure the baggage was logged + this.assertBaggageLogs(span, key, expected, false, true); + + assertEquals( + 1L, metricsReporter.counters.get("jaeger.baggage-truncate").longValue()); } @Test @@ -295,14 +340,14 @@ public void testSpanDetectsSamplingPriorityLessThanZero() { public void testBaggageOneReference() { io.opentracing.Span parent = tracer.buildSpan("foo").startManual(); parent.setBaggageItem("foo", "bar"); - this.assertBaggageLogs(parent, "foo", "bar", false); + this.assertBaggageLogs(parent, "foo", "bar", false, false); io.opentracing.Span child = tracer.buildSpan("foo") .asChildOf(parent) .startManual(); child.setBaggageItem("a", "a"); - this.assertBaggageLogs(child, "a", "a", false); + this.assertBaggageLogs(child, "a", "a", false, false); assertNull(parent.getBaggageItem("a")); assertEquals("a", child.getBaggageItem("a")); @@ -313,10 +358,10 @@ public void testBaggageOneReference() { public void testBaggageMultipleReferences() { io.opentracing.Span parent1 = tracer.buildSpan("foo").startManual(); parent1.setBaggageItem("foo", "bar"); - this.assertBaggageLogs(parent1, "foo", "bar", false); + this.assertBaggageLogs(parent1, "foo", "bar", false, false); io.opentracing.Span parent2 = tracer.buildSpan("foo").startManual(); parent2.setBaggageItem("foo2", "bar"); - this.assertBaggageLogs(parent2, "foo2", "bar", false); + this.assertBaggageLogs(parent2, "foo2", "bar", false, false); io.opentracing.Span child = tracer.buildSpan("foo") .asChildOf(parent1) @@ -324,9 +369,9 @@ public void testBaggageMultipleReferences() { .startManual(); child.setBaggageItem("a", "a"); - this.assertBaggageLogs(child, "a", "a", false); + this.assertBaggageLogs(child, "a", "a", false, false); child.setBaggageItem("foo2", "b"); - this.assertBaggageLogs(child, "foo2", "b", true); + this.assertBaggageLogs(child, "foo2", "b", true, false); assertNull(parent1.getBaggageItem("a")); assertNull(parent2.getBaggageItem("a")); @@ -350,17 +395,25 @@ public void testImmutableBaggage() { assertFalse(baggageIter.hasNext()); } - private void assertBaggageLogs(io.opentracing.Span span, String key, String value, boolean override) { + private void assertBaggageLogs( + io.opentracing.Span span, + String key, + String value, + boolean override, + boolean truncate + ) { Span sp = (Span)span; List logs = sp.getLogs(); assertEquals(false, logs.isEmpty()); Map fields = logs.get(logs.size() - 1).getFields(); - assertEquals(override ? 4 : 3, fields.size()); assertEquals("baggage", fields.get("event")); assertEquals(key, fields.get("key")); assertEquals(value, fields.get("value")); if (override) { assertEquals("true", fields.get("override")); } + if (truncate) { + assertEquals("true", fields.get("truncated")); + } } } diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java new file mode 100644 index 000000000..57eb516c7 --- /dev/null +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2017, Uber Technologies, Inc + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package com.uber.jaeger.baggage; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class BaggageRestrictionManagerTest { + private static final String BAGGAGE_KEY = "key"; + private static final String BAGGAGE_VALUE = "value"; + + private DefaultBaggageRestrictionManager undertest = new DefaultBaggageRestrictionManager(); + + @Test + public void testSanitizeBaggage() { + SanitizedBaggage actual = undertest.sanitizeBaggage(BAGGAGE_KEY, BAGGAGE_VALUE); + SanitizedBaggage expected = SanitizedBaggage.of(true, BAGGAGE_VALUE, false); + assertEquals(expected, actual); + + final String value = "01234567890123456789"; + final String truncated = "0123456789"; + final int max_value_length = 10; + + actual = undertest.truncateBaggage(value, max_value_length); + expected = SanitizedBaggage.of(true, truncated, true); + assertEquals(expected, actual); + } +} From 6379066e8c9439198c5925f44cdd12a73f171c38 Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Mon, 24 Jul 2017 16:09:56 -0400 Subject: [PATCH 02/14] feature envy --- .../src/main/java/com/uber/jaeger/Span.java | 28 +------ .../baggage/BaggageRestrictionManager.java | 9 +-- .../uber/jaeger/baggage/BaggageValidity.java | 77 +++++++++++++++++++ .../DefaultBaggageRestrictionManager.java | 8 +- .../uber/jaeger/baggage/SanitizedBaggage.java | 32 -------- .../test/java/com/uber/jaeger/SpanTest.java | 4 +- .../BaggageRestrictionManagerTest.java | 23 ++---- 7 files changed, 93 insertions(+), 88 deletions(-) create mode 100644 jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageValidity.java delete mode 100644 jaeger-core/src/main/java/com/uber/jaeger/baggage/SanitizedBaggage.java diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Span.java b/jaeger-core/src/main/java/com/uber/jaeger/Span.java index 3ec452824..57ad004da 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/Span.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/Span.java @@ -22,7 +22,7 @@ package com.uber.jaeger; -import com.uber.jaeger.baggage.SanitizedBaggage; +import com.uber.jaeger.baggage.BaggageValidity; import io.opentracing.tag.Tags; import java.util.ArrayList; import java.util.Collections; @@ -121,30 +121,8 @@ public Span setBaggageItem(String key, String value) { if (key == null || value == null) { return this; } - SanitizedBaggage baggage = this.tracer.getBaggageRestrictionManager().sanitizeBaggage(key, value); - if (!baggage.isValid()) { - this.tracer.getMetrics().baggageUpdateFailure.inc(1); - return this; - } - if (baggage.isTruncated()) { - this.tracer.getMetrics().baggageTruncate.inc(1); - } - String prevItem = this.getBaggageItem(key); - this.context = this.context.withBaggageItem(key, baggage.getSanitizedValue()); - this.tracer.getMetrics().baggageUpdateSuccess.inc(1); - if (context.isSampled()) { - Map fields = new HashMap(); - fields.put("event", "baggage"); - fields.put("key", key); - fields.put("value", baggage.getSanitizedValue()); - if (prevItem != null) { - fields.put("override", "true"); - } - if (baggage.isTruncated()) { - fields.put("truncated", "true"); - } - return this.log(fields); - } + BaggageValidity baggageValidity = this.getTracer().getBaggageRestrictionManager().isBaggageValid(key, value); + this.context = baggageValidity.sanitizeBaggage(this, key, value); return this; } } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java index 4bc48f713..33b6cc0e4 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java @@ -25,12 +25,5 @@ public abstract class BaggageRestrictionManager { static final int DEFAULT_MAX_VALUE_LENGTH = 2048; - public abstract SanitizedBaggage sanitizeBaggage(String key, String value); - - SanitizedBaggage truncateBaggage(String value, int maxValueLength) { - if (value.length() > maxValueLength) { - return SanitizedBaggage.of(true, value.substring(0, maxValueLength), true); - } - return SanitizedBaggage.of(true, value, false); - } + public abstract BaggageValidity isBaggageValid(String key, String value); } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageValidity.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageValidity.java new file mode 100644 index 000000000..818d2bd8d --- /dev/null +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageValidity.java @@ -0,0 +1,77 @@ +/* + * Copyright (c) 2017, Uber Technologies, Inc + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package com.uber.jaeger.baggage; + +import com.uber.jaeger.Span; +import com.uber.jaeger.SpanContext; +import com.uber.jaeger.metrics.Metrics; +import lombok.Value; + +import java.util.HashMap; +import java.util.Map; + +@Value(staticConstructor = "of") +public class BaggageValidity { + final boolean valid; + final int maxValueLength; + + public SpanContext sanitizeBaggage(Span span, String key, String value) { + Metrics metrics = span.getTracer().getMetrics(); + if (!this.isValid()) { + metrics.baggageUpdateFailure.inc(1); + this.logFields(span, key, value, null, false, true); + return span.context(); + } + boolean truncated = false; + if (value.length() > maxValueLength) { + truncated = true; + value = value.substring(0, maxValueLength); + metrics.baggageTruncate.inc(1); + } + + String prevItem = span.getBaggageItem(key); + this.logFields(span, key, value, prevItem, truncated, false); + SpanContext context = span.context().withBaggageItem(key, value); + metrics.baggageUpdateSuccess.inc(1); + return context; + } + + void logFields(Span span, String key, String value, String prevItem, boolean truncated, boolean invalid) { + if (span.context().isSampled()) { + Map fields = new HashMap(); + fields.put("event", "baggage"); + fields.put("key", key); + fields.put("value", value); + if (prevItem != null) { + fields.put("override", "true"); + } + if (truncated) { + fields.put("truncated", "true"); + } + if (invalid) { + fields.put("invalid", "true"); + } + span.log(fields); + } + } +} diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java index fbd09d61f..45e4eb16e 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java @@ -24,18 +24,18 @@ public class DefaultBaggageRestrictionManager extends BaggageRestrictionManager { - private final int maxValueLength; + private final BaggageValidity baggageValidity; public DefaultBaggageRestrictionManager() { this(DEFAULT_MAX_VALUE_LENGTH); } public DefaultBaggageRestrictionManager(int maxValueLength) { - this.maxValueLength = maxValueLength; + baggageValidity = BaggageValidity.of(true, maxValueLength); } @Override - public SanitizedBaggage sanitizeBaggage(String key, String value) { - return truncateBaggage(value, maxValueLength); + public BaggageValidity isBaggageValid(String key, String value) { + return baggageValidity; } } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/SanitizedBaggage.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/SanitizedBaggage.java deleted file mode 100644 index 31991b70a..000000000 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/SanitizedBaggage.java +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright (c) 2017, Uber Technologies, Inc - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -package com.uber.jaeger.baggage; - -import lombok.Value; - -@Value(staticConstructor = "of") -public class SanitizedBaggage { - final boolean valid; - final String sanitizedValue; - final boolean truncated; -} diff --git a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java index 9dae33557..fb8356d23 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java @@ -30,7 +30,7 @@ import com.uber.jaeger.baggage.BaggageRestrictionManager; import com.uber.jaeger.baggage.DefaultBaggageRestrictionManager; -import com.uber.jaeger.baggage.SanitizedBaggage; +import com.uber.jaeger.baggage.BaggageValidity; import com.uber.jaeger.metrics.InMemoryStatsReporter; import com.uber.jaeger.reporters.InMemoryReporter; import com.uber.jaeger.samplers.ConstSampler; @@ -104,7 +104,7 @@ public void testInvalidBaggageKey() { .build(); span = (Span) tracer.buildSpan("some-operation").startManual(); - when(mgr.sanitizeBaggage(key, value)).thenReturn(SanitizedBaggage.of(false, null, false)); + when(mgr.isBaggageValid(key, value)).thenReturn(BaggageValidity.of(false, 0)); span.setBaggageItem(key, value); assertNull(span.getBaggageItem(key)); diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java index 57eb516c7..88ec9abc4 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java @@ -25,28 +25,17 @@ import static org.junit.Assert.assertEquals; import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; -@RunWith(MockitoJUnitRunner.class) public class BaggageRestrictionManagerTest { - private static final String BAGGAGE_KEY = "key"; - private static final String BAGGAGE_VALUE = "value"; - - private DefaultBaggageRestrictionManager undertest = new DefaultBaggageRestrictionManager(); @Test - public void testSanitizeBaggage() { - SanitizedBaggage actual = undertest.sanitizeBaggage(BAGGAGE_KEY, BAGGAGE_VALUE); - SanitizedBaggage expected = SanitizedBaggage.of(true, BAGGAGE_VALUE, false); - assertEquals(expected, actual); - - final String value = "01234567890123456789"; - final String truncated = "0123456789"; - final int max_value_length = 10; + public void testIsBaggageValid() { + final String BAGGAGE_KEY = "key"; + final String BAGGAGE_VALUE = "value"; + final DefaultBaggageRestrictionManager undertest = new DefaultBaggageRestrictionManager(); - actual = undertest.truncateBaggage(value, max_value_length); - expected = SanitizedBaggage.of(true, truncated, true); + BaggageValidity actual = undertest.isBaggageValid(BAGGAGE_KEY, BAGGAGE_VALUE); + BaggageValidity expected = BaggageValidity.of(true, 2048); assertEquals(expected, actual); } } From e29d837e8e7491cbd51dd13b262afd8d82887689 Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Mon, 24 Jul 2017 23:27:06 -0400 Subject: [PATCH 03/14] comments --- .../src/main/java/com/uber/jaeger/Span.java | 12 +- .../src/main/java/com/uber/jaeger/Tracer.java | 6 +- .../baggage/BaggageRestrictionManager.java | 2 +- ...aggageValidity.java => BaggageSetter.java} | 9 +- .../DefaultBaggageRestrictionManager.java | 17 +-- .../test/java/com/uber/jaeger/SpanTest.java | 37 +++--- .../BaggageRestrictionManagerTest.java | 13 +- .../jaeger/baggage/BaggageSetterTest.java | 116 ++++++++++++++++++ 8 files changed, 162 insertions(+), 50 deletions(-) rename jaeger-core/src/main/java/com/uber/jaeger/baggage/{BaggageValidity.java => BaggageSetter.java} (94%) create mode 100644 jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Span.java b/jaeger-core/src/main/java/com/uber/jaeger/Span.java index 57ad004da..5603586d7 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/Span.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/Span.java @@ -22,7 +22,7 @@ package com.uber.jaeger; -import com.uber.jaeger.baggage.BaggageValidity; +import com.uber.jaeger.baggage.BaggageSetter; import io.opentracing.tag.Tags; import java.util.ArrayList; import java.util.Collections; @@ -117,12 +117,12 @@ public List getLogs() { @Override public Span setBaggageItem(String key, String value) { + if (key == null || value == null) { + return this; + } + BaggageSetter baggageValidity = this.getTracer().getBaggageRestrictionManager().getBaggageSetter(key); synchronized (this) { - if (key == null || value == null) { - return this; - } - BaggageValidity baggageValidity = this.getTracer().getBaggageRestrictionManager().isBaggageValid(key, value); - this.context = baggageValidity.sanitizeBaggage(this, key, value); + this.context = baggageValidity.setBaggage(this, key, value); return this; } } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java b/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java index 0852d234f..64e3a4082 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java @@ -450,13 +450,14 @@ public static final class Builder { private final Sampler sampler; private final Reporter reporter; private final PropagationRegistry registry = new PropagationRegistry(); - private Metrics metrics; + private Metrics metrics = new Metrics(new StatsFactoryImpl(new NullStatsReporter())); private String serviceName; private Clock clock = new SystemClock(); private Map tags = new HashMap(); private boolean zipkinSharedRpcSpan; private ActiveSpanSource activeSpanSource = new ThreadLocalActiveSpanSource(); - private BaggageRestrictionManager baggageRestrictionManager = new DefaultBaggageRestrictionManager(); + private BaggageRestrictionManager baggageRestrictionManager = + new DefaultBaggageRestrictionManager(this.metrics); public Builder(String serviceName, Reporter reporter, Sampler sampler) { if (serviceName == null || serviceName.trim().length() == 0) { @@ -465,7 +466,6 @@ public Builder(String serviceName, Reporter reporter, Sampler sampler) { this.serviceName = serviceName; this.reporter = reporter; this.sampler = sampler; - this.metrics = new Metrics(new StatsFactoryImpl(new NullStatsReporter())); TextMapCodec textMapCodec = new TextMapCodec(false); this.registerInjector(Format.Builtin.TEXT_MAP, textMapCodec); diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java index 33b6cc0e4..1ed53ef34 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java @@ -25,5 +25,5 @@ public abstract class BaggageRestrictionManager { static final int DEFAULT_MAX_VALUE_LENGTH = 2048; - public abstract BaggageValidity isBaggageValid(String key, String value); + public abstract BaggageSetter getBaggageSetter(String key); } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageValidity.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java similarity index 94% rename from jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageValidity.java rename to jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java index 818d2bd8d..2191fdbf8 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageValidity.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java @@ -25,18 +25,19 @@ import com.uber.jaeger.Span; import com.uber.jaeger.SpanContext; import com.uber.jaeger.metrics.Metrics; -import lombok.Value; import java.util.HashMap; import java.util.Map; +import lombok.Value; + @Value(staticConstructor = "of") -public class BaggageValidity { +public class BaggageSetter { final boolean valid; final int maxValueLength; + final Metrics metrics; - public SpanContext sanitizeBaggage(Span span, String key, String value) { - Metrics metrics = span.getTracer().getMetrics(); + public SpanContext setBaggage(Span span, String key, String value) { if (!this.isValid()) { metrics.baggageUpdateFailure.inc(1); this.logFields(span, key, value, null, false, true); diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java index 45e4eb16e..63aa0567c 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java @@ -22,20 +22,21 @@ package com.uber.jaeger.baggage; -public class DefaultBaggageRestrictionManager extends BaggageRestrictionManager { +import com.uber.jaeger.metrics.Metrics; - private final BaggageValidity baggageValidity; +public class DefaultBaggageRestrictionManager extends BaggageRestrictionManager { + private final BaggageSetter baggageSetter; - public DefaultBaggageRestrictionManager() { - this(DEFAULT_MAX_VALUE_LENGTH); + public DefaultBaggageRestrictionManager(Metrics metrics) { + this(metrics, DEFAULT_MAX_VALUE_LENGTH); } - public DefaultBaggageRestrictionManager(int maxValueLength) { - baggageValidity = BaggageValidity.of(true, maxValueLength); + public DefaultBaggageRestrictionManager(Metrics metrics, int maxValueLength) { + baggageSetter = BaggageSetter.of(true, maxValueLength, metrics); } @Override - public BaggageValidity isBaggageValid(String key, String value) { - return baggageValidity; + public BaggageSetter getBaggageSetter(String key) { + return baggageSetter; } } diff --git a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java index fb8356d23..47e3f02bd 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java @@ -29,9 +29,11 @@ import static org.mockito.Mockito.when; import com.uber.jaeger.baggage.BaggageRestrictionManager; +import com.uber.jaeger.baggage.BaggageSetter; import com.uber.jaeger.baggage.DefaultBaggageRestrictionManager; -import com.uber.jaeger.baggage.BaggageValidity; import com.uber.jaeger.metrics.InMemoryStatsReporter; +import com.uber.jaeger.metrics.Metrics; +import com.uber.jaeger.metrics.StatsFactoryImpl; import com.uber.jaeger.reporters.InMemoryReporter; import com.uber.jaeger.samplers.ConstSampler; import com.uber.jaeger.utils.Clock; @@ -51,17 +53,19 @@ public class SpanTest { private Tracer tracer; private Span span; private InMemoryStatsReporter metricsReporter; + private Metrics metrics; @Before public void setUp() throws Exception { metricsReporter = new InMemoryStatsReporter(); reporter = new InMemoryReporter(); clock = mock(Clock.class); + metrics = new Metrics(new StatsFactoryImpl(metricsReporter)); tracer = new Tracer.Builder("SamplerTest", reporter, new ConstSampler(true)) .withStatsReporter(metricsReporter) .withClock(clock) - .withBaggageRestrictionManager(new DefaultBaggageRestrictionManager(15)) + .withBaggageRestrictionManager(new DefaultBaggageRestrictionManager(metrics, 15)) .build(); span = (Span) tracer.buildSpan("some-operation").startManual(); } @@ -78,14 +82,17 @@ public void testSpanMetrics() { @Test public void testSetAndGetBaggageItem() { - String expected = "expected"; + String value = "01234567890123456789"; + String expected = "012345678901234"; String key = "some.BAGGAGE"; - span.setBaggageItem(key, expected); + span.setBaggageItem(key, value); assertEquals(expected, span.getBaggageItem(key)); // Ensure the baggage was logged - this.assertBaggageLogs(span, key, expected, false, false); + this.assertBaggageLogs(span, key, expected, false, true); + assertEquals( + 1L, metricsReporter.counters.get("jaeger.baggage-truncate").longValue()); assertEquals( 1L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue()); } @@ -93,7 +100,6 @@ public void testSetAndGetBaggageItem() { @Test public void testInvalidBaggageKey() { String key = "some.BAGGAGE"; - String value = "luggage"; BaggageRestrictionManager mgr = mock(BaggageRestrictionManager.class); tracer = @@ -104,30 +110,15 @@ public void testInvalidBaggageKey() { .build(); span = (Span) tracer.buildSpan("some-operation").startManual(); - when(mgr.isBaggageValid(key, value)).thenReturn(BaggageValidity.of(false, 0)); + when(mgr.getBaggageSetter(key)).thenReturn(BaggageSetter.of(false, 0, metrics)); - span.setBaggageItem(key, value); + span.setBaggageItem(key, "luggage"); assertNull(span.getBaggageItem(key)); assertEquals( 1L, metricsReporter.counters.get("jaeger.baggage-update.result=err").longValue()); } - @Test - public void testTruncateBaggage() { - String value = "01234567890123456789"; - String expected = "012345678901234"; - String key = "some.BAGGAGE"; - span.setBaggageItem(key, value); - assertEquals(expected, span.getBaggageItem(key)); - - // Ensure the baggage was logged - this.assertBaggageLogs(span, key, expected, false, true); - - assertEquals( - 1L, metricsReporter.counters.get("jaeger.baggage-truncate").longValue()); - } - @Test public void testSetBooleanTag() { Boolean expected = true; diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java index 88ec9abc4..fd41cfeb8 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java @@ -24,18 +24,21 @@ import static org.junit.Assert.assertEquals; +import com.uber.jaeger.metrics.Metrics; +import com.uber.jaeger.metrics.NullStatsReporter; +import com.uber.jaeger.metrics.StatsFactoryImpl; import org.junit.Test; public class BaggageRestrictionManagerTest { + @Test public void testIsBaggageValid() { - final String BAGGAGE_KEY = "key"; - final String BAGGAGE_VALUE = "value"; - final DefaultBaggageRestrictionManager undertest = new DefaultBaggageRestrictionManager(); + final Metrics nullMetrics = new Metrics(new StatsFactoryImpl(new NullStatsReporter())); + final DefaultBaggageRestrictionManager undertest = new DefaultBaggageRestrictionManager(nullMetrics); - BaggageValidity actual = undertest.isBaggageValid(BAGGAGE_KEY, BAGGAGE_VALUE); - BaggageValidity expected = BaggageValidity.of(true, 2048); + BaggageSetter actual = undertest.getBaggageSetter("key"); + BaggageSetter expected = BaggageSetter.of(true, 2048, nullMetrics); assertEquals(expected, actual); } } diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java new file mode 100644 index 000000000..98aca1d62 --- /dev/null +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java @@ -0,0 +1,116 @@ +/* + * Copyright (c) 2017, Uber Technologies, Inc + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package com.uber.jaeger.baggage; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +import com.uber.jaeger.LogData; +import com.uber.jaeger.Span; +import com.uber.jaeger.SpanContext; +import com.uber.jaeger.Tracer; +import com.uber.jaeger.metrics.InMemoryStatsReporter; +import com.uber.jaeger.metrics.Metrics; +import com.uber.jaeger.metrics.StatsFactoryImpl; +import com.uber.jaeger.reporters.InMemoryReporter; +import com.uber.jaeger.samplers.ConstSampler; + +import java.util.List; +import java.util.Map; + +import org.junit.Before; +import org.junit.Test; + +public class BaggageSetterTest { + + private InMemoryReporter reporter; + private Tracer tracer; + private Span span; + private InMemoryStatsReporter metricsReporter; + private Metrics metrics; + + private static final String KEY = "key"; + + @Before + public void setUp() throws Exception { + metricsReporter = new InMemoryStatsReporter(); + reporter = new InMemoryReporter(); + metrics = new Metrics(new StatsFactoryImpl(metricsReporter)); + tracer = + new Tracer.Builder("SamplerTest", reporter, new ConstSampler(true)) + .withStatsReporter(metricsReporter) + .withBaggageRestrictionManager(new DefaultBaggageRestrictionManager(metrics, 15)) + .build(); + span = (Span) tracer.buildSpan("some-operation").startManual(); + } + + @Test + public void testInvalidBaggage() { + BaggageSetter setter = BaggageSetter.of(false, 0, metrics); + final String value = "value"; + SpanContext ctx = setter.setBaggage(span, KEY, value); + + assertBaggageLogs(span, KEY, value, false, true); + assertNull(ctx.getBaggageItem(KEY)); + + assertEquals( + 1L, metricsReporter.counters.get("jaeger.baggage-update.result=err").longValue()); + } + + @Test + public void testTruncatedBaggage() { + BaggageSetter setter = BaggageSetter.of(true, 5, metrics); + final String value = "0123456789"; + final String expected = "01234"; + SpanContext ctx = setter.setBaggage(span, KEY, value); + + assertBaggageLogs(span, KEY, expected, true, false); + assertEquals(expected, ctx.getBaggageItem(KEY)); + + assertEquals( + 1L, metricsReporter.counters.get("jaeger.baggage-truncate").longValue()); + assertEquals( + 1L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue()); + } + + private void assertBaggageLogs( + Span span, + String key, + String value, + boolean truncate, + boolean invalid + ) { + List logs = span.getLogs(); + assertEquals(false, logs.isEmpty()); + Map fields = logs.get(logs.size() - 1).getFields(); + assertEquals("baggage", fields.get("event")); + assertEquals(key, fields.get("key")); + assertEquals(value, fields.get("value")); + if (truncate) { + assertEquals("true", fields.get("truncated")); + } + if (invalid) { + assertEquals("true", fields.get("invalid")); + } + } +} From 11c6d86086455da815debca12669c28c47bd2c50 Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Mon, 24 Jul 2017 23:32:34 -0400 Subject: [PATCH 04/14] too many spaces --- .../com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java index fd41cfeb8..90301d208 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java @@ -31,7 +31,6 @@ public class BaggageRestrictionManagerTest { - @Test public void testIsBaggageValid() { final Metrics nullMetrics = new Metrics(new StatsFactoryImpl(new NullStatsReporter())); From f113e558e0e7922e9f85e7f6742c681ea4b055dd Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Tue, 25 Jul 2017 11:58:02 -0400 Subject: [PATCH 05/14] meh --- .../src/main/java/com/uber/jaeger/Tracer.java | 3 +++ .../test/java/com/uber/jaeger/TracerTest.java | 18 +++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java b/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java index 64e3a4082..13b3c65bc 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java @@ -508,6 +508,9 @@ public Builder withZipkinSharedRpcSpan() { public Builder withMetrics(Metrics metrics) { this.metrics = metrics; + if (this.baggageRestrictionManager instanceof DefaultBaggageRestrictionManager) { + this.baggageRestrictionManager = new DefaultBaggageRestrictionManager(this.metrics); + } return this; } diff --git a/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java b/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java index 4e3766209..3bf1cedb0 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java @@ -30,6 +30,8 @@ import static org.mockito.Mockito.verify; import com.uber.jaeger.metrics.InMemoryStatsReporter; +import com.uber.jaeger.metrics.Metrics; +import com.uber.jaeger.metrics.StatsFactoryImpl; import com.uber.jaeger.propagation.Injector; import com.uber.jaeger.reporters.InMemoryReporter; import com.uber.jaeger.reporters.Reporter; @@ -41,7 +43,6 @@ import org.junit.Before; import org.junit.Test; - public class TracerTest { Tracer tracer; @@ -120,6 +121,21 @@ public void testBuilderIsNotServerRpc() { assertFalse(spanBuilder.isRpcServer()); } + @Test + public void testWithBaggageRestrictionManager() { + metricsReporter = new InMemoryStatsReporter(); + Metrics metrics = new Metrics(new StatsFactoryImpl(metricsReporter)); + tracer = + new Tracer.Builder("TracerTestService", new InMemoryReporter(), new ConstSampler(true)) + .withMetrics(metrics) + .build(); + Span span = (Span) tracer.buildSpan("some-operation").startManual(); + tracer.getBaggageRestrictionManager().getBaggageSetter("key").setBaggage(span, "key", "value"); + + assertEquals( + 1L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue()); + } + @Test public void testClose() { Reporter reporter = mock(Reporter.class); From df9b6537fe97b075b9a6c0a7ed837b1ca76216d6 Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Tue, 25 Jul 2017 12:46:18 -0400 Subject: [PATCH 06/14] wow --- jaeger-core/src/main/java/com/uber/jaeger/Tracer.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java b/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java index 13b3c65bc..17063b203 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java @@ -456,8 +456,7 @@ public static final class Builder { private Map tags = new HashMap(); private boolean zipkinSharedRpcSpan; private ActiveSpanSource activeSpanSource = new ThreadLocalActiveSpanSource(); - private BaggageRestrictionManager baggageRestrictionManager = - new DefaultBaggageRestrictionManager(this.metrics); + private BaggageRestrictionManager baggageRestrictionManager; public Builder(String serviceName, Reporter reporter, Sampler sampler) { if (serviceName == null || serviceName.trim().length() == 0) { @@ -508,9 +507,6 @@ public Builder withZipkinSharedRpcSpan() { public Builder withMetrics(Metrics metrics) { this.metrics = metrics; - if (this.baggageRestrictionManager instanceof DefaultBaggageRestrictionManager) { - this.baggageRestrictionManager = new DefaultBaggageRestrictionManager(this.metrics); - } return this; } @@ -542,6 +538,9 @@ public Builder withBaggageRestrictionManager(BaggageRestrictionManager baggageRe } public Tracer build() { + if (baggageRestrictionManager == null) { + baggageRestrictionManager = new DefaultBaggageRestrictionManager(this.metrics); + } return new Tracer(this.serviceName, reporter, sampler, registry, clock, metrics, tags, zipkinSharedRpcSpan, activeSpanSource, baggageRestrictionManager); } From 4af5607b1f473a6565e7cc0042dfc7b0b8dda746 Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Wed, 26 Jul 2017 10:35:54 -0400 Subject: [PATCH 07/14] fix variable name --- jaeger-core/src/main/java/com/uber/jaeger/Span.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Span.java b/jaeger-core/src/main/java/com/uber/jaeger/Span.java index 5603586d7..41b867ae8 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/Span.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/Span.java @@ -120,9 +120,9 @@ public Span setBaggageItem(String key, String value) { if (key == null || value == null) { return this; } - BaggageSetter baggageValidity = this.getTracer().getBaggageRestrictionManager().getBaggageSetter(key); + BaggageSetter setter = this.getTracer().getBaggageRestrictionManager().getBaggageSetter(key); synchronized (this) { - this.context = baggageValidity.setBaggage(this, key, value); + this.context = setter.setBaggage(this, key, value); return this; } } From b644bb6c29562d451e33c460031e945e604fb05c Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Wed, 26 Jul 2017 11:04:21 -0400 Subject: [PATCH 08/14] private --- .../src/main/java/com/uber/jaeger/baggage/BaggageSetter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java index 2191fdbf8..898dcd808 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java @@ -57,7 +57,7 @@ public SpanContext setBaggage(Span span, String key, String value) { return context; } - void logFields(Span span, String key, String value, String prevItem, boolean truncated, boolean invalid) { + private void logFields(Span span, String key, String value, String prevItem, boolean truncated, boolean invalid) { if (span.context().isSampled()) { Map fields = new HashMap(); fields.put("event", "baggage"); From 72c1b849a2e54cb2370abccdb70de572c69adfd8 Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Wed, 26 Jul 2017 14:28:26 -0400 Subject: [PATCH 09/14] oops --- .../baggage/BaggageRestrictionManager.java | 6 ++ .../uber/jaeger/baggage/BaggageSetter.java | 24 ++++++- .../test/java/com/uber/jaeger/SpanTest.java | 68 +++---------------- .../jaeger/baggage/BaggageSetterTest.java | 23 ++++++- 4 files changed, 59 insertions(+), 62 deletions(-) diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java index 1ed53ef34..46f60d492 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java @@ -22,6 +22,12 @@ package com.uber.jaeger.baggage; +/** + * BaggageRestrictionManager is an abstract class that manages baggage + * restrictions for baggage keys. The manager will return a {@link BaggageSetter} + * for a specific baggage key which will set the baggage on the {@link com.uber.jaeger.Span} + * given the baggage restrictions for that key. + */ public abstract class BaggageRestrictionManager { static final int DEFAULT_MAX_VALUE_LENGTH = 2048; diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java index 898dcd808..eaeb73422 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java @@ -31,16 +31,34 @@ import lombok.Value; +/** + * BaggageSetter is a class that sets baggage and the logs associated + * with the baggage on a given {@link Span}. + */ @Value(staticConstructor = "of") public class BaggageSetter { final boolean valid; final int maxValueLength; final Metrics metrics; + /** + * Sets the baggage key:value on the {@link Span} and the corresponding + * logs. Whether the baggage is set on the span depends on if the key + * is valid. + *

+ * A {@link SpanContext} is returned with the new baggage key:value set + * if key is valid, else returns the existing {@link SpanContext} + * on the {@link Span}. + * + * @param span the span to set the baggage on + * @param key the baggage key to set + * @param value the baggage value to set + * @return the SpanContext with the baggage set + */ public SpanContext setBaggage(Span span, String key, String value) { - if (!this.isValid()) { + if (!valid) { metrics.baggageUpdateFailure.inc(1); - this.logFields(span, key, value, null, false, true); + logFields(span, key, value, null, false, true); return span.context(); } boolean truncated = false; @@ -51,7 +69,7 @@ public SpanContext setBaggage(Span span, String key, String value) { } String prevItem = span.getBaggageItem(key); - this.logFields(span, key, value, prevItem, truncated, false); + logFields(span, key, value, prevItem, truncated, false); SpanContext context = span.context().withBaggageItem(key, value); metrics.baggageUpdateSuccess.inc(1); return context; diff --git a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java index 47e3f02bd..4b89e7b20 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.uber.jaeger.baggage.BaggageRestrictionManager; @@ -46,6 +47,8 @@ import java.util.Map.Entry; import org.junit.Before; import org.junit.Test; +import org.mockito.Mock; +import org.mockito.Mockito; public class SpanTest { private Clock clock; @@ -65,7 +68,7 @@ public void setUp() throws Exception { new Tracer.Builder("SamplerTest", reporter, new ConstSampler(true)) .withStatsReporter(metricsReporter) .withClock(clock) - .withBaggageRestrictionManager(new DefaultBaggageRestrictionManager(metrics, 15)) + .withBaggageRestrictionManager(new DefaultBaggageRestrictionManager(metrics)) .build(); span = (Span) tracer.buildSpan("some-operation").startManual(); } @@ -82,41 +85,20 @@ public void testSpanMetrics() { @Test public void testSetAndGetBaggageItem() { - String value = "01234567890123456789"; - String expected = "012345678901234"; - String key = "some.BAGGAGE"; - span.setBaggageItem(key, value); - assertEquals(expected, span.getBaggageItem(key)); - - // Ensure the baggage was logged - this.assertBaggageLogs(span, key, expected, false, true); - - assertEquals( - 1L, metricsReporter.counters.get("jaeger.baggage-truncate").longValue()); - assertEquals( - 1L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue()); - } - - @Test - public void testInvalidBaggageKey() { - String key = "some.BAGGAGE"; - BaggageRestrictionManager mgr = mock(BaggageRestrictionManager.class); - + final BaggageRestrictionManager mgr = Mockito.mock(DefaultBaggageRestrictionManager.class); tracer = new Tracer.Builder("SamplerTest", reporter, new ConstSampler(true)) - .withStatsReporter(metricsReporter) .withClock(clock) .withBaggageRestrictionManager(mgr) .build(); span = (Span) tracer.buildSpan("some-operation").startManual(); - when(mgr.getBaggageSetter(key)).thenReturn(BaggageSetter.of(false, 0, metrics)); - - span.setBaggageItem(key, "luggage"); - assertNull(span.getBaggageItem(key)); - - assertEquals( - 1L, metricsReporter.counters.get("jaeger.baggage-update.result=err").longValue()); + final String key = "key"; + final String value = "value"; + when(mgr.getBaggageSetter(key)).thenReturn(BaggageSetter.of(true, 10, metrics)); + span.setBaggageItem(key, "value"); + verify(mgr).getBaggageSetter(key); + assertEquals(value, span.getBaggageItem(key)); } @Test @@ -331,14 +313,12 @@ public void testSpanDetectsSamplingPriorityLessThanZero() { public void testBaggageOneReference() { io.opentracing.Span parent = tracer.buildSpan("foo").startManual(); parent.setBaggageItem("foo", "bar"); - this.assertBaggageLogs(parent, "foo", "bar", false, false); io.opentracing.Span child = tracer.buildSpan("foo") .asChildOf(parent) .startManual(); child.setBaggageItem("a", "a"); - this.assertBaggageLogs(child, "a", "a", false, false); assertNull(parent.getBaggageItem("a")); assertEquals("a", child.getBaggageItem("a")); @@ -349,10 +329,8 @@ public void testBaggageOneReference() { public void testBaggageMultipleReferences() { io.opentracing.Span parent1 = tracer.buildSpan("foo").startManual(); parent1.setBaggageItem("foo", "bar"); - this.assertBaggageLogs(parent1, "foo", "bar", false, false); io.opentracing.Span parent2 = tracer.buildSpan("foo").startManual(); parent2.setBaggageItem("foo2", "bar"); - this.assertBaggageLogs(parent2, "foo2", "bar", false, false); io.opentracing.Span child = tracer.buildSpan("foo") .asChildOf(parent1) @@ -360,9 +338,7 @@ public void testBaggageMultipleReferences() { .startManual(); child.setBaggageItem("a", "a"); - this.assertBaggageLogs(child, "a", "a", false, false); child.setBaggageItem("foo2", "b"); - this.assertBaggageLogs(child, "foo2", "b", true, false); assertNull(parent1.getBaggageItem("a")); assertNull(parent2.getBaggageItem("a")); @@ -385,26 +361,4 @@ public void testImmutableBaggage() { baggageIter.next(); assertFalse(baggageIter.hasNext()); } - - private void assertBaggageLogs( - io.opentracing.Span span, - String key, - String value, - boolean override, - boolean truncate - ) { - Span sp = (Span)span; - List logs = sp.getLogs(); - assertEquals(false, logs.isEmpty()); - Map fields = logs.get(logs.size() - 1).getFields(); - assertEquals("baggage", fields.get("event")); - assertEquals(key, fields.get("key")); - assertEquals(value, fields.get("value")); - if (override) { - assertEquals("true", fields.get("override")); - } - if (truncate) { - assertEquals("true", fields.get("truncated")); - } - } } diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java index 98aca1d62..067efb5f5 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java @@ -70,7 +70,7 @@ public void testInvalidBaggage() { final String value = "value"; SpanContext ctx = setter.setBaggage(span, KEY, value); - assertBaggageLogs(span, KEY, value, false, true); + assertBaggageLogs(span, KEY, value, false, false, true); assertNull(ctx.getBaggageItem(KEY)); assertEquals( @@ -84,7 +84,7 @@ public void testTruncatedBaggage() { final String expected = "01234"; SpanContext ctx = setter.setBaggage(span, KEY, value); - assertBaggageLogs(span, KEY, expected, true, false); + assertBaggageLogs(span, KEY, expected, true, false, false); assertEquals(expected, ctx.getBaggageItem(KEY)); assertEquals( @@ -93,11 +93,27 @@ public void testTruncatedBaggage() { 1L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue()); } + @Test + public void testOverrideBaggage() { + BaggageSetter setter = BaggageSetter.of(true, 5, metrics); + final String value = "value"; + SpanContext ctx = setter.setBaggage(span, KEY, value); + Span child = (Span) tracer.buildSpan("some-operation").asChildOf(ctx).startManual(); + ctx = setter.setBaggage(child, KEY, value); + + assertBaggageLogs(child, KEY, value, false, true, false); + assertEquals(value, ctx.getBaggageItem(KEY)); + + assertEquals( + 2L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue()); + } + private void assertBaggageLogs( Span span, String key, String value, boolean truncate, + boolean override, boolean invalid ) { List logs = span.getLogs(); @@ -109,6 +125,9 @@ private void assertBaggageLogs( if (truncate) { assertEquals("true", fields.get("truncated")); } + if (override) { + assertEquals("true", fields.get("override")); + } if (invalid) { assertEquals("true", fields.get("invalid")); } From b24d7de3fe487cc4e4dffe99a0d011dc8e09c271 Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Thu, 27 Jul 2017 13:47:14 -0400 Subject: [PATCH 10/14] make setter more reasonable --- .../src/main/java/com/uber/jaeger/Span.java | 2 +- .../uber/jaeger/baggage/BaggageSetter.java | 27 +++++++++++-------- .../DefaultBaggageRestrictionManager.java | 18 ++++++++++--- .../test/java/com/uber/jaeger/SpanTest.java | 2 +- .../test/java/com/uber/jaeger/TracerTest.java | 2 +- .../BaggageRestrictionManagerTest.java | 12 ++++++--- .../jaeger/baggage/BaggageSetterTest.java | 14 +++++----- 7 files changed, 50 insertions(+), 27 deletions(-) diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Span.java b/jaeger-core/src/main/java/com/uber/jaeger/Span.java index 41b867ae8..03133f7e5 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/Span.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/Span.java @@ -122,7 +122,7 @@ public Span setBaggageItem(String key, String value) { } BaggageSetter setter = this.getTracer().getBaggageRestrictionManager().getBaggageSetter(key); synchronized (this) { - this.context = setter.setBaggage(this, key, value); + this.context = setter.setBaggage(this, value); return this; } } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java index eaeb73422..c387b8baf 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java @@ -37,9 +37,15 @@ */ @Value(staticConstructor = "of") public class BaggageSetter { - final boolean valid; - final int maxValueLength; - final Metrics metrics; + + private final String key; + /** + * This flag represents whether the key is a valid baggage key. If valid + * the baggage key:value will be written to the span. + */ + private final boolean valid; + private final int maxValueLength; + private final Metrics metrics; /** * Sets the baggage key:value on the {@link Span} and the corresponding @@ -51,31 +57,30 @@ public class BaggageSetter { * on the {@link Span}. * * @param span the span to set the baggage on - * @param key the baggage key to set * @param value the baggage value to set * @return the SpanContext with the baggage set */ - public SpanContext setBaggage(Span span, String key, String value) { + public SpanContext setBaggage(Span span, String value) { + boolean truncated = false; + String prevItem = span.getBaggageItem(key); if (!valid) { metrics.baggageUpdateFailure.inc(1); - logFields(span, key, value, null, false, true); + logFields(span, value, prevItem, truncated); return span.context(); } - boolean truncated = false; if (value.length() > maxValueLength) { truncated = true; value = value.substring(0, maxValueLength); metrics.baggageTruncate.inc(1); } - String prevItem = span.getBaggageItem(key); - logFields(span, key, value, prevItem, truncated, false); + logFields(span, value, prevItem, truncated); SpanContext context = span.context().withBaggageItem(key, value); metrics.baggageUpdateSuccess.inc(1); return context; } - private void logFields(Span span, String key, String value, String prevItem, boolean truncated, boolean invalid) { + private void logFields(Span span, String value, String prevItem, boolean truncated) { if (span.context().isSampled()) { Map fields = new HashMap(); fields.put("event", "baggage"); @@ -87,7 +92,7 @@ private void logFields(Span span, String key, String value, String prevItem, boo if (truncated) { fields.put("truncated", "true"); } - if (invalid) { + if (!valid) { fields.put("invalid", "true"); } span.log(fields); diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java index 63aa0567c..1bb8849d7 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java @@ -24,19 +24,31 @@ import com.uber.jaeger.metrics.Metrics; +import java.util.HashMap; +import java.util.Map; + public class DefaultBaggageRestrictionManager extends BaggageRestrictionManager { - private final BaggageSetter baggageSetter; + private Map baggageSetters; + private final int maxValueLength; + private final Metrics metrics; public DefaultBaggageRestrictionManager(Metrics metrics) { this(metrics, DEFAULT_MAX_VALUE_LENGTH); } public DefaultBaggageRestrictionManager(Metrics metrics, int maxValueLength) { - baggageSetter = BaggageSetter.of(true, maxValueLength, metrics); + this.maxValueLength = maxValueLength; + this.metrics = metrics; + this.baggageSetters = new HashMap(); } @Override public BaggageSetter getBaggageSetter(String key) { - return baggageSetter; + BaggageSetter setter = baggageSetters.get(key); + if (setter == null) { + setter = BaggageSetter.of(key, true, maxValueLength, metrics); + baggageSetters.put(key, setter); + } + return setter; } } diff --git a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java index 4b89e7b20..5a9d50203 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java @@ -95,7 +95,7 @@ public void testSetAndGetBaggageItem() { final String key = "key"; final String value = "value"; - when(mgr.getBaggageSetter(key)).thenReturn(BaggageSetter.of(true, 10, metrics)); + when(mgr.getBaggageSetter(key)).thenReturn(BaggageSetter.of(key, true, 10, metrics)); span.setBaggageItem(key, "value"); verify(mgr).getBaggageSetter(key); assertEquals(value, span.getBaggageItem(key)); diff --git a/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java b/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java index 3bf1cedb0..56ac891fb 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java @@ -130,7 +130,7 @@ public void testWithBaggageRestrictionManager() { .withMetrics(metrics) .build(); Span span = (Span) tracer.buildSpan("some-operation").startManual(); - tracer.getBaggageRestrictionManager().getBaggageSetter("key").setBaggage(span, "key", "value"); + tracer.getBaggageRestrictionManager().getBaggageSetter("key").setBaggage(span, "value"); assertEquals( 1L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue()); diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java index 90301d208..033a33114 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java @@ -23,6 +23,7 @@ package com.uber.jaeger.baggage; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; import com.uber.jaeger.metrics.Metrics; import com.uber.jaeger.metrics.NullStatsReporter; @@ -32,12 +33,17 @@ public class BaggageRestrictionManagerTest { @Test - public void testIsBaggageValid() { + public void testGetBaggageSetter() { final Metrics nullMetrics = new Metrics(new StatsFactoryImpl(new NullStatsReporter())); final DefaultBaggageRestrictionManager undertest = new DefaultBaggageRestrictionManager(nullMetrics); - BaggageSetter actual = undertest.getBaggageSetter("key"); - BaggageSetter expected = BaggageSetter.of(true, 2048, nullMetrics); + final String key = "key"; + BaggageSetter actual = undertest.getBaggageSetter(key); + BaggageSetter expected = BaggageSetter.of(key, true, 2048, nullMetrics); assertEquals(expected, actual); + + expected = actual; + actual = undertest.getBaggageSetter(key); + assertSame(actual, expected); } } diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java index 067efb5f5..e39e591c3 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java @@ -66,9 +66,9 @@ public void setUp() throws Exception { @Test public void testInvalidBaggage() { - BaggageSetter setter = BaggageSetter.of(false, 0, metrics); + BaggageSetter setter = BaggageSetter.of(KEY, false, 0, metrics); final String value = "value"; - SpanContext ctx = setter.setBaggage(span, KEY, value); + SpanContext ctx = setter.setBaggage(span, value); assertBaggageLogs(span, KEY, value, false, false, true); assertNull(ctx.getBaggageItem(KEY)); @@ -79,10 +79,10 @@ public void testInvalidBaggage() { @Test public void testTruncatedBaggage() { - BaggageSetter setter = BaggageSetter.of(true, 5, metrics); + BaggageSetter setter = BaggageSetter.of(KEY, true, 5, metrics); final String value = "0123456789"; final String expected = "01234"; - SpanContext ctx = setter.setBaggage(span, KEY, value); + SpanContext ctx = setter.setBaggage(span, value); assertBaggageLogs(span, KEY, expected, true, false, false); assertEquals(expected, ctx.getBaggageItem(KEY)); @@ -95,11 +95,11 @@ public void testTruncatedBaggage() { @Test public void testOverrideBaggage() { - BaggageSetter setter = BaggageSetter.of(true, 5, metrics); + BaggageSetter setter = BaggageSetter.of(KEY, true, 5, metrics); final String value = "value"; - SpanContext ctx = setter.setBaggage(span, KEY, value); + SpanContext ctx = setter.setBaggage(span, value); Span child = (Span) tracer.buildSpan("some-operation").asChildOf(ctx).startManual(); - ctx = setter.setBaggage(child, KEY, value); + ctx = setter.setBaggage(child, value); assertBaggageLogs(child, KEY, value, false, true, false); assertEquals(value, ctx.getBaggageItem(KEY)); From 4d5d98b5ab6ef71b306098d6309423dba9df984b Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Thu, 27 Jul 2017 15:23:08 -0400 Subject: [PATCH 11/14] lil clean up --- .../baggage/BaggageRestrictionManager.java | 4 +-- .../uber/jaeger/baggage/BaggageSetter.java | 34 +++++++++---------- .../DefaultBaggageRestrictionManager.java | 2 +- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java index 46f60d492..78c5d59ac 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java @@ -23,12 +23,12 @@ package com.uber.jaeger.baggage; /** - * BaggageRestrictionManager is an abstract class that manages baggage + * BaggageRestrictionManager is an interface for a class that manages baggage * restrictions for baggage keys. The manager will return a {@link BaggageSetter} * for a specific baggage key which will set the baggage on the {@link com.uber.jaeger.Span} * given the baggage restrictions for that key. */ -public abstract class BaggageRestrictionManager { +public interface BaggageRestrictionManager { static final int DEFAULT_MAX_VALUE_LENGTH = 2048; public abstract BaggageSetter getBaggageSetter(String key); diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java index c387b8baf..19b918750 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java @@ -75,27 +75,27 @@ public SpanContext setBaggage(Span span, String value) { } logFields(span, value, prevItem, truncated); - SpanContext context = span.context().withBaggageItem(key, value); metrics.baggageUpdateSuccess.inc(1); - return context; + return span.context().withBaggageItem(key, value); } private void logFields(Span span, String value, String prevItem, boolean truncated) { - if (span.context().isSampled()) { - Map fields = new HashMap(); - fields.put("event", "baggage"); - fields.put("key", key); - fields.put("value", value); - if (prevItem != null) { - fields.put("override", "true"); - } - if (truncated) { - fields.put("truncated", "true"); - } - if (!valid) { - fields.put("invalid", "true"); - } - span.log(fields); + if (!span.context().isSampled()) { + return; } + Map fields = new HashMap(); + fields.put("event", "baggage"); + fields.put("key", key); + fields.put("value", value); + if (prevItem != null) { + fields.put("override", "true"); + } + if (truncated) { + fields.put("truncated", "true"); + } + if (!valid) { + fields.put("invalid", "true"); + } + span.log(fields); } } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java index 1bb8849d7..9aa7d0224 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java @@ -27,7 +27,7 @@ import java.util.HashMap; import java.util.Map; -public class DefaultBaggageRestrictionManager extends BaggageRestrictionManager { +public class DefaultBaggageRestrictionManager implements BaggageRestrictionManager { private Map baggageSetters; private final int maxValueLength; private final Metrics metrics; From 495a6565f08966d2c091bbdf98b1ec373494a12b Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Fri, 28 Jul 2017 10:47:09 -0400 Subject: [PATCH 12/14] revert some of the previous changes that added the key directly to the SpanSetter since this wont be able to scale with the number of random baggage people can set --- .../src/main/java/com/uber/jaeger/Span.java | 2 +- .../baggage/BaggageRestrictionManager.java | 4 ++-- .../uber/jaeger/baggage/BaggageSetter.java | 9 ++++---- .../DefaultBaggageRestrictionManager.java | 22 ++++++------------- .../test/java/com/uber/jaeger/SpanTest.java | 2 +- .../test/java/com/uber/jaeger/TracerTest.java | 3 ++- .../BaggageRestrictionManagerTest.java | 2 +- .../jaeger/baggage/BaggageSetterTest.java | 14 ++++++------ 8 files changed, 25 insertions(+), 33 deletions(-) diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Span.java b/jaeger-core/src/main/java/com/uber/jaeger/Span.java index 03133f7e5..41b867ae8 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/Span.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/Span.java @@ -122,7 +122,7 @@ public Span setBaggageItem(String key, String value) { } BaggageSetter setter = this.getTracer().getBaggageRestrictionManager().getBaggageSetter(key); synchronized (this) { - this.context = setter.setBaggage(this, value); + this.context = setter.setBaggage(this, key, value); return this; } } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java index 78c5d59ac..1068a84a3 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java @@ -29,7 +29,7 @@ * given the baggage restrictions for that key. */ public interface BaggageRestrictionManager { - static final int DEFAULT_MAX_VALUE_LENGTH = 2048; + int DEFAULT_MAX_VALUE_LENGTH = 2048; - public abstract BaggageSetter getBaggageSetter(String key); + BaggageSetter getBaggageSetter(String key); } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java index 19b918750..01b04132e 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java @@ -38,7 +38,6 @@ @Value(staticConstructor = "of") public class BaggageSetter { - private final String key; /** * This flag represents whether the key is a valid baggage key. If valid * the baggage key:value will be written to the span. @@ -60,12 +59,12 @@ public class BaggageSetter { * @param value the baggage value to set * @return the SpanContext with the baggage set */ - public SpanContext setBaggage(Span span, String value) { + public SpanContext setBaggage(Span span, String key, String value) { boolean truncated = false; String prevItem = span.getBaggageItem(key); if (!valid) { metrics.baggageUpdateFailure.inc(1); - logFields(span, value, prevItem, truncated); + logFields(span, key, value, prevItem, truncated); return span.context(); } if (value.length() > maxValueLength) { @@ -74,12 +73,12 @@ public SpanContext setBaggage(Span span, String value) { metrics.baggageTruncate.inc(1); } - logFields(span, value, prevItem, truncated); + logFields(span, key, value, prevItem, truncated); metrics.baggageUpdateSuccess.inc(1); return span.context().withBaggageItem(key, value); } - private void logFields(Span span, String value, String prevItem, boolean truncated) { + private void logFields(Span span, String key, String value, String prevItem, boolean truncated) { if (!span.context().isSampled()) { return; } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java index 9aa7d0224..af6ae3cfc 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java @@ -24,31 +24,23 @@ import com.uber.jaeger.metrics.Metrics; -import java.util.HashMap; -import java.util.Map; - +/** + * DefaultBaggageRestrictionManager is a manager that returns a {@link BaggageSetter} + * that allows all baggage. + */ public class DefaultBaggageRestrictionManager implements BaggageRestrictionManager { - private Map baggageSetters; - private final int maxValueLength; - private final Metrics metrics; + private BaggageSetter setter; public DefaultBaggageRestrictionManager(Metrics metrics) { this(metrics, DEFAULT_MAX_VALUE_LENGTH); } - public DefaultBaggageRestrictionManager(Metrics metrics, int maxValueLength) { - this.maxValueLength = maxValueLength; - this.metrics = metrics; - this.baggageSetters = new HashMap(); + DefaultBaggageRestrictionManager(Metrics metrics, int maxValueLength) { + this.setter = BaggageSetter.of(true, maxValueLength, metrics); } @Override public BaggageSetter getBaggageSetter(String key) { - BaggageSetter setter = baggageSetters.get(key); - if (setter == null) { - setter = BaggageSetter.of(key, true, maxValueLength, metrics); - baggageSetters.put(key, setter); - } return setter; } } diff --git a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java index 5a9d50203..4b89e7b20 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java @@ -95,7 +95,7 @@ public void testSetAndGetBaggageItem() { final String key = "key"; final String value = "value"; - when(mgr.getBaggageSetter(key)).thenReturn(BaggageSetter.of(key, true, 10, metrics)); + when(mgr.getBaggageSetter(key)).thenReturn(BaggageSetter.of(true, 10, metrics)); span.setBaggageItem(key, "value"); verify(mgr).getBaggageSetter(key); assertEquals(value, span.getBaggageItem(key)); diff --git a/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java b/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java index 56ac891fb..5c759af27 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java @@ -130,7 +130,8 @@ public void testWithBaggageRestrictionManager() { .withMetrics(metrics) .build(); Span span = (Span) tracer.buildSpan("some-operation").startManual(); - tracer.getBaggageRestrictionManager().getBaggageSetter("key").setBaggage(span, "value"); + final String key = "key"; + tracer.getBaggageRestrictionManager().getBaggageSetter(key).setBaggage(span, key, "value"); assertEquals( 1L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue()); diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java index 033a33114..91f069acb 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java @@ -39,7 +39,7 @@ public void testGetBaggageSetter() { final String key = "key"; BaggageSetter actual = undertest.getBaggageSetter(key); - BaggageSetter expected = BaggageSetter.of(key, true, 2048, nullMetrics); + BaggageSetter expected = BaggageSetter.of(true, 2048, nullMetrics); assertEquals(expected, actual); expected = actual; diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java index e39e591c3..067efb5f5 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java @@ -66,9 +66,9 @@ public void setUp() throws Exception { @Test public void testInvalidBaggage() { - BaggageSetter setter = BaggageSetter.of(KEY, false, 0, metrics); + BaggageSetter setter = BaggageSetter.of(false, 0, metrics); final String value = "value"; - SpanContext ctx = setter.setBaggage(span, value); + SpanContext ctx = setter.setBaggage(span, KEY, value); assertBaggageLogs(span, KEY, value, false, false, true); assertNull(ctx.getBaggageItem(KEY)); @@ -79,10 +79,10 @@ public void testInvalidBaggage() { @Test public void testTruncatedBaggage() { - BaggageSetter setter = BaggageSetter.of(KEY, true, 5, metrics); + BaggageSetter setter = BaggageSetter.of(true, 5, metrics); final String value = "0123456789"; final String expected = "01234"; - SpanContext ctx = setter.setBaggage(span, value); + SpanContext ctx = setter.setBaggage(span, KEY, value); assertBaggageLogs(span, KEY, expected, true, false, false); assertEquals(expected, ctx.getBaggageItem(KEY)); @@ -95,11 +95,11 @@ public void testTruncatedBaggage() { @Test public void testOverrideBaggage() { - BaggageSetter setter = BaggageSetter.of(KEY, true, 5, metrics); + BaggageSetter setter = BaggageSetter.of(true, 5, metrics); final String value = "value"; - SpanContext ctx = setter.setBaggage(span, value); + SpanContext ctx = setter.setBaggage(span, KEY, value); Span child = (Span) tracer.buildSpan("some-operation").asChildOf(ctx).startManual(); - ctx = setter.setBaggage(child, value); + ctx = setter.setBaggage(child, KEY, value); assertBaggageLogs(child, KEY, value, false, true, false); assertEquals(value, ctx.getBaggageItem(KEY)); From 72cbdc5e3e14bcea2672846c8b41fc989cdb32ec Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Tue, 8 Aug 2017 17:13:10 -0400 Subject: [PATCH 13/14] new API --- .../src/main/java/com/uber/jaeger/Span.java | 3 +- .../src/main/java/com/uber/jaeger/Tracer.java | 17 ++++----- .../baggage/BaggageRestrictionManager.java | 8 ++--- .../uber/jaeger/baggage/BaggageSetter.java | 33 +++++++++-------- .../DefaultBaggageRestrictionManager.java | 18 +++++----- .../com/uber/jaeger/baggage/Restriction.java | 35 +++++++++++++++++++ .../test/java/com/uber/jaeger/SpanTest.java | 7 ++-- .../test/java/com/uber/jaeger/TracerTest.java | 2 +- .../jaeger/baggage/BaggageSetterTest.java | 31 +++++++++++++--- ...DefaultBaggageRestrictionManagerTest.java} | 13 ++++--- 10 files changed, 109 insertions(+), 58 deletions(-) create mode 100644 jaeger-core/src/main/java/com/uber/jaeger/baggage/Restriction.java rename jaeger-core/src/test/java/com/uber/jaeger/baggage/{BaggageRestrictionManagerTest.java => DefaultBaggageRestrictionManagerTest.java} (80%) diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Span.java b/jaeger-core/src/main/java/com/uber/jaeger/Span.java index 41b867ae8..6f2c8d5d3 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/Span.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/Span.java @@ -120,9 +120,8 @@ public Span setBaggageItem(String key, String value) { if (key == null || value == null) { return this; } - BaggageSetter setter = this.getTracer().getBaggageRestrictionManager().getBaggageSetter(key); synchronized (this) { - this.context = setter.setBaggage(this, key, value); + this.context = getTracer().setBaggage(this, key, value); return this; } } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java b/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java index 17063b203..8022cc1db 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/Tracer.java @@ -23,6 +23,7 @@ package com.uber.jaeger; import com.uber.jaeger.baggage.BaggageRestrictionManager; +import com.uber.jaeger.baggage.BaggageSetter; import com.uber.jaeger.baggage.DefaultBaggageRestrictionManager; import com.uber.jaeger.exceptions.UnsupportedFormatException; import com.uber.jaeger.metrics.Metrics; @@ -75,7 +76,7 @@ public class Tracer implements io.opentracing.Tracer { private final Map tags; private final boolean zipkinSharedRpcSpan; private final ActiveSpanSource activeSpanSource; - private final BaggageRestrictionManager baggageRestrictionManager; + private final BaggageSetter baggageSetter; private Tracer( String serviceName, @@ -96,7 +97,7 @@ private Tracer( this.metrics = metrics; this.zipkinSharedRpcSpan = zipkinSharedRpcSpan; this.activeSpanSource = activeSpanSource; - this.baggageRestrictionManager = baggageRestrictionManager; + this.baggageSetter = new BaggageSetter(baggageRestrictionManager, metrics); this.version = loadVersion(); @@ -144,10 +145,6 @@ Reporter getReporter() { return reporter; } - BaggageRestrictionManager getBaggageRestrictionManager() { - return baggageRestrictionManager; - } - void reportSpan(Span span) { reporter.report(span); metrics.spansFinished.inc(1); @@ -456,7 +453,7 @@ public static final class Builder { private Map tags = new HashMap(); private boolean zipkinSharedRpcSpan; private ActiveSpanSource activeSpanSource = new ThreadLocalActiveSpanSource(); - private BaggageRestrictionManager baggageRestrictionManager; + private BaggageRestrictionManager baggageRestrictionManager = new DefaultBaggageRestrictionManager(); public Builder(String serviceName, Reporter reporter, Sampler sampler) { if (serviceName == null || serviceName.trim().length() == 0) { @@ -538,9 +535,6 @@ public Builder withBaggageRestrictionManager(BaggageRestrictionManager baggageRe } public Tracer build() { - if (baggageRestrictionManager == null) { - baggageRestrictionManager = new DefaultBaggageRestrictionManager(this.metrics); - } return new Tracer(this.serviceName, reporter, sampler, registry, clock, metrics, tags, zipkinSharedRpcSpan, activeSpanSource, baggageRestrictionManager); } @@ -598,4 +592,7 @@ String getHostName() { } } + SpanContext setBaggage(Span span, String key, String value) { + return baggageSetter.setBaggage(span, key, value); + } } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java index 1068a84a3..e52ab98cb 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java @@ -24,12 +24,12 @@ /** * BaggageRestrictionManager is an interface for a class that manages baggage - * restrictions for baggage keys. The manager will return a {@link BaggageSetter} - * for a specific baggage key which will set the baggage on the {@link com.uber.jaeger.Span} - * given the baggage restrictions for that key. + * restrictions for baggage keys. The manager will return a {@link Restriction} + * for a specific baggage key which will determine whether the baggage key is + * allowed and any other applicable restrictions on the baggage value. */ public interface BaggageRestrictionManager { int DEFAULT_MAX_VALUE_LENGTH = 2048; - BaggageSetter getBaggageSetter(String key); + Restriction getRestriction(String key); } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java index 01b04132e..343e0f3fb 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java @@ -24,28 +24,26 @@ import com.uber.jaeger.Span; import com.uber.jaeger.SpanContext; +import com.uber.jaeger.metrics.Metric; import com.uber.jaeger.metrics.Metrics; import java.util.HashMap; import java.util.Map; -import lombok.Value; - /** * BaggageSetter is a class that sets baggage and the logs associated * with the baggage on a given {@link Span}. */ -@Value(staticConstructor = "of") public class BaggageSetter { - /** - * This flag represents whether the key is a valid baggage key. If valid - * the baggage key:value will be written to the span. - */ - private final boolean valid; - private final int maxValueLength; + private final BaggageRestrictionManager restrictionManager; private final Metrics metrics; + public BaggageSetter(BaggageRestrictionManager restrictionManager, Metrics metrics) { + this.restrictionManager = restrictionManager; + this.metrics = metrics; + } + /** * Sets the baggage key:value on the {@link Span} and the corresponding * logs. Whether the baggage is set on the span depends on if the key @@ -60,25 +58,26 @@ public class BaggageSetter { * @return the SpanContext with the baggage set */ public SpanContext setBaggage(Span span, String key, String value) { + Restriction restriction = restrictionManager.getRestriction(key); boolean truncated = false; - String prevItem = span.getBaggageItem(key); - if (!valid) { + String prevItem = null; + if (!restriction.isKeyAllowed()) { metrics.baggageUpdateFailure.inc(1); - logFields(span, key, value, prevItem, truncated); + logFields(span, key, value, prevItem, truncated, restriction.isKeyAllowed()); return span.context(); } - if (value.length() > maxValueLength) { + if (value.length() > restriction.getMaxValueLength()) { truncated = true; - value = value.substring(0, maxValueLength); + value = value.substring(0, restriction.getMaxValueLength()); metrics.baggageTruncate.inc(1); } - - logFields(span, key, value, prevItem, truncated); + prevItem = span.getBaggageItem(key); + logFields(span, key, value, prevItem, truncated, restriction.isKeyAllowed()); metrics.baggageUpdateSuccess.inc(1); return span.context().withBaggageItem(key, value); } - private void logFields(Span span, String key, String value, String prevItem, boolean truncated) { + private void logFields(Span span, String key, String value, String prevItem, boolean truncated, boolean valid) { if (!span.context().isSampled()) { return; } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java index af6ae3cfc..acf41cd2a 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java @@ -22,25 +22,23 @@ package com.uber.jaeger.baggage; -import com.uber.jaeger.metrics.Metrics; - /** - * DefaultBaggageRestrictionManager is a manager that returns a {@link BaggageSetter} + * DefaultBaggageRestrictionManager is a manager that returns a {@link Restriction} * that allows all baggage. */ public class DefaultBaggageRestrictionManager implements BaggageRestrictionManager { - private BaggageSetter setter; + private final Restriction restriction; - public DefaultBaggageRestrictionManager(Metrics metrics) { - this(metrics, DEFAULT_MAX_VALUE_LENGTH); + public DefaultBaggageRestrictionManager() { + this(DEFAULT_MAX_VALUE_LENGTH); } - DefaultBaggageRestrictionManager(Metrics metrics, int maxValueLength) { - this.setter = BaggageSetter.of(true, maxValueLength, metrics); + DefaultBaggageRestrictionManager(int maxValueLength) { + this.restriction = Restriction.of(true, maxValueLength); } @Override - public BaggageSetter getBaggageSetter(String key) { - return setter; + public Restriction getRestriction(String key) { + return restriction; } } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/Restriction.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/Restriction.java new file mode 100644 index 000000000..b217f3601 --- /dev/null +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/Restriction.java @@ -0,0 +1,35 @@ +/* + * Copyright (c) 2017, Uber Technologies, Inc + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package com.uber.jaeger.baggage; + +import lombok.Value; + +/** + * Restriction determines whether a baggage key is allowed and contains any + * restrictions on the baggage value. + */ +@Value(staticConstructor = "of") +public class Restriction { + boolean keyAllowed; + int maxValueLength; +} diff --git a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java index 4b89e7b20..9f48d27f8 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java @@ -32,6 +32,7 @@ import com.uber.jaeger.baggage.BaggageRestrictionManager; import com.uber.jaeger.baggage.BaggageSetter; import com.uber.jaeger.baggage.DefaultBaggageRestrictionManager; +import com.uber.jaeger.baggage.Restriction; import com.uber.jaeger.metrics.InMemoryStatsReporter; import com.uber.jaeger.metrics.Metrics; import com.uber.jaeger.metrics.StatsFactoryImpl; @@ -68,7 +69,7 @@ public void setUp() throws Exception { new Tracer.Builder("SamplerTest", reporter, new ConstSampler(true)) .withStatsReporter(metricsReporter) .withClock(clock) - .withBaggageRestrictionManager(new DefaultBaggageRestrictionManager(metrics)) + .withBaggageRestrictionManager(new DefaultBaggageRestrictionManager()) .build(); span = (Span) tracer.buildSpan("some-operation").startManual(); } @@ -95,9 +96,9 @@ public void testSetAndGetBaggageItem() { final String key = "key"; final String value = "value"; - when(mgr.getBaggageSetter(key)).thenReturn(BaggageSetter.of(true, 10, metrics)); + when(mgr.getRestriction(key)).thenReturn(Restriction.of(true, 10)); span.setBaggageItem(key, "value"); - verify(mgr).getBaggageSetter(key); + verify(mgr).getRestriction(key); assertEquals(value, span.getBaggageItem(key)); } diff --git a/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java b/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java index 5c759af27..c18247e47 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/TracerTest.java @@ -131,7 +131,7 @@ public void testWithBaggageRestrictionManager() { .build(); Span span = (Span) tracer.buildSpan("some-operation").startManual(); final String key = "key"; - tracer.getBaggageRestrictionManager().getBaggageSetter(key).setBaggage(span, key, "value"); + tracer.setBaggage(span, key, "value"); assertEquals( 1L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue()); diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java index 067efb5f5..a4e00e144 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java @@ -24,6 +24,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import com.uber.jaeger.LogData; import com.uber.jaeger.Span; @@ -48,6 +50,8 @@ public class BaggageSetterTest { private Span span; private InMemoryStatsReporter metricsReporter; private Metrics metrics; + private BaggageRestrictionManager mgr; + private BaggageSetter setter; private static final String KEY = "key"; @@ -56,17 +60,19 @@ public void setUp() throws Exception { metricsReporter = new InMemoryStatsReporter(); reporter = new InMemoryReporter(); metrics = new Metrics(new StatsFactoryImpl(metricsReporter)); + mgr = mock(DefaultBaggageRestrictionManager.class); + setter = new BaggageSetter(mgr, metrics); tracer = new Tracer.Builder("SamplerTest", reporter, new ConstSampler(true)) .withStatsReporter(metricsReporter) - .withBaggageRestrictionManager(new DefaultBaggageRestrictionManager(metrics, 15)) .build(); span = (Span) tracer.buildSpan("some-operation").startManual(); } @Test public void testInvalidBaggage() { - BaggageSetter setter = BaggageSetter.of(false, 0, metrics); + when(mgr.getRestriction(KEY)).thenReturn(Restriction.of(false, 0)); + final String value = "value"; SpanContext ctx = setter.setBaggage(span, KEY, value); @@ -79,7 +85,7 @@ public void testInvalidBaggage() { @Test public void testTruncatedBaggage() { - BaggageSetter setter = BaggageSetter.of(true, 5, metrics); + when(mgr.getRestriction(KEY)).thenReturn(Restriction.of(true, 5)); final String value = "0123456789"; final String expected = "01234"; SpanContext ctx = setter.setBaggage(span, KEY, value); @@ -95,7 +101,7 @@ public void testTruncatedBaggage() { @Test public void testOverrideBaggage() { - BaggageSetter setter = BaggageSetter.of(true, 5, metrics); + when(mgr.getRestriction(KEY)).thenReturn(Restriction.of(true, 5)); final String value = "value"; SpanContext ctx = setter.setBaggage(span, KEY, value); Span child = (Span) tracer.buildSpan("some-operation").asChildOf(ctx).startManual(); @@ -108,6 +114,23 @@ public void testOverrideBaggage() { 2L, metricsReporter.counters.get("jaeger.baggage-update.result=ok").longValue()); } + @Test + public void testUnsampledSpan() { + tracer = + new Tracer.Builder("SamplerTest", reporter, new ConstSampler(false)) + .withStatsReporter(metricsReporter) + .build(); + span = (Span) tracer.buildSpan("some-operation").startManual(); + + when(mgr.getRestriction(KEY)).thenReturn(Restriction.of(true, 5)); + final String value = "value"; + SpanContext ctx = setter.setBaggage(span, KEY, value); + + assertEquals(value, ctx.getBaggageItem(KEY)); + // No logs should be written if the span is not sampled + assertNull(span.getLogs()); + } + private void assertBaggageLogs( Span span, String key, diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManagerTest.java similarity index 80% rename from jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java rename to jaeger-core/src/test/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManagerTest.java index 91f069acb..e7545fe2e 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageRestrictionManagerTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManagerTest.java @@ -30,20 +30,19 @@ import com.uber.jaeger.metrics.StatsFactoryImpl; import org.junit.Test; -public class BaggageRestrictionManagerTest { +public class DefaultBaggageRestrictionManagerTest { @Test - public void testGetBaggageSetter() { - final Metrics nullMetrics = new Metrics(new StatsFactoryImpl(new NullStatsReporter())); - final DefaultBaggageRestrictionManager undertest = new DefaultBaggageRestrictionManager(nullMetrics); + public void testGetRestriction() { + final DefaultBaggageRestrictionManager undertest = new DefaultBaggageRestrictionManager(); final String key = "key"; - BaggageSetter actual = undertest.getBaggageSetter(key); - BaggageSetter expected = BaggageSetter.of(true, 2048, nullMetrics); + Restriction actual = undertest.getRestriction(key); + Restriction expected = Restriction.of(true, 2048); assertEquals(expected, actual); expected = actual; - actual = undertest.getBaggageSetter(key); + actual = undertest.getRestriction(key); assertSame(actual, expected); } } From 45d2b581832d63f199e2ea209dcecc131f7643ca Mon Sep 17 00:00:00 2001 From: Won Jun Jang Date: Thu, 10 Aug 2017 14:58:30 -0400 Subject: [PATCH 14/14] address comments --- jaeger-core/src/main/java/com/uber/jaeger/Span.java | 2 +- .../jaeger/baggage/BaggageRestrictionManager.java | 5 +++-- .../java/com/uber/jaeger/baggage/BaggageSetter.java | 4 ++-- .../baggage/DefaultBaggageRestrictionManager.java | 2 +- .../src/test/java/com/uber/jaeger/SpanTest.java | 7 ++++--- .../com/uber/jaeger/baggage/BaggageSetterTest.java | 11 ++++++----- .../baggage/DefaultBaggageRestrictionManagerTest.java | 4 ++-- 7 files changed, 19 insertions(+), 16 deletions(-) diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Span.java b/jaeger-core/src/main/java/com/uber/jaeger/Span.java index 6f2c8d5d3..11aff3b4a 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/Span.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/Span.java @@ -121,7 +121,7 @@ public Span setBaggageItem(String key, String value) { return this; } synchronized (this) { - this.context = getTracer().setBaggage(this, key, value); + context = tracer.setBaggage(this, key, value); return this; } } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java index e52ab98cb..58549addc 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageRestrictionManager.java @@ -26,10 +26,11 @@ * BaggageRestrictionManager is an interface for a class that manages baggage * restrictions for baggage keys. The manager will return a {@link Restriction} * for a specific baggage key which will determine whether the baggage key is - * allowed and any other applicable restrictions on the baggage value. + * allowed for the current service and any other applicable restrictions on the + * baggage value. */ public interface BaggageRestrictionManager { int DEFAULT_MAX_VALUE_LENGTH = 2048; - Restriction getRestriction(String key); + Restriction getRestriction(String service, String key); } diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java index 343e0f3fb..2405f3654 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/BaggageSetter.java @@ -47,7 +47,7 @@ public BaggageSetter(BaggageRestrictionManager restrictionManager, Metrics metri /** * Sets the baggage key:value on the {@link Span} and the corresponding * logs. Whether the baggage is set on the span depends on if the key - * is valid. + * is allowed to be set by this service. *

* A {@link SpanContext} is returned with the new baggage key:value set * if key is valid, else returns the existing {@link SpanContext} @@ -58,7 +58,7 @@ public BaggageSetter(BaggageRestrictionManager restrictionManager, Metrics metri * @return the SpanContext with the baggage set */ public SpanContext setBaggage(Span span, String key, String value) { - Restriction restriction = restrictionManager.getRestriction(key); + Restriction restriction = restrictionManager.getRestriction(span.getTracer().getServiceName(), key); boolean truncated = false; String prevItem = null; if (!restriction.isKeyAllowed()) { diff --git a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java index acf41cd2a..64c379c81 100644 --- a/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java +++ b/jaeger-core/src/main/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManager.java @@ -38,7 +38,7 @@ public DefaultBaggageRestrictionManager() { } @Override - public Restriction getRestriction(String key) { + public Restriction getRestriction(String service, String key) { return restriction; } } diff --git a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java index 9f48d27f8..1324eeb45 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/SpanTest.java @@ -86,9 +86,10 @@ public void testSpanMetrics() { @Test public void testSetAndGetBaggageItem() { + final String service = "SamplerTest"; final BaggageRestrictionManager mgr = Mockito.mock(DefaultBaggageRestrictionManager.class); tracer = - new Tracer.Builder("SamplerTest", reporter, new ConstSampler(true)) + new Tracer.Builder(service, reporter, new ConstSampler(true)) .withClock(clock) .withBaggageRestrictionManager(mgr) .build(); @@ -96,9 +97,9 @@ public void testSetAndGetBaggageItem() { final String key = "key"; final String value = "value"; - when(mgr.getRestriction(key)).thenReturn(Restriction.of(true, 10)); + when(mgr.getRestriction(service, key)).thenReturn(Restriction.of(true, 10)); span.setBaggageItem(key, "value"); - verify(mgr).getRestriction(key); + verify(mgr).getRestriction(service, key); assertEquals(value, span.getBaggageItem(key)); } diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java index a4e00e144..4a1a4f130 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/BaggageSetterTest.java @@ -54,6 +54,7 @@ public class BaggageSetterTest { private BaggageSetter setter; private static final String KEY = "key"; + private static final String SERVICE = "SamplerTest"; @Before public void setUp() throws Exception { @@ -63,7 +64,7 @@ public void setUp() throws Exception { mgr = mock(DefaultBaggageRestrictionManager.class); setter = new BaggageSetter(mgr, metrics); tracer = - new Tracer.Builder("SamplerTest", reporter, new ConstSampler(true)) + new Tracer.Builder(SERVICE, reporter, new ConstSampler(true)) .withStatsReporter(metricsReporter) .build(); span = (Span) tracer.buildSpan("some-operation").startManual(); @@ -71,7 +72,7 @@ public void setUp() throws Exception { @Test public void testInvalidBaggage() { - when(mgr.getRestriction(KEY)).thenReturn(Restriction.of(false, 0)); + when(mgr.getRestriction(SERVICE, KEY)).thenReturn(Restriction.of(false, 0)); final String value = "value"; SpanContext ctx = setter.setBaggage(span, KEY, value); @@ -85,7 +86,7 @@ public void testInvalidBaggage() { @Test public void testTruncatedBaggage() { - when(mgr.getRestriction(KEY)).thenReturn(Restriction.of(true, 5)); + when(mgr.getRestriction(SERVICE, KEY)).thenReturn(Restriction.of(true, 5)); final String value = "0123456789"; final String expected = "01234"; SpanContext ctx = setter.setBaggage(span, KEY, value); @@ -101,7 +102,7 @@ public void testTruncatedBaggage() { @Test public void testOverrideBaggage() { - when(mgr.getRestriction(KEY)).thenReturn(Restriction.of(true, 5)); + when(mgr.getRestriction(SERVICE, KEY)).thenReturn(Restriction.of(true, 5)); final String value = "value"; SpanContext ctx = setter.setBaggage(span, KEY, value); Span child = (Span) tracer.buildSpan("some-operation").asChildOf(ctx).startManual(); @@ -122,7 +123,7 @@ public void testUnsampledSpan() { .build(); span = (Span) tracer.buildSpan("some-operation").startManual(); - when(mgr.getRestriction(KEY)).thenReturn(Restriction.of(true, 5)); + when(mgr.getRestriction(SERVICE, KEY)).thenReturn(Restriction.of(true, 5)); final String value = "value"; SpanContext ctx = setter.setBaggage(span, KEY, value); diff --git a/jaeger-core/src/test/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManagerTest.java b/jaeger-core/src/test/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManagerTest.java index e7545fe2e..decda42aa 100644 --- a/jaeger-core/src/test/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManagerTest.java +++ b/jaeger-core/src/test/java/com/uber/jaeger/baggage/DefaultBaggageRestrictionManagerTest.java @@ -37,12 +37,12 @@ public void testGetRestriction() { final DefaultBaggageRestrictionManager undertest = new DefaultBaggageRestrictionManager(); final String key = "key"; - Restriction actual = undertest.getRestriction(key); + Restriction actual = undertest.getRestriction("", key); Restriction expected = Restriction.of(true, 2048); assertEquals(expected, actual); expected = actual; - actual = undertest.getRestriction(key); + actual = undertest.getRestriction("", key); assertSame(actual, expected); } }