From fabff6c2f5159fea845f585140e256708c1fbec1 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sun, 10 Sep 2017 10:39:35 +0200 Subject: [PATCH] Adds v2 StorageComponent and allows the server to use it directly (#1729) This adds a type for the v2 StorageComponent so that future storage implementations can be made without a compile dependency on the v1 jar. This is internal until everything is verified. --- .../benchmarks/Span2ConverterBenchmarks.java | 6 +- .../java/zipkin/junit/ZipkinDispatcher.java | 10 ++- .../main/java/zipkin/junit/ZipkinRule.java | 16 ++-- .../java/zipkin/junit/v2/HttpV2Storage.java | 9 ++- .../java/zipkin/junit/v2/ITHttpV2Storage.java | 10 ++- .../java/zipkin/server/ZipkinQueryApiV2.java | 18 ++--- .../server/ZipkinServerConfiguration.java | 25 +++++- .../server/ZipkinServerIntegrationTest.java | 4 +- .../server/ZipkinServerV2StorageTest.java | 59 ++++++++++++++ .../storage/cassandra/ITCassandraStorage.java | 2 +- .../integration/ITCassandra3Storage.java | 2 +- .../http/ElasticsearchHttpStorage.java | 45 ++++++++--- .../ElasticsearchHttpSpanConsumerTest.java | 5 +- .../http/VersionSpecificTemplatesTest.java | 3 +- .../ElasticsearchHttpSpanConsumerTest.java | 43 +++++----- .../ITElasticsearchHttpStorageV2.java | 2 +- .../ITElasticsearchHttpStorageV5.java | 2 +- .../ITElasticsearchHttpStorageV6.java | 2 +- .../LazyElasticsearchHttpStorage.java | 8 +- .../zipkin/storage/mysql/ITMySQLStorage.java | 2 +- .../main/java/zipkin/collector/Collector.java | 5 +- .../main/java/zipkin/internal/JsonCodec.java | 4 +- .../java/zipkin/internal/V2Collector.java | 10 ++- .../zipkin/internal/V2InMemoryStorage.java | 71 ----------------- .../java/zipkin/internal/V2SpanConverter.java | 26 ++++--- .../zipkin/internal/V2StorageComponent.java | 76 +++++++++++++++--- .../java/zipkin/internal/v2/CheckResult.java | 45 +++++++++++ .../java/zipkin/internal/v2/Component.java | 49 ++++++++++++ .../java/zipkin/internal/v2/Endpoint.java | 4 +- .../zipkin/internal/v2/internal/Buffer.java | 3 +- .../internal/v2/internal/V1SpanWriter.java | 11 ++- .../internal/v2/internal/V2SpanWriter.java | 2 + .../internal/v2/storage/InMemoryStorage.java | 22 ++++-- .../internal/v2/storage/StorageComponent.java | 78 +++++++++++++++++++ .../java/zipkin/collector/CollectorTest.java | 10 +-- .../zipkin/internal/V2SpanConverterTest.java | 73 +++++++++-------- .../internal/V2SpanStoreAdapterTest.java | 5 +- .../java/zipkin/internal/v2/SpanTest.java | 4 +- .../v2/codec/SpanBytesEncoderTest.java | 12 +-- .../{ => v2/storage}/ITV2InMemoryStorage.java | 21 ++--- 40 files changed, 543 insertions(+), 261 deletions(-) create mode 100644 zipkin-server/src/test/java/zipkin/server/ZipkinServerV2StorageTest.java delete mode 100644 zipkin/src/main/java/zipkin/internal/V2InMemoryStorage.java create mode 100644 zipkin/src/main/java/zipkin/internal/v2/CheckResult.java create mode 100644 zipkin/src/main/java/zipkin/internal/v2/Component.java create mode 100644 zipkin/src/main/java/zipkin/internal/v2/storage/StorageComponent.java rename zipkin/src/test/java/zipkin/internal/{ => v2/storage}/ITV2InMemoryStorage.java (74%) diff --git a/benchmarks/src/main/java/zipkin/benchmarks/Span2ConverterBenchmarks.java b/benchmarks/src/main/java/zipkin/benchmarks/Span2ConverterBenchmarks.java index c3ffd8eb568..faf1b3d3329 100644 --- a/benchmarks/src/main/java/zipkin/benchmarks/Span2ConverterBenchmarks.java +++ b/benchmarks/src/main/java/zipkin/benchmarks/Span2ConverterBenchmarks.java @@ -38,8 +38,6 @@ import zipkin.internal.V2SpanConverter; import zipkin.internal.Util; -import static zipkin.internal.V2SpanConverter.toEndpoint; - @Measurement(iterations = 5, time = 1) @Warmup(iterations = 10, time = 1) @Fork(3) @@ -97,8 +95,8 @@ public class Span2ConverterBenchmarks { .name("get") .kind(Span.Kind.SERVER) .shared(true) - .localEndpoint(toEndpoint(backend)) - .remoteEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(backend)) + .remoteEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996250000L) .duration(100000L) .putTag(TraceKeys.HTTP_PATH, "/backend") diff --git a/zipkin-junit/src/main/java/zipkin/junit/ZipkinDispatcher.java b/zipkin-junit/src/main/java/zipkin/junit/ZipkinDispatcher.java index 6532a383660..c3a6ccd6bc1 100644 --- a/zipkin-junit/src/main/java/zipkin/junit/ZipkinDispatcher.java +++ b/zipkin-junit/src/main/java/zipkin/junit/ZipkinDispatcher.java @@ -34,6 +34,7 @@ import zipkin.internal.v2.codec.DependencyLinkBytesCodec; import zipkin.internal.v2.codec.SpanBytesCodec; import zipkin.internal.v2.internal.Platform; +import zipkin.internal.v2.storage.StorageComponent; import zipkin.storage.Callback; import zipkin.storage.QueryRequest; import zipkin.storage.SpanStore; @@ -51,10 +52,11 @@ final class ZipkinDispatcher extends Dispatcher { private final CollectorMetrics metrics; private final MockWebServer server; - ZipkinDispatcher(V2StorageComponent storage, CollectorMetrics metrics, MockWebServer server) { - this.store = storage.spanStore(); - this.store2 = storage.v2SpanStore(); - this.consumer = Collector.builder(getClass()).storage(storage).metrics(metrics).build(); + ZipkinDispatcher(StorageComponent storage, CollectorMetrics metrics, MockWebServer server) { + V2StorageComponent adapted = V2StorageComponent.create(storage); + this.store = adapted.spanStore(); + this.store2 = storage.spanStore(); + this.consumer = Collector.builder(getClass()).storage(adapted).metrics(metrics).build(); this.metrics = metrics; this.server = server; } diff --git a/zipkin-junit/src/main/java/zipkin/junit/ZipkinRule.java b/zipkin-junit/src/main/java/zipkin/junit/ZipkinRule.java index 0646c9c0c0c..62cce7bb130 100644 --- a/zipkin-junit/src/main/java/zipkin/junit/ZipkinRule.java +++ b/zipkin-junit/src/main/java/zipkin/junit/ZipkinRule.java @@ -31,10 +31,10 @@ import org.junit.runners.model.Statement; import zipkin.Span; import zipkin.collector.InMemoryCollectorMetrics; -import zipkin.internal.CallbackCaptor; import zipkin.internal.GroupByTraceId; -import zipkin.internal.V2InMemoryStorage; import zipkin.internal.V2SpanConverter; +import zipkin.internal.v2.internal.Platform; +import zipkin.internal.v2.storage.InMemoryStorage; import static okhttp3.mockwebserver.SocketPolicy.KEEP_OPEN; import static zipkin.internal.GroupByTraceId.TRACE_DESCENDING; @@ -48,7 +48,7 @@ * See http://openzipkin.github.io/zipkin-api/#/ */ public final class ZipkinRule implements TestRule { - private final V2InMemoryStorage storage = V2InMemoryStorage.newBuilder().build(); + private final InMemoryStorage storage = InMemoryStorage.newBuilder().build(); private final InMemoryCollectorMetrics metrics = new InMemoryCollectorMetrics(); private final MockWebServer server = new MockWebServer(); private final BlockingQueue failureQueue = new LinkedBlockingQueue<>(); @@ -114,9 +114,11 @@ public InMemoryCollectorMetrics collectorMetrics() { * you'd add the parent here. */ public ZipkinRule storeSpans(List spans) { - CallbackCaptor callback = new CallbackCaptor<>(); - storage.asyncSpanConsumer().accept(spans, callback); - callback.get(); + try { + storage.accept(V2SpanConverter.fromSpans(spans)).execute(); + } catch (IOException e) { + throw Platform.get().uncheckedIOException(e); + } return this; } @@ -138,7 +140,7 @@ public ZipkinRule enqueueFailure(HttpFailure failure) { /** Retrieves all traces this zipkin server has received. */ public List> getTraces() { - List> traces = storage.v2SpanStore().getTraces(); + List> traces = storage.spanStore().getTraces(); List> result = new ArrayList<>(traces.size()); for (List trace2 : traces) { List sameTraceId = new ArrayList<>(); diff --git a/zipkin-junit/src/test/java/zipkin/junit/v2/HttpV2Storage.java b/zipkin-junit/src/test/java/zipkin/junit/v2/HttpV2Storage.java index 1ece6fa720b..72b7cc2f99c 100644 --- a/zipkin-junit/src/test/java/zipkin/junit/v2/HttpV2Storage.java +++ b/zipkin-junit/src/test/java/zipkin/junit/v2/HttpV2Storage.java @@ -15,16 +15,17 @@ import okhttp3.HttpUrl; import okhttp3.OkHttpClient; -import zipkin.internal.V2StorageComponent; +import zipkin.internal.v2.CheckResult; import zipkin.internal.v2.storage.SpanConsumer; import zipkin.internal.v2.storage.SpanStore; +import zipkin.internal.v2.storage.StorageComponent; /** * Test storage component that forwards requests to an HTTP endpoint. * *

Note: this inherits the {@link Builder#strictTraceId(boolean)} from the backend. */ -final class HttpV2Storage extends V2StorageComponent { +final class HttpV2Storage extends StorageComponent { private final OkHttpClient client; private final HttpUrl baseUrl; private final HttpV2SpanStore spanStore; @@ -40,11 +41,11 @@ final class HttpV2Storage extends V2StorageComponent { this.spanConsumer = new HttpV2SpanConsumer(this.client, this.baseUrl); } - @Override public SpanStore v2SpanStore() { + @Override public SpanStore spanStore() { return spanStore; } - @Override public SpanConsumer v2SpanConsumer() { + @Override public SpanConsumer spanConsumer() { return spanConsumer; } diff --git a/zipkin-junit/src/test/java/zipkin/junit/v2/ITHttpV2Storage.java b/zipkin-junit/src/test/java/zipkin/junit/v2/ITHttpV2Storage.java index 7acb4209f0f..ba11093bf51 100644 --- a/zipkin-junit/src/test/java/zipkin/junit/v2/ITHttpV2Storage.java +++ b/zipkin-junit/src/test/java/zipkin/junit/v2/ITHttpV2Storage.java @@ -17,7 +17,9 @@ import org.junit.Rule; import org.junit.experimental.runners.Enclosed; import org.junit.runner.RunWith; +import zipkin.internal.V2StorageComponent; import zipkin.junit.ZipkinRule; +import zipkin.storage.StorageComponent; @RunWith(Enclosed.class) public class ITHttpV2Storage { @@ -26,8 +28,8 @@ public static class DependenciesTest extends zipkin.storage.DependenciesTest { @Rule public ZipkinRule server = new ZipkinRule(); HttpV2Storage storage = new HttpV2Storage(server.httpUrl()); - @Override protected HttpV2Storage storage() { - return storage; + @Override protected StorageComponent storage() { + return V2StorageComponent.create(storage); } @Override public void clear() { @@ -39,8 +41,8 @@ public static class SpanStoreTest extends zipkin.storage.SpanStoreTest { @Rule public ZipkinRule server = new ZipkinRule(); HttpV2Storage storage = new HttpV2Storage(server.httpUrl()); - @Override protected HttpV2Storage storage() { - return storage; + @Override protected StorageComponent storage() { + return V2StorageComponent.create(storage); } @Override public void clear() throws IOException { diff --git a/zipkin-server/src/main/java/zipkin/server/ZipkinQueryApiV2.java b/zipkin-server/src/main/java/zipkin/server/ZipkinQueryApiV2.java index c7f590b93c2..2ddcc52a0cd 100644 --- a/zipkin-server/src/main/java/zipkin/server/ZipkinQueryApiV2.java +++ b/zipkin-server/src/main/java/zipkin/server/ZipkinQueryApiV2.java @@ -39,7 +39,7 @@ import zipkin.internal.v2.codec.DependencyLinkBytesCodec; import zipkin.internal.v2.codec.SpanBytesCodec; import zipkin.internal.v2.storage.QueryRequest; -import zipkin.storage.StorageComponent; +import zipkin.internal.v2.storage.StorageComponent; import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; @@ -51,7 +51,7 @@ public class ZipkinQueryApiV2 { static final Charset UTF_8 = Charset.forName("UTF-8"); final String storageType; - final V2StorageComponent storage; // don't cache spanStore here as it can cause the app to crash! + final StorageComponent storage; // don't cache spanStore here as it can cause the app to crash! final long defaultLookback; /** The Cache-Control max-age (seconds) for /api/v2/services and /api/v2/spans */ final int namesMaxAge; @@ -59,13 +59,13 @@ public class ZipkinQueryApiV2 { volatile int serviceCount; // used as a threshold to start returning cache-control headers ZipkinQueryApiV2( - StorageComponent storage, + zipkin.storage.StorageComponent storage, @Value("${zipkin.storage.type:mem}") String storageType, @Value("${zipkin.query.lookback:86400000}") long defaultLookback, // 1 day in millis @Value("${zipkin.query.names-max-age:300}") int namesMaxAge // 5 minutes ) { if (storage instanceof V2StorageComponent) { - this.storage = (V2StorageComponent) storage; + this.storage = ((V2StorageComponent) storage).internalDelegate(); } else { this.storage = null; } @@ -81,7 +81,7 @@ public byte[] getDependencies( ) throws IOException { if (storage == null) throw new Version2StorageNotConfigured(); - Call> call = storage.v2SpanStore() + Call> call = storage.spanStore() .getDependencies(endTs, lookback != null ? lookback : defaultLookback); return DependencyLinkBytesCodec.JSON.encodeList(call.execute()); } @@ -90,7 +90,7 @@ public byte[] getDependencies( public ResponseEntity> getServiceNames() throws IOException { if (storage == null) throw new Version2StorageNotConfigured(); - List serviceNames = storage.v2SpanStore().getServiceNames().execute(); + List serviceNames = storage.spanStore().getServiceNames().execute(); serviceCount = serviceNames.size(); return maybeCacheNames(serviceNames); } @@ -101,7 +101,7 @@ public ResponseEntity> getSpanNames( ) throws IOException { if (storage == null) throw new Version2StorageNotConfigured(); - return maybeCacheNames(storage.v2SpanStore().getSpanNames(serviceName).execute()); + return maybeCacheNames(storage.spanStore().getSpanNames(serviceName).execute()); } @RequestMapping(value = "/traces", method = RequestMethod.GET, produces = APPLICATION_JSON_VALUE) @@ -127,7 +127,7 @@ public String getTraces( .lookback(lookback != null ? lookback : defaultLookback) .limit(limit).build(); - List> traces = storage.v2SpanStore().getTraces(queryRequest).execute(); + List> traces = storage.spanStore().getTraces(queryRequest).execute(); return new String(SpanBytesCodec.JSON_V2.encodeNestedList(traces), UTF_8); } @@ -135,7 +135,7 @@ public String getTraces( public String getTrace(@PathVariable String traceIdHex, WebRequest request) throws IOException { if (storage == null) throw new Version2StorageNotConfigured(); - List trace = storage.v2SpanStore().getTrace(traceIdHex).execute(); + List trace = storage.spanStore().getTrace(traceIdHex).execute(); if (trace.isEmpty()) throw new TraceNotFoundException(traceIdHex); return new String(SpanBytesCodec.JSON_V2.encodeList(trace), UTF_8); } diff --git a/zipkin-server/src/main/java/zipkin/server/ZipkinServerConfiguration.java b/zipkin-server/src/main/java/zipkin/server/ZipkinServerConfiguration.java index 5099bad7a93..6cb5f5a503d 100644 --- a/zipkin-server/src/main/java/zipkin/server/ZipkinServerConfiguration.java +++ b/zipkin-server/src/main/java/zipkin/server/ZipkinServerConfiguration.java @@ -21,14 +21,15 @@ import org.springframework.boot.actuate.health.HealthAggregator; import org.springframework.boot.actuate.metrics.buffer.CounterBuffers; import org.springframework.boot.actuate.metrics.buffer.GaugeBuffers; +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import zipkin.collector.CollectorMetrics; import zipkin.collector.CollectorSampler; -import zipkin.internal.V2InMemoryStorage; import zipkin.internal.V2StorageComponent; +import zipkin.internal.v2.storage.InMemoryStorage; import zipkin.server.brave.TracedStorageComponent; import zipkin.storage.StorageComponent; @@ -80,6 +81,10 @@ public Object postProcessAfterInitialization(Object bean, String beanName) { } } + /** + * This is a special-case configuration if there's no StorageComponent of any kind. In-Mem can + * supply both read apis, so we add two beans here. + */ @Configuration // "matchIfMissing = true" ensures this is used when there's no configured storage type @ConditionalOnProperty(name = "zipkin.storage.type", havingValue = "mem", matchIfMissing = true) @@ -88,10 +93,24 @@ static class InMemoryConfiguration { @Bean StorageComponent storage( @Value("${zipkin.storage.strict-trace-id:true}") boolean strictTraceId, @Value("${zipkin.storage.mem.max-spans:500000}") int maxSpans) { - return V2InMemoryStorage.newBuilder() + return V2StorageComponent.create(InMemoryStorage.newBuilder() .strictTraceId(strictTraceId) .maxSpanCount(maxSpans) - .build(); + .build()); + } + + @Bean InMemoryStorage v2Storage(V2StorageComponent component) { + return (InMemoryStorage) component.internalDelegate(); + } + } + + /** This allows zipkin v2 components to be adapted to v1 components */ + @Configuration + @ConditionalOnBean(zipkin.internal.v2.storage.StorageComponent.class) + @ConditionalOnMissingBean(StorageComponent.class) + static class AdapterConfiguration { + @Bean StorageComponent storage(zipkin.internal.v2.storage.StorageComponent in) { + return V2StorageComponent.create(in); } } } diff --git a/zipkin-server/src/test/java/zipkin/server/ZipkinServerIntegrationTest.java b/zipkin-server/src/test/java/zipkin/server/ZipkinServerIntegrationTest.java index 17c16cbd7dc..2de7f87d21c 100644 --- a/zipkin-server/src/test/java/zipkin/server/ZipkinServerIntegrationTest.java +++ b/zipkin-server/src/test/java/zipkin/server/ZipkinServerIntegrationTest.java @@ -36,9 +36,9 @@ import zipkin.Codec; import zipkin.Span; import zipkin.internal.ApplyTimestampAndDuration; -import zipkin.internal.V2InMemoryStorage; import zipkin.internal.V2SpanConverter; import zipkin.internal.v2.codec.SpanBytesEncoder; +import zipkin.internal.v2.storage.InMemoryStorage; import static java.lang.String.format; import static java.util.Arrays.asList; @@ -67,7 +67,7 @@ public class ZipkinServerIntegrationTest { @Autowired ConfigurableWebApplicationContext context; @Autowired - V2InMemoryStorage storage; + InMemoryStorage storage; @Autowired ActuateCollectorMetrics metrics; @LocalServerPort diff --git a/zipkin-server/src/test/java/zipkin/server/ZipkinServerV2StorageTest.java b/zipkin-server/src/test/java/zipkin/server/ZipkinServerV2StorageTest.java new file mode 100644 index 00000000000..ad157875f14 --- /dev/null +++ b/zipkin-server/src/test/java/zipkin/server/ZipkinServerV2StorageTest.java @@ -0,0 +1,59 @@ +/** + * Copyright 2015-2017 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 zipkin.server; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.springframework.boot.actuate.health.HealthAggregator; +import org.springframework.boot.actuate.health.OrderedHealthAggregator; +import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import zipkin.internal.v2.storage.InMemoryStorage; +import zipkin.internal.v2.storage.StorageComponent; + +public class ZipkinServerV2StorageTest { + AnnotationConfigApplicationContext context; + + @Before public void init() { + context = new AnnotationConfigApplicationContext(); + } + + @After public void close() { + if (context != null) context.close(); + } + + @Test public void adaptsStorageComponent() { + context.register( + PropertyPlaceholderAutoConfiguration.class, + V2Storage.class, + ZipkinServerConfiguration.class); + context.refresh(); + + context.getBean(zipkin.storage.StorageComponent.class); + } + + @Configuration + public static class V2Storage { + @Bean public HealthAggregator healthAggregator() { + return new OrderedHealthAggregator(); + } + + @Bean public StorageComponent component() { + return InMemoryStorage.newBuilder().build(); + } + } +} diff --git a/zipkin-storage/cassandra/src/test/java/zipkin/storage/cassandra/ITCassandraStorage.java b/zipkin-storage/cassandra/src/test/java/zipkin/storage/cassandra/ITCassandraStorage.java index 152d8ee1c41..63510f01c6a 100644 --- a/zipkin-storage/cassandra/src/test/java/zipkin/storage/cassandra/ITCassandraStorage.java +++ b/zipkin-storage/cassandra/src/test/java/zipkin/storage/cassandra/ITCassandraStorage.java @@ -30,7 +30,7 @@ public class ITCassandraStorage { @ClassRule public static LazyCassandraStorage storage = - new LazyCassandraStorage("openzipkin/zipkin-cassandra:1.29.1", "test_zipkin"); + new LazyCassandraStorage("openzipkin/zipkin-cassandra:1.31.1", "test_zipkin"); public static class DependenciesTest extends CassandraDependenciesTest { @Override protected CassandraStorage storage() { diff --git a/zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/ITCassandra3Storage.java b/zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/ITCassandra3Storage.java index fdce4c12c0f..0a8fec9dadf 100644 --- a/zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/ITCassandra3Storage.java +++ b/zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/ITCassandra3Storage.java @@ -30,7 +30,7 @@ public class ITCassandra3Storage { @ClassRule public static LazyCassandra3Storage storage = - new LazyCassandra3Storage("openzipkin/zipkin-cassandra:1.29.1", "test_zipkin3"); + new LazyCassandra3Storage("openzipkin/zipkin-cassandra:1.31.1", "test_zipkin3"); public static class DependenciesTest extends CassandraDependenciesTest { @Override protected Cassandra3Storage storage() { diff --git a/zipkin-storage/elasticsearch-http/src/main/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpStorage.java b/zipkin-storage/elasticsearch-http/src/main/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpStorage.java index 9e057746301..ff923031224 100644 --- a/zipkin-storage/elasticsearch-http/src/main/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpStorage.java +++ b/zipkin-storage/elasticsearch-http/src/main/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpStorage.java @@ -33,6 +33,7 @@ import zipkin.internal.v2.storage.SpanConsumer; import zipkin.internal.v2.storage.SpanStore; import zipkin.storage.AsyncSpanStore; +import zipkin.storage.StorageComponent; import zipkin.storage.elasticsearch.http.internal.client.HttpCall; import static zipkin.moshi.JsonReaders.enterPath; @@ -40,7 +41,9 @@ import static zipkin.storage.elasticsearch.http.ElasticsearchHttpSpanStore.SPAN; @AutoValue -public abstract class ElasticsearchHttpStorage extends V2StorageComponent { +public abstract class ElasticsearchHttpStorage extends V2StorageComponent + implements V2StorageComponent.LegacySpanStoreProvider { + /** * A list of elasticsearch nodes to connect to, in http://host:port or https://host:port format. * Note this value is only read once. @@ -76,7 +79,7 @@ public static Builder builder() { abstract Builder toBuilder(); @AutoValue.Builder - public static abstract class Builder implements zipkin.storage.StorageComponent.Builder { + public static abstract class Builder implements StorageComponent.Builder { abstract Builder client(OkHttpClient client); abstract Builder shutdownClientOnClose(boolean shutdownClientOnClose); @@ -204,12 +207,37 @@ public final Builder dateSeparator(char dateSeparator) { abstract boolean legacyReadsEnabled(); - @Override public SpanStore v2SpanStore() { - ensureIndexTemplates(); - return new ElasticsearchHttpSpanStore(this); + @Override protected LegacySpanStoreProvider legacyProvider() { + return this; + } + + @Override public zipkin.internal.v2.storage.StorageComponent internalDelegate() { + return new Delegate(this); } - @Override @Nullable protected AsyncSpanStore legacyAsyncSpanStore() { + /** + * This type adapts to the new storage apis, without changing the enclosing hierarchy. This is + * done for api compat reasons and will be unwrapped in Zipkin v2. + */ + static final class Delegate extends zipkin.internal.v2.storage.StorageComponent { + final ElasticsearchHttpStorage delegate; + + Delegate(ElasticsearchHttpStorage delegate) { + this.delegate = delegate; + } + + @Override public SpanStore spanStore() { + delegate.ensureIndexTemplates(); + return new ElasticsearchHttpSpanStore(delegate); + } + + @Override public SpanConsumer spanConsumer() { + delegate.ensureIndexTemplates(); + return new ElasticsearchHttpSpanConsumer(delegate); + } + } + + @Override @Nullable public AsyncSpanStore legacyAsyncSpanStore() { float version = ensureIndexTemplates().version(); if (version >= 6 /* multi-type (legacy) index isn't possible */ || !legacyReadsEnabled()) { return null; @@ -217,11 +245,6 @@ public final Builder dateSeparator(char dateSeparator) { return new LegacyElasticsearchHttpSpanStore(this); } - @Override public SpanConsumer v2SpanConsumer() { - ensureIndexTemplates(); - return new ElasticsearchHttpSpanConsumer(this); - } - /** This is a blocking call, only used in tests. */ void clear() throws IOException { Set toClear = new LinkedHashSet<>(); diff --git a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpSpanConsumerTest.java b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpSpanConsumerTest.java index 8bacdbf84cc..b0ba4c4f085 100644 --- a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpSpanConsumerTest.java +++ b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpSpanConsumerTest.java @@ -123,7 +123,8 @@ public void close() throws IOException { Span span = Span.newBuilder().traceId("20").id("20").name("get") .timestamp(TODAY * 1000).build(); - assertThat(SpanBytesCodec.JSON_V2.decode(prefixWithTimestampMillisAndQuery(span, span.timestamp()))) + assertThat( + SpanBytesCodec.JSON_V2.decode(prefixWithTimestampMillisAndQuery(span, span.timestamp()))) .isEqualTo(span); // ignores timestamp_millis field } @@ -208,6 +209,6 @@ public void close() throws IOException { } void accept(Span... spans) throws Exception { - storage.v2SpanConsumer().accept(asList(spans)).execute(); + storage.internalDelegate().spanConsumer().accept(asList(spans)).execute(); } } diff --git a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/VersionSpecificTemplatesTest.java b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/VersionSpecificTemplatesTest.java index 1bc791cf913..7d515ca3d28 100644 --- a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/VersionSpecificTemplatesTest.java +++ b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/VersionSpecificTemplatesTest.java @@ -34,8 +34,7 @@ public class VersionSpecificTemplatesTest { .hosts(asList(es.url("").toString())) .build(); - VersionSpecificTemplates - client = new VersionSpecificTemplates(storage); + VersionSpecificTemplates client = new VersionSpecificTemplates(storage); @After public void close() throws IOException { diff --git a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ElasticsearchHttpSpanConsumerTest.java b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ElasticsearchHttpSpanConsumerTest.java index a30da1bda35..3a5d64245e4 100644 --- a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ElasticsearchHttpSpanConsumerTest.java +++ b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ElasticsearchHttpSpanConsumerTest.java @@ -25,12 +25,11 @@ import org.junit.Test; import zipkin.Annotation; import zipkin.Span; -import zipkin.internal.CallbackCaptor; import zipkin.internal.Util; +import zipkin.internal.V2SpanConverter; import zipkin.storage.elasticsearch.http.ElasticsearchHttpStorage; import zipkin.storage.elasticsearch.http.InternalForTests; -import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; import static zipkin.Constants.SERVER_RECV; import static zipkin.Constants.SERVER_SEND; @@ -61,24 +60,24 @@ public void spanGoesIntoADailyIndex_whenTimestampIsDerived() throws Exception { long twoDaysAgo = (TODAY - 2 * DAY); Span span = Span.builder().traceId(20L).id(20L).name("get") - .addAnnotation(Annotation.create(twoDaysAgo * 1000, SERVER_RECV, WEB_ENDPOINT)) - .addAnnotation(Annotation.create(TODAY * 1000, SERVER_SEND, WEB_ENDPOINT)) - .build(); + .addAnnotation(Annotation.create(twoDaysAgo * 1000, SERVER_RECV, WEB_ENDPOINT)) + .addAnnotation(Annotation.create(TODAY * 1000, SERVER_SEND, WEB_ENDPOINT)) + .build(); accept(span); // make sure the span went into an index corresponding to its first annotation timestamp assertThat(findSpans(twoDaysAgo, span.traceId)) - .contains("\"hits\":{\"total\":1"); + .contains("\"hits\":{\"total\":1"); } String findSpans(long endTs, long traceId) throws IOException { return new OkHttpClient().newCall(new Request.Builder().url( - HttpUrl.parse(baseUrl()).newBuilder() - .addPathSegment(INDEX + ":span-" + dateFormat.format(new Date(endTs))) - .addPathSegment("_search") - .addQueryParameter("q", "traceId:" + Util.toLowerHex(traceId)).build()) - .get().build()).execute().body().string(); + HttpUrl.parse(baseUrl()).newBuilder() + .addPathSegment(INDEX + ":span-" + dateFormat.format(new Date(endTs))) + .addPathSegment("_search") + .addQueryParameter("q", "traceId:" + Util.toLowerHex(traceId)).build()) + .get().build()).execute().body().string(); } @Test @@ -86,13 +85,13 @@ public void spanGoesIntoADailyIndex_whenTimestampIsExplicit() throws Exception { long twoDaysAgo = (TODAY - 2 * DAY); Span span = Span.builder().traceId(20L).id(20L).name("get") - .timestamp(twoDaysAgo * 1000).build(); + .timestamp(twoDaysAgo * 1000).build(); accept(span); // make sure the span went into an index corresponding to its timestamp, not collection time assertThat(findSpans(twoDaysAgo, span.traceId)) - .contains("\"hits\":{\"total\":1"); + .contains("\"hits\":{\"total\":1"); } @Test @@ -103,7 +102,7 @@ public void spanGoesIntoADailyIndex_fallsBackToTodayWhenNoTimestamps() throws Ex // make sure the span went into an index corresponding to collection time assertThat(findSpans(TODAY, span.traceId)) - .contains("\"hits\":{\"total\":1"); + .contains("\"hits\":{\"total\":1"); } @Test @@ -113,21 +112,19 @@ public void searchByTimestampMillis() throws Exception { accept(span); Call searchRequest = new OkHttpClient().newCall(new Request.Builder().url( - HttpUrl.parse(baseUrl()).newBuilder() - .addPathSegment(INDEX + ":span-*") - .addPathSegment("_search") - .addQueryParameter("q", "timestamp_millis:" + TODAY).build()) - .get().tag("search-terms").build()); + HttpUrl.parse(baseUrl()).newBuilder() + .addPathSegment(INDEX + ":span-*") + .addPathSegment("_search") + .addQueryParameter("q", "timestamp_millis:" + TODAY).build()) + .get().tag("search-terms").build()); assertThat(searchRequest.execute().body().string()) - .contains("\"hits\":{\"total\":1"); + .contains("\"hits\":{\"total\":1"); } abstract String baseUrl(); void accept(Span span) throws Exception { - CallbackCaptor callback = new CallbackCaptor<>(); - storage().asyncSpanConsumer().accept(asList(span), callback); - callback.get(); + storage().internalDelegate().spanConsumer().accept(V2SpanConverter.fromSpan(span)).execute(); } } diff --git a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ITElasticsearchHttpStorageV2.java b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ITElasticsearchHttpStorageV2.java index 8709a8410bc..7928fdc5ef6 100644 --- a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ITElasticsearchHttpStorageV2.java +++ b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ITElasticsearchHttpStorageV2.java @@ -25,7 +25,7 @@ public class ITElasticsearchHttpStorageV2 { @ClassRule public static LazyElasticsearchHttpStorage storage = - new LazyElasticsearchHttpStorage("openzipkin/zipkin-elasticsearch:1.29.1"); + new LazyElasticsearchHttpStorage("openzipkin/zipkin-elasticsearch:1.31.1"); public static class DependenciesTest extends ElasticsearchHttpDependenciesTest { @Override protected ElasticsearchHttpStorage storage() { diff --git a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ITElasticsearchHttpStorageV5.java b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ITElasticsearchHttpStorageV5.java index 7602204280f..7a22062ec5c 100644 --- a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ITElasticsearchHttpStorageV5.java +++ b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ITElasticsearchHttpStorageV5.java @@ -25,7 +25,7 @@ public class ITElasticsearchHttpStorageV5 { @ClassRule public static LazyElasticsearchHttpStorage storage = - new LazyElasticsearchHttpStorage("openzipkin/zipkin-elasticsearch5:1.29.1"); + new LazyElasticsearchHttpStorage("openzipkin/zipkin-elasticsearch5:1.31.1"); public static class DependenciesTest extends ElasticsearchHttpDependenciesTest { @Override protected ElasticsearchHttpStorage storage() { diff --git a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ITElasticsearchHttpStorageV6.java b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ITElasticsearchHttpStorageV6.java index 57797b07dc4..ccc7496451a 100644 --- a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ITElasticsearchHttpStorageV6.java +++ b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/ITElasticsearchHttpStorageV6.java @@ -25,7 +25,7 @@ public class ITElasticsearchHttpStorageV6 { @ClassRule public static LazyElasticsearchHttpStorage storage = - new LazyElasticsearchHttpStorage("openzipkin/zipkin-elasticsearch6:1.29.1"); + new LazyElasticsearchHttpStorage("openzipkin/zipkin-elasticsearch6:1.31.1"); public static class DependenciesTest extends ElasticsearchHttpDependenciesTest { @Override protected ElasticsearchHttpStorage storage() { diff --git a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/LazyElasticsearchHttpStorage.java b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/LazyElasticsearchHttpStorage.java index 3a8a6e08027..2bd2ecadbe1 100644 --- a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/LazyElasticsearchHttpStorage.java +++ b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/integration/LazyElasticsearchHttpStorage.java @@ -24,8 +24,8 @@ import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.output.Slf4jLogConsumer; import org.testcontainers.containers.wait.HttpWaitStrategy; -import zipkin.Component; import zipkin.internal.LazyCloseable; +import zipkin.internal.v2.CheckResult; import zipkin.storage.elasticsearch.http.ElasticsearchHttpStorage; import zipkin.storage.elasticsearch.http.InternalForTests; @@ -55,11 +55,11 @@ class LazyElasticsearchHttpStorage extends LazyCloseable { // not final for mock +public class Collector + extends zipkin.internal.Collector { // not final for mock /** Needed to scope this to the correct logging category */ public static Builder builder(Class loggingClass) { @@ -91,7 +92,7 @@ public Collector build() { builder.logger, builder.metrics, builder.sampler, - (V2StorageComponent) storage + ((V2StorageComponent) storage).internalDelegate() ); } else { storage2 = null; diff --git a/zipkin/src/main/java/zipkin/internal/JsonCodec.java b/zipkin/src/main/java/zipkin/internal/JsonCodec.java index 9b6a7d5f158..ea92538ff10 100644 --- a/zipkin/src/main/java/zipkin/internal/JsonCodec.java +++ b/zipkin/src/main/java/zipkin/internal/JsonCodec.java @@ -85,7 +85,7 @@ public final class JsonCodec implements Codec { static final Buffer.Writer ENDPOINT_WRITER = new Buffer.Writer() { @Override public int sizeInBytes(Endpoint v) { - zipkin.internal.v2.Endpoint value = V2SpanConverter.toEndpoint(v); + zipkin.internal.v2.Endpoint value = V2SpanConverter.fromEndpoint(v); int sizeInBytes = 17; // {"serviceName":"" if (value.serviceName() != null) { sizeInBytes += jsonEscapedSizeInBytes(value.serviceName()); @@ -109,7 +109,7 @@ public final class JsonCodec implements Codec { } @Override public void write(Endpoint v, Buffer b) { - zipkin.internal.v2.Endpoint value = V2SpanConverter.toEndpoint(v); + zipkin.internal.v2.Endpoint value = V2SpanConverter.fromEndpoint(v); b.writeAscii("{\"serviceName\":\""); if (value.serviceName() != null) { b.writeUtf8(jsonEscape(value.serviceName())); diff --git a/zipkin/src/main/java/zipkin/internal/V2Collector.java b/zipkin/src/main/java/zipkin/internal/V2Collector.java index f763a6e12f6..1f871f65d80 100644 --- a/zipkin/src/main/java/zipkin/internal/V2Collector.java +++ b/zipkin/src/main/java/zipkin/internal/V2Collector.java @@ -20,23 +20,25 @@ import zipkin.collector.CollectorSampler; import zipkin.internal.v2.Span; import zipkin.internal.v2.codec.BytesDecoder; +import zipkin.internal.v2.storage.StorageComponent; import zipkin.storage.Callback; import static zipkin.internal.Util.checkNotNull; public final class V2Collector extends Collector, Span> { - final V2StorageComponent storage; + final StorageComponent storage; final CollectorSampler sampler; public V2Collector(Logger logger, @Nullable CollectorMetrics metrics, - @Nullable CollectorSampler sampler, V2StorageComponent storage) { + @Nullable CollectorSampler sampler, StorageComponent storage) { super(logger, metrics); this.storage = checkNotNull(storage, "storage"); this.sampler = sampler == null ? CollectorSampler.ALWAYS_SAMPLE : sampler; } @Override - public void acceptSpans(byte[] serializedSpans, BytesDecoder decoder, Callback callback) { + public void acceptSpans(byte[] serializedSpans, BytesDecoder decoder, + Callback callback) { super.acceptSpans(serializedSpans, decoder, callback); } @@ -49,7 +51,7 @@ public void acceptSpans(byte[] serializedSpans, BytesDecoder decoder, Call } @Override protected void record(List sampled, Callback callback) { - storage.v2SpanConsumer().accept(sampled).enqueue(new V2CallbackAdapter(callback)); + storage.spanConsumer().accept(sampled).enqueue(new V2CallbackAdapter<>(callback)); } @Override protected String idString(Span span) { diff --git a/zipkin/src/main/java/zipkin/internal/V2InMemoryStorage.java b/zipkin/src/main/java/zipkin/internal/V2InMemoryStorage.java deleted file mode 100644 index 04d0609acfd..00000000000 --- a/zipkin/src/main/java/zipkin/internal/V2InMemoryStorage.java +++ /dev/null @@ -1,71 +0,0 @@ -/** - * Copyright 2015-2017 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 zipkin.internal; - -import java.io.IOException; -import zipkin.internal.v2.storage.InMemoryStorage; - -public final class V2InMemoryStorage extends V2StorageComponent { - - public static Builder newBuilder() { - return new V2InMemoryStorage.Builder(); - } - - public static final class Builder implements zipkin.storage.StorageComponent.Builder { - final InMemoryStorage.Builder delegate = InMemoryStorage.newBuilder(); - - @Override public Builder strictTraceId(boolean strictTraceId) { - delegate.strictTraceId(strictTraceId); - return this; - } - - /** Eldest traces are removed to ensure spans in memory don't exceed this value */ - public Builder maxSpanCount(int maxSpanCount) { - delegate.maxSpanCount(maxSpanCount); - return this; - } - - @Override public V2InMemoryStorage build() { - return new V2InMemoryStorage(delegate.build()); - } - - Builder() { - } - } - - final InMemoryStorage delegate; - - V2InMemoryStorage(InMemoryStorage delegate) { - this.delegate = delegate; - } - - @Override public InMemoryStorage v2SpanStore() { - return delegate; - } - - @Override public InMemoryStorage v2SpanConsumer() { - return delegate; - } - - @Override public CheckResult check() { - return CheckResult.OK; - } - - public void clear() { - delegate.clear(); - } - - @Override public void close() throws IOException { - } -} diff --git a/zipkin/src/main/java/zipkin/internal/V2SpanConverter.java b/zipkin/src/main/java/zipkin/internal/V2SpanConverter.java index 81cc0a10b57..0d6457f1a86 100644 --- a/zipkin/src/main/java/zipkin/internal/V2SpanConverter.java +++ b/zipkin/src/main/java/zipkin/internal/V2SpanConverter.java @@ -110,7 +110,7 @@ void processAnnotations(zipkin.Span source) { if (closeEnough(cs.endpoint, sr.endpoint)) { client.kind(Kind.CLIENT); // fork a new span for the server side - server = newSpanBuilder(source, toEndpoint(sr.endpoint)).kind(Kind.SERVER); + server = newSpanBuilder(source, fromEndpoint(sr.endpoint)).kind(Kind.SERVER); } else { server = forEndpoint(source, sr.endpoint); } @@ -151,7 +151,7 @@ void processAnnotations(zipkin.Span source) { if (closeEnough(ms.endpoint, mr.endpoint)) { producer.kind(Kind.PRODUCER); // fork a new span for the consumer side - consumer = newSpanBuilder(source, toEndpoint(mr.endpoint)).kind(Kind.CONSUMER); + consumer = newSpanBuilder(source, fromEndpoint(mr.endpoint)).kind(Kind.CONSUMER); } else { consumer = forEndpoint(source, mr.endpoint); } @@ -234,30 +234,30 @@ void processBinaryAnnotations(zipkin.Span source) { } if (cs != null && sa != null && !closeEnough(sa, cs.endpoint)) { - forEndpoint(source, cs.endpoint).remoteEndpoint(toEndpoint(sa)); + forEndpoint(source, cs.endpoint).remoteEndpoint(fromEndpoint(sa)); } if (sr != null && ca != null && !closeEnough(ca, sr.endpoint)) { - forEndpoint(source, sr.endpoint).remoteEndpoint(toEndpoint(ca)); + forEndpoint(source, sr.endpoint).remoteEndpoint(fromEndpoint(ca)); } if (ms != null && ma != null && !closeEnough(ma, ms.endpoint)) { - forEndpoint(source, ms.endpoint).remoteEndpoint(toEndpoint(ma)); + forEndpoint(source, ms.endpoint).remoteEndpoint(fromEndpoint(ma)); } if (mr != null && ma != null && !closeEnough(ma, mr.endpoint)) { - forEndpoint(source, mr.endpoint).remoteEndpoint(toEndpoint(ma)); + forEndpoint(source, mr.endpoint).remoteEndpoint(fromEndpoint(ma)); } // special-case when we are missing core annotations, but we have both address annotations if ((cs == null && sr == null) && (ca != null && sa != null)) { - forEndpoint(source, ca).remoteEndpoint(toEndpoint(sa)); + forEndpoint(source, ca).remoteEndpoint(fromEndpoint(sa)); } } Span.Builder forEndpoint(zipkin.Span source, @Nullable zipkin.Endpoint e) { if (e == null) return spans.get(0); // allocate missing endpoint data to first span - Endpoint converted = toEndpoint(e); + Endpoint converted = fromEndpoint(e); for (int i = 0, length = spans.size(); i < length; i++) { Span.Builder next = spans.get(i); Endpoint nextLocalEndpoint = next.localEndpoint(); @@ -443,7 +443,7 @@ public static zipkin.Span toSpan(Span in) { return result.build(); } - public static Endpoint toEndpoint(zipkin.Endpoint input) { + public static Endpoint fromEndpoint(zipkin.Endpoint input) { Endpoint.Builder result = Endpoint.newBuilder() .serviceName(input.serviceName) .port(input.port != null ? input.port & 0xffff : null); @@ -529,4 +529,12 @@ public static List fromLinks(Iterable lin } return result; } + + public static List fromSpans(Iterable spans) { + List result = new ArrayList<>(); + for (zipkin.Span span1 : spans) { + result.addAll(fromSpan(span1)); + } + return result; + } } diff --git a/zipkin/src/main/java/zipkin/internal/V2StorageComponent.java b/zipkin/src/main/java/zipkin/internal/V2StorageComponent.java index 610bbf522f9..6b7bdae963a 100644 --- a/zipkin/src/main/java/zipkin/internal/V2StorageComponent.java +++ b/zipkin/src/main/java/zipkin/internal/V2StorageComponent.java @@ -13,39 +13,91 @@ */ package zipkin.internal; +import java.io.IOException; +import java.util.concurrent.ExecutionException; import javax.annotation.Nullable; -import zipkin.internal.v2.storage.SpanConsumer; -import zipkin.internal.v2.storage.SpanStore; import zipkin.storage.AsyncSpanConsumer; import zipkin.storage.AsyncSpanStore; import zipkin.storage.StorageAdapters; import zipkin.storage.StorageComponent; +/** + * This is an internal type used to bridge two versions of the storage component. + * + *

This type is extensible at the moment as we cannot break api in v1. In Zipkin v2, a storage + * component, such as Elasticsearch, can break api and change the overall type it implements. + */ public abstract class V2StorageComponent implements StorageComponent { + + public interface LegacySpanStoreProvider { + /** Returns null when legacy reads are not supported */ + @Nullable AsyncSpanStore legacyAsyncSpanStore(); + } + + public static V2StorageComponent create(zipkin.internal.v2.storage.StorageComponent delegate) { + if (delegate == null) throw new NullPointerException("delegate == null"); + LegacySpanStoreProvider legacyProvider; + if (delegate instanceof LegacySpanStoreProvider) { + legacyProvider = (LegacySpanStoreProvider) delegate; + } else { + legacyProvider = null; + } + return new V2StorageComponent() { + @Override protected LegacySpanStoreProvider legacyProvider() { + return legacyProvider; + } + + @Override public zipkin.internal.v2.storage.StorageComponent internalDelegate() { + return delegate; + } + }; + } + + protected abstract LegacySpanStoreProvider legacyProvider(); + + /** + * This is a public method, but should not be used outside Zipkin internal code. If you need to + * use this method, please use shade or another way to protect from api change, as it is declared + * on an internal type. + */ + public abstract zipkin.internal.v2.storage.StorageComponent internalDelegate(); + @Override public zipkin.storage.SpanStore spanStore() { - if (legacyAsyncSpanStore() != null) { + AsyncSpanStore legacy = + legacyProvider() != null ? legacyProvider().legacyAsyncSpanStore() : null; + if (legacy != null) { return StorageAdapters.asyncToBlocking(asyncSpanStore()); } - return new V2SpanStoreAdapter(v2SpanStore()); + return new V2SpanStoreAdapter(internalDelegate().spanStore()); } @Override public AsyncSpanStore asyncSpanStore() { - V2SpanStoreAdapter v2 = new V2SpanStoreAdapter(v2SpanStore()); - AsyncSpanStore legacy = legacyAsyncSpanStore(); + V2SpanStoreAdapter v2 = new V2SpanStoreAdapter(internalDelegate().spanStore()); + AsyncSpanStore legacy = + legacyProvider() != null ? legacyProvider().legacyAsyncSpanStore() : null; if (legacy == null) return v2; // fan out queries as we don't know if old legacy collectors are in use return new LenientDoubleCallbackAsyncSpanStore(v2, legacy); } - @Nullable protected AsyncSpanStore legacyAsyncSpanStore() { - return null; + @Override public final AsyncSpanConsumer asyncSpanConsumer() { + return new V2SpanConsumerAdapter(internalDelegate().spanConsumer()); } - public abstract SpanStore v2SpanStore(); + @Override public CheckResult check() { + zipkin.internal.v2.CheckResult result = internalDelegate().check(); + return result.ok() ? CheckResult.OK : CheckResult.failed( + result.error() instanceof Exception + ? ((Exception) result.error()) + : new ExecutionException(result.error()) + ); + } - @Override public final AsyncSpanConsumer asyncSpanConsumer() { - return new V2SpanConsumerAdapter(v2SpanConsumer()); + @Override public void close() throws IOException { + internalDelegate().close(); } - public abstract SpanConsumer v2SpanConsumer(); + @Override public String toString() { + return internalDelegate().toString(); + } } diff --git a/zipkin/src/main/java/zipkin/internal/v2/CheckResult.java b/zipkin/src/main/java/zipkin/internal/v2/CheckResult.java new file mode 100644 index 00000000000..b351d07b952 --- /dev/null +++ b/zipkin/src/main/java/zipkin/internal/v2/CheckResult.java @@ -0,0 +1,45 @@ +/** + * Copyright 2015-2017 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 zipkin.internal.v2; + +import com.google.auto.value.AutoValue; +import javax.annotation.Nullable; +import jdk.nashorn.internal.ir.annotations.Immutable; + +/** + * Answers the question: Are operations on this component likely to succeed? + * + *

Implementations should initialize the component if necessary. It should test a remote + * connection, or consult a trusted source to derive the result. They should use least resources + * possible to establish a meaningful result, and be safe to call many times, even concurrently. + * + * @see CheckResult#OK + */ +@Immutable +@AutoValue +public abstract class CheckResult { + public static final CheckResult OK = new AutoValue_CheckResult(true, null); + + public static CheckResult failed(Throwable error) { + return new AutoValue_CheckResult(false, error); + } + + public abstract boolean ok(); + + /** Present when not ok */ + @Nullable public abstract Throwable error(); + + CheckResult() { + } +} diff --git a/zipkin/src/main/java/zipkin/internal/v2/Component.java b/zipkin/src/main/java/zipkin/internal/v2/Component.java new file mode 100644 index 00000000000..f81832c03e3 --- /dev/null +++ b/zipkin/src/main/java/zipkin/internal/v2/Component.java @@ -0,0 +1,49 @@ +/** + * Copyright 2015-2017 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 zipkin.internal.v2; + +import java.io.Closeable; +import java.io.IOException; + +/** + * Components are object graphs used to compose a zipkin service or client. For example, a storage + * component might return a query api. + * + *

Components are lazy with regards to I/O. They can be injected directly to other components so + * as to avoid crashing the application graph if a network service is unavailable. + */ +public abstract class Component implements Closeable { + + /** + * Answers the question: Are operations on this component likely to succeed? + * + *

Implementations should initialize the component if necessary. It should test a remote + * connection, or consult a trusted source to derive the result. They should use least resources + * possible to establish a meaningful result, and be safe to call many times, even concurrently. + * + * @see CheckResult#OK + */ + public CheckResult check() { + return CheckResult.OK; + } + + /** + * Closes any network resources created implicitly by the component. + * + *

For example, if this created a connection, it would close it. If it was provided one, this + * would close any sessions, but leave the connection open. + */ + @Override public void close() throws IOException { + } +} diff --git a/zipkin/src/main/java/zipkin/internal/v2/Endpoint.java b/zipkin/src/main/java/zipkin/internal/v2/Endpoint.java index 27a72f931ac..68c9531058a 100644 --- a/zipkin/src/main/java/zipkin/internal/v2/Endpoint.java +++ b/zipkin/src/main/java/zipkin/internal/v2/Endpoint.java @@ -425,8 +425,8 @@ private static boolean isValidIpV4Word(CharSequence word, int from, int toExclus if (len == 3) { return (c1 = word.charAt(from + 1)) >= '0' && (c2 = word.charAt(from + 2)) >= '0' && - (c0 <= '1' && c1 <= '9' && c2 <= '9' || - c0 == '2' && c1 <= '5' && (c2 <= '5' || c1 < '5' && c2 <= '9')); + ((c0 <= '1' && c1 <= '9' && c2 <= '9') || + (c0 == '2' && c1 <= '5' && (c2 <= '5' || (c1 < '5' && c2 <= '9')))); } return c0 <= '9' && (len == 1 || isValidNumericChar(word.charAt(from + 1))); } diff --git a/zipkin/src/main/java/zipkin/internal/v2/internal/Buffer.java b/zipkin/src/main/java/zipkin/internal/v2/internal/Buffer.java index 6e443fdb645..8d823c2eed0 100644 --- a/zipkin/src/main/java/zipkin/internal/v2/internal/Buffer.java +++ b/zipkin/src/main/java/zipkin/internal/v2/internal/Buffer.java @@ -14,12 +14,11 @@ package zipkin.internal.v2.internal; import java.nio.charset.Charset; -import javax.annotation.concurrent.Immutable; public final class Buffer { static final Charset UTF_8 = Charset.forName("UTF-8"); - @Immutable public interface Writer { + public interface Writer { int sizeInBytes(T value); void write(T value, Buffer buffer); diff --git a/zipkin/src/main/java/zipkin/internal/v2/internal/V1SpanWriter.java b/zipkin/src/main/java/zipkin/internal/v2/internal/V1SpanWriter.java index 3f78368f308..cb30a63e53c 100644 --- a/zipkin/src/main/java/zipkin/internal/v2/internal/V1SpanWriter.java +++ b/zipkin/src/main/java/zipkin/internal/v2/internal/V1SpanWriter.java @@ -13,9 +13,11 @@ */ package zipkin.internal.v2.internal; +import java.nio.charset.Charset; import java.util.Iterator; import java.util.Map; import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; import zipkin.Constants; import zipkin.internal.v2.Annotation; import zipkin.internal.v2.Endpoint; @@ -29,6 +31,7 @@ import static zipkin.internal.v2.internal.V2SpanWriter.writeAnnotation; import static zipkin.internal.v2.internal.V2SpanWriter.writeEndpoint; +@Immutable public final class V1SpanWriter implements Buffer.Writer { @Override public int sizeInBytes(Span value) { Parsed parsed = parse(value); @@ -185,15 +188,17 @@ public final class V1SpanWriter implements Buffer.Writer { return "Span"; } + static final byte[] EMPTY_SERVICE = "{\"serviceName\":\"\"".getBytes(Charset.forName("UTF-8")); + static byte[] legacyEndpointBytes(@Nullable Endpoint localEndpoint) { if (localEndpoint == null) return null; Buffer buffer = new Buffer(endpointSizeInBytes(localEndpoint)); writeEndpoint(localEndpoint, buffer); byte[] endpointBytes = buffer.toByteArray(); if (localEndpoint.serviceName() != null) return endpointBytes; - byte[] newSpanBytes = new byte[17 /* {"serviceName":"" */ + endpointBytes.length]; - System.arraycopy("{\"serviceName\":\"\"".getBytes(), 0, newSpanBytes, 0, 17); - newSpanBytes[17] = ','; + byte[] newSpanBytes = new byte[EMPTY_SERVICE.length + endpointBytes.length]; + System.arraycopy(EMPTY_SERVICE, 0, newSpanBytes, 0, EMPTY_SERVICE.length); + newSpanBytes[EMPTY_SERVICE.length] = ','; System.arraycopy(endpointBytes, 1, newSpanBytes, 18, endpointBytes.length - 1); return newSpanBytes; } diff --git a/zipkin/src/main/java/zipkin/internal/v2/internal/V2SpanWriter.java b/zipkin/src/main/java/zipkin/internal/v2/internal/V2SpanWriter.java index aa30c1f516f..82548f4e78f 100644 --- a/zipkin/src/main/java/zipkin/internal/v2/internal/V2SpanWriter.java +++ b/zipkin/src/main/java/zipkin/internal/v2/internal/V2SpanWriter.java @@ -16,6 +16,7 @@ import java.util.Iterator; import java.util.Map; import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; import zipkin.internal.v2.Annotation; import zipkin.internal.v2.Endpoint; import zipkin.internal.v2.Span; @@ -24,6 +25,7 @@ import static zipkin.internal.v2.internal.JsonEscaper.jsonEscape; import static zipkin.internal.v2.internal.JsonEscaper.jsonEscapedSizeInBytes; +@Immutable public final class V2SpanWriter implements Buffer.Writer { @Override public int sizeInBytes(Span value) { int sizeInBytes = 13; // {"traceId":"" diff --git a/zipkin/src/main/java/zipkin/internal/v2/storage/InMemoryStorage.java b/zipkin/src/main/java/zipkin/internal/v2/storage/InMemoryStorage.java index c4108307164..4c569f8aa02 100644 --- a/zipkin/src/main/java/zipkin/internal/v2/storage/InMemoryStorage.java +++ b/zipkin/src/main/java/zipkin/internal/v2/storage/InMemoryStorage.java @@ -14,6 +14,7 @@ package zipkin.internal.v2.storage; import com.google.auto.value.AutoValue; +import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -64,18 +65,18 @@ * foo --> ( GET, POST ) * } */ -public final class InMemoryStorage implements SpanConsumer, SpanStore { +public final class InMemoryStorage extends StorageComponent implements SpanStore, SpanConsumer { public static Builder newBuilder() { return new Builder(); } - public static final class Builder { + public static final class Builder extends StorageComponent.Builder { boolean strictTraceId = true; int maxSpanCount = 500000; /** {@inheritDoc} */ - public Builder strictTraceId(boolean strictTraceId) { + @Override public Builder strictTraceId(boolean strictTraceId) { this.strictTraceId = strictTraceId; return this; } @@ -87,7 +88,7 @@ public Builder maxSpanCount(int maxSpanCount) { return this; } - public InMemoryStorage build() { + @Override public InMemoryStorage build() { return new InMemoryStorage(this); } } @@ -412,6 +413,17 @@ private Collection traceIdTimestampsByServiceName(String servi } static String lowTraceId(String traceId) { - return traceId.length() == 32 ? traceId.substring(16) :traceId; + return traceId.length() == 32 ? traceId.substring(16) : traceId; + } + + @Override public InMemoryStorage spanStore() { + return this; + } + + @Override public SpanConsumer spanConsumer() { + return this; + } + + @Override public void close() throws IOException { } } diff --git a/zipkin/src/main/java/zipkin/internal/v2/storage/StorageComponent.java b/zipkin/src/main/java/zipkin/internal/v2/storage/StorageComponent.java new file mode 100644 index 00000000000..6f92c66ea3e --- /dev/null +++ b/zipkin/src/main/java/zipkin/internal/v2/storage/StorageComponent.java @@ -0,0 +1,78 @@ +/** + * Copyright 2015-2017 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 zipkin.internal.v2.storage; + +import zipkin.internal.v2.Component; +import zipkin.internal.v2.Span; + +/** + * A component that provides storage interfaces used for spans and aggregations. Implementations are + * free to provide other interfaces, but the ones declared here must be supported. + * + * @see InMemoryStorage + */ +public abstract class StorageComponent extends Component { + + public abstract SpanStore spanStore(); + + public abstract SpanConsumer spanConsumer(); + + public static abstract class Builder { + + /** + * Zipkin supports 64 and 128-bit trace identifiers, typically serialized as 16 or 32 character + * hex strings. When false, this setting only considers the low 64-bits (right-most 16 + * characters) of a trace ID when grouping or retrieving traces. This should be set to false + * while some applications issue 128-bit trace IDs and while other truncate them to 64-bit. If + * 128-bit trace IDs are not in use, this setting is not required. + * + *

Details

+ * + *

Zipkin historically had 64-bit {@link Span#traceId trace IDs}, but it now supports 128-bit + * trace IDs via 32-character hex representation. While instrumentation update to propagate + * 128-bit IDs, it can be ambiguous whether a 64-bit trace ID was sent intentionally, or as an + * accident of truncation. This setting allows Zipkin to be usable until application + * instrumentation are upgraded to support 128-bit trace IDs. + * + *

Here are a few trace IDs the help explain this setting. + * + * 

    
+     *   
  • Trace ID A: 463ac35c9f6413ad48485a3953bb6124
    • 
+     *   
  • Trace ID B: 48485a3953bb6124
    • 
+     *   
  • Trace ID C: 463ac35c9f6413adf1a48a8cff464e0e
    • 
+     *   
  • Trace ID D: 463ac35c9f6413ad
    • 
+     *
+ * + *

In the above example, Trace ID A and Trace ID B might mean they are in the same trace, + * since the lower-64 bits of the IDs are the same. This could happen if a server A created the + * trace and propagated it to server B which ran an older tracing library. Server B could have + * truncated the trace ID to lower-64 bits. When {@code strictTraceId == false}, spans matching + * either trace ID A or B would be returned in the same trace when searching by ID A or B. Spans + * with trace ID C or D wouldn't be when searching by ID A or B because trace IDs C and D don't + * share lower 64-bits (right-most 16 characters) with trace IDs A or B. + * + *

It is also possible that all servers are capable of handling 128-bit trace identifiers, + * but are configured to only send 64-bit ones. In this case, if {@code strictTraceId == false} + * trace ID A and B would clash and be put into the same trace, causing confusion. Moreover, + * there is overhead associated with indexing spans both by 64 and 128-bit trace IDs. When a + * site has finished upgrading to 128-bit trace IDs, they should enable this setting. + * + *

See https://github.com/openzipkin/b3-propagation/issues/6 for the status of known open + * source libraries on 128-bit trace identifiers. + */ + public abstract Builder strictTraceId(boolean strictTraceId); + + public abstract StorageComponent build(); + } +} diff --git a/zipkin/src/test/java/zipkin/collector/CollectorTest.java b/zipkin/src/test/java/zipkin/collector/CollectorTest.java index 1c6b2bf022a..fbc60355b80 100644 --- a/zipkin/src/test/java/zipkin/collector/CollectorTest.java +++ b/zipkin/src/test/java/zipkin/collector/CollectorTest.java @@ -96,15 +96,13 @@ public class CollectorTest { * double-conversion. */ @Test public void routesToSpan2Collector() { - abstract class WithSpan2 extends V2StorageComponent implements zipkin.storage.StorageComponent { - @Override public abstract SpanConsumer v2SpanConsumer(); - } - WithSpan2 storage = mock(WithSpan2.class); + zipkin.internal.v2.storage.StorageComponent storage = + mock(zipkin.internal.v2.storage.StorageComponent.class); SpanConsumer span2Consumer = mock(SpanConsumer.class); - when(storage.v2SpanConsumer()).thenReturn(span2Consumer); + when(storage.spanConsumer()).thenReturn(span2Consumer); collector = spy(Collector.builder(Collector.class) - .storage(storage).build()); + .storage(V2StorageComponent.create(storage)).build()); byte[] bytes = SpanBytesEncoder.JSON_V2.encodeList(asList(span2_1)); collector.acceptSpans(bytes, SpanDecoder.DETECTING_DECODER, NOOP); diff --git a/zipkin/src/test/java/zipkin/internal/V2SpanConverterTest.java b/zipkin/src/test/java/zipkin/internal/V2SpanConverterTest.java index 9ccf5772c64..96a2e962180 100644 --- a/zipkin/src/test/java/zipkin/internal/V2SpanConverterTest.java +++ b/zipkin/src/test/java/zipkin/internal/V2SpanConverterTest.java @@ -26,7 +26,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static zipkin.Constants.LOCAL_COMPONENT; -import static zipkin.internal.V2SpanConverter.toEndpoint; public class V2SpanConverterTest { Endpoint frontend = Endpoint.create("frontend", 127 << 24 | 1); @@ -44,8 +43,8 @@ public class V2SpanConverterTest { .id("5b4185666d50f68b") .name("get") .kind(Kind.CLIENT) - .localEndpoint(toEndpoint(frontend)) - .remoteEndpoint(toEndpoint(backend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) + .remoteEndpoint(V2SpanConverter.fromEndpoint(backend)) .timestamp(1472470996199000L) .duration(207000L) .addAnnotation(1472470996238000L, Constants.WIRE_SEND) @@ -84,7 +83,7 @@ public class V2SpanConverterTest { .id("5b4185666d50f68b") .name("get") .kind(Kind.CLIENT) - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996199000L) .addAnnotation(1472470996238000L, Constants.WIRE_SEND) .build(); @@ -112,7 +111,7 @@ public class V2SpanConverterTest { .parentId("6b221d5bc9e6496c") .id("5b4185666d50f68b") .name("get") - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996199000L) .duration(1472470996238000L - 1472470996199000L) .addAnnotation(1472470996199000L, Constants.CLIENT_SEND) @@ -140,8 +139,8 @@ public class V2SpanConverterTest { .parentId("6b221d5bc9e6496c") .id("5b4185666d50f68b") .name("get") - .localEndpoint(toEndpoint(frontend)) - .remoteEndpoint(toEndpoint(backend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) + .remoteEndpoint(V2SpanConverter.fromEndpoint(backend)) .timestamp(1472470996199000L) .duration(207000L) .build(); @@ -171,7 +170,7 @@ public class V2SpanConverterTest { .id("5b4185666d50f68b") .kind(Kind.CLIENT) .name("get") - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996199000L) .duration(207000L) .build(); @@ -200,8 +199,8 @@ public class V2SpanConverterTest { .id("216a2aea45d08fc9") .name("get") .kind(Kind.SERVER) - .localEndpoint(toEndpoint(backend)) - .remoteEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(backend)) + .remoteEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996199000L) .duration(207000L) .putTag(TraceKeys.HTTP_PATH, "/api") @@ -285,7 +284,7 @@ public class V2SpanConverterTest { .parentId("1") .id("2") .name("local") - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996199000L) .duration(207000L) .build(); @@ -336,8 +335,8 @@ public class V2SpanConverterTest { // the client side owns timestamp and duration Span client = builder.clone() .kind(Kind.CLIENT) - .localEndpoint(toEndpoint(frontend)) - .remoteEndpoint(toEndpoint(backend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) + .remoteEndpoint(V2SpanConverter.fromEndpoint(backend)) .timestamp(1472470996199000L) .duration(207000L) .addAnnotation(1472470996238000L, Constants.WIRE_SEND) @@ -350,8 +349,8 @@ public class V2SpanConverterTest { Span server = builder.clone() .kind(Kind.SERVER) .shared(true) - .localEndpoint(toEndpoint(backend)) - .remoteEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(backend)) + .remoteEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996250000L) .duration(100000L) .putTag(TraceKeys.HTTP_PATH, "/backend") @@ -384,7 +383,7 @@ public class V2SpanConverterTest { .name("get") .kind(Kind.SERVER) .shared(true) - .localEndpoint(toEndpoint(backend)) + .localEndpoint(V2SpanConverter.fromEndpoint(backend)) .timestamp(1472470996250000L) .duration(100000L) .build(); @@ -416,7 +415,7 @@ public class V2SpanConverterTest { Span client = builder.clone() .kind(Kind.CLIENT) - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996199000L) .duration(207000L) .build(); @@ -424,7 +423,7 @@ public class V2SpanConverterTest { Span server = builder.clone() .kind(Kind.SERVER) .shared(true) - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996250000L) .duration(100000L) .build(); @@ -452,14 +451,14 @@ public class V2SpanConverterTest { Span client = builder.clone() .kind(Kind.CLIENT) - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996199000L) .build(); Span server = builder.clone() .kind(Kind.SERVER) .shared(true) - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996250000L) .build(); @@ -483,7 +482,7 @@ public class V2SpanConverterTest { .id("5b4185666d50f68b") .name("send") .kind(Kind.PRODUCER) - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996199000L) .build(); @@ -509,9 +508,9 @@ public class V2SpanConverterTest { .id("5b4185666d50f68b") .name("send") .kind(Kind.PRODUCER) - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996199000L) - .remoteEndpoint(toEndpoint(kafka)) + .remoteEndpoint(V2SpanConverter.fromEndpoint(kafka)) .build(); assertThat(V2SpanConverter.toSpan(span2)) @@ -539,7 +538,7 @@ public class V2SpanConverterTest { .id("5b4185666d50f68b") .name("send") .kind(Kind.PRODUCER) - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996199000L) .duration(51000L) .build(); @@ -567,7 +566,7 @@ public class V2SpanConverterTest { .id("5b4185666d50f68b") .name("send") .kind(Kind.CONSUMER) - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996199000L) .build(); @@ -595,8 +594,8 @@ public class V2SpanConverterTest { .id("5b4185666d50f68b") .name("send") .kind(Kind.CONSUMER) - .localEndpoint(toEndpoint(frontend)) - .remoteEndpoint(toEndpoint(kafka)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) + .remoteEndpoint(V2SpanConverter.fromEndpoint(kafka)) .timestamp(1472470996199000L) .build(); @@ -625,7 +624,7 @@ public class V2SpanConverterTest { .id("5b4185666d50f68b") .name("send") .kind(Kind.CONSUMER) - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996199000L) .duration(51000L) .build(); @@ -659,8 +658,8 @@ public class V2SpanConverterTest { Span producer = builder.clone() .kind(Kind.PRODUCER) - .localEndpoint(toEndpoint(frontend)) - .remoteEndpoint(toEndpoint(kafka)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) + .remoteEndpoint(V2SpanConverter.fromEndpoint(kafka)) .timestamp(1472470996199000L) .duration(1472470996238000L - 1472470996199000L) .build(); @@ -668,8 +667,8 @@ public class V2SpanConverterTest { Span consumer = builder.clone() .kind(Kind.CONSUMER) .shared(true) - .localEndpoint(toEndpoint(backend)) - .remoteEndpoint(toEndpoint(kafka)) + .localEndpoint(V2SpanConverter.fromEndpoint(backend)) + .remoteEndpoint(V2SpanConverter.fromEndpoint(kafka)) .timestamp(1472470996403000L) .duration(1472470996406000L - 1472470996403000L) .build(); @@ -700,7 +699,7 @@ public class V2SpanConverterTest { Span producer = builder.clone() .kind(Kind.PRODUCER) - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996199000L) .duration(1472470996238000L - 1472470996199000L) .build(); @@ -708,7 +707,7 @@ public class V2SpanConverterTest { Span consumer = builder.clone() .kind(Kind.CONSUMER) .shared(true) - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp(1472470996403000L) .duration(1472470996406000L - 1472470996403000L) .build(); @@ -738,7 +737,7 @@ public class V2SpanConverterTest { .name("missing"); Span first = builder.clone() - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .addAnnotation(1472470996199000L, "foo") .addAnnotation(1472470996238000L, "bar") .addAnnotation(1472470996403000L, "missing") @@ -747,7 +746,7 @@ public class V2SpanConverterTest { .build(); Span second = builder.clone() - .localEndpoint(toEndpoint(backend)) + .localEndpoint(V2SpanConverter.fromEndpoint(backend)) .addAnnotation(1472470996250000L, "baz") .addAnnotation(1472470996350000L, "qux") .putTag("baz", "qux") @@ -781,7 +780,7 @@ public class V2SpanConverterTest { .traceId("1") .name("test") .id("2") - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .putTag("bool", "true") .putTag("short", "20") .putTag("int", "32800") diff --git a/zipkin/src/test/java/zipkin/internal/V2SpanStoreAdapterTest.java b/zipkin/src/test/java/zipkin/internal/V2SpanStoreAdapterTest.java index 0ec502089c6..34f93172020 100644 --- a/zipkin/src/test/java/zipkin/internal/V2SpanStoreAdapterTest.java +++ b/zipkin/src/test/java/zipkin/internal/V2SpanStoreAdapterTest.java @@ -41,7 +41,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static zipkin.TestObjects.TODAY; -import static zipkin.internal.V2SpanConverter.toEndpoint; public class V2SpanStoreAdapterTest { @Rule public MockitoRule mocks = MockitoJUnit.rule(); @@ -61,14 +60,14 @@ public class V2SpanStoreAdapterTest { List skewedTrace2 = asList( builder.clone() .kind(Span.Kind.CLIENT) - .localEndpoint(toEndpoint(frontend)) + .localEndpoint(V2SpanConverter.fromEndpoint(frontend)) .timestamp((TODAY + 200) * 1000) .duration(120_000L) .build(), builder.clone() .kind(Span.Kind.SERVER) .shared(true) - .localEndpoint(toEndpoint(backend)) + .localEndpoint(V2SpanConverter.fromEndpoint(backend)) .timestamp((TODAY + 100) * 1000) // received before sent! .duration(60_000L) .build() diff --git a/zipkin/src/test/java/zipkin/internal/v2/SpanTest.java b/zipkin/src/test/java/zipkin/internal/v2/SpanTest.java index 1a2f51ce9d1..2a562f1706a 100644 --- a/zipkin/src/test/java/zipkin/internal/v2/SpanTest.java +++ b/zipkin/src/test/java/zipkin/internal/v2/SpanTest.java @@ -17,14 +17,14 @@ import java.io.ObjectOutputStream; import okio.Buffer; import org.junit.Test; +import zipkin.internal.V2SpanConverter; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.data.MapEntry.entry; import static zipkin.TestObjects.APP_ENDPOINT; -import static zipkin.internal.V2SpanConverter.toEndpoint; public class SpanTest { - Span base = Span.newBuilder().traceId("1").id("1").localEndpoint(toEndpoint(APP_ENDPOINT)).build(); + Span base = Span.newBuilder().traceId("1").id("1").localEndpoint(V2SpanConverter.fromEndpoint(APP_ENDPOINT)).build(); @Test public void traceIdString() { Span with128BitId = base.toBuilder() diff --git a/zipkin/src/test/java/zipkin/internal/v2/codec/SpanBytesEncoderTest.java b/zipkin/src/test/java/zipkin/internal/v2/codec/SpanBytesEncoderTest.java index 588ffcdb141..411253893f8 100644 --- a/zipkin/src/test/java/zipkin/internal/v2/codec/SpanBytesEncoderTest.java +++ b/zipkin/src/test/java/zipkin/internal/v2/codec/SpanBytesEncoderTest.java @@ -22,11 +22,11 @@ import zipkin.Constants; import zipkin.Endpoint; import zipkin.TraceKeys; -import zipkin.internal.V2SpanConverter; import zipkin.internal.v2.Span; import static org.assertj.core.api.Assertions.assertThat; import static zipkin.internal.Util.UTF_8; +import static zipkin.internal.V2SpanConverter.fromEndpoint; import static zipkin.internal.V2SpanConverter.toEndpoint; public class SpanBytesEncoderTest { @@ -43,8 +43,8 @@ public class SpanBytesEncoderTest { .id("5b4185666d50f68b") .name("get") .kind(Span.Kind.CLIENT) - .localEndpoint(toEndpoint(frontend)) - .remoteEndpoint(toEndpoint(backend)) + .localEndpoint(fromEndpoint(frontend)) + .remoteEndpoint(fromEndpoint(backend)) .timestamp(1472470996199000L) .duration(207000L) .addAnnotation(1472470996238000L, Constants.WIRE_SEND) @@ -181,8 +181,7 @@ public class SpanBytesEncoderTest { + " }\n" + "}"; - assertThat( - V2SpanConverter.toEndpoint(SpanBytesCodec.JSON_V2.decode(json.getBytes(UTF_8)).localEndpoint())) + assertThat(toEndpoint(SpanBytesCodec.JSON_V2.decode(json.getBytes(UTF_8)).localEndpoint())) .isEqualTo(Endpoint.create("", 127 << 24 | 1)); } @@ -324,7 +323,8 @@ public class SpanBytesEncoderTest { @Test public void spanRoundTrip_noRemoteServiceName() throws IOException { span = span.toBuilder() - .remoteEndpoint(toEndpoint(backend.toBuilder().serviceName("").build())).build(); + .remoteEndpoint(fromEndpoint(backend.toBuilder().serviceName("").build())) + .build(); assertThat(SpanBytesCodec.JSON_V2.decode(SpanBytesEncoder.JSON_V2.encode(span))) .isEqualTo(span); diff --git a/zipkin/src/test/java/zipkin/internal/ITV2InMemoryStorage.java b/zipkin/src/test/java/zipkin/internal/v2/storage/ITV2InMemoryStorage.java similarity index 74% rename from zipkin/src/test/java/zipkin/internal/ITV2InMemoryStorage.java rename to zipkin/src/test/java/zipkin/internal/v2/storage/ITV2InMemoryStorage.java index ce77c2f26c9..f14293b58c9 100644 --- a/zipkin/src/test/java/zipkin/internal/ITV2InMemoryStorage.java +++ b/zipkin/src/test/java/zipkin/internal/v2/storage/ITV2InMemoryStorage.java @@ -11,7 +11,7 @@ * or implied. See the License for the specific language governing permissions and limitations under * the License. */ -package zipkin.internal; +package zipkin.internal.v2.storage; import java.io.IOException; import org.junit.Test; @@ -19,6 +19,7 @@ import org.junit.runner.RunWith; import zipkin.Span; import zipkin.TestObjects; +import zipkin.internal.V2StorageComponent; import zipkin.storage.StorageComponent; import static org.assertj.core.api.Assertions.assertThat; @@ -27,10 +28,10 @@ public class ITV2InMemoryStorage { public static class DependenciesTest extends zipkin.storage.DependenciesTest { - final V2InMemoryStorage storage = V2InMemoryStorage.newBuilder().build(); + final InMemoryStorage storage = InMemoryStorage.newBuilder().build(); - @Override protected V2InMemoryStorage storage() { - return storage; + @Override protected StorageComponent storage() { + return V2StorageComponent.create(storage); } @Override public void clear() { @@ -39,10 +40,10 @@ public static class DependenciesTest extends zipkin.storage.DependenciesTest { } public static class SpanStoreTest extends zipkin.storage.SpanStoreTest { - final V2InMemoryStorage storage = V2InMemoryStorage.newBuilder().build(); + final InMemoryStorage storage = InMemoryStorage.newBuilder().build(); - @Override protected V2InMemoryStorage storage() { - return storage; + @Override protected StorageComponent storage() { + return V2StorageComponent.create(storage); } @Override public void clear() throws IOException { @@ -50,7 +51,7 @@ public static class SpanStoreTest extends zipkin.storage.SpanStoreTest { } /** This shows when spans are sent multiple times. Doing so can reveal instrumentation bugs. */ - @Test public void getRawTrace_sameSpanTwice(){ + @Test public void getRawTrace_sameSpanTwice() { Span span = TestObjects.LOTS_OF_SPANS[0]; accept(span); accept(span); @@ -61,10 +62,10 @@ public static class SpanStoreTest extends zipkin.storage.SpanStoreTest { } public static class StrictTraceIdFalseTest extends zipkin.storage.StrictTraceIdFalseTest { - final V2InMemoryStorage storage = V2InMemoryStorage.newBuilder().strictTraceId(false).build(); + final InMemoryStorage storage = InMemoryStorage.newBuilder().strictTraceId(false).build(); @Override protected StorageComponent storage() { - return storage; + return V2StorageComponent.create(storage); } @Override public void clear() throws IOException {