From 058510d43204019f1722e1b1c7dd89235aba2548 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 25 Jun 2019 16:14:54 +0800 Subject: [PATCH 1/7] Base implementation for opentracing 0.32 and 0.33 --- pom.xml | 34 ++-- src/it/opentracing-0.33/README.md | 2 + src/it/opentracing-0.33/pom.xml | 101 ++++++++++++ .../opentracing/BraveScopeManagerTest.java | 17 ++ .../opentracing/BraveSpanBuilderTest.java | 17 ++ .../java/brave/opentracing/BraveSpanTest.java | 17 ++ .../brave/opentracing/BraveTracerTest.java | 25 +++ src/it/settings.xml | 50 ++++++ .../java/brave/opentracing/BraveScope.java | 5 +- .../brave/opentracing/BraveScopeManager.java | 27 +++- .../java/brave/opentracing/BraveSpan.java | 10 +- .../brave/opentracing/BraveSpanBuilder.java | 29 ++-- .../brave/opentracing/BraveSpanContext.java | 34 +++- .../java/brave/opentracing/BraveTracer.java | 38 +++-- .../brave/opentracing/OpenTracingVersion.java | 81 ++++++++++ ...penTracing0_32_BraveScopeManagerTest.java} | 14 +- .../OpenTracing0_32_BraveTracerTest.java | 148 ++++++++++++++++++ ...OpenTracing0_33_BraveScopeManagerTest.java | 81 ++++++++++ ...OpenTracing0_33_BraveSpanBuilderTest.java} | 4 +- ...ava => OpenTracing0_33_BraveSpanTest.java} | 66 +++----- ...a => OpenTracing0_33_BraveTracerTest.java} | 132 +++++++--------- .../brave/opentracing/TextMapSetterTest.java | 6 +- 22 files changed, 751 insertions(+), 187 deletions(-) create mode 100644 src/it/opentracing-0.33/README.md create mode 100644 src/it/opentracing-0.33/pom.xml create mode 100644 src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveScopeManagerTest.java create mode 100644 src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveSpanBuilderTest.java create mode 100644 src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveSpanTest.java create mode 100644 src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveTracerTest.java create mode 100644 src/it/settings.xml create mode 100644 src/main/java/brave/opentracing/OpenTracingVersion.java rename src/test/java/brave/opentracing/{BraveScopeManagerTest.java => OpenTracing0_32_BraveScopeManagerTest.java} (94%) create mode 100644 src/test/java/brave/opentracing/OpenTracing0_32_BraveTracerTest.java create mode 100644 src/test/java/brave/opentracing/OpenTracing0_33_BraveScopeManagerTest.java rename src/test/java/brave/opentracing/{BraveSpanBuilderTest.java => OpenTracing0_33_BraveSpanBuilderTest.java} (95%) rename src/test/java/brave/opentracing/{BraveSpanTest.java => OpenTracing0_33_BraveSpanTest.java} (80%) rename src/test/java/brave/opentracing/{BraveTracerTest.java => OpenTracing0_33_BraveTracerTest.java} (70%) diff --git a/pom.xml b/pom.xml index 4f25fbe..a162300 100644 --- a/pom.xml +++ b/pom.xml @@ -27,6 +27,7 @@ 0.34.0-SNAPSHOT + ${project.basedir} UTF-8 UTF-8 @@ -40,10 +41,17 @@ 1.8 1.8 - + + -Xep:MissingOverride:OFF 2.3.3 - ${project.basedir} + + 4.12 + 3.12.2 + 2.28.2 + 1.13.1 + 3.8.1 + 3.0.0-M3 @@ -107,7 +115,7 @@ io.opentracing opentracing-api - 0.31.0 + 0.32.0 io.zipkin.brave @@ -117,7 +125,7 @@ junit junit - 4.12 + ${junit.version} test @@ -128,19 +136,19 @@ org.mockito mockito-core - 2.28.2 + ${mockito.version} test org.assertj assertj-core - 3.12.1 + ${assertj.version} test com.tngtech.java junit-dataprovider - 1.13.1 + ${junit-dataprovider.version} test @@ -159,7 +167,7 @@ maven-compiler-plugin - 3.8.1 + ${maven-compiler-plugin.version} true @@ -171,6 +179,11 @@ + + maven-surefire-plugin + ${maven-surefire-plugin.version} + + net.orfjackal.retrolambda retrolambda-maven-plugin @@ -284,11 +297,6 @@ - - maven-surefire-plugin - 3.0.0-M3 - - maven-invoker-plugin 3.2.0 diff --git a/src/it/opentracing-0.33/README.md b/src/it/opentracing-0.33/README.md new file mode 100644 index 0000000..6597c4c --- /dev/null +++ b/src/it/opentracing-0.33/README.md @@ -0,0 +1,2 @@ +# OpenTracing 0.31 +This tests that BraveTracer can be used with OpenTracing 0.31 diff --git a/src/it/opentracing-0.33/pom.xml b/src/it/opentracing-0.33/pom.xml new file mode 100644 index 0000000..772f4d4 --- /dev/null +++ b/src/it/opentracing-0.33/pom.xml @@ -0,0 +1,101 @@ + + + + 4.0.0 + + @project.groupId@ + opentracing-0.33 + @project.version@ + opentracing-0.33 + + + 1.8 + 1.8 + + + + + ${project.groupId} + brave-opentracing + ${project.version} + + + + io.opentracing + opentracing-api + 0.33.0 + + + + junit + junit + @junit.version@ + + + + org.assertj + assertj-core + @assertj.version@ + + + + org.mockito + mockito-core + @mockito.version@ + + + + com.tngtech.java + junit-dataprovider + @junit-dataprovider.version@ + + + + io.zipkin.brave + brave-tests + @brave.version@ + + + + + + @project.build.testSourceDirectory@ + + + maven-compiler-plugin + @maven-compiler-plugin.version@ + + + **/OpenTracing0_33_*Test.java + + + + + maven-surefire-plugin + @maven-surefire-plugin.version@ + + true + + **/Brave*Test.java + + + + + + diff --git a/src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveScopeManagerTest.java b/src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveScopeManagerTest.java new file mode 100644 index 0000000..a9eb8dc --- /dev/null +++ b/src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveScopeManagerTest.java @@ -0,0 +1,17 @@ +/* + * Copyright 2016-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.opentracing; + +public class BraveScopeManagerTest extends OpenTracing0_33_BraveScopeManagerTest { +} diff --git a/src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveSpanBuilderTest.java b/src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveSpanBuilderTest.java new file mode 100644 index 0000000..84d2ead --- /dev/null +++ b/src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveSpanBuilderTest.java @@ -0,0 +1,17 @@ +/* + * Copyright 2016-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.opentracing; + +public class BraveSpanBuilderTest extends OpenTracing0_33_BraveSpanBuilderTest { +} diff --git a/src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveSpanTest.java b/src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveSpanTest.java new file mode 100644 index 0000000..34bfa1e --- /dev/null +++ b/src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveSpanTest.java @@ -0,0 +1,17 @@ +/* + * Copyright 2016-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.opentracing; + +public class BraveSpanTest extends OpenTracing0_33_BraveSpanTest { +} diff --git a/src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveTracerTest.java b/src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveTracerTest.java new file mode 100644 index 0000000..631300c --- /dev/null +++ b/src/it/opentracing-0.33/src/test/java/brave/opentracing/BraveTracerTest.java @@ -0,0 +1,25 @@ +/* + * Copyright 2016-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.opentracing; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class BraveTracerTest extends OpenTracing0_33_BraveTracerTest { + @Test public void versionIsCorrect() { + assertThat(OpenTracingVersion.get()) + .isInstanceOf(OpenTracingVersion.v0_33.class); + } +} diff --git a/src/it/settings.xml b/src/it/settings.xml new file mode 100644 index 0000000..eb1fbfb --- /dev/null +++ b/src/it/settings.xml @@ -0,0 +1,50 @@ + + + + + + it-repo + + true + + + + local.central + @localRepositoryUrl@ + + true + + + true + + + + + + local.central + @localRepositoryUrl@ + + true + + + true + + + + + + diff --git a/src/main/java/brave/opentracing/BraveScope.java b/src/main/java/brave/opentracing/BraveScope.java index 06f30f2..5c184eb 100644 --- a/src/main/java/brave/opentracing/BraveScope.java +++ b/src/main/java/brave/opentracing/BraveScope.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018 The OpenZipkin Authors + * Copyright 2016-2019 The OpenZipkin Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -50,7 +50,8 @@ public final class BraveScope implements Scope { source.deregister(this); } - @Override public BraveSpan span() { + /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ + @Deprecated public BraveSpan span() { return wrapped; } diff --git a/src/main/java/brave/opentracing/BraveScopeManager.java b/src/main/java/brave/opentracing/BraveScopeManager.java index a05d772..03d679c 100644 --- a/src/main/java/brave/opentracing/BraveScopeManager.java +++ b/src/main/java/brave/opentracing/BraveScopeManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018 The OpenZipkin Authors + * Copyright 2016-2019 The OpenZipkin Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -41,11 +41,12 @@ public final class BraveScopeManager implements ScopeManager { } /** - * This api's only purpose is to retrieve the {@link Scope#span() span}. + * This api's only purpose is to retrieve the {@link BraveScope#span() span}. * * Calling {@link Scope#close() close } on the returned scope has no effect on the active span */ - @Override public Scope active() { + /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ + @Deprecated public Scope active() { BraveSpan span = currentSpan(); if (span == null) return null; return new Scope() { @@ -53,14 +54,23 @@ public final class BraveScopeManager implements ScopeManager { // no-op } - @Override public Span span() { + /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ + @Deprecated public Span span() { return span; } }; } + @Override public BraveSpan activeSpan() { + brave.Span braveSpan = tracer.currentSpan(); + if (braveSpan != null) { + return new BraveSpan(tracer, braveSpan); + } + return null; + } + /** Attempts to get a span from the current api, falling back to brave's native one */ - BraveSpan currentSpan() { + @Deprecated BraveSpan currentSpan() { BraveScope scope = currentScopes.get().peekFirst(); if (scope != null) { return scope.span(); @@ -73,7 +83,12 @@ BraveSpan currentSpan() { return null; } - @Override public BraveScope activate(Span span, boolean finishSpanOnClose) { + @Override public BraveScope activate(Span span) { + return activate(span, false); + } + + /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ + @Deprecated public BraveScope activate(Span span, boolean finishSpanOnClose) { if (span == null) return null; if (!(span instanceof BraveSpan)) { throw new IllegalArgumentException( diff --git a/src/main/java/brave/opentracing/BraveSpan.java b/src/main/java/brave/opentracing/BraveSpan.java index 2e4bc2e..299c644 100644 --- a/src/main/java/brave/opentracing/BraveSpan.java +++ b/src/main/java/brave/opentracing/BraveSpan.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018 The OpenZipkin Authors + * Copyright 2016-2019 The OpenZipkin Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -93,6 +93,14 @@ public final brave.Span unwrap() { return setTag(key, value.toString()); } + @Override public BraveSpan setTag(io.opentracing.tag.Tag tag, T value) { + // Strange there's a new api only to dispatch something that can be done as easily directly + // eg instead of tag.set(span, value) this allows span.setTag(tag, value) (3 more characters!) + // Would be nice to see documentation clarify why this was important enough to break api over. + tag.set(this, value); + return this; + } + @Override public BraveSpan log(Map fields) { if (finishCalled) return this; diff --git a/src/main/java/brave/opentracing/BraveSpanBuilder.java b/src/main/java/brave/opentracing/BraveSpanBuilder.java index 7699a5d..29ee694 100644 --- a/src/main/java/brave/opentracing/BraveSpanBuilder.java +++ b/src/main/java/brave/opentracing/BraveSpanBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018 The OpenZipkin Authors + * Copyright 2016-2019 The OpenZipkin Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -36,7 +36,7 @@ */ public final class BraveSpanBuilder implements Tracer.SpanBuilder { - private final Tracer tracer; + private final BraveTracer tracer; private final brave.Tracer braveTracer; private final Map tags = new LinkedHashMap<>(); @@ -46,7 +46,7 @@ public final class BraveSpanBuilder implements Tracer.SpanBuilder { private BraveSpanContext reference; private boolean ignoreActiveSpan = false; - BraveSpanBuilder(Tracer tracer, brave.Tracer braveTracer, String operationName) { + BraveSpanBuilder(BraveTracer tracer, brave.Tracer braveTracer, String operationName) { this.tracer = tracer; this.braveTracer = braveTracer; this.operationName = operationName; @@ -88,20 +88,31 @@ public final class BraveSpanBuilder implements Tracer.SpanBuilder { return withTag(key, value.toString()); } + @Override public BraveSpanBuilder withTag(io.opentracing.tag.Tag tag, T value) { + if (tag == null) throw new NullPointerException("tag == null"); + if (value == null) throw new NullPointerException("value == null"); + if (value instanceof String) return withTag(tag.getKey(), (String) value); + if (value instanceof Number) return withTag(tag.getKey(), (Number) value); + if (value instanceof Boolean) return withTag(tag.getKey(), (Boolean) value); + throw new IllegalArgumentException("tag value not a string, number or boolean: " + tag); + } + @Override public BraveSpanBuilder withStartTimestamp(long microseconds) { this.timestamp = microseconds; return this; } - @Override @Deprecated public BraveSpan startManual() { + /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ + @Deprecated public BraveSpan startManual() { return start(); } - @Override public Scope startActive(boolean finishSpanOnClose) { + /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ + @Deprecated public BraveScope startActive(boolean finishSpanOnClose) { if (!ignoreActiveSpan) { - Scope parent = tracer.scopeManager().active(); + BraveSpan parent = tracer.scopeManager().activeSpan(); if (parent != null) { - asChildOf(parent.span()); + asChildOf(parent.context()); } } return tracer.scopeManager().activate(start(), finishSpanOnClose); @@ -117,9 +128,9 @@ public final class BraveSpanBuilder implements Tracer.SpanBuilder { // Check if active span should be established as CHILD_OF relationship if (reference == null && !ignoreActiveSpan) { - Scope parent = tracer.scopeManager().active(); + BraveSpan parent = tracer.scopeManager().activeSpan(); if (parent != null) { - asChildOf(parent.span()); + asChildOf(parent.context()); } } diff --git a/src/main/java/brave/opentracing/BraveSpanContext.java b/src/main/java/brave/opentracing/BraveSpanContext.java index d295cba..9672c25 100644 --- a/src/main/java/brave/opentracing/BraveSpanContext.java +++ b/src/main/java/brave/opentracing/BraveSpanContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018 The OpenZipkin Authors + * Copyright 2016-2019 The OpenZipkin Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -13,6 +13,7 @@ */ package brave.opentracing; +import brave.Span; import brave.propagation.ExtraFieldPropagation; import brave.propagation.TraceContext; import brave.propagation.TraceContextOrSamplingFlags; @@ -22,8 +23,8 @@ /** * Holds the {@linkplain TraceContext} used by the underlying {@linkplain brave.Tracer}, or an {link - * TraceContextOrSamplingFlags extraction result if an incoming context}. An {@link TraceContext#sampled() unsampled} - * context results in a {@link brave.NoopSpan}. + * TraceContextOrSamplingFlags extraction result if an incoming context}. An {@link + * TraceContext#sampled() unsampled} context results in a {@link Span#isNoop() noop span}. * *

This type also includes hooks to integrate with the underlying {@linkplain brave.Tracer}. Ex * you can access the underlying trace context with {@link #unwrap} @@ -35,11 +36,12 @@ public abstract class BraveSpanContext implements SpanContext { * *

When a span context is returned from {@link BraveSpan#context()}, there's no ambiguity. It * represents the current span. However, a span context can be in an intermediate state when - * extracted from headers. In other words, unwrap might not have a {@link TraceContext} to return. + * extracted from headers. In other words, unwrap might not have a {@link TraceContext} to + * return. * *

Why? {@link BraveTracer#extract(Format, Object) Extraction from headers} can return partial - * info. For example, in Amazon Web Services, you may be suggested just a trace ID. In other cases, you - * might just inherit baggage or a sampling hint. + * info. For example, in Amazon Web Services, you may be suggested just a trace ID. In other + * cases, you might just inherit baggage or a sampling hint. */ public abstract TraceContext unwrap(); @@ -67,6 +69,15 @@ static final class Complete extends BraveSpanContext { return context; } + // notice: no sampling or parent span ID here! + @Override public String toTraceId() { + return context.traceIdString(); + } + + @Override public String toSpanId() { + return context.spanIdString(); + } + @Override public Iterable> baggageItems() { return ExtraFieldPropagation.getAll(context).entrySet(); } @@ -88,6 +99,17 @@ TraceContextOrSamplingFlags extractionResult() { // temporarily hidden return extractionResult.context(); } + // notice: no sampling or parent span ID here! + @Override public String toTraceId() { + TraceContext context = extractionResult.context(); + return context != null ? context.traceIdString() : null; + } + + @Override public String toSpanId() { + TraceContext context = extractionResult.context(); + return context != null ? context.spanIdString() : null; + } + /** Returns empty unless {@link ExtraFieldPropagation} is in use */ @Override public Iterable> baggageItems() { return ExtraFieldPropagation.getAll(extractionResult).entrySet(); diff --git a/src/main/java/brave/opentracing/BraveTracer.java b/src/main/java/brave/opentracing/BraveTracer.java index 656ec37..043834f 100644 --- a/src/main/java/brave/opentracing/BraveTracer.java +++ b/src/main/java/brave/opentracing/BraveTracer.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018 The OpenZipkin Authors + * Copyright 2016-2019 The OpenZipkin Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -23,8 +23,8 @@ import brave.propagation.TraceContext.Extractor; import brave.propagation.TraceContext.Injector; import brave.propagation.TraceContextOrSamplingFlags; -import io.opentracing.Scope; import io.opentracing.ScopeManager; +import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.Tracer; import io.opentracing.propagation.Format; @@ -60,6 +60,7 @@ */ public final class BraveTracer implements Tracer { + private final Tracing tracing; private final brave.Tracer brave4; private final BraveScopeManager scopeManager; @@ -89,8 +90,10 @@ public static final class Builder { Builder(Tracing tracing) { if (tracing == null) throw new NullPointerException("brave tracing component == null"); this.tracing = tracing; + formatToPropagation.put(Format.Builtin.HTTP_HEADERS, tracing.propagation()); formatToPropagation.put(Format.Builtin.TEXT_MAP, tracing.propagation()); + // TODO: TEXT_MAP_INJECT and TEXT_MAP_EXTRACT (not excited about this) } /** @@ -122,16 +125,22 @@ public BraveTracer build() { } } - final Map, Injector> formatToInjector = new LinkedHashMap<>(); - final Map, Extractor> formatToExtractor = new LinkedHashMap<>(); + final Map, Injector> formatToInjector = new LinkedHashMap<>(); + final Map, Extractor> formatToExtractor = new LinkedHashMap<>(); BraveTracer(Builder b) { + tracing = b.tracing; brave4 = b.tracing.tracer(); scopeManager = new BraveScopeManager(b.tracing); for (Map.Entry, Propagation> entry : b.formatToPropagation.entrySet()) { formatToInjector.put(entry.getKey(), entry.getValue().injector(TEXT_MAP_SETTER)); formatToExtractor.put(entry.getKey(), new TextMapExtractorAdaptor(entry.getValue())); } + + for (Propagation propagation : b.formatToPropagation.values()) { + formatToInjector.put(Format.Builtin.TEXT_MAP_INJECT, propagation.injector(TEXT_MAP_SETTER)); + formatToExtractor.put(Format.Builtin.TEXT_MAP_EXTRACT, new TextMapExtractorAdaptor(propagation)); + } } @Override public BraveScopeManager scopeManager() { @@ -139,8 +148,11 @@ public BraveTracer build() { } @Override public BraveSpan activeSpan() { - Scope scope = this.scopeManager.active(); - return scope != null ? (BraveSpan) scope.span() : null; + return scopeManager.activeSpan(); + } + + @Override public BraveScope activateSpan(Span span) { + return scopeManager.activate(span); } @Override public BraveSpanBuilder buildSpan(String operationName) { @@ -151,12 +163,12 @@ public BraveTracer build() { * Injects the underlying context using B3 encoding by default. */ @Override public void inject(SpanContext spanContext, Format format, C carrier) { - Injector injector = formatToInjector.get(format); + Injector injector = (Injector) formatToInjector.get(format); if (injector == null) { throw new UnsupportedOperationException(format + " not in " + formatToInjector.keySet()); } TraceContext traceContext = ((BraveSpanContext) spanContext).unwrap(); - injector.inject(traceContext, (TextMap) carrier); + injector.inject(traceContext, carrier); } /** @@ -164,15 +176,19 @@ public BraveTracer build() { * encoded context in the carrier, or upon error extracting it. */ @Override public BraveSpanContext extract(Format format, C carrier) { - Extractor extractor = formatToExtractor.get(format); + Extractor extractor = (Extractor) formatToExtractor.get(format); if (extractor == null) { throw new UnsupportedOperationException(format + " not in " + formatToExtractor.keySet()); } - TraceContextOrSamplingFlags extractionResult = extractor.extract((TextMap) carrier); + TraceContextOrSamplingFlags extractionResult = extractor.extract(carrier); return BraveSpanContext.create(extractionResult); } - static final Setter TEXT_MAP_SETTER = new Setter(){ + @Override public void close() { + tracing.close(); + } + + static final Setter TEXT_MAP_SETTER = new Setter() { @Override public void put(TextMap carrier, String key, String value) { carrier.put(key, value); } diff --git a/src/main/java/brave/opentracing/OpenTracingVersion.java b/src/main/java/brave/opentracing/OpenTracingVersion.java new file mode 100644 index 0000000..d522791 --- /dev/null +++ b/src/main/java/brave/opentracing/OpenTracingVersion.java @@ -0,0 +1,81 @@ +/* + * Copyright 2016-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.opentracing; + +import io.opentracing.ScopeManager; +import io.opentracing.Span; + +/** + * Access to version-specific features. + * + *

Originally designed by OkHttp team, derived from {@code okhttp3.internal.platform.OpenTracingVersion} + */ +abstract class OpenTracingVersion { + private static final OpenTracingVersion INSTANCE = findVersion(); + + static OpenTracingVersion get() { + return INSTANCE; + } + + /** Attempt to match the host runtime to a capable OpenTracingVersion implementation. */ + static OpenTracingVersion findVersion() { + OpenTracingVersion version = v0_32.buildIfSupported(); + if (version != null) return version; + + version = v0_33.buildIfSupported(); + if (version != null) return version; + + throw new UnsupportedOperationException("Unsupported opentracing-api version"); + } + + static class v0_33 extends OpenTracingVersion { + static v0_33 buildIfSupported() { + // Find OpenTracing 0.32 new type + try { + Class.forName("io.opentracing.tag.Tag"); + return new v0_33(); + } catch (ClassNotFoundException e) { + } + return null; + } + + @Override public String toString() { + return "v0_33{}"; + } + + v0_33() { + } + } + + static class v0_32 extends OpenTracingVersion { + static v0_32 buildIfSupported() { + // Find OpenTracing 0.32 deprecated method + try { + if (ScopeManager.class.getMethod("activate", Span.class, boolean.class) + .getAnnotation(Deprecated.class) != null) { + return new v0_32(); + } + } catch (NoSuchMethodException e) { + } + return null; + } + + @Override public String toString() { + return "v0_32{}"; + } + + v0_32() { + } + } +} diff --git a/src/test/java/brave/opentracing/BraveScopeManagerTest.java b/src/test/java/brave/opentracing/OpenTracing0_32_BraveScopeManagerTest.java similarity index 94% rename from src/test/java/brave/opentracing/BraveScopeManagerTest.java rename to src/test/java/brave/opentracing/OpenTracing0_32_BraveScopeManagerTest.java index 69f06e5..9052263 100644 --- a/src/test/java/brave/opentracing/BraveScopeManagerTest.java +++ b/src/test/java/brave/opentracing/OpenTracing0_32_BraveScopeManagerTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018 The OpenZipkin Authors + * Copyright 2016-2019 The OpenZipkin Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -25,8 +25,7 @@ import static org.assertj.core.api.Assertions.assertThat; -public class BraveScopeManagerTest { - +public class OpenTracing0_32_BraveScopeManagerTest { List spans = new ArrayList<>(); Tracing brave = Tracing.newBuilder() .currentTraceContext(ThreadLocalCurrentTraceContext.newBuilder() @@ -37,6 +36,10 @@ public class BraveScopeManagerTest { BraveTracer opentracing = BraveTracer.create(brave); + @After public void clear() { + brave.close(); + } + @Test public void scopeManagerActive() { BraveSpan span = opentracing.buildSpan("spanA").start(); @@ -87,9 +90,4 @@ public class BraveScopeManagerTest { scopeB.close(); } } - - @After public void clear() { - Tracing current = Tracing.current(); - if (current != null) current.close(); - } } diff --git a/src/test/java/brave/opentracing/OpenTracing0_32_BraveTracerTest.java b/src/test/java/brave/opentracing/OpenTracing0_32_BraveTracerTest.java new file mode 100644 index 0000000..fec4931 --- /dev/null +++ b/src/test/java/brave/opentracing/OpenTracing0_32_BraveTracerTest.java @@ -0,0 +1,148 @@ +/* + * Copyright 2016-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.opentracing; + +import brave.Tracing; +import brave.propagation.B3Propagation; +import brave.propagation.ExtraFieldPropagation; +import brave.propagation.StrictScopeDecorator; +import brave.propagation.ThreadLocalCurrentTraceContext; +import brave.propagation.TraceContext; +import io.opentracing.Scope; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import org.junit.After; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; + +public class OpenTracing0_32_BraveTracerTest { + List spans = new ArrayList<>(); + Tracing brave = Tracing.newBuilder() + .currentTraceContext(ThreadLocalCurrentTraceContext.newBuilder() + .addScopeDecorator(StrictScopeDecorator.create()) + .build()) + .propagationFactory(ExtraFieldPropagation.newFactoryBuilder(B3Propagation.FACTORY) + .addPrefixedFields("baggage-", Arrays.asList("country-code", "user-id")) + .build()) + .spanReporter(spans::add) + .build(); + BraveTracer opentracing = BraveTracer.create(brave); + + @After public void clear() { + brave.close(); + } + + @Test public void versionIsCorrect() { + assertThat(OpenTracingVersion.get()) + .isInstanceOf(OpenTracingVersion.v0_32.class); + } + + /** OpenTracing span implements auto-closeable, and implies reporting on close */ + @Test public void startActive_autoCloseOnTryFinally() { + try (Scope scope = opentracing.buildSpan("foo").startActive(true)) { + } + + assertThat(spans) + .hasSize(1); + } + + @Test public void startActive_autoCloseOnTryFinally_doesntReportTwice() { + try (Scope scope = opentracing.buildSpan("foo").startActive(true)) { + opentracing.activeSpan().finish(); // user closes and also auto-close closes + } + + assertThat(spans) + .hasSize(1); + } + + @Test public void startActive_autoCloseOnTryFinally_dontClose() { + try (Scope scope = opentracing.buildSpan("foo").startActive(false)) { + } + + assertThat(spans) + .isEmpty(); + } + + @Test public void subsequentChildrenNestProperly_OTStyle() { + // this test is semantically identical to subsequentChildrenNestProperly_BraveStyle, but uses + // the OpenTracingAPI instead of the Brave API. + + Long idOfSpanA; + Long shouldBeIdOfSpanA; + Long idOfSpanB; + Long shouldBeIdOfSpanB; + Long parentIdOfSpanB; + Long parentIdOfSpanC; + + try (Scope scopeA = opentracing.buildSpan("spanA").startActive(false)) { + idOfSpanA = brave.currentTraceContext().get().spanId(); + try (Scope scopeB = opentracing.buildSpan("spanB").startActive(false)) { + idOfSpanB = brave.currentTraceContext().get().spanId(); + parentIdOfSpanB = brave.currentTraceContext().get().parentId(); + shouldBeIdOfSpanB = brave.currentTraceContext().get().spanId(); + } + shouldBeIdOfSpanA = brave.currentTraceContext().get().spanId(); + try (Scope scopeC = opentracing.buildSpan("spanC").startActive(false)) { + parentIdOfSpanC = brave.currentTraceContext().get().parentId(); + } + } + + assertEquals("SpanA should have been active again after closing B", idOfSpanA, + shouldBeIdOfSpanA); + assertEquals("SpanB should have been active prior to its closure", idOfSpanB, + shouldBeIdOfSpanB); + assertEquals("SpanB's parent should be SpanA", idOfSpanA, parentIdOfSpanB); + assertEquals("SpanC's parent should be SpanA", idOfSpanA, parentIdOfSpanC); + } + + @Test public void implicitParentFromSpanManager_startActive() { + try (BraveScope scopeA = opentracing.buildSpan("spanA").startActive(true)) { + try (BraveScope scopeB = opentracing.buildSpan("spanB").startActive(true)) { + TraceContext current = brave.currentTraceContext().get(); + assertThat(scopeB.span().context().unwrap().parentId()) + .isEqualTo(scopeA.span().context().unwrap().spanId()); + } + } + } + + @Test public void implicitParentFromSpanManager_start() { + try (Scope scopeA = opentracing.buildSpan("spanA").startActive(true)) { + BraveSpan span = opentracing.buildSpan("spanB").start(); + assertThat(span.unwrap().context().parentId()) + .isEqualTo(brave.currentTraceContext().get().spanId()); + } + } + + @Test public void implicitParentFromSpanManager_startActive_ignoreActiveSpan() { + try (Scope scopeA = opentracing.buildSpan("spanA").startActive(true)) { + try (Scope scopeB = opentracing.buildSpan("spanA") + .ignoreActiveSpan().startActive(true)) { + assertThat(brave.currentTraceContext().get().parentId()) + .isNull(); // new trace + } + } + } + + @Test public void implicitParentFromSpanManager_start_ignoreActiveSpan() { + try (Scope scopeA = opentracing.buildSpan("spanA").startActive(true)) { + BraveSpan span = opentracing.buildSpan("spanB") + .ignoreActiveSpan().start(); + assertThat(span.unwrap().context().parentId()) + .isNull(); // new trace + } + } +} diff --git a/src/test/java/brave/opentracing/OpenTracing0_33_BraveScopeManagerTest.java b/src/test/java/brave/opentracing/OpenTracing0_33_BraveScopeManagerTest.java new file mode 100644 index 0000000..000be1f --- /dev/null +++ b/src/test/java/brave/opentracing/OpenTracing0_33_BraveScopeManagerTest.java @@ -0,0 +1,81 @@ +/* + * Copyright 2016-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.opentracing; + +import brave.ScopedSpan; +import brave.Tracing; +import brave.propagation.StrictScopeDecorator; +import brave.propagation.ThreadLocalCurrentTraceContext; +import io.opentracing.Scope; +import java.util.ArrayList; +import java.util.List; +import org.junit.After; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class OpenTracing0_33_BraveScopeManagerTest { + + List spans = new ArrayList<>(); + Tracing brave = Tracing.newBuilder() + .currentTraceContext(ThreadLocalCurrentTraceContext.newBuilder() + .addScopeDecorator(StrictScopeDecorator.create()) + .build()) + .spanReporter(spans::add) + .build(); + BraveTracer opentracing = BraveTracer.create(brave); + + @After public void clear() { + brave.close(); + } + + @Test public void activate() { + BraveSpan span = opentracing.buildSpan("spanA").start(); + + try (Scope scopeA = opentracing.scopeManager().activate(span)) { + assertThat(opentracing.scopeManager().activeSpan().context().unwrap()) + .isEqualTo(span.context().unwrap()); + } + + assertThat(opentracing.scopeManager().activeSpan()) + .isNull(); + } + + /** This ensures downstream code using OpenTracing api can see Brave's scope */ + @Test public void activeSpan_bridgesNormalBrave() { + ScopedSpan spanInScope = brave.tracer().startScopedSpan("spanA"); + try { + assertThat(opentracing.scopeManager().activeSpan()) + .extracting("delegate.context") + .containsExactly(spanInScope.context()); + } finally { + spanInScope.finish(); + } + } + + @Test public void activate_nested() { + BraveSpan spanA = opentracing.buildSpan("spanA").start(); + BraveSpan spanB = opentracing.buildSpan("spanB").start(); + + try (Scope scopeA = opentracing.scopeManager().activate(spanA)) { + try (Scope scopeB = opentracing.scopeManager().activate(spanB)) { + assertThat(opentracing.scopeManager().activeSpan().context().unwrap()) + .isEqualTo(spanB.context().unwrap()); + } + + assertThat(opentracing.scopeManager().activeSpan().context().unwrap()) + .isEqualTo(spanA.context().unwrap()); + } + } +} diff --git a/src/test/java/brave/opentracing/BraveSpanBuilderTest.java b/src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanBuilderTest.java similarity index 95% rename from src/test/java/brave/opentracing/BraveSpanBuilderTest.java rename to src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanBuilderTest.java index 7bd45e0..364f9d4 100644 --- a/src/test/java/brave/opentracing/BraveSpanBuilderTest.java +++ b/src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanBuilderTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018 The OpenZipkin Authors + * Copyright 2016-2019 The OpenZipkin Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -20,7 +20,7 @@ import static org.assertj.core.api.Java6Assertions.assertThat; -public class BraveSpanBuilderTest { +public class OpenTracing0_33_BraveSpanBuilderTest { /** Ensures when the caller invokes with null, nothing happens */ @Test public void asChildOf_nullParentContext_noop() { diff --git a/src/test/java/brave/opentracing/BraveSpanTest.java b/src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanTest.java similarity index 80% rename from src/test/java/brave/opentracing/BraveSpanTest.java rename to src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanTest.java index 177bc26..38f88fd 100644 --- a/src/test/java/brave/opentracing/BraveSpanTest.java +++ b/src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018 The OpenZipkin Authors + * Copyright 2016-2019 The OpenZipkin Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -21,12 +21,9 @@ import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; -import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.SpanContext; -import io.opentracing.propagation.Format; -import io.opentracing.propagation.TextMapExtractAdapter; -import io.opentracing.propagation.TextMapInjectAdapter; +import io.opentracing.propagation.TextMapAdapter; import io.opentracing.tag.Tags; import java.util.ArrayList; import java.util.LinkedHashMap; @@ -38,47 +35,26 @@ import zipkin2.Endpoint; import zipkin2.Span.Kind; +import static io.opentracing.propagation.Format.Builtin.TEXT_MAP; import static io.opentracing.tag.Tags.SAMPLING_PRIORITY; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; @RunWith(DataProviderRunner.class) -public class BraveSpanTest { +public class OpenTracing0_33_BraveSpanTest { List spans = new ArrayList<>(); - BraveTracer tracer = BraveTracer.create( - Tracing.newBuilder() - .localServiceName("tracer") - .currentTraceContext(ThreadLocalCurrentTraceContext.newBuilder() - .addScopeDecorator(StrictScopeDecorator.create()) - .build()) - .propagationFactory(ExtraFieldPropagation.newFactory(B3Propagation.FACTORY, "client-id")) - .spanReporter(spans::add).build() - ); - - /** OpenTracing span implements auto-closeable, and implies reporting on close */ - @Test public void autoCloseOnTryFinally() { - try (Scope scope = tracer.buildSpan("foo").startActive(true)) { - } + Tracing brave = Tracing.newBuilder() + .localServiceName("tracer") + .currentTraceContext(ThreadLocalCurrentTraceContext.newBuilder() + .addScopeDecorator(StrictScopeDecorator.create()) + .build()) + .propagationFactory(ExtraFieldPropagation.newFactory(B3Propagation.FACTORY, "client-id")) + .spanReporter(spans::add).build(); - assertThat(spans) - .hasSize(1); - } - - @Test public void autoCloseOnTryFinally_doesntReportTwice() { - try (Scope scope = tracer.buildSpan("foo").startActive(true)) { - scope.span().finish(); // user closes and also auto-close closes - } - - assertThat(spans) - .hasSize(1); - } - - @Test public void autoCloseOnTryFinally_dontClose() { - try (Scope scope = tracer.buildSpan("foo").startActive(false)) { - } + BraveTracer tracer = BraveTracer.create(brave); - assertThat(spans) - .isEmpty(); + @After public void clear() { + brave.close(); } @DataProvider @@ -155,7 +131,7 @@ public void spanKind_afterStart(String tagValue, Kind kind) { assertThat(spans) .flatExtracting(s -> s.tags().entrySet()) - .containsExactly(entry("hello", "monster")); + .containsOnly(entry("hello", "monster")); } @Test public void afterFinish_dataIgnored() { @@ -178,7 +154,7 @@ public void spanKind_afterStart(String tagValue, Kind kind) { .start(); Map carrier = new LinkedHashMap<>(); - tracer.inject(spanClient.context(), Format.Builtin.TEXT_MAP, new TextMapInjectAdapter(carrier)); + tracer.inject(spanClient.context(), TEXT_MAP, new TextMapAdapter(carrier)); BraveTracer tracer2 = BraveTracer.create( Tracing.newBuilder() @@ -186,8 +162,7 @@ public void spanKind_afterStart(String tagValue, Kind kind) { .spanReporter(spans::add).build() ); - SpanContext extractedContext = - tracer2.extract(Format.Builtin.TEXT_MAP, new TextMapExtractAdapter(carrier)); + SpanContext extractedContext = tracer2.extract(TEXT_MAP, new TextMapAdapter(carrier)); Span spanServer = tracer2.buildSpan("foo") .asChildOf(extractedContext) @@ -219,7 +194,7 @@ public void spanKind_afterStart(String tagValue, Kind kind) { carrier.put("client-id", "aloha"); SpanContext extractedContext = - tracer.extract(Format.Builtin.TEXT_MAP, new TextMapExtractAdapter(carrier)); + tracer.extract(TEXT_MAP, new TextMapAdapter(carrier)); assertThat(extractedContext.baggageItems()) .contains(entry("client-id", "aloha")); @@ -296,9 +271,4 @@ public void spanKind_afterStart(String tagValue, Kind kind) { .ip("2001:db8::c001") .port(8080).build()); } - - @After public void clear() { - Tracing current = Tracing.current(); - if (current != null) current.close(); - } } diff --git a/src/test/java/brave/opentracing/BraveTracerTest.java b/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java similarity index 70% rename from src/test/java/brave/opentracing/BraveTracerTest.java rename to src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java index 5b15165..50c48a4 100644 --- a/src/test/java/brave/opentracing/BraveTracerTest.java +++ b/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018 The OpenZipkin Authors + * Copyright 2016-2019 The OpenZipkin Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -25,8 +25,7 @@ import io.opentracing.Scope; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; -import io.opentracing.propagation.TextMapExtractAdapter; -import io.opentracing.propagation.TextMapInjectAdapter; +import io.opentracing.propagation.TextMapAdapter; import java.util.ArrayList; import java.util.Arrays; import java.util.LinkedHashMap; @@ -36,6 +35,8 @@ import org.junit.Test; import zipkin2.Annotation; +import static io.opentracing.propagation.Format.Builtin.HTTP_HEADERS; +import static io.opentracing.propagation.Format.Builtin.TEXT_MAP; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.data.MapEntry.entry; import static org.junit.Assert.assertEquals; @@ -44,7 +45,7 @@ * This shows how one might make an OpenTracing adapter for Brave, and how to navigate in and out of * the core concepts. */ -public class BraveTracerTest { +public class OpenTracing0_33_BraveTracerTest { List spans = new ArrayList<>(); Tracing brave = Tracing.newBuilder() @@ -58,6 +59,10 @@ public class BraveTracerTest { .build(); BraveTracer opentracing = BraveTracer.create(brave); + @After public void clear() { + brave.close(); + } + @Test public void startWithOpenTracingAndFinishWithBrave() { io.opentracing.Span openTracingSpan = opentracing.buildSpan("encode") .withTag("lc", "codec") @@ -85,91 +90,83 @@ public class BraveTracerTest { checkSpanReportedToZipkin(); } - @Test public void extractTraceContext() throws Exception { + @Test public void extractTraceContext() { Map map = new LinkedHashMap<>(); map.put("X-B3-TraceId", "0000000000000001"); map.put("X-B3-SpanId", "0000000000000002"); map.put("X-B3-Sampled", "1"); - BraveSpanContext openTracingContext = - (BraveSpanContext) opentracing.extract(Format.Builtin.HTTP_HEADERS, - new TextMapExtractAdapter(map)); + BraveSpanContext otContext = opentracing.extract(HTTP_HEADERS, new TextMapAdapter(map)); - assertThat(openTracingContext.unwrap()) + assertThat(otContext.unwrap()) .isEqualTo(TraceContext.newBuilder() .traceId(1L) .spanId(2L) .sampled(true).build()); } - @Test public void extractBaggage() throws Exception { + @Test public void extractBaggage() { Map map = new LinkedHashMap<>(); map.put("X-B3-TraceId", "0000000000000001"); map.put("X-B3-SpanId", "0000000000000002"); map.put("X-B3-Sampled", "1"); map.put("baggage-country-code", "FO"); - BraveSpanContext openTracingContext = opentracing.extract(Format.Builtin.HTTP_HEADERS, - new TextMapExtractAdapter(map)); + BraveSpanContext otContext = opentracing.extract(HTTP_HEADERS, new TextMapAdapter(map)); - assertThat(openTracingContext.baggageItems()) + assertThat(otContext.baggageItems()) .containsExactly(entry("country-code", "FO")); } - @Test public void extractTraceContextTextMap() throws Exception { + @Test public void extractTraceContextTextMap() { Map map = new LinkedHashMap<>(); map.put("X-B3-TraceId", "0000000000000001"); map.put("X-B3-SpanId", "0000000000000002"); map.put("X-B3-Sampled", "1"); - BraveSpanContext openTracingContext = - (BraveSpanContext) opentracing.extract(Format.Builtin.TEXT_MAP, - new TextMapExtractAdapter(map)); + BraveSpanContext otContext = opentracing.extract(TEXT_MAP, new TextMapAdapter(map)); - assertThat(openTracingContext.unwrap()) + assertThat(otContext.unwrap()) .isEqualTo(TraceContext.newBuilder() .traceId(1L) .spanId(2L) .sampled(true).build()); } - @Test public void extractTraceContextCaseInsensitive() throws Exception { + @Test public void extractTraceContextCaseInsensitive() { Map map = new LinkedHashMap<>(); map.put("X-B3-TraceId", "0000000000000001"); map.put("x-b3-spanid", "0000000000000002"); map.put("x-b3-SaMpLeD", "1"); map.put("other", "1"); - BraveSpanContext openTracingContext = - (BraveSpanContext) opentracing.extract(Format.Builtin.HTTP_HEADERS, - new TextMapExtractAdapter(map)); + BraveSpanContext otContext = opentracing.extract(HTTP_HEADERS, new TextMapAdapter(map)); - assertThat(openTracingContext.unwrap()) + assertThat(otContext.unwrap()) .isEqualTo(TraceContext.newBuilder() .traceId(1L) .spanId(2L) .sampled(true).build()); } - @Test public void extractTraceContext_unwrapReturnsNull() throws Exception { + @Test public void extractTraceContext_unwrapReturnsNull() { Map map = new LinkedHashMap<>(); map.put("other", "1"); - BraveSpanContext openTracingContext = opentracing.extract(Format.Builtin.HTTP_HEADERS, - new TextMapExtractAdapter(map)); + BraveSpanContext otContext = opentracing.extract(HTTP_HEADERS, new TextMapAdapter(map)); - assertThat(openTracingContext.unwrap()).isNull(); + assertThat(otContext.unwrap()).isNull(); } - @Test public void injectTraceContext() throws Exception { + @Test public void injectTraceContext() { TraceContext context = TraceContext.newBuilder() .traceId(1L) .spanId(2L) .sampled(true).build(); Map map = new LinkedHashMap<>(); - TextMapInjectAdapter carrier = new TextMapInjectAdapter(map); - opentracing.inject(BraveSpanContext.create(context), Format.Builtin.HTTP_HEADERS, carrier); + TextMapAdapter carrier = new TextMapAdapter(map); + opentracing.inject(BraveSpanContext.create(context), HTTP_HEADERS, carrier); assertThat(map).containsExactly( entry("X-B3-TraceId", "0000000000000001"), @@ -178,26 +175,26 @@ public class BraveTracerTest { ); } - @Test public void injectTraceContext_baggage() throws Exception { + @Test public void injectTraceContext_baggage() { BraveSpan span = opentracing.buildSpan("foo").start(); span.setBaggageItem("country-code", "FO"); Map map = new LinkedHashMap<>(); - TextMapInjectAdapter carrier = new TextMapInjectAdapter(map); - opentracing.inject(span.context(), Format.Builtin.HTTP_HEADERS, carrier); + TextMapAdapter carrier = new TextMapAdapter(map); + opentracing.inject(span.context(), HTTP_HEADERS, carrier); assertThat(map).containsEntry("baggage-country-code", "FO"); } - @Test public void injectTraceContextTextMap() throws Exception { + @Test public void injectTraceContextTextMap() { TraceContext context = TraceContext.newBuilder() .traceId(1L) .spanId(2L) .sampled(true).build(); Map map = new LinkedHashMap<>(); - TextMapInjectAdapter carrier = new TextMapInjectAdapter(map); - opentracing.inject(BraveSpanContext.create(context), Format.Builtin.TEXT_MAP, carrier); + TextMapAdapter carrier = new TextMapAdapter(map); + opentracing.inject(BraveSpanContext.create(context), TEXT_MAP, carrier); assertThat(map).containsExactly( entry("X-B3-TraceId", "0000000000000001"), @@ -206,7 +203,7 @@ public class BraveTracerTest { ); } - @Test public void canUseCustomFormatKeys() throws Exception { + @Test public void canUseCustomFormatKeys() { Format B3 = new Format() { }; opentracing = BraveTracer.newBuilder(brave) @@ -218,7 +215,7 @@ public class BraveTracerTest { .sampled(true).build(); Map map = new LinkedHashMap<>(); - TextMapInjectAdapter carrier = new TextMapInjectAdapter(map); + TextMapAdapter carrier = new TextMapAdapter(map); opentracing.inject(BraveSpanContext.create(context), B3, carrier); assertThat(map).containsExactly( @@ -252,16 +249,21 @@ void checkSpanReportedToZipkin() { Long parentIdOfSpanB; Long parentIdOfSpanC; - try (Scope scopeA = opentracing.buildSpan("spanA").startActive(false)) { - idOfSpanA = getTraceContext(scopeA).spanId(); - try (Scope scopeB = opentracing.buildSpan("spanB").startActive(false)) { - idOfSpanB = getTraceContext(scopeB).spanId(); - parentIdOfSpanB = getTraceContext(scopeB).parentId(); - shouldBeIdOfSpanB = getTraceContext(opentracing.scopeManager().active()).spanId(); + BraveSpan spanA = opentracing.buildSpan("spanA").start(); + try (Scope scopeA = opentracing.activateSpan(spanA)) { + idOfSpanA = brave.currentTraceContext().get().spanId(); + + BraveSpan spanB = opentracing.buildSpan("spanB").start(); + try (Scope scopeB = opentracing.activateSpan(spanB)) { + idOfSpanB = brave.currentTraceContext().get().spanId(); + parentIdOfSpanB = brave.currentTraceContext().get().parentId(); + shouldBeIdOfSpanB = brave.currentTraceContext().get().spanId(); } - shouldBeIdOfSpanA = getTraceContext(opentracing.scopeManager().active()).spanId(); - try (Scope scopeC = opentracing.buildSpan("spanC").startActive(false)) { - parentIdOfSpanC = getTraceContext(scopeC).parentId(); + shouldBeIdOfSpanA = brave.currentTraceContext().get().spanId(); + + BraveSpan spanC = opentracing.buildSpan("spanC").start(); + try (Scope scopeC = opentracing.activateSpan(spanC)) { + parentIdOfSpanC = brave.currentTraceContext().get().parentId(); } } @@ -317,35 +319,18 @@ void checkSpanReportedToZipkin() { assertEquals("SpanC's parent should be SpanA", idOfSpanA, parentIdOfSpanC); } - @Test public void implicitParentFromSpanManager_startActive() { - try (Scope scopeA = opentracing.buildSpan("spanA").startActive(true)) { - try (Scope scopeB = opentracing.buildSpan("spanA").startActive(true)) { - assertThat(getTraceContext(scopeB).parentId()) - .isEqualTo(getTraceContext(scopeA).spanId()); - } - } - } - @Test public void implicitParentFromSpanManager_start() { - try (Scope scopeA = opentracing.buildSpan("spanA").startActive(true)) { + BraveSpan spanA = opentracing.buildSpan("spanA").start(); + try (Scope scopeA = opentracing.activateSpan(spanA)) { BraveSpan span = opentracing.buildSpan("spanB").start(); assertThat(span.unwrap().context().parentId()) - .isEqualTo(getTraceContext(scopeA).spanId()); - } - } - - @Test public void implicitParentFromSpanManager_startActive_ignoreActiveSpan() { - try (Scope scopeA = opentracing.buildSpan("spanA").startActive(true)) { - try (Scope scopeB = opentracing.buildSpan("spanA") - .ignoreActiveSpan().startActive(true)) { - assertThat(getTraceContext(scopeB).parentId()) - .isNull(); // new trace - } + .isEqualTo(brave.currentTraceContext().get().spanId()); } } @Test public void implicitParentFromSpanManager_start_ignoreActiveSpan() { - try (Scope scopeA = opentracing.buildSpan("spanA").startActive(true)) { + BraveSpan spanA = opentracing.buildSpan("spanA").start(); + try (Scope scopeA = opentracing.activateSpan(spanA)) { BraveSpan span = opentracing.buildSpan("spanB") .ignoreActiveSpan().start(); assertThat(span.unwrap().context().parentId()) @@ -371,13 +356,4 @@ void checkSpanReportedToZipkin() { assertThat(spans.get(0).tags()) .isEmpty(); } - - private static TraceContext getTraceContext(Scope scope) { - return ((BraveSpanContext) scope.span().context()).unwrap(); - } - - @After public void clear() { - Tracing current = Tracing.current(); - if (current != null) current.close(); - } } diff --git a/src/test/java/brave/opentracing/TextMapSetterTest.java b/src/test/java/brave/opentracing/TextMapSetterTest.java index f9bd8c7..102cece 100644 --- a/src/test/java/brave/opentracing/TextMapSetterTest.java +++ b/src/test/java/brave/opentracing/TextMapSetterTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018 The OpenZipkin Authors + * Copyright 2016-2019 The OpenZipkin Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -16,14 +16,14 @@ import brave.propagation.Propagation; import brave.test.propagation.PropagationSetterTest; import io.opentracing.propagation.TextMap; -import io.opentracing.propagation.TextMapInjectAdapter; +import io.opentracing.propagation.TextMapAdapter; import java.util.Collections; import java.util.LinkedHashMap; /** Verifies the method reference {@link TextMap#put} works as a Brave propagation setter */ public class TextMapSetterTest extends PropagationSetterTest { LinkedHashMap delegate = new LinkedHashMap<>(); - TextMap carrier = new TextMapInjectAdapter(delegate); + TextMap carrier = new TextMapAdapter(delegate); @Override public Propagation.KeyFactory keyFactory() { return Propagation.KeyFactory.STRING; From 80a427498da9d388c3bfdd6fdc3a6af75fbba4aa Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 25 Jun 2019 16:55:55 +0800 Subject: [PATCH 2/7] Avoids super dodgy code when using OpenTracing 0.33+ --- .../java/brave/opentracing/BraveScope.java | 36 +++---- .../brave/opentracing/BraveScopeManager.java | 93 ++++--------------- .../brave/opentracing/BraveSpanBuilder.java | 72 ++++++-------- .../java/brave/opentracing/BraveTracer.java | 17 ++-- .../brave/opentracing/OpenTracingVersion.java | 55 +++++++---- .../brave/opentracing/v0_32_BraveScope.java | 49 ++++++++++ .../opentracing/v0_32_BraveScopeManager.java | 93 +++++++++++++++++++ .../opentracing/v0_32_BraveSpanBuilder.java | 35 +++++++ .../OpenTracing0_33_BraveSpanBuilderTest.java | 2 +- 9 files changed, 282 insertions(+), 170 deletions(-) create mode 100644 src/main/java/brave/opentracing/v0_32_BraveScope.java create mode 100644 src/main/java/brave/opentracing/v0_32_BraveScopeManager.java create mode 100644 src/main/java/brave/opentracing/v0_32_BraveSpanBuilder.java diff --git a/src/main/java/brave/opentracing/BraveScope.java b/src/main/java/brave/opentracing/BraveScope.java index 5c184eb..ede5128 100644 --- a/src/main/java/brave/opentracing/BraveScope.java +++ b/src/main/java/brave/opentracing/BraveScope.java @@ -17,45 +17,31 @@ import io.opentracing.Scope; /** - * {@link BraveScope} is a simple {@link Scope} implementation that wraps the corresponding - * BraveSpan. + * {@link BraveScope} is a simple {@link Scope} implementation that wraps the corresponding {@link + * SpanInScope Brave scope}. * - * @see BraveSpan - * @see BraveScopeManager + * @see SpanInScope */ -public final class BraveScope implements Scope { - private final BraveScopeManager source; - private final SpanInScope scope; - private final BraveSpan wrapped; - private final boolean finishSpanOnClose; +public class BraveScope implements Scope { + final SpanInScope delegate; /** - * @param source the BraveActiveSpanSource that created this BraveActiveSpan - * @param scope a SpanInScope to be closed upon deactivation of this ActiveSpan - * @param wrapped the wrapped BraveSpan to which we will delegate all span operations + * @param delegate a SpanInScope to be closed upon deactivation of this ActiveSpan */ - BraveScope(BraveScopeManager source, SpanInScope scope, BraveSpan wrapped, - boolean finishSpanOnClose) { - this.source = source; - this.scope = scope; - this.wrapped = wrapped; - this.finishSpanOnClose = finishSpanOnClose; + BraveScope(SpanInScope delegate) { + this.delegate = delegate; } @Override public void close() { - if (finishSpanOnClose) { - wrapped.finish(); - } - scope.close(); - source.deregister(this); + delegate.close(); } /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ @Deprecated public BraveSpan span() { - return wrapped; + throw new UnsupportedOperationException("Not supported in OpenTracing 0.33+"); } @Override public String toString() { - return "BraveScope{scope=" + scope + ", wrapped=" + wrapped.delegate + '}'; + return "BraveScope(" + delegate + ")"; } } diff --git a/src/main/java/brave/opentracing/BraveScopeManager.java b/src/main/java/brave/opentracing/BraveScopeManager.java index 03d679c..361460d 100644 --- a/src/main/java/brave/opentracing/BraveScopeManager.java +++ b/src/main/java/brave/opentracing/BraveScopeManager.java @@ -14,101 +14,46 @@ package brave.opentracing; import brave.Tracer; -import brave.Tracing; import brave.propagation.CurrentTraceContext; import io.opentracing.Scope; import io.opentracing.ScopeManager; import io.opentracing.Span; -import java.util.ArrayDeque; -import java.util.Deque; /** This integrates with Brave's {@link CurrentTraceContext}. */ -public final class BraveScopeManager implements ScopeManager { - // This probably needs to be redesigned to stash the OpenTracing span in brave's .extra() - // We wouldn't have to do this if it weren't a requirement to return the same instance... - // - // When scopes are leaked this thread local will prevent this type from being unloaded. This can - // cause problems in redeployment scenarios. https://github.com/openzipkin/brave/issues/785 - final ThreadLocal> currentScopes = new ThreadLocal>() { - @Override protected Deque initialValue() { - return new ArrayDeque<>(); - } - }; - private final Tracer tracer; +public class BraveScopeManager implements ScopeManager { + final brave.Tracer tracer; - BraveScopeManager(Tracing tracing) { - tracer = tracing.tracer(); + BraveScopeManager(Tracer tracer) { + this.tracer = tracer; } - /** - * This api's only purpose is to retrieve the {@link BraveScope#span() span}. - * - * Calling {@link Scope#close() close } on the returned scope has no effect on the active span - */ - /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ - @Deprecated public Scope active() { - BraveSpan span = currentSpan(); + @Override public BraveScope activate(Span span) { if (span == null) return null; - return new Scope() { - @Override public void close() { - // no-op - } - - /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ - @Deprecated public Span span() { - return span; - } - }; + if (!(span instanceof BraveSpan)) { + throw new IllegalArgumentException( + "Span must be an instance of brave.opentracing.BraveSpan, but was " + span.getClass()); + } + return new BraveScope(tracer.withSpanInScope(((BraveSpan) span).delegate)); } @Override public BraveSpan activeSpan() { brave.Span braveSpan = tracer.currentSpan(); - if (braveSpan != null) { - return new BraveSpan(tracer, braveSpan); - } - return null; - } - - /** Attempts to get a span from the current api, falling back to brave's native one */ - @Deprecated BraveSpan currentSpan() { - BraveScope scope = currentScopes.get().peekFirst(); - if (scope != null) { - return scope.span(); - } else { - brave.Span braveSpan = tracer.currentSpan(); - if (braveSpan != null) { - return new BraveSpan(tracer, braveSpan); - } - } - return null; + return braveSpan != null ? new BraveSpan(tracer, braveSpan) : null; } - @Override public BraveScope activate(Span span) { - return activate(span, false); + /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ + @Deprecated public Scope active() { + throw new UnsupportedOperationException("Not supported in OpenTracing 0.33+"); } /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ @Deprecated public BraveScope activate(Span span, boolean finishSpanOnClose) { - if (span == null) return null; - if (!(span instanceof BraveSpan)) { - throw new IllegalArgumentException( - "Span must be an instance of brave.opentracing.BraveSpan, but was " + span.getClass()); - } - return newScope((BraveSpan) span, finishSpanOnClose); - } - - BraveScope newScope(BraveSpan span, boolean finishSpanOnClose) { - BraveScope result = new BraveScope( - this, - tracer.withSpanInScope(span.delegate), - span, - finishSpanOnClose - ); - currentScopes.get().addFirst(result); - return result; + throw new UnsupportedOperationException("Not supported in OpenTracing 0.33+"); } - void deregister(BraveScope span) { - currentScopes.get().remove(span); + /** Attempts to get a span from the current api, falling back to brave's native one */ + /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ + @Deprecated BraveSpan currentSpan() { + throw new UnsupportedOperationException("Not supported in OpenTracing 0.33+"); } } diff --git a/src/main/java/brave/opentracing/BraveSpanBuilder.java b/src/main/java/brave/opentracing/BraveSpanBuilder.java index 29ee694..7b907a1 100644 --- a/src/main/java/brave/opentracing/BraveSpanBuilder.java +++ b/src/main/java/brave/opentracing/BraveSpanBuilder.java @@ -16,7 +16,6 @@ import brave.propagation.TraceContext; import brave.sampler.Sampler; import io.opentracing.References; -import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.Tracer; @@ -34,21 +33,18 @@ *

Brave does not support multiple parents so this has been implemented to use the first parent * defined. */ -public final class BraveSpanBuilder implements Tracer.SpanBuilder { +public class BraveSpanBuilder implements Tracer.SpanBuilder { + final brave.Tracer tracer; + final Map tags = new LinkedHashMap<>(); - private final BraveTracer tracer; - private final brave.Tracer braveTracer; - private final Map tags = new LinkedHashMap<>(); + String operationName; + long timestamp; + int remotePort; + BraveSpanContext reference; + boolean ignoreActiveSpan = false; - private String operationName; - private long timestamp; - private int remotePort; - private BraveSpanContext reference; - private boolean ignoreActiveSpan = false; - - BraveSpanBuilder(BraveTracer tracer, brave.Tracer braveTracer, String operationName) { + BraveSpanBuilder(brave.Tracer tracer, String operationName) { this.tracer = tracer; - this.braveTracer = braveTracer; this.operationName = operationName; } @@ -61,9 +57,7 @@ public final class BraveSpanBuilder implements Tracer.SpanBuilder { } @Override public BraveSpanBuilder addReference(String type, SpanContext context) { - if (reference != null || context == null) { - return this; - } + if (reference != null || context == null) return this; if (References.CHILD_OF.equals(type) || References.FOLLOWS_FROM.equals(type)) { this.reference = (BraveSpanContext) context; } @@ -102,22 +96,6 @@ public final class BraveSpanBuilder implements Tracer.SpanBuilder { return this; } - /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ - @Deprecated public BraveSpan startManual() { - return start(); - } - - /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ - @Deprecated public BraveScope startActive(boolean finishSpanOnClose) { - if (!ignoreActiveSpan) { - BraveSpan parent = tracer.scopeManager().activeSpan(); - if (parent != null) { - asChildOf(parent.context()); - } - } - return tracer.scopeManager().activate(start(), finishSpanOnClose); - } - @Override public BraveSpanBuilder ignoreActiveSpan() { ignoreActiveSpan = true; return this; @@ -128,10 +106,8 @@ public final class BraveSpanBuilder implements Tracer.SpanBuilder { // Check if active span should be established as CHILD_OF relationship if (reference == null && !ignoreActiveSpan) { - BraveSpan parent = tracer.scopeManager().activeSpan(); - if (parent != null) { - asChildOf(parent.context()); - } + brave.Span parent = tracer.currentSpan(); + if (parent != null) asChildOf(BraveSpanContext.create(parent.context())); } brave.Span span; @@ -139,32 +115,32 @@ public final class BraveSpanBuilder implements Tracer.SpanBuilder { if (reference == null) { // adjust sampling decision, this reflects Zipkin's "before the fact" sampling policy // https://github.com/openzipkin/brave/tree/master/brave#sampling - brave.Tracer scopedBraveTracer = braveTracer; + brave.Tracer scopedTracer = tracer; String sampling = tags.get(Tags.SAMPLING_PRIORITY.getKey()); if (sampling != null) { try { Integer samplingPriority = Integer.valueOf(sampling); if (samplingPriority == 0) { - scopedBraveTracer = braveTracer.withSampler(Sampler.NEVER_SAMPLE); + scopedTracer = tracer.withSampler(Sampler.NEVER_SAMPLE); } else if (samplingPriority > 0) { - scopedBraveTracer = braveTracer.withSampler(Sampler.ALWAYS_SAMPLE); + scopedTracer = tracer.withSampler(Sampler.ALWAYS_SAMPLE); } } catch (NumberFormatException ex) { // ignore } } - span = scopedBraveTracer.newTrace(); + span = scopedTracer.newTrace(); } else if ((context = reference.unwrap()) != null) { // Zipkin's default is to share a span ID between the client and the server in an RPC. // When we start a server span with a parent, we assume the "parent" is actually the // client on the other side of the RPC. Accordingly, we join that span instead of fork. - span = server ? braveTracer.joinSpan(context) : braveTracer.newChild(context); + span = server ? tracer.joinSpan(context) : tracer.newChild(context); } else { - span = braveTracer.nextSpan(((BraveSpanContext.Incomplete) reference).extractionResult()); + span = tracer.nextSpan(((BraveSpanContext.Incomplete) reference).extractionResult()); } if (operationName != null) span.name(operationName); - BraveSpan result = new BraveSpan(braveTracer, span); + BraveSpan result = new BraveSpan(tracer, span); result.remotePort = remotePort; for (Map.Entry tag : tags.entrySet()) { result.setTag(tag.getKey(), tag.getValue()); @@ -178,4 +154,14 @@ public final class BraveSpanBuilder implements Tracer.SpanBuilder { return result; } + + /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ + @Deprecated public BraveSpan startManual() { + throw new UnsupportedOperationException("Not supported in OpenTracing 0.33+"); + } + + /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ + @Deprecated public BraveScope startActive(boolean finishSpanOnClose) { + throw new UnsupportedOperationException("Not supported in OpenTracing 0.33+"); + } } diff --git a/src/main/java/brave/opentracing/BraveTracer.java b/src/main/java/brave/opentracing/BraveTracer.java index 043834f..96c61cb 100644 --- a/src/main/java/brave/opentracing/BraveTracer.java +++ b/src/main/java/brave/opentracing/BraveTracer.java @@ -36,6 +36,7 @@ import java.util.Locale; import java.util.Map; import java.util.Set; +import javafx.application.Platform; /** * Using a tracer, you can create a spans, inject span contexts into a transport, and extract span @@ -59,10 +60,9 @@ * @see Propagation */ public final class BraveTracer implements Tracer { - - private final Tracing tracing; - private final brave.Tracer brave4; - private final BraveScopeManager scopeManager; + final Tracing tracing; + final brave.Tracer delegate; + final BraveScopeManager scopeManager; /** * Returns an implementation of {@link Tracer} which delegates to the provided Brave {@link @@ -130,8 +130,8 @@ public BraveTracer build() { BraveTracer(Builder b) { tracing = b.tracing; - brave4 = b.tracing.tracer(); - scopeManager = new BraveScopeManager(b.tracing); + delegate = b.tracing.tracer(); + scopeManager = OpenTracingVersion.get().scopeManager(b.tracing); for (Map.Entry, Propagation> entry : b.formatToPropagation.entrySet()) { formatToInjector.put(entry.getKey(), entry.getValue().injector(TEXT_MAP_SETTER)); formatToExtractor.put(entry.getKey(), new TextMapExtractorAdaptor(entry.getValue())); @@ -139,7 +139,8 @@ public BraveTracer build() { for (Propagation propagation : b.formatToPropagation.values()) { formatToInjector.put(Format.Builtin.TEXT_MAP_INJECT, propagation.injector(TEXT_MAP_SETTER)); - formatToExtractor.put(Format.Builtin.TEXT_MAP_EXTRACT, new TextMapExtractorAdaptor(propagation)); + formatToExtractor.put(Format.Builtin.TEXT_MAP_EXTRACT, + new TextMapExtractorAdaptor(propagation)); } } @@ -156,7 +157,7 @@ public BraveTracer build() { } @Override public BraveSpanBuilder buildSpan(String operationName) { - return new BraveSpanBuilder(this, brave4, operationName); + return OpenTracingVersion.get().spanBuilder(this, operationName); } /** diff --git a/src/main/java/brave/opentracing/OpenTracingVersion.java b/src/main/java/brave/opentracing/OpenTracingVersion.java index d522791..00a5db5 100644 --- a/src/main/java/brave/opentracing/OpenTracingVersion.java +++ b/src/main/java/brave/opentracing/OpenTracingVersion.java @@ -13,6 +13,7 @@ */ package brave.opentracing; +import brave.Tracing; import io.opentracing.ScopeManager; import io.opentracing.Span; @@ -28,8 +29,16 @@ static OpenTracingVersion get() { return INSTANCE; } + BraveScopeManager scopeManager(Tracing tracing) { + return new BraveScopeManager(tracing.tracer()); + } + + BraveSpanBuilder spanBuilder(BraveTracer braveTracer, String operationName) { + return new BraveSpanBuilder(braveTracer.delegate, operationName); + } + /** Attempt to match the host runtime to a capable OpenTracingVersion implementation. */ - static OpenTracingVersion findVersion() { + private static OpenTracingVersion findVersion() { OpenTracingVersion version = v0_32.buildIfSupported(); if (version != null) return version; @@ -39,43 +48,51 @@ static OpenTracingVersion findVersion() { throw new UnsupportedOperationException("Unsupported opentracing-api version"); } - static class v0_33 extends OpenTracingVersion { - static v0_33 buildIfSupported() { - // Find OpenTracing 0.32 new type + static class v0_32 extends OpenTracingVersion { + static v0_32 buildIfSupported() { + // Find OpenTracing 0.32 deprecated method try { - Class.forName("io.opentracing.tag.Tag"); - return new v0_33(); - } catch (ClassNotFoundException e) { + if (ScopeManager.class.getMethod("activate", Span.class, boolean.class) + .getAnnotation(Deprecated.class) != null) { + return new v0_32(); + } + } catch (NoSuchMethodException e) { } return null; } + @Override BraveScopeManager scopeManager(Tracing tracing) { + return new v0_32_BraveScopeManager(tracing.tracer()); + } + + @Override BraveSpanBuilder spanBuilder(BraveTracer braveTracer, String operationName) { + return new v0_32_BraveSpanBuilder(braveTracer.scopeManager, operationName); + } + @Override public String toString() { - return "v0_33{}"; + return "v0_32{}"; } - v0_33() { + v0_32() { } } - static class v0_32 extends OpenTracingVersion { - static v0_32 buildIfSupported() { - // Find OpenTracing 0.32 deprecated method + static class v0_33 extends OpenTracingVersion { + static v0_33 buildIfSupported() { + // Find OpenTracing 0.32 new type try { - if (ScopeManager.class.getMethod("activate", Span.class, boolean.class) - .getAnnotation(Deprecated.class) != null) { - return new v0_32(); - } - } catch (NoSuchMethodException e) { + Class.forName("io.opentracing.tag.Tag"); + return new v0_33(); + } catch (ClassNotFoundException e) { } return null; } @Override public String toString() { - return "v0_32{}"; + return "v0_33{}"; } - v0_32() { + v0_33() { } } } diff --git a/src/main/java/brave/opentracing/v0_32_BraveScope.java b/src/main/java/brave/opentracing/v0_32_BraveScope.java new file mode 100644 index 0000000..24bdd85 --- /dev/null +++ b/src/main/java/brave/opentracing/v0_32_BraveScope.java @@ -0,0 +1,49 @@ +/* + * Copyright 2016-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.opentracing; + +import brave.Tracer.SpanInScope; + +final class v0_32_BraveScope extends BraveScope { + final v0_32_BraveScopeManager source; + final BraveSpan wrapped; + final boolean finishSpanOnClose; + + /** + * @param delegate a SpanInScope to be closed upon deactivation of this ActiveSpan + * @param source the BraveActiveSpanSource that created this BraveActiveSpan + * @param wrapped the wrapped BraveSpan to which we will delegate all span operations + */ + v0_32_BraveScope(SpanInScope delegate, v0_32_BraveScopeManager source, BraveSpan wrapped, + boolean finishSpanOnClose) { + super(delegate); + this.source = source; + this.wrapped = wrapped; + this.finishSpanOnClose = finishSpanOnClose; + } + + @Override public void close() { + super.close(); + if (finishSpanOnClose) wrapped.finish(); + source.deregister(this); + } + + @Override @Deprecated public BraveSpan span() { + return wrapped; + } + + @Override public String toString() { + return "BraveScope{scope=" + delegate + ", wrapped=" + wrapped.delegate + '}'; + } +} diff --git a/src/main/java/brave/opentracing/v0_32_BraveScopeManager.java b/src/main/java/brave/opentracing/v0_32_BraveScopeManager.java new file mode 100644 index 0000000..30db739 --- /dev/null +++ b/src/main/java/brave/opentracing/v0_32_BraveScopeManager.java @@ -0,0 +1,93 @@ +/* + * Copyright 2016-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.opentracing; + +import brave.Tracer; +import brave.propagation.CurrentTraceContext; +import io.opentracing.Scope; +import io.opentracing.Span; +import java.util.ArrayDeque; +import java.util.Deque; + +/** This integrates with Brave's {@link CurrentTraceContext}. */ +final class v0_32_BraveScopeManager extends BraveScopeManager { + // This probably needs to be redesigned to stash the OpenTracing span in brave's .extra() + // We wouldn't have to do this if it weren't a requirement to return the same instance... + // + // When scopes are leaked this thread local will prevent this type from being unloaded. This can + // cause problems in redeployment scenarios. https://github.com/openzipkin/brave/issues/785 + final ThreadLocal> currentScopes = + new ThreadLocal>() { + @Override protected Deque initialValue() { + return new ArrayDeque<>(); + } + }; + + v0_32_BraveScopeManager(Tracer tracer) { + super(tracer); + } + + @Override @Deprecated public Scope active() { + BraveSpan span = currentSpan(); + if (span == null) return null; + return new Scope() { + @Override public void close() { + // no-op + } + + /* @Override deprecated 0.32 method: Intentionally no override to ensure 0.33 works! */ + @Deprecated public Span span() { + return span; + } + }; + } + + @Override @Deprecated BraveSpan currentSpan() { + BraveScope scope = currentScopes.get().peekFirst(); + if (scope != null) { + return scope.span(); + } else { + brave.Span braveSpan = tracer.currentSpan(); + if (braveSpan != null) { + return new BraveSpan(tracer, braveSpan); + } + } + return null; + } + + @Override public BraveScope activate(Span span) { + return activate(span, false); + } + + @Override @Deprecated public BraveScope activate(Span span, boolean finishSpanOnClose) { + if (span == null) return null; + if (!(span instanceof BraveSpan)) { + throw new IllegalArgumentException( + "Span must be an instance of brave.opentracing.BraveSpan, but was " + span.getClass()); + } + return newScope((BraveSpan) span, finishSpanOnClose); + } + + BraveScope newScope(BraveSpan span, boolean finishSpanOnClose) { + v0_32_BraveScope result = new v0_32_BraveScope( + tracer.withSpanInScope(span.delegate), this, span, finishSpanOnClose + ); + currentScopes.get().addFirst(result); + return result; + } + + void deregister(BraveScope span) { + currentScopes.get().remove(span); + } +} diff --git a/src/main/java/brave/opentracing/v0_32_BraveSpanBuilder.java b/src/main/java/brave/opentracing/v0_32_BraveSpanBuilder.java new file mode 100644 index 0000000..c01f301 --- /dev/null +++ b/src/main/java/brave/opentracing/v0_32_BraveSpanBuilder.java @@ -0,0 +1,35 @@ +/* + * Copyright 2016-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.opentracing; + +final class v0_32_BraveSpanBuilder extends BraveSpanBuilder { + final BraveScopeManager scopeManager; + + v0_32_BraveSpanBuilder(BraveScopeManager scopeManager, String operationName) { + super(scopeManager.tracer, operationName); + this.scopeManager = scopeManager; + } + + @Override @Deprecated public BraveSpan startManual() { + return start(); + } + + @Override @Deprecated public BraveScope startActive(boolean finishSpanOnClose) { + if (!ignoreActiveSpan) { + BraveSpan parent = scopeManager.activeSpan(); + if (parent != null) asChildOf(parent.context()); + } + return scopeManager.activate(start(), finishSpanOnClose); + } +} diff --git a/src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanBuilderTest.java b/src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanBuilderTest.java index 364f9d4..cd0b0b4 100644 --- a/src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanBuilderTest.java +++ b/src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanBuilderTest.java @@ -48,6 +48,6 @@ public class OpenTracing0_33_BraveSpanBuilderTest { BraveSpanBuilder newSpanBuilder() { // hijacking nullability as tracer isn't referenced until build, making easier comparisons - return new BraveSpanBuilder(null, null, "foo"); + return new BraveSpanBuilder(null, "foo"); } } From bb3f1320b96f6112a66a45e5f33cd0d4ab43393f Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 25 Jun 2019 17:20:07 +0800 Subject: [PATCH 3/7] Adds an error if opentracing-api 0.31.0 is in use It is very likely people will misconfigure their classpaths as there are compatibility issues between versions of opentracing. This raises the following message to tell people one way to resolve it. > OpenTracing 0.31 detected. This version is compatible with io.opentracing:opentracing-api 0.32 or 0.33. Use latest io.opentracing.brave:brave-opentracing:0.33 or update you opentracing-api --- .../OpenTracing0_31_UnsupportedTest.java | 32 +++++++++++++++++++ .../java/brave/opentracing/BraveTracer.java | 10 ++++++ .../brave/opentracing/OpenTracingVersion.java | 21 +++++++++++- .../OpenTracing0_33_BraveTracerTest.java | 5 +++ 4 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 src/it/opentracing-0.31/src/test/java/brave/opentracing/OpenTracing0_31_UnsupportedTest.java diff --git a/src/it/opentracing-0.31/src/test/java/brave/opentracing/OpenTracing0_31_UnsupportedTest.java b/src/it/opentracing-0.31/src/test/java/brave/opentracing/OpenTracing0_31_UnsupportedTest.java new file mode 100644 index 0000000..02c094b --- /dev/null +++ b/src/it/opentracing-0.31/src/test/java/brave/opentracing/OpenTracing0_31_UnsupportedTest.java @@ -0,0 +1,32 @@ +/* + * Copyright 2016-2019 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.opentracing; + +import brave.Tracing; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown; + +public class OpenTracing0_31_UnsupportedTest { + @Test public void unsupported() { + try (Tracing brave = Tracing.newBuilder().build()) { + BraveTracer.create(brave); + + failBecauseExceptionWasNotThrown(ExceptionInInitializerError.class); + } catch (UnsupportedOperationException e) { + assertThat(e.getMessage()).startsWith("OpenTracing 0.31 detected."); + } + } +} diff --git a/src/main/java/brave/opentracing/BraveTracer.java b/src/main/java/brave/opentracing/BraveTracer.java index 96c61cb..0ceb917 100644 --- a/src/main/java/brave/opentracing/BraveTracer.java +++ b/src/main/java/brave/opentracing/BraveTracer.java @@ -79,6 +79,16 @@ public static BraveTracer create(Tracing brave4) { * ScopeManager}. */ public static Builder newBuilder(Tracing brave4) { + // This is the only public entrypoint into the brave-opentracing bridge. The following will + // raise an exception when using an incompatible version of opentracing-api. Notably, this + // unwraps ExceptionInInitializerError to avoid confusing users, as this is an implementation + // detail of the version singleton. + try { + OpenTracingVersion.get(); + } catch (ExceptionInInitializerError e) { + if (e.getCause() instanceof RuntimeException) throw (RuntimeException) e.getCause(); + throw e; + } return new Builder(brave4); } diff --git a/src/main/java/brave/opentracing/OpenTracingVersion.java b/src/main/java/brave/opentracing/OpenTracingVersion.java index 00a5db5..89b5dae 100644 --- a/src/main/java/brave/opentracing/OpenTracingVersion.java +++ b/src/main/java/brave/opentracing/OpenTracingVersion.java @@ -39,13 +39,32 @@ BraveSpanBuilder spanBuilder(BraveTracer braveTracer, String operationName) { /** Attempt to match the host runtime to a capable OpenTracingVersion implementation. */ private static OpenTracingVersion findVersion() { + if (isV0_31()) { + throw new UnsupportedOperationException("OpenTracing 0.31 detected. " + + "This version is compatible with io.opentracing:opentracing-api 0.32 or 0.33. " + + "Use latest io.opentracing.brave:brave-opentracing:0.33 or update you opentracing-api"); + } + OpenTracingVersion version = v0_32.buildIfSupported(); if (version != null) return version; version = v0_33.buildIfSupported(); if (version != null) return version; - throw new UnsupportedOperationException("Unsupported opentracing-api version"); + throw new UnsupportedOperationException( + "This is only compatible with io.opentracing:opentracing-api 0.32 or 0.33"); + } + + static boolean isV0_31() { + // Find OpenTracing 0.31 method + try { + if (ScopeManager.class.getMethod("activate", Span.class, boolean.class) + .getAnnotation(Deprecated.class) == null) { + return true; + } + } catch (NoSuchMethodException e) { + } + return false; } static class v0_32 extends OpenTracingVersion { diff --git a/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java b/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java index 50c48a4..038440c 100644 --- a/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java +++ b/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java @@ -63,6 +63,11 @@ public class OpenTracing0_33_BraveTracerTest { brave.close(); } + @Test public void versionIsCorrect() { + assertThat(OpenTracingVersion.get()) + .isInstanceOf(OpenTracingVersion.v0_32.class); + } + @Test public void startWithOpenTracingAndFinishWithBrave() { io.opentracing.Span openTracingSpan = opentracing.buildSpan("encode") .withTag("lc", "codec") From 1d361c22964ea76757f68a5589946c0e412680ef Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 25 Jun 2019 17:24:24 +0800 Subject: [PATCH 4/7] polishes error message --- src/main/java/brave/opentracing/OpenTracingVersion.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/brave/opentracing/OpenTracingVersion.java b/src/main/java/brave/opentracing/OpenTracingVersion.java index 89b5dae..3846e54 100644 --- a/src/main/java/brave/opentracing/OpenTracingVersion.java +++ b/src/main/java/brave/opentracing/OpenTracingVersion.java @@ -42,7 +42,7 @@ private static OpenTracingVersion findVersion() { if (isV0_31()) { throw new UnsupportedOperationException("OpenTracing 0.31 detected. " + "This version is compatible with io.opentracing:opentracing-api 0.32 or 0.33. " - + "Use latest io.opentracing.brave:brave-opentracing:0.33 or update you opentracing-api"); + + "io.opentracing.brave:brave-opentracing:0.33.13+ works with version 0.31"); } OpenTracingVersion version = v0_32.buildIfSupported(); From 097705da4661f7ee385596116b22e3a5c925e11f Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 25 Jun 2019 17:54:08 +0800 Subject: [PATCH 5/7] scrubs bad import --- src/main/java/brave/opentracing/BraveTracer.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/brave/opentracing/BraveTracer.java b/src/main/java/brave/opentracing/BraveTracer.java index 0ceb917..4e3cd65 100644 --- a/src/main/java/brave/opentracing/BraveTracer.java +++ b/src/main/java/brave/opentracing/BraveTracer.java @@ -36,7 +36,9 @@ import java.util.Locale; import java.util.Map; import java.util.Set; -import javafx.application.Platform; + +import static io.opentracing.propagation.Format.Builtin.TEXT_MAP_EXTRACT; +import static io.opentracing.propagation.Format.Builtin.TEXT_MAP_INJECT; /** * Using a tracer, you can create a spans, inject span contexts into a transport, and extract span @@ -103,7 +105,6 @@ public static final class Builder { formatToPropagation.put(Format.Builtin.HTTP_HEADERS, tracing.propagation()); formatToPropagation.put(Format.Builtin.TEXT_MAP, tracing.propagation()); - // TODO: TEXT_MAP_INJECT and TEXT_MAP_EXTRACT (not excited about this) } /** @@ -148,9 +149,8 @@ public BraveTracer build() { } for (Propagation propagation : b.formatToPropagation.values()) { - formatToInjector.put(Format.Builtin.TEXT_MAP_INJECT, propagation.injector(TEXT_MAP_SETTER)); - formatToExtractor.put(Format.Builtin.TEXT_MAP_EXTRACT, - new TextMapExtractorAdaptor(propagation)); + formatToInjector.put(TEXT_MAP_INJECT, propagation.injector(TEXT_MAP_SETTER)); + formatToExtractor.put(TEXT_MAP_EXTRACT, new TextMapExtractorAdaptor(propagation)); } } From 41a21868ab698f0b50385717c12916a6da2b4888 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 25 Jun 2019 20:51:02 +0800 Subject: [PATCH 6/7] Backfills a crapload of tests --- .../brave/opentracing/BraveSpanBuilder.java | 5 +- .../java/brave/opentracing/BraveTracer.java | 1 + .../OpenTracing0_33_BraveSpanTest.java | 115 ++++++++++++- .../OpenTracing0_33_BraveTracerTest.java | 157 ++++++++++-------- 4 files changed, 202 insertions(+), 76 deletions(-) diff --git a/src/main/java/brave/opentracing/BraveSpanBuilder.java b/src/main/java/brave/opentracing/BraveSpanBuilder.java index 7b907a1..b17f00d 100644 --- a/src/main/java/brave/opentracing/BraveSpanBuilder.java +++ b/src/main/java/brave/opentracing/BraveSpanBuilder.java @@ -19,6 +19,7 @@ import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.Tracer; +import io.opentracing.tag.Tag; import io.opentracing.tag.Tags; import java.util.LinkedHashMap; import java.util.Map; @@ -82,13 +83,13 @@ public class BraveSpanBuilder implements Tracer.SpanBuilder { return withTag(key, value.toString()); } - @Override public BraveSpanBuilder withTag(io.opentracing.tag.Tag tag, T value) { + @Override public BraveSpanBuilder withTag(Tag tag, T value) { if (tag == null) throw new NullPointerException("tag == null"); if (value == null) throw new NullPointerException("value == null"); if (value instanceof String) return withTag(tag.getKey(), (String) value); if (value instanceof Number) return withTag(tag.getKey(), (Number) value); if (value instanceof Boolean) return withTag(tag.getKey(), (Boolean) value); - throw new IllegalArgumentException("tag value not a string, number or boolean: " + tag); + throw new IllegalArgumentException("tag value not a string, number or boolean: " + value); } @Override public BraveSpanBuilder withStartTimestamp(long microseconds) { diff --git a/src/main/java/brave/opentracing/BraveTracer.java b/src/main/java/brave/opentracing/BraveTracer.java index 4e3cd65..14bd8cd 100644 --- a/src/main/java/brave/opentracing/BraveTracer.java +++ b/src/main/java/brave/opentracing/BraveTracer.java @@ -148,6 +148,7 @@ public BraveTracer build() { formatToExtractor.put(entry.getKey(), new TextMapExtractorAdaptor(entry.getValue())); } + // Now, go back and make sure the special inject/extract forms work for (Propagation propagation : b.formatToPropagation.values()) { formatToInjector.put(TEXT_MAP_INJECT, propagation.injector(TEXT_MAP_SETTER)); formatToExtractor.put(TEXT_MAP_EXTRACT, new TextMapExtractorAdaptor(propagation)); diff --git a/src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanTest.java b/src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanTest.java index 38f88fd..70cf176 100644 --- a/src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanTest.java +++ b/src/test/java/brave/opentracing/OpenTracing0_33_BraveSpanTest.java @@ -18,18 +18,21 @@ import brave.propagation.ExtraFieldPropagation; import brave.propagation.StrictScopeDecorator; import brave.propagation.ThreadLocalCurrentTraceContext; +import brave.sampler.Sampler; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.propagation.TextMapAdapter; +import io.opentracing.tag.Tag; import io.opentracing.tag.Tags; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import zipkin2.Endpoint; @@ -43,15 +46,24 @@ @RunWith(DataProviderRunner.class) public class OpenTracing0_33_BraveSpanTest { List spans = new ArrayList<>(); - Tracing brave = Tracing.newBuilder() - .localServiceName("tracer") - .currentTraceContext(ThreadLocalCurrentTraceContext.newBuilder() - .addScopeDecorator(StrictScopeDecorator.create()) - .build()) - .propagationFactory(ExtraFieldPropagation.newFactory(B3Propagation.FACTORY, "client-id")) - .spanReporter(spans::add).build(); + Tracing brave; + BraveTracer tracer; - BraveTracer tracer = BraveTracer.create(brave); + @Before public void init() { + init(Tracing.newBuilder()); + } + + void init(Tracing.Builder tracingBuilder) { + if (brave != null) brave.close(); + brave = tracingBuilder + .localServiceName("tracer") + .currentTraceContext(ThreadLocalCurrentTraceContext.newBuilder() + .addScopeDecorator(StrictScopeDecorator.create()) + .build()) + .propagationFactory(ExtraFieldPropagation.newFactory(B3Propagation.FACTORY, "client-id")) + .spanReporter(spans::add).build(); + tracer = BraveTracer.create(brave); + } @After public void clear() { brave.close(); @@ -210,6 +222,20 @@ public void spanKind_afterStart(String tagValue, Kind kind) { serverSpan.finish(); } + @Test public void samplingPriority_sampledWhenAtStart() { + init(Tracing.newBuilder().sampler(Sampler.NEVER_SAMPLE)); + + BraveSpan span = tracer.buildSpan("foo") + .withTag(SAMPLING_PRIORITY.getKey(), 1) + .start(); + + assertThat(span.context().unwrap().sampled()) + .isTrue(); + + span.finish(); + assertThat(spans).hasSize(1); + } + @Test public void samplingPriority_unsampledWhenAtStart() { BraveSpan span = tracer.buildSpan("foo") .withTag(SAMPLING_PRIORITY.getKey(), 0) @@ -271,4 +297,77 @@ public void spanKind_afterStart(String tagValue, Kind kind) { .ip("2001:db8::c001") .port(8080).build()); } + + @Test public void withTag() { + tracer.buildSpan("encode") + .withTag(Tags.HTTP_METHOD.getKey(), "GET") + .withTag(Tags.ERROR.getKey(), true) + .withTag(Tags.HTTP_STATUS.getKey(), 404) + .start().finish(); + + assertContainsTags(); + } + + @Test public void withTag_object() { + tracer.buildSpan("encode") + .withTag(Tags.HTTP_METHOD, "GET") + .withTag(Tags.ERROR, true) + .withTag(Tags.HTTP_STATUS, 404) + .start().finish(); + + assertContainsTags(); + } + + @Test public void setTag() { + tracer.buildSpan("encode").start() + .setTag(Tags.HTTP_METHOD.getKey(), "GET") + .setTag(Tags.ERROR.getKey(), true) + .setTag(Tags.HTTP_STATUS.getKey(), 404) + .finish(); + + assertContainsTags(); + } + + @Test public void setTag_object() { + tracer.buildSpan("encode").start() + .setTag(Tags.HTTP_METHOD, "GET") + .setTag(Tags.ERROR, true) + .setTag(Tags.HTTP_STATUS, 404) + .finish(); + + assertContainsTags(); + } + + void assertContainsTags() { + assertThat(spans.get(0).tags()) + .containsEntry("http.method", "GET") + .containsEntry("error", "true") + .containsEntry("http.status_code", "404"); + } + + Tag exceptionTag = new Tag() { + @Override public String getKey() { + return "exception"; + } + + @Override public void set(Span span, Exception value) { + span.setTag(getKey(), value.getClass().getSimpleName()); + } + }; + + @Test public void setTag_custom() { + tracer.buildSpan("encode").start() + .setTag(exceptionTag, new RuntimeException("ice cream")).finish(); + + assertThat(spans.get(0).tags()) + .containsEntry("exception", "RuntimeException"); + } + + /** There is no javadoc, but we were told only string, bool or number? */ + @Test(expected = IllegalArgumentException.class) + public void withTag_custom_unsupported() { + tracer.buildSpan("encode") + .withTag(exceptionTag, new RuntimeException("ice cream")); + } + } diff --git a/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java b/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java index 038440c..0c4a5c7 100644 --- a/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java +++ b/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java @@ -22,6 +22,9 @@ import brave.propagation.StrictScopeDecorator; import brave.propagation.ThreadLocalCurrentTraceContext; import brave.propagation.TraceContext; +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; import io.opentracing.Scope; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; @@ -33,11 +36,15 @@ import java.util.Map; import org.junit.After; import org.junit.Test; +import org.junit.runner.RunWith; import zipkin2.Annotation; import static io.opentracing.propagation.Format.Builtin.HTTP_HEADERS; import static io.opentracing.propagation.Format.Builtin.TEXT_MAP; +import static io.opentracing.propagation.Format.Builtin.TEXT_MAP_EXTRACT; +import static io.opentracing.propagation.Format.Builtin.TEXT_MAP_INJECT; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown; import static org.assertj.core.data.MapEntry.entry; import static org.junit.Assert.assertEquals; @@ -45,7 +52,12 @@ * This shows how one might make an OpenTracing adapter for Brave, and how to navigate in and out of * the core concepts. */ +@RunWith(DataProviderRunner.class) public class OpenTracing0_33_BraveTracerTest { + TraceContext context = TraceContext.newBuilder() + .traceId(1L) + .spanId(2L) + .sampled(true).build(); List spans = new ArrayList<>(); Tracing brave = Tracing.newBuilder() @@ -95,83 +107,91 @@ public class OpenTracing0_33_BraveTracerTest { checkSpanReportedToZipkin(); } - @Test public void extractTraceContext() { + @DataProvider public static Object[] dataProviderExtractTextFormats() { + return new Object[] {HTTP_HEADERS, TEXT_MAP, TEXT_MAP_EXTRACT}; + } + + @Test @UseDataProvider("dataProviderExtractTextFormats") + public void extractTraceContext(Format format) { Map map = new LinkedHashMap<>(); map.put("X-B3-TraceId", "0000000000000001"); map.put("X-B3-SpanId", "0000000000000002"); map.put("X-B3-Sampled", "1"); - BraveSpanContext otContext = opentracing.extract(HTTP_HEADERS, new TextMapAdapter(map)); - - assertThat(otContext.unwrap()) - .isEqualTo(TraceContext.newBuilder() - .traceId(1L) - .spanId(2L) - .sampled(true).build()); + assertExtractedContext(format, new TextMapAdapter(map)); } - @Test public void extractBaggage() { + @Test @UseDataProvider("dataProviderExtractTextFormats") + public void extractBaggage(Format format) { Map map = new LinkedHashMap<>(); map.put("X-B3-TraceId", "0000000000000001"); map.put("X-B3-SpanId", "0000000000000002"); map.put("X-B3-Sampled", "1"); map.put("baggage-country-code", "FO"); - BraveSpanContext otContext = opentracing.extract(HTTP_HEADERS, new TextMapAdapter(map)); + BraveSpanContext otContext = opentracing.extract(format, new TextMapAdapter(map)); assertThat(otContext.baggageItems()) .containsExactly(entry("country-code", "FO")); } - @Test public void extractTraceContextTextMap() { + @Test @UseDataProvider("dataProviderExtractTextFormats") + public void extractOnlyBaggage(Format format) { + Map map = new LinkedHashMap<>(); + map.put("baggage-country-code", "FO"); + + BraveSpanContext otContext = opentracing.extract(format, new TextMapAdapter(map)); + + assertThat(otContext.toTraceId()).isNull(); + assertThat(otContext.toSpanId()).isNull(); + assertThat(otContext.unwrap()).isNull(); + assertThat(otContext.baggageItems()) + .containsExactly(entry("country-code", "FO")); + } + + @Test @UseDataProvider("dataProviderExtractTextFormats") + public void extractOnlySampled(Format format) { Map map = new LinkedHashMap<>(); - map.put("X-B3-TraceId", "0000000000000001"); - map.put("X-B3-SpanId", "0000000000000002"); map.put("X-B3-Sampled", "1"); - BraveSpanContext otContext = opentracing.extract(TEXT_MAP, new TextMapAdapter(map)); + BraveSpanContext otContext = opentracing.extract(format, new TextMapAdapter(map)); - assertThat(otContext.unwrap()) - .isEqualTo(TraceContext.newBuilder() - .traceId(1L) - .spanId(2L) - .sampled(true).build()); + assertThat(otContext.toTraceId()).isNull(); + assertThat(otContext.toSpanId()).isNull(); + assertThat(otContext.unwrap()).isNull(); } - @Test public void extractTraceContextCaseInsensitive() { + @Test @UseDataProvider("dataProviderExtractTextFormats") + public void extractTraceContextCaseInsensitive(Format format) { Map map = new LinkedHashMap<>(); map.put("X-B3-TraceId", "0000000000000001"); map.put("x-b3-spanid", "0000000000000002"); map.put("x-b3-SaMpLeD", "1"); map.put("other", "1"); - BraveSpanContext otContext = opentracing.extract(HTTP_HEADERS, new TextMapAdapter(map)); - - assertThat(otContext.unwrap()) - .isEqualTo(TraceContext.newBuilder() - .traceId(1L) - .spanId(2L) - .sampled(true).build()); + assertExtractedContext(format, new TextMapAdapter(map)); } - @Test public void extractTraceContext_unwrapReturnsNull() { - Map map = new LinkedHashMap<>(); - map.put("other", "1"); + void assertExtractedContext(Format format, C carrier) { + BraveSpanContext otContext = opentracing.extract(format, carrier); - BraveSpanContext otContext = opentracing.extract(HTTP_HEADERS, new TextMapAdapter(map)); - - assertThat(otContext.unwrap()).isNull(); + assertThat(otContext.toTraceId()) + .isEqualTo(otContext.unwrap().traceIdString()); + assertThat(otContext.toSpanId()) + .isEqualTo(otContext.unwrap().spanIdString()); + assertThat(otContext.unwrap()) + .isEqualTo(TraceContext.newBuilder().traceId(1L).spanId(2L).sampled(true).build()); } - @Test public void injectTraceContext() { - TraceContext context = TraceContext.newBuilder() - .traceId(1L) - .spanId(2L) - .sampled(true).build(); + @DataProvider public static Object[] dataProviderInjectTextFormats() { + return new Object[] {HTTP_HEADERS, TEXT_MAP, TEXT_MAP_INJECT}; + } + @Test @UseDataProvider("dataProviderInjectTextFormats") + public void injectTraceContext(Format format) { Map map = new LinkedHashMap<>(); TextMapAdapter carrier = new TextMapAdapter(map); - opentracing.inject(BraveSpanContext.create(context), HTTP_HEADERS, carrier); + opentracing.inject(BraveSpanContext.create(context), format, carrier); assertThat(map).containsExactly( entry("X-B3-TraceId", "0000000000000001"), @@ -180,54 +200,51 @@ public class OpenTracing0_33_BraveTracerTest { ); } - @Test public void injectTraceContext_baggage() { + @Test @UseDataProvider("dataProviderInjectTextFormats") + public void injectTraceContext_baggage(Format format) { BraveSpan span = opentracing.buildSpan("foo").start(); span.setBaggageItem("country-code", "FO"); Map map = new LinkedHashMap<>(); TextMapAdapter carrier = new TextMapAdapter(map); - opentracing.inject(span.context(), HTTP_HEADERS, carrier); + opentracing.inject(span.context(), format, carrier); assertThat(map).containsEntry("baggage-country-code", "FO"); } - @Test public void injectTraceContextTextMap() { - TraceContext context = TraceContext.newBuilder() - .traceId(1L) - .spanId(2L) - .sampled(true).build(); - + @Test public void unsupportedFormat() { Map map = new LinkedHashMap<>(); TextMapAdapter carrier = new TextMapAdapter(map); - opentracing.inject(BraveSpanContext.create(context), TEXT_MAP, carrier); + Format B3 = new Format() { + }; - assertThat(map).containsExactly( - entry("X-B3-TraceId", "0000000000000001"), - entry("X-B3-SpanId", "0000000000000002"), - entry("X-B3-Sampled", "1") - ); + try { + opentracing.inject(BraveSpanContext.create(context), B3, carrier); + failBecauseExceptionWasNotThrown(UnsupportedOperationException.class); + } catch (UnsupportedOperationException e){ + } + + try { + opentracing.extract(B3, carrier); + failBecauseExceptionWasNotThrown(UnsupportedOperationException.class); + } catch (UnsupportedOperationException e){ + } } @Test public void canUseCustomFormatKeys() { + Map map = new LinkedHashMap<>(); + TextMapAdapter carrier = new TextMapAdapter(map); Format B3 = new Format() { }; - opentracing = BraveTracer.newBuilder(brave) - .textMapPropagation(B3, Propagation.B3_STRING).build(); - TraceContext context = TraceContext.newBuilder() - .traceId(1L) - .spanId(2L) - .sampled(true).build(); + opentracing = BraveTracer.newBuilder(brave) + .textMapPropagation(B3, Propagation.B3_SINGLE_STRING).build(); - Map map = new LinkedHashMap<>(); - TextMapAdapter carrier = new TextMapAdapter(map); opentracing.inject(BraveSpanContext.create(context), B3, carrier); - assertThat(map).containsExactly( - entry("X-B3-TraceId", "0000000000000001"), - entry("X-B3-SpanId", "0000000000000002"), - entry("X-B3-Sampled", "1") - ); + assertThat(map).containsEntry("b3", "0000000000000001-0000000000000002-1"); + + assertExtractedContext(B3, new TextMapAdapter(map)); } void checkSpanReportedToZipkin() { @@ -324,6 +341,14 @@ void checkSpanReportedToZipkin() { assertEquals("SpanC's parent should be SpanA", idOfSpanA, parentIdOfSpanC); } + @Test public void activeSpan() { + BraveSpan spanA = opentracing.buildSpan("spanA").start(); + try (Scope scopeA = opentracing.activateSpan(spanA)) { + assertThat(opentracing.activeSpan()) + .isEqualToComparingFieldByField(opentracing.scopeManager().activeSpan()); + } + } + @Test public void implicitParentFromSpanManager_start() { BraveSpan spanA = opentracing.buildSpan("spanA").start(); try (Scope scopeA = opentracing.activateSpan(spanA)) { From 76b006a2d8543dc1a38227fc56d92d2299d28001 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 25 Jun 2019 21:14:12 +0800 Subject: [PATCH 7/7] Adds binary format support --- .../java/brave/opentracing/BraveTracer.java | 38 +++++++++++++++++++ .../OpenTracing0_33_BraveTracerTest.java | 22 ++++++++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/main/java/brave/opentracing/BraveTracer.java b/src/main/java/brave/opentracing/BraveTracer.java index 14bd8cd..3c46806 100644 --- a/src/main/java/brave/opentracing/BraveTracer.java +++ b/src/main/java/brave/opentracing/BraveTracer.java @@ -14,6 +14,7 @@ package brave.opentracing; import brave.Tracing; +import brave.propagation.B3SingleFormat; import brave.propagation.CurrentTraceContext; import brave.propagation.ExtraFieldPropagation; import brave.propagation.Propagation; @@ -27,8 +28,11 @@ import io.opentracing.Span; import io.opentracing.SpanContext; import io.opentracing.Tracer; +import io.opentracing.propagation.BinaryExtract; +import io.opentracing.propagation.BinaryInject; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; +import java.nio.charset.Charset; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -37,6 +41,9 @@ import java.util.Map; import java.util.Set; +import static io.opentracing.propagation.Format.Builtin.BINARY; +import static io.opentracing.propagation.Format.Builtin.BINARY_EXTRACT; +import static io.opentracing.propagation.Format.Builtin.BINARY_INJECT; import static io.opentracing.propagation.Format.Builtin.TEXT_MAP_EXTRACT; import static io.opentracing.propagation.Format.Builtin.TEXT_MAP_INJECT; @@ -58,6 +65,11 @@ * Span clientSpan = tracer.buildSpan('...').asChildOf(clientContext).start(); * * + *

Propagation

+ * This uses the same propagation as defined in zipkin for text formats. B3 Single is used for + * binary formats. + * * @see BraveSpan * @see Propagation */ @@ -153,6 +165,12 @@ public BraveTracer build() { formatToInjector.put(TEXT_MAP_INJECT, propagation.injector(TEXT_MAP_SETTER)); formatToExtractor.put(TEXT_MAP_EXTRACT, new TextMapExtractorAdaptor(propagation)); } + + // Finally add binary support + formatToInjector.put(BINARY, BinaryCodec.INSTANCE); + formatToInjector.put(BINARY_INJECT, BinaryCodec.INSTANCE); + formatToExtractor.put(BINARY, BinaryCodec.INSTANCE); + formatToExtractor.put(BINARY_EXTRACT, BinaryCodec.INSTANCE); } @Override public BraveScopeManager scopeManager() { @@ -259,4 +277,24 @@ static Set lowercaseSet(List fields) { } return lcSet; } + + // Temporary until https://github.com/openzipkin/brave/issues/928 + enum BinaryCodec implements Injector, Extractor { + INSTANCE; + + final Charset ascii = Charset.forName("US-ASCII"); + + @Override public TraceContextOrSamplingFlags extract(BinaryExtract binaryExtract) { + try { + return B3SingleFormat.parseB3SingleFormat(ascii.decode(binaryExtract.extractionBuffer())); + } catch (RuntimeException e) { + return TraceContextOrSamplingFlags.EMPTY; + } + } + + @Override public void inject(TraceContext traceContext, BinaryInject binaryInject) { + byte[] injected = B3SingleFormat.writeB3SingleFormatAsBytes(traceContext); + binaryInject.injectionBuffer(injected.length).put(injected); + } + } } diff --git a/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java b/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java index 0c4a5c7..1b6661f 100644 --- a/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java +++ b/src/test/java/brave/opentracing/OpenTracing0_33_BraveTracerTest.java @@ -26,9 +26,12 @@ import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; import io.opentracing.Scope; +import io.opentracing.propagation.BinaryAdapters; +import io.opentracing.propagation.BinaryInject; import io.opentracing.propagation.Format; import io.opentracing.propagation.TextMap; import io.opentracing.propagation.TextMapAdapter; +import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; import java.util.LinkedHashMap; @@ -39,6 +42,11 @@ import org.junit.runner.RunWith; import zipkin2.Annotation; +import static io.opentracing.propagation.BinaryAdapters.extractionCarrier; +import static io.opentracing.propagation.BinaryAdapters.injectionCarrier; +import static io.opentracing.propagation.Format.Builtin.BINARY; +import static io.opentracing.propagation.Format.Builtin.BINARY_EXTRACT; +import static io.opentracing.propagation.Format.Builtin.BINARY_INJECT; import static io.opentracing.propagation.Format.Builtin.HTTP_HEADERS; import static io.opentracing.propagation.Format.Builtin.TEXT_MAP; import static io.opentracing.propagation.Format.Builtin.TEXT_MAP_EXTRACT; @@ -221,13 +229,13 @@ public void injectTraceContext_baggage(Format format) { try { opentracing.inject(BraveSpanContext.create(context), B3, carrier); failBecauseExceptionWasNotThrown(UnsupportedOperationException.class); - } catch (UnsupportedOperationException e){ + } catch (UnsupportedOperationException e) { } try { opentracing.extract(B3, carrier); failBecauseExceptionWasNotThrown(UnsupportedOperationException.class); - } catch (UnsupportedOperationException e){ + } catch (UnsupportedOperationException e) { } } @@ -247,6 +255,16 @@ public void injectTraceContext_baggage(Format format) { assertExtractedContext(B3, new TextMapAdapter(map)); } + @Test public void binaryFormat() { + ByteBuffer buffer = ByteBuffer.allocate(128); + + opentracing.inject(BraveSpanContext.create(context), BINARY_INJECT, injectionCarrier(buffer)); + buffer.rewind(); + + assertThat(opentracing.extract(BINARY_EXTRACT, extractionCarrier(buffer)).unwrap()) + .isEqualTo(context); + } + void checkSpanReportedToZipkin() { assertThat(spans).first().satisfies(s -> { assertThat(s.name()).isEqualTo("encode");