From beaefc1518f87f8c69a0eb03e59f11a7d70326c7 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Wed, 13 Jan 2021 15:11:45 +0100 Subject: [PATCH 1/4] Supporting `from` and `to` properties in relative time ranges. This change is implementing functionality to support an alternative way to use relative time ranges. Instead of a `range` property that defines the length of the range in seconds going backwards from now, it allows defining `from` and `to` properties defining the endpoints of a range in relation to `now`. This allows e.g. defining a range that goes from 5 minutes ago to 1 minute ago by specifying a relative range as: ``` { "type": "relative", "from": 300, "to": 60 } ``` Fixes #9898. --- .../views/search/engine/QueryBackend.java | 2 +- .../searches/timeranges/RelativeRange.java | 79 +++++++++++++------ 2 files changed, 56 insertions(+), 25 deletions(-) diff --git a/graylog2-server/src/main/java/org/graylog/plugins/views/search/engine/QueryBackend.java b/graylog2-server/src/main/java/org/graylog/plugins/views/search/engine/QueryBackend.java index 6f04df66ed37..7d721779601d 100644 --- a/graylog2-server/src/main/java/org/graylog/plugins/views/search/engine/QueryBackend.java +++ b/graylog2-server/src/main/java/org/graylog/plugins/views/search/engine/QueryBackend.java @@ -50,7 +50,7 @@ public interface QueryBackend { T generate(SearchJob job, Query query, Set predecessorResults); default boolean isAllMessages(TimeRange timeRange) { - return timeRange instanceof RelativeRange && ((RelativeRange)timeRange).range() == 0; + return timeRange instanceof RelativeRange && ((RelativeRange)timeRange).isAllMessages(); } default AbsoluteRange effectiveTimeRangeForResult(Query query, QueryResult queryResult) { diff --git a/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java b/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java index 742dc98a87b1..7d1e4d9fcf5d 100644 --- a/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java +++ b/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java @@ -18,19 +18,20 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeName; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.google.auto.value.AutoValue; -import com.google.common.collect.ImmutableMap; import org.graylog2.plugin.Tools; import org.joda.time.DateTime; -import org.joda.time.DateTimeZone; -import org.joda.time.Seconds; -import java.util.Map; +import java.util.OptionalInt; @AutoValue @JsonTypeName(RelativeRange.RELATIVE) +@JsonInclude(JsonInclude.Include.NON_ABSENT) +@JsonDeserialize(builder = RelativeRange.Builder.class) public abstract class RelativeRange extends TimeRange { public static final String RELATIVE = "relative"; @@ -40,56 +41,86 @@ public abstract class RelativeRange extends TimeRange { public abstract String type(); @JsonProperty - public abstract int range(); + public abstract OptionalInt range(); + + @JsonProperty + public abstract OptionalInt from(); + + @JsonProperty + public abstract OptionalInt to(); public int getRange() { - return range(); + return range().orElse(0); } @Override @JsonIgnore public DateTime getFrom() { // TODO this should be computed once - if (range() > 0) { - return Tools.nowUTC().minus(Seconds.seconds(range())); + if (range().isPresent()) { + return Tools.nowUTC().minusSeconds(range().getAsInt()); } - return new DateTime(0, DateTimeZone.UTC); + + return Tools.nowUTC().minusSeconds(from().orElseThrow(() -> new IllegalStateException("Neither `range` nor `from` specified!"))); } @Override @JsonIgnore public DateTime getTo() { // TODO this should be fixed - return Tools.nowUTC(); - } + if (range().isPresent()) { + return Tools.nowUTC(); + } - @JsonCreator - public static RelativeRange create(@JsonProperty("type") String type, @JsonProperty("range") int range) throws InvalidRangeParametersException { - return builder().type(type).checkRange(range).build(); + return Tools.nowUTC().minusSeconds(to().orElseThrow(() -> new IllegalStateException("Neither `range` nor `to` specified!"))); } - public static RelativeRange create(int range) throws InvalidRangeParametersException { - return create(RELATIVE, range); + @JsonIgnore + public boolean isAllMessages() { + return range().orElse(-1) == 0; } - public static Builder builder() { - return new AutoValue_RelativeRange.Builder(); + public static RelativeRange create(int range) throws InvalidRangeParametersException { + return Builder.builder() + .range(range) + .build(); } @AutoValue.Builder public abstract static class Builder { - public abstract RelativeRange build(); + abstract RelativeRange autoBuild(); + @JsonProperty("type") public abstract Builder type(String type); + @JsonProperty("range") public abstract Builder range(int range); + abstract OptionalInt range(); + + @JsonProperty("from") + public abstract Builder from(int from); + abstract OptionalInt from(); + + @JsonProperty("to") + public abstract Builder to(int to); + abstract OptionalInt to(); - // TODO replace with custom build() - public Builder checkRange(int range) throws InvalidRangeParametersException { - if (range < 0) { - throw new InvalidRangeParametersException("Range must not be negative"); + public RelativeRange build() throws InvalidRangeParametersException { + if (range().isPresent() && (from().isPresent() || to().isPresent())) { + throw new InvalidRangeParametersException("Either `range` OR `from`/`to` must be specifed, not both!"); } - return range(range); + + if (range().isPresent()) { + if (range().getAsInt() < 0) { + throw new InvalidRangeParametersException("Range must not be negative"); + } + } + return autoBuild(); + } + + @JsonCreator + public static Builder builder() { + return new AutoValue_RelativeRange.Builder().type(RELATIVE); } } From ce24dee27a6e5078b8cf1204cae07d72760116e9 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Wed, 13 Jan 2021 15:26:03 +0100 Subject: [PATCH 2/4] Sanity checking presence and relation of `from`/`to`. --- .../indexer/searches/timeranges/RelativeRange.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java b/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java index 7d1e4d9fcf5d..def8c048b09a 100644 --- a/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java +++ b/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java @@ -115,6 +115,15 @@ public RelativeRange build() throws InvalidRangeParametersException { throw new InvalidRangeParametersException("Range must not be negative"); } } + + if ((from().isPresent() && !to().isPresent()) || (to().isPresent() && !from().isPresent()) { + throw new InvalidRangeParametersException("Both `from` and `to` must be specified!"); + } + + if ((from().isPresent() && to().isPresent()) && (to().getAsInt() > from().getAsInt())) { + throw new InvalidRangeParametersException("`from` must be greater than `to`!"); + } + return autoBuild(); } From 970e54807d75a35ba3929a7297fc45dd6194f862 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Wed, 13 Jan 2021 15:49:54 +0100 Subject: [PATCH 3/4] Fixing missing brace. --- .../plugin/indexer/searches/timeranges/RelativeRange.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java b/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java index def8c048b09a..bca95c15b22e 100644 --- a/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java +++ b/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java @@ -116,14 +116,13 @@ public RelativeRange build() throws InvalidRangeParametersException { } } - if ((from().isPresent() && !to().isPresent()) || (to().isPresent() && !from().isPresent()) { + if ((from().isPresent() && !to().isPresent()) || (to().isPresent() && !from().isPresent())) { throw new InvalidRangeParametersException("Both `from` and `to` must be specified!"); } if ((from().isPresent() && to().isPresent()) && (to().getAsInt() > from().getAsInt())) { throw new InvalidRangeParametersException("`from` must be greater than `to`!"); } - return autoBuild(); } From ba1ff1276692ca459818394bedc9382da53509a5 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Thu, 14 Jan 2021 13:48:37 +0100 Subject: [PATCH 4/4] Handle all message ranges as before. --- .../plugin/indexer/searches/timeranges/RelativeRange.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java b/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java index bca95c15b22e..10186b1e571b 100644 --- a/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java +++ b/graylog2-server/src/main/java/org/graylog2/plugin/indexer/searches/timeranges/RelativeRange.java @@ -25,6 +25,7 @@ import com.google.auto.value.AutoValue; import org.graylog2.plugin.Tools; import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; import java.util.OptionalInt; @@ -58,7 +59,10 @@ public int getRange() { public DateTime getFrom() { // TODO this should be computed once if (range().isPresent()) { - return Tools.nowUTC().minusSeconds(range().getAsInt()); + final int range = range().getAsInt(); + return range > 0 + ? Tools.nowUTC().minusSeconds(range().getAsInt()) + : new DateTime(0, DateTimeZone.UTC); } return Tools.nowUTC().minusSeconds(from().orElseThrow(() -> new IllegalStateException("Neither `range` nor `from` specified!")));