From 112c99c565268f13541d4845f581868dc0548a9f Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Fri, 25 May 2018 08:42:25 +0900 Subject: [PATCH 1/2] Removes auto-value dependency and maven metadata from core jar This removes the auto-value dependency from the zipkin2 core jar as we only use it in a couple places now. This prevents a version problem where new versions require a hard dependency. It also reduces the number of classes and the size (orders of tens of kilobytes uncompressed). By default, the maven plugin includes a copy of the pom file, which can be a kilobyte or more of text. This excludes it in favor of normal download location. --- pom.xml | 6 + zipkin/pom.xml | 2 + .../internal/V2SpanStoreAdapterTest.java | 4 +- zipkin2/pom.xml | 14 +- zipkin2/src/main/java/zipkin2/Call.java | 8 +- .../src/main/java/zipkin2/CheckResult.java | 33 ++- .../main/java/zipkin2/internal/Buffer.java | 2 +- .../zipkin2/internal/DependencyLinker.java | 39 +++- .../src/main/java/zipkin2/internal/Node.java | 44 ++-- .../java/zipkin2/storage/InMemoryStorage.java | 59 ++++-- .../java/zipkin2/storage/QueryRequest.java | 194 +++++++++++++----- 11 files changed, 272 insertions(+), 133 deletions(-) diff --git a/pom.xml b/pom.xml index 290491eea9a..40c02466c15 100755 --- a/pom.xml +++ b/pom.xml @@ -526,6 +526,12 @@ maven-jar-plugin ${maven-jar-plugin.version} + + + + false + + diff --git a/zipkin/pom.xml b/zipkin/pom.xml index 1b9d3a0c427..bfcc1fb1ec7 100644 --- a/zipkin/pom.xml +++ b/zipkin/pom.xml @@ -143,6 +143,8 @@ default-jar + + false ${project.build.outputDirectory}/META-INF/MANIFEST.MF diff --git a/zipkin/src/test/java/zipkin/internal/V2SpanStoreAdapterTest.java b/zipkin/src/test/java/zipkin/internal/V2SpanStoreAdapterTest.java index e93e6d71c00..529b311b9d2 100644 --- a/zipkin/src/test/java/zipkin/internal/V2SpanStoreAdapterTest.java +++ b/zipkin/src/test/java/zipkin/internal/V2SpanStoreAdapterTest.java @@ -1,5 +1,5 @@ /** - * Copyright 2015-2017 The OpenZipkin Authors + * Copyright 2015-2018 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 @@ -436,7 +436,7 @@ public void getDependencies_sync_wrapsIOE() throws IOException { .lookback(60L) .limit(100) .build())) - .isEqualTo(zipkin2.storage.QueryRequest.newBuilder() + .isEqualToComparingFieldByField(zipkin2.storage.QueryRequest.newBuilder() .serviceName("service") .spanName("span") .parseAnnotationQuery("annotation and tag=value") diff --git a/zipkin2/pom.xml b/zipkin2/pom.xml index 67651c4a032..7f1304587fe 100644 --- a/zipkin2/pom.xml +++ b/zipkin2/pom.xml @@ -34,18 +34,6 @@ - - com.google.auto.value - auto-value - provided - - - - javax.annotation - javax.annotation-api - provided - - org.jvnet animal-sniffer-annotation @@ -145,6 +133,8 @@ default-jar + + false ${project.build.outputDirectory}/META-INF/MANIFEST.MF diff --git a/zipkin2/src/main/java/zipkin2/Call.java b/zipkin2/src/main/java/zipkin2/Call.java index 6a1d278e236..80375e62fe4 100644 --- a/zipkin2/src/main/java/zipkin2/Call.java +++ b/zipkin2/src/main/java/zipkin2/Call.java @@ -1,5 +1,5 @@ /** - * Copyright 2015-2017 The OpenZipkin Authors + * Copyright 2015-2018 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 @@ -228,8 +228,8 @@ static final class Mapping extends Base { final Mapper mapper; final Call delegate; - Mapping(Mapper Mapper, Call delegate) { - this.mapper = Mapper; + Mapping(Mapper mapper, Call delegate) { + this.mapper = mapper; this.delegate = delegate; } @@ -320,7 +320,7 @@ static final class ErrorHandling extends Base { try { return delegate.execute(); } catch (IOException | RuntimeException | Error e) { - final AtomicReference ref = new AtomicReference(SENTINEL); + final AtomicReference ref = new AtomicReference<>(SENTINEL); errorHandler.onErrorReturn(e, new Callback() { @Override public void onSuccess(V value) { ref.set(value); diff --git a/zipkin2/src/main/java/zipkin2/CheckResult.java b/zipkin2/src/main/java/zipkin2/CheckResult.java index dc32d08be60..e1e25e40459 100644 --- a/zipkin2/src/main/java/zipkin2/CheckResult.java +++ b/zipkin2/src/main/java/zipkin2/CheckResult.java @@ -1,5 +1,5 @@ /** - * Copyright 2015-2017 The OpenZipkin Authors + * Copyright 2015-2018 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,7 +13,6 @@ */ package zipkin2; -import com.google.auto.value.AutoValue; import zipkin2.internal.Nullable; /** @@ -25,20 +24,34 @@ * * @see CheckResult#OK */ -@AutoValue -//@Immutable -public abstract class CheckResult { - public static final CheckResult OK = new AutoValue_CheckResult(true, null); +// @Immutable +public final class CheckResult { + public static final CheckResult OK = new CheckResult(true, null); public static CheckResult failed(Throwable error) { - return new AutoValue_CheckResult(false, error); + return new CheckResult(false, error); } - public abstract boolean ok(); + public boolean ok() { + return ok; + } /** Present when not ok */ - @Nullable public abstract Throwable error(); + @Nullable + public Throwable error() { + return error; + } + + final boolean ok; + final Throwable error; + + CheckResult(boolean ok, @Nullable Throwable error) { + this.ok = ok; + this.error = error; + } - CheckResult() { + @Override + public String toString() { + return "CheckResult{ok=" + ok + ", " + "error=" + error + "}"; } } diff --git a/zipkin2/src/main/java/zipkin2/internal/Buffer.java b/zipkin2/src/main/java/zipkin2/internal/Buffer.java index 2259cef638a..6764a73b943 100644 --- a/zipkin2/src/main/java/zipkin2/internal/Buffer.java +++ b/zipkin2/src/main/java/zipkin2/internal/Buffer.java @@ -341,7 +341,7 @@ long readVarint64() { if (i == 9 && (b & 0xf0) != 0) { throw new IllegalArgumentException("Greater than 64-bit varint at position " + (pos - 1)); } - result |= (long) (b & 0x7f) << i * 7; + result |= (long) (b & 0x7f) << (i * 7); } return result; } diff --git a/zipkin2/src/main/java/zipkin2/internal/DependencyLinker.java b/zipkin2/src/main/java/zipkin2/internal/DependencyLinker.java index 137f070bef8..f54bff45d4b 100644 --- a/zipkin2/src/main/java/zipkin2/internal/DependencyLinker.java +++ b/zipkin2/src/main/java/zipkin2/internal/DependencyLinker.java @@ -13,7 +13,6 @@ */ package zipkin2.internal; -import com.google.auto.value.AutoValue; import java.util.ArrayList; import java.util.Iterator; import java.util.LinkedHashMap; @@ -217,7 +216,7 @@ void addLink(String parent, String child, boolean isError) { if (logger.isLoggable(FINE)) { logger.fine("incrementing " + (isError ? "error " : "") + "link " + parent + " -> " + child); } - Pair key = Pair.of(parent, child); + Pair key = new Pair(parent, child); if (callCounts.containsKey(key)) { callCounts.put(key, callCounts.get(key) + 1); } else { @@ -241,7 +240,7 @@ public static List merge(Iterable in) { Map errorCounts = new LinkedHashMap<>(); for (DependencyLink link : in) { - Pair parentChild = Pair.of(link.parent(), link.child()); + Pair parentChild = new Pair(link.parent(), link.child()); long callCount = callCounts.containsKey(parentChild) ? callCounts.get(parentChild) : 0L; callCount += link.callCount(); callCounts.put(parentChild, callCount); @@ -259,8 +258,8 @@ static List link(Map callCounts, for (Map.Entry entry : callCounts.entrySet()) { Pair parentChild = entry.getKey(); result.add(DependencyLink.newBuilder() - .parent(parentChild.left()) - .child(parentChild.right()) + .parent(parentChild.left) + .child(parentChild.right) .callCount(entry.getValue()) .errorCount(errorCounts.containsKey(parentChild) ? errorCounts.get(parentChild) : 0L) .build()); @@ -268,12 +267,30 @@ static List link(Map callCounts, return result; } - @AutoValue - static abstract class Pair { - static Pair of(String left, String right) { - return new AutoValue_DependencyLinker_Pair(left, right); + static final class Pair { + final String left, right; + + Pair(String left, String right) { + this.left = left; + this.right = right; + } + + @Override + public boolean equals(Object o) { + if (o == this) return true; + if (!(o instanceof Pair)) return false; + Pair that = (DependencyLinker.Pair) o; + return left.equals(that.left) && right.equals(that.right); + } + + @Override + public int hashCode() { + int h$ = 1; + h$ *= 1000003; + h$ ^= left.hashCode(); + h$ *= 1000003; + h$ ^= right.hashCode(); + return h$; } - abstract String left(); - abstract String right(); } } diff --git a/zipkin2/src/main/java/zipkin2/internal/Node.java b/zipkin2/src/main/java/zipkin2/internal/Node.java index 540a350b05f..ab93dce28cd 100644 --- a/zipkin2/src/main/java/zipkin2/internal/Node.java +++ b/zipkin2/src/main/java/zipkin2/internal/Node.java @@ -13,7 +13,6 @@ */ package zipkin2.internal; -import com.google.auto.value.AutoValue; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collection; @@ -39,12 +38,12 @@ public final class Node { /** Set via {@link #addChild(Node)} */ - private Node parent; + Node parent; /** mutable as some transformations, such as clock skew, adjust this. */ - private V value; + V value; /** mutable to avoid allocating lists for childless nodes */ - private List> children = Collections.emptyList(); - private boolean missingRootDummyNode; + List> children = Collections.emptyList(); + boolean missingRootDummyNode; /** Returns the parent, or null if root */ @Nullable public Node parent() { @@ -161,14 +160,14 @@ public boolean addNode(@Nullable String parentId, String id, V value) { } } idToParent.put(id, parentId); - entries.add(Entry.create(parentId, id, value)); + entries.add(new Entry<>(parentId, id, value)); return true; } void processNode(Entry entry) { - String parentId = entry.parentId() != null ? entry.parentId() : idToParent.get(entry.id()); - String id = entry.id(); - V value = entry.value(); + String parentId = entry.parentId != null ? entry.parentId : idToParent.get(entry.id); + String id = entry.id; + V value = entry.value; if (parentId == null) { if (rootId != null) { @@ -224,16 +223,27 @@ public Node build() { } } - @AutoValue - static abstract class Entry { - static Entry create(@Nullable String parentId, String id, V value) { - return new AutoValue_Node_Entry(parentId, id, value); + static final class Entry { + @Nullable final String parentId; + final String id; + final V value; + + Entry(@Nullable String parentId, String id, V value) { + if (id == null) throw new NullPointerException("id == null"); + if (value == null) throw new NullPointerException("value == null"); + this.parentId = parentId; + this.id = id; + this.value = value; } - @Nullable abstract String parentId(); - - abstract String id(); + @Override + public String toString() { + return "Entry{parentId=" + parentId + ", id=" + id + ", value=" + value + "}"; + } + } - abstract V value(); + @Override + public String toString() { + return "Node{parent=" + parent + ", value=" + value + ", children=" + children + "}"; } } diff --git a/zipkin2/src/main/java/zipkin2/storage/InMemoryStorage.java b/zipkin2/src/main/java/zipkin2/storage/InMemoryStorage.java index 31fe74c11ea..ee82d07a3c4 100644 --- a/zipkin2/src/main/java/zipkin2/storage/InMemoryStorage.java +++ b/zipkin2/src/main/java/zipkin2/storage/InMemoryStorage.java @@ -13,8 +13,6 @@ */ package zipkin2.storage; -import com.google.auto.value.AutoValue; -import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -152,7 +150,7 @@ public synchronized void clear() { for (Span span : spans) { long timestamp = span.timestampAsLong(); String lowTraceId = lowTraceId(span.traceId()); - TraceIdTimestamp traceIdTimeStamp = TraceIdTimestamp.create(lowTraceId, timestamp); + TraceIdTimestamp traceIdTimeStamp = new TraceIdTimestamp(lowTraceId, timestamp); spansByTraceIdTimeStamp.put(traceIdTimeStamp, span); traceIdToTraceIdTimeStamps.put(lowTraceId, traceIdTimeStamp); acceptedSpanCount++; @@ -171,17 +169,6 @@ public synchronized void clear() { return Call.create(null /* Void == null */); } - @AutoValue - static abstract class TraceIdTimestamp { - static TraceIdTimestamp create(String traceId, long timestamp) { - return new AutoValue_InMemoryStorage_TraceIdTimestamp(traceId, timestamp); - } - - abstract String lowTraceId(); - - abstract long timestamp(); - } - /** Returns the count of spans evicted. */ int evictToRecoverSpans(int spansToRecover) { int spansEvicted = 0; @@ -196,7 +183,7 @@ int evictToRecoverSpans(int spansToRecover) { /** Returns the count of spans evicted. */ private int deleteOldestTrace() { int spansEvicted = 0; - String lowTraceId = spansByTraceIdTimeStamp.delegate.lastKey().lowTraceId(); + String lowTraceId = spansByTraceIdTimeStamp.delegate.lastKey().lowTraceId; Collection traceIdTimeStamps = traceIdToTraceIdTimeStamps.remove(lowTraceId); for (Iterator traceIdTimeStampIter = traceIdTimeStamps.iterator(); traceIdTimeStampIter.hasNext(); ) { @@ -279,8 +266,8 @@ Set traceIdsDescendingByTimestamp(QueryRequest request) { if (traceIdTimestamps == null || traceIdTimestamps.isEmpty()) return Collections.emptySet(); Set result = new LinkedHashSet<>(); for (TraceIdTimestamp traceIdTimestamp : traceIdTimestamps) { - if (traceIdTimestamp.timestamp() >= startTs || traceIdTimestamp.timestamp() <= endTs) { - result.add(traceIdTimestamp.lowTraceId()); + if (traceIdTimestamp.timestamp >= startTs || traceIdTimestamp.timestamp <= endTs) { + result.add(traceIdTimestamp.lowTraceId); } } return result; @@ -357,10 +344,10 @@ enum LinkDependencies implements Call.Mapper>, List TIMESTAMP_DESCENDING = new Comparator() { @Override public int compare(TraceIdTimestamp left, TraceIdTimestamp right) { - long x = left.timestamp(), y = right.timestamp(); + long x = left.timestamp, y = right.timestamp; int result = (x < y) ? -1 : ((x == y) ? 0 : 1); // Long.compareTo is JRE 7+ if (result != 0) return -result; // use negative as we are descending - return right.lowTraceId().compareTo(left.lowTraceId()); + return right.lowTraceId.compareTo(left.lowTraceId); } @Override public String toString() { @@ -435,7 +422,7 @@ Collection get(K key) { } } - private List spansByTraceId(String lowTraceId) { + List spansByTraceId(String lowTraceId) { List sameTraceId = new ArrayList<>(); for (TraceIdTimestamp traceIdTimestamp : traceIdToTraceIdTimeStamps.get(lowTraceId)) { sameTraceId.addAll(spansByTraceIdTimeStamp.get(traceIdTimestamp)); @@ -443,7 +430,7 @@ private List spansByTraceId(String lowTraceId) { return sameTraceId; } - private Collection traceIdTimestampsByServiceName(String serviceName) { + Collection traceIdTimestampsByServiceName(String serviceName) { List traceIdTimestamps = new ArrayList<>(); for (String lowTraceId : serviceToTraceIds.get(serviceName)) { traceIdTimestamps.addAll(traceIdToTraceIdTimeStamps.get(lowTraceId)); @@ -464,6 +451,34 @@ static String lowTraceId(String traceId) { return this; } - @Override public void close() throws IOException { + @Override public void close() { + } + + static final class TraceIdTimestamp { + final String lowTraceId; + final long timestamp; + + TraceIdTimestamp(String lowTraceId, long timestamp) { + this.lowTraceId = lowTraceId; + this.timestamp = timestamp; + } + + @Override + public boolean equals(Object o) { + if (o == this) return true; + if (!(o instanceof TraceIdTimestamp)) return false; + TraceIdTimestamp that = (TraceIdTimestamp) o; + return lowTraceId.equals(that.lowTraceId) && timestamp == that.timestamp; + } + + @Override + public int hashCode() { + int h$ = 1; + h$ *= 1000003; + h$ ^= lowTraceId.hashCode(); + h$ *= 1000003; + h$ ^= (int) ((timestamp >>> 32) ^ timestamp); + return h$; + } } } diff --git a/zipkin2/src/main/java/zipkin2/storage/QueryRequest.java b/zipkin2/src/main/java/zipkin2/storage/QueryRequest.java index 736bc944ed4..7bb31d608f3 100644 --- a/zipkin2/src/main/java/zipkin2/storage/QueryRequest.java +++ b/zipkin2/src/main/java/zipkin2/storage/QueryRequest.java @@ -13,7 +13,6 @@ */ package zipkin2.storage; -import com.google.auto.value.AutoValue; import java.util.Collections; import java.util.Iterator; import java.util.LinkedHashMap; @@ -37,16 +36,19 @@ * microseconds, the grain of {@link Span#timestamp()}. Milliseconds is a more familiar and * supported granularity for query, index and windowing functions. */ -@AutoValue -public abstract class QueryRequest { +public final class QueryRequest { /** * When present, corresponds to {@link zipkin2.Endpoint#serviceName} and constrains all other * parameters. */ - @Nullable public abstract String serviceName(); + @Nullable public String serviceName() { + return serviceName; + } /** When present, only include traces with this {@link Span#name} */ - @Nullable public abstract String spanName(); + @Nullable public String spanName() { + return spanName; + } /** * When an input value is the empty string, include traces whose {@link Span#annotations()} @@ -55,34 +57,46 @@ public abstract class QueryRequest { * *

Multiple entries are combined with AND, and AND against other conditions. */ - public abstract Map annotationQuery(); + public Map annotationQuery(){ + return annotationQuery; + } /** * Only return traces whose {@link Span#duration()} is greater than or equal to minDuration * microseconds. */ - @Nullable public abstract Long minDuration(); + @Nullable public Long minDuration() { + return minDuration; + } /** * Only return traces whose {@link Span#duration()} is less than or equal to maxDuration * microseconds. Only valid with {@link #minDuration}. */ - @Nullable public abstract Long maxDuration(); + @Nullable public Long maxDuration() { + return maxDuration; + } /** * Only return traces where all {@link Span#timestamp()} are at or before this time in epoch * milliseconds. Defaults to current time. */ - public abstract long endTs(); + public long endTs() { + return endTs; + } /** * Only return traces where all {@link Span#timestamp()} are at or after (endTs - lookback) in * milliseconds. Defaults to endTs. */ - public abstract long lookback(); + public long lookback() { + return lookback; + } /** Maximum number of traces to return. Defaults to 10 */ - public abstract int limit(); + public int limit() { + return limit; + } /** * Corresponds to query parameter "annotationQuery". Ex. "http.method=GET and error" @@ -103,24 +117,47 @@ public abstract class QueryRequest { return result.length() > 0 ? result.toString() : null; } - public abstract Builder toBuilder(); + public Builder toBuilder() { + return new Builder(this); + } public static Builder newBuilder() { - return new AutoValue_QueryRequest.Builder().annotationQuery(Collections.emptyMap()); + return new Builder(); } - @AutoValue.Builder - public abstract static class Builder { + public static final class Builder { + String serviceName, spanName; + Map annotationQuery = Collections.emptyMap(); + Long minDuration, maxDuration; + long endTs, lookback; + int limit; + + Builder(QueryRequest source) { + serviceName = source.serviceName; + spanName = source.spanName; + annotationQuery = source.annotationQuery; + minDuration = source.minDuration; + maxDuration = source.maxDuration; + endTs = source.endTs; + lookback = source.lookback; + limit = source.limit; + } /** @see QueryRequest#serviceName() */ - public abstract Builder serviceName(@Nullable String serviceName); + public Builder serviceName(@Nullable String serviceName) { + this.serviceName = serviceName; + return this; + } /** * This ignores the reserved span name "all". * * @see QueryRequest#spanName() */ - public abstract Builder spanName(@Nullable String spanName); + public Builder spanName(@Nullable String spanName) { + this.spanName = spanName; + return this; + } /** * Corresponds to query parameter "annotationQuery". Ex. "http.method=GET and error" @@ -143,61 +180,73 @@ public Builder parseAnnotationQuery(@Nullable String annotationQuery) { } /** @see QueryRequest#annotationQuery() */ - public abstract Builder annotationQuery(Map annotationQuery); + public Builder annotationQuery(Map annotationQuery) { + if (annotationQuery == null) throw new NullPointerException("annotationQuery == null"); + this.annotationQuery = annotationQuery; + return this; + } /** @see QueryRequest#minDuration() */ - public abstract Builder minDuration(@Nullable Long minDuration); + public Builder minDuration(@Nullable Long minDuration) { + this.minDuration = minDuration; + return this; + } /** @see QueryRequest#maxDuration() */ - public abstract Builder maxDuration(@Nullable Long maxDuration); + public Builder maxDuration(@Nullable Long maxDuration) { + this.maxDuration = maxDuration; + return this; + } /** @see QueryRequest#endTs() */ - public abstract Builder endTs(long endTs); + public Builder endTs(long endTs) { + this.endTs = endTs; + return this; + } /** @see QueryRequest#lookback() */ - public abstract Builder lookback(long lookback); + public Builder lookback(long lookback) { + this.lookback = lookback; + return this; + } /** @see QueryRequest#limit() */ - public abstract Builder limit(int limit); - - // getters for validation - @Nullable abstract String serviceName(); - - @Nullable abstract String spanName(); - - abstract Map annotationQuery(); - - @Nullable abstract Long minDuration(); - - @Nullable abstract Long maxDuration(); - - abstract long endTs(); - - abstract int limit(); - - abstract QueryRequest autoBuild(); + public Builder limit(int limit) { + this.limit = limit; + return this; + } public final QueryRequest build() { // coerce service and span names to lowercase - if (serviceName() != null) serviceName(serviceName().toLowerCase(Locale.ROOT)); - if (spanName() != null) spanName(spanName().toLowerCase(Locale.ROOT)); + if (serviceName != null) serviceName = serviceName.toLowerCase(Locale.ROOT); + if (spanName != null) spanName = spanName.toLowerCase(Locale.ROOT); // remove any accidental empty strings - annotationQuery().remove(""); - if ("".equals(serviceName())) serviceName(null); - if ("".equals(spanName()) || "all".equals(spanName())) spanName(null); - - if (endTs() <= 0) throw new IllegalArgumentException("endTs <= 0"); - if (limit() <= 0) throw new IllegalArgumentException("limit <= 0"); - if (minDuration() != null) { - if (minDuration() <= 0) throw new IllegalArgumentException("minDuration <= 0"); - if (maxDuration() != null && maxDuration() < minDuration()) { + annotationQuery.remove(""); + if ("".equals(serviceName)) serviceName = null ; + if ("".equals(spanName) || "all".equals(spanName)) spanName = null; + + if (endTs <= 0) throw new IllegalArgumentException("endTs <= 0"); + if (limit <= 0) throw new IllegalArgumentException("limit <= 0"); + if (minDuration != null) { + if (minDuration <= 0) throw new IllegalArgumentException("minDuration <= 0"); + if (maxDuration != null && maxDuration < minDuration) { throw new IllegalArgumentException("maxDuration < minDuration"); } - } else if (maxDuration() != null) { + } else if (maxDuration != null) { throw new IllegalArgumentException("maxDuration is only valid with minDuration"); } - return autoBuild(); + + return new QueryRequest( + serviceName, + spanName, + annotationQuery, + minDuration, + maxDuration, + endTs, + lookback, + limit + ); } Builder() { @@ -270,6 +319,43 @@ timestamp > endTs() * 1000) { && testedDuration; } - QueryRequest() { + + final String serviceName, spanName; + final Map annotationQuery; + final Long minDuration, maxDuration; + final long endTs, lookback; + final int limit; + + QueryRequest( + @Nullable String serviceName, + @Nullable String spanName, + Map annotationQuery, + @Nullable Long minDuration, + @Nullable Long maxDuration, + long endTs, + long lookback, + int limit) { + this.serviceName = serviceName; + this.spanName = spanName; + this.annotationQuery = annotationQuery; + this.minDuration = minDuration; + this.maxDuration = maxDuration; + this.endTs = endTs; + this.lookback = lookback; + this.limit = limit; + } + + @Override + public String toString() { + return "QueryRequest{" + + "serviceName=" + serviceName + ", " + + "spanName=" + spanName + ", " + + "annotationQuery=" + annotationQuery + ", " + + "minDuration=" + minDuration + ", " + + "maxDuration=" + maxDuration + ", " + + "endTs=" + endTs + ", " + + "lookback=" + lookback + ", " + + "limit=" + limit + + "}"; } } From 2b44ade3526d79863cb8148063ee546fdcc46b97 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Fri, 25 May 2018 10:19:26 +0900 Subject: [PATCH 2/2] adds missing check --- .../zipkin/internal/V2SpanStoreAdapterTest.java | 14 ++++++++++++++ .../main/java/zipkin2/storage/QueryRequest.java | 1 + 2 files changed, 15 insertions(+) diff --git a/zipkin/src/test/java/zipkin/internal/V2SpanStoreAdapterTest.java b/zipkin/src/test/java/zipkin/internal/V2SpanStoreAdapterTest.java index 529b311b9d2..ef3c14d113d 100644 --- a/zipkin/src/test/java/zipkin/internal/V2SpanStoreAdapterTest.java +++ b/zipkin/src/test/java/zipkin/internal/V2SpanStoreAdapterTest.java @@ -40,6 +40,7 @@ import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static zipkin.TestObjects.DAY; import static zipkin.TestObjects.TODAY; public class V2SpanStoreAdapterTest { @@ -448,6 +449,19 @@ public void getDependencies_sync_wrapsIOE() throws IOException { .build()); } + @Test public void convert_queryRequest_less() { + // from CassandraSpanStoreTest.overFetchesToCompensateForDuplicateIndexData + QueryRequest input = QueryRequest.builder() + .serviceName("web") + .lookback(DAY).limit(2000).build(); + zipkin2.storage.QueryRequest converted = V2SpanStoreAdapter.convertRequest(input); + + assertThat(converted.serviceName()).isEqualTo(input.serviceName); + assertThat(converted.endTs()).isEqualTo(input.endTs); + assertThat(converted.lookback()).isEqualTo(input.lookback); + assertThat(converted.limit()).isEqualTo(input.limit); + } + void doEnqueue(Consumer answer) { doAnswer(invocation -> { answer.accept((zipkin2.Callback) invocation.getArguments()[0]); diff --git a/zipkin2/src/main/java/zipkin2/storage/QueryRequest.java b/zipkin2/src/main/java/zipkin2/storage/QueryRequest.java index 7bb31d608f3..a0fa1a74487 100644 --- a/zipkin2/src/main/java/zipkin2/storage/QueryRequest.java +++ b/zipkin2/src/main/java/zipkin2/storage/QueryRequest.java @@ -228,6 +228,7 @@ public final QueryRequest build() { if (endTs <= 0) throw new IllegalArgumentException("endTs <= 0"); if (limit <= 0) throw new IllegalArgumentException("limit <= 0"); + if (lookback <= 0) throw new IllegalArgumentException("lookback <= 0"); if (minDuration != null) { if (minDuration <= 0) throw new IllegalArgumentException("minDuration <= 0"); if (maxDuration != null && maxDuration < minDuration) {