Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sorting pivot values numerically when field is numeric. #20918

Open
wants to merge 35 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
d4b1a2f
Sorting pivot values numerically when field is numeric.
dennisoelkers Nov 8, 2024
a9cbff9
Improving test code infrastructure.
dennisoelkers Nov 8, 2024
92599d8
Adding license header.
dennisoelkers Nov 8, 2024
b2ae0db
Adding changelog snippet.
dennisoelkers Nov 8, 2024
b901868
Adding mandatory description.
dennisoelkers Nov 11, 2024
715c0f0
Cleaning up tests.
dennisoelkers Nov 11, 2024
3f1380f
Completing test case
dennisoelkers Nov 11, 2024
6d6221a
Fixing test.
dennisoelkers Nov 11, 2024
f82989d
Cleaning up.
dennisoelkers Nov 11, 2024
f81b98b
Using query again to limit search scope.
dennisoelkers Nov 11, 2024
91f5bd4
Changing order to see error earlier.
dennisoelkers Nov 11, 2024
777231d
Merge remote-tracking branch 'origin/master' into fix/numeric-pivot-s…
dennisoelkers Nov 11, 2024
a38bce9
Removing duplicate assertion.
dennisoelkers Nov 12, 2024
5713da1
Cleaning up method signatures.
dennisoelkers Nov 12, 2024
64c6a55
Adding clarifying comments.
dennisoelkers Nov 12, 2024
473e747
Adding additional tests.
dennisoelkers Nov 12, 2024
fcc397a
Generating proper aggregation sort for if multiple pivots.
dennisoelkers Nov 13, 2024
7f6470a
Wrapping query with stream, so pivot streams can be defined in tests.
dennisoelkers Nov 13, 2024
ea868f6
Normalize streams properly before fetching field type.
dennisoelkers Nov 13, 2024
0931271
Merge remote-tracking branch 'origin/master' into fix/numeric-pivot-s…
dennisoelkers Nov 13, 2024
671fa4b
Improving test robustness.
dennisoelkers Nov 13, 2024
71c1327
Renaming variable to communicate its intent better.
dennisoelkers Nov 13, 2024
ca781c4
Cleaning up.
dennisoelkers Nov 13, 2024
01abd53
Waiting longer for field types.
dennisoelkers Nov 13, 2024
89fe6c0
Refreshing before executing search.
dennisoelkers Nov 13, 2024
52cd672
Revert "Refreshing before executing search."
dennisoelkers Nov 14, 2024
c064cd5
Wait for messages scopes to stream.
dennisoelkers Nov 14, 2024
55e0bdf
Waiting longer for messages.
dennisoelkers Nov 14, 2024
1a22d7a
Merge branch 'master' into fix/numeric-pivot-sorting
dennisoelkers Nov 14, 2024
3c734e1
Increasing wait for messages by a lot for testing.
dennisoelkers Nov 14, 2024
17af2d9
Using backend failure config for tests.
dennisoelkers Nov 15, 2024
4868912
Ingesting each message three times.
dennisoelkers Nov 15, 2024
fed438e
Cleaning up.
dennisoelkers Nov 15, 2024
feadd0a
Show backend log when waiting for messages fails.
dennisoelkers Nov 15, 2024
d866d3a
Merge branch 'master' into fix/numeric-pivot-sorting
dennisoelkers Nov 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/unreleased/issue-enterprise-6479.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type = "fixed"
message = "Sorting pivot values numerically when field is numeric."

issues = ["Graylog2/graylog-plugin-enterprise#6479"]
pulls = ["20918"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
/*
* Copyright (C) 2020 Graylog, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the Server Side Public License, version 1,
* as published by MongoDB, Inc.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* Server Side Public License for more details.
*
* You should have received a copy of the Server Side Public License
* along with this program. If not, see
* <http://www.mongodb.com/licensing/server-side-public-license>.
*/
package org.graylog.plugins.views.aggregations;

import com.github.rholder.retry.RetryException;
import io.restassured.response.ValidatableResponse;
import org.graylog.plugins.views.search.searchtypes.pivot.Pivot;
import org.graylog.plugins.views.search.searchtypes.pivot.PivotSort;
import org.graylog.plugins.views.search.searchtypes.pivot.SeriesSort;
import org.graylog.plugins.views.search.searchtypes.pivot.SortSpec;
import org.graylog.plugins.views.search.searchtypes.pivot.buckets.Values;
import org.graylog.plugins.views.search.searchtypes.pivot.series.Count;
import org.graylog.testing.completebackend.apis.GraylogApis;
import org.graylog.testing.completebackend.apis.inputs.PortBoundGelfInputApi;
import org.graylog.testing.containermatrix.annotations.ContainerMatrixTest;
import org.graylog.testing.containermatrix.annotations.ContainerMatrixTestsConfiguration;
import org.junit.jupiter.api.BeforeAll;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.stream.IntStream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.graylog.testing.containermatrix.SearchServer.ES7;
import static org.graylog.testing.containermatrix.SearchServer.OS1;
import static org.graylog.testing.containermatrix.SearchServer.OS2_LATEST;
import static org.hamcrest.Matchers.is;

@ContainerMatrixTestsConfiguration(searchVersions = {ES7, OS1, OS2_LATEST})
public class AggregationSortingIT {
private static final Logger LOG = LoggerFactory.getLogger(AggregationSortingIT.class);
private static final String numericField = "numeric_field";
private static final String nonNumericField = "non_numeric_field";

private final GraylogApis api;
private PortBoundGelfInputApi gelfInput;

public AggregationSortingIT(GraylogApis api) {
this.api = api;
}

@BeforeAll
void setUp() {
this.gelfInput = api.gelf().createGelfHttpInput();
}

@ContainerMatrixTest
void sortingOnNumericPivotFieldSortsNumerically() throws ExecutionException, RetryException {
final var values = Set.of(9, 8, 4, 25, 2, 15, 1);
final var messagePrefix = "sorting on numeric pivot test ";
try (final var env = createEnvironment()) {
IntStream.range(0, 3).forEach((i) -> {
for (final var value : values) {
env.ingestMessage(Map.of(
nonNumericField, "foo",
numericField, value,
"short_message", messagePrefix + value
));
}
});

final var pivotBuilder = Pivot.builder()
.rowGroups(Values.builder()
.fields(List.of(nonNumericField, numericField)).limit(10).build())
.series(List.of())
.rollup(false);

waitForMessages(env, values.stream().map(value -> messagePrefix + value).toList());

env.waitForFieldTypes(numericField);

final var resultDesc = env.executePivot(
pivotBuilder
.sort(PivotSort.create(numericField, SortSpec.Direction.Descending))
.build()
);
assertThat(resultDesc).isNotNull();

expectKeys(resultDesc, "25", "15", "9", "8", "4", "2", "1");

final var resultAsc = env.executePivot(
pivotBuilder
.sort(PivotSort.create(numericField, SortSpec.Direction.Ascending))
.build());

expectKeys(resultAsc, "1", "2", "4", "8", "9", "15", "25");
}
}

@ContainerMatrixTest
void sortingOnNonNumericPivotFieldSortsLexicographically() throws ExecutionException, RetryException {
final var values = Set.of("B", "C", "D", "A", "E");
final var messagePrefix = "sorting on non-numeric pivot test ";
try (final var env = createEnvironment()) {
IntStream.range(0, 3).forEach((i) -> {
for (final var value : values) {
env.ingestMessage(Map.of(
nonNumericField, value,
numericField, 42,
"short_message", messagePrefix + value
));
}
});

final var pivotBuilder = Pivot.builder()
.rowGroups(Values.builder()
.fields(List.of(numericField, nonNumericField)).limit(10).build())
.series(List.of())
.rollup(false);

waitForMessages(env, values.stream().map(value -> messagePrefix + value).toList());

env.waitForFieldTypes(numericField);

final var resultDesc = env.executePivot(
pivotBuilder
.sort(PivotSort.create(nonNumericField, SortSpec.Direction.Ascending))
.build()
);
assertThat(resultDesc).isNotNull();

expectKeys(resultDesc, "A", "B", "C", "D", "E");

final var resultAsc = env.executePivot(
pivotBuilder
.sort(PivotSort.create(nonNumericField, SortSpec.Direction.Descending))
.build());

expectKeys(resultAsc, "E", "D", "C", "B", "A");
}
}

@ContainerMatrixTest
void sortingOnBothNumericFieldAndMetric() throws ExecutionException, RetryException {
final var values = List.of(2, 4, 9, 1, 25, 2, 9, 4, 15);
final var messagePrefix = "Ingesting value ";
try (final var env = createEnvironment()) {
IntStream.range(0, 3).forEach((i) -> {
for (final var value : values) {
env.ingestMessage(Map.of(
nonNumericField, "Test",
numericField, value,
"short_message", messagePrefix + value
));
}
});

waitForMessages(env, values.stream().distinct().map(value -> messagePrefix + value).toList());

env.waitForFieldTypes(numericField);

final var pivotBuilder = Pivot.builder()
.rowGroups(Values.builder()
.fields(List.of(nonNumericField, numericField)).limit(10).build())
.series(List.of(Count.builder().build()))
.rollup(false);

final var resultAsc = env.executePivot(
pivotBuilder
.sort(
PivotSort.create(numericField, SortSpec.Direction.Ascending),
SeriesSort.create("count()", SortSpec.Direction.Descending)
)
.build()
);

expectKeys(resultAsc, "1", "2", "4", "9", "15", "25");

final var resultDesc = env.executePivot(
pivotBuilder
.sort(
PivotSort.create(numericField, SortSpec.Direction.Descending),
SeriesSort.create("count()", SortSpec.Direction.Descending)
)
.build()
);

expectKeys(resultDesc, "25", "15", "9", "4", "2", "1");
}
}

private void waitForMessages(GraylogApis.SearchEnvironment env, Collection<String> messages) {
try {
env.waitForMessages(messages);
} catch (AssertionError e) {
LOG.error("Waiting for messages failed: {}", api.backend().getLogs());
}
}

private void expectKeys(ValidatableResponse response, String... values) {
for (int i = 0; i < values.length; i++) {
response.body(".rows[" + i + "].key[1]", is(values[i]));
}
}

private GraylogApis.SearchEnvironment createEnvironment() throws ExecutionException, RetryException {
return api.createEnvironment(gelfInput);
}
}
Loading
Loading