Skip to content

Commit

Permalink
Fix label values limits
Browse files Browse the repository at this point in the history
Signed-off-by: Andrea Lamparelli <[email protected]>
  • Loading branch information
lampajr authored and stalep committed Feb 20, 2025
1 parent 84caca3 commit d208817
Show file tree
Hide file tree
Showing 4 changed files with 248 additions and 66 deletions.
1 change: 1 addition & 0 deletions horreum-backend/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@
<artifactId>commons-math3</artifactId>
<version>${commons.math3.version}</version>
</dependency>

<dependency>
<groupId>org.testcontainers</groupId>
<artifactId>postgresql</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ combined as (
LEFT JOIN label ON label.id = lv.label_id
WHERE dataset.testid = :testId
AND (label.id IS NULL OR (:filteringLabels AND label.filtering) OR (:metricLabels AND label.metrics)) INCLUDE_EXCLUDE_PLACEHOLDER
) SELECT * from combined FILTER_PLACEHOLDER ORDER_PLACEHOLDER LIMIT_PLACEHOLDER
) SELECT * from combined FILTER_PLACEHOLDER ORDER_PLACEHOLDER
""";

protected static final String LABEL_VALUES_QUERY_BY_RUN = """
Expand All @@ -67,7 +67,7 @@ combined as (
LEFT JOIN label_values lv ON dataset.id = lv.dataset_id
LEFT JOIN label ON label.id = lv.label_id
WHERE dataset.runid = :runId INCLUDE_EXCLUDE_PLACEHOLDER
) SELECT * from combined FILTER_PLACEHOLDER ORDER_PLACEHOLDER LIMIT_PLACEHOLDER
) SELECT * from combined FILTER_PLACEHOLDER ORDER_PLACEHOLDER
""";

protected static final String LABEL_VALUES_DATASETS_BY_TEST_AND_FILTER = """
Expand Down Expand Up @@ -278,7 +278,7 @@ public List<ExportedLabelValues> labelValuesByTest(int testId, String filter, St
includeExcludeSql = "AND label.name NOT in :exclude";
}

if (filterSql.isBlank() && filter != null && !filter.isBlank()) {
if (filterSql.isBlank() && filter != null && !filter.equals("{}") && !filter.isBlank()) {
Log.errorf("Filter SQL is empty but received not empty filter: %s", filter);
//TODO there was an error with the filter, do we return that info to the user?
}
Expand All @@ -295,16 +295,10 @@ public List<ExportedLabelValues> labelValuesByTest(int testId, String filter, St
Log.warnf("Invalid sort order received: %s", sort);
}

// --- limit
String limitSql = "";
if (limit != null) {
limitSql = "limit " + limit + " offset " + limit * Math.max(0, page);
}
String sql = LABEL_VALUES_QUERY_BY_TEST
.replace("FILTER_PLACEHOLDER", filterSql)
.replace("INCLUDE_EXCLUDE_PLACEHOLDER", includeExcludeSql)
.replace("ORDER_PLACEHOLDER", orderSql)
.replace("LIMIT_PLACEHOLDER", limitSql);
.replace("ORDER_PLACEHOLDER", orderSql);

NativeQuery<Object[]> query = (NativeQuery<Object[]>) (em.createNativeQuery(sql))
.setParameter("testId", testId)
Expand Down Expand Up @@ -368,7 +362,7 @@ public List<ExportedLabelValues> labelValuesByTest(int testId, String filter, St
.addScalar("start", StandardBasicTypes.INSTANT)
.addScalar("stop", StandardBasicTypes.INSTANT);

return LabelValuesService.parse(query.getResultList());
return LabelValuesService.parse(query.getResultList(), limit, page);
}

/**
Expand Down Expand Up @@ -405,7 +399,7 @@ public List<ExportedLabelValues> labelValuesByRun(int runId, String filter, Stri
includeExcludeSql = "AND label.name NOT in :exclude";
}

if (filterSql.isBlank() && filter != null && !filter.isBlank()) {
if (filterSql.isBlank() && filter != null && !filter.equals("{}") && !filter.isBlank()) {
Log.errorf("Filter SQL is empty but received not empty filter: %s", filter);
//TODO there was an error with the filter, do we return that info to the user?
}
Expand All @@ -419,14 +413,10 @@ public List<ExportedLabelValues> labelValuesByRun(int runId, String filter, Stri
Log.warnf("Provided sort param which is no longer supported: %s", sort);
}

// --- limit
String limitSql = "limit " + limit + " offset " + limit * Math.max(0, page);

String sql = LABEL_VALUES_QUERY_BY_RUN
.replace("FILTER_PLACEHOLDER", filterSql)
.replace("INCLUDE_EXCLUDE_PLACEHOLDER", includeExcludeSql)
.replace("ORDER_PLACEHOLDER", orderSql)
.replace("LIMIT_PLACEHOLDER", limitSql);
.replace("ORDER_PLACEHOLDER", orderSql);

NativeQuery<Object[]> query = (NativeQuery<Object[]>) (em.createNativeQuery(sql))
.setParameter("runId", runId);
Expand Down Expand Up @@ -484,7 +474,7 @@ public List<ExportedLabelValues> labelValuesByRun(int runId, String filter, Stri
.addScalar("start", StandardBasicTypes.INSTANT)
.addScalar("stop", StandardBasicTypes.INSTANT);

return LabelValuesService.parse(query.getResultList());
return LabelValuesService.parse(query.getResultList(), limit, page);
}

/**
Expand All @@ -502,7 +492,7 @@ public List<ExportedLabelValues> labelValuesByRun(int runId, String filter, Stri
* @param nodes
* @return
*/
protected static List<ExportedLabelValues> parse(List<Object[]> nodes) {
protected static List<ExportedLabelValues> parse(List<Object[]> nodes, Integer limit, Integer page) {
if (nodes == null || nodes.isEmpty())
return new ArrayList<>();

Expand Down Expand Up @@ -531,6 +521,11 @@ protected static List<ExportedLabelValues> parse(List<Object[]> nodes) {
return exportedLabelValue;
})));

if (limit != null && limit > 0) {
long toSkip = (long) limit * ((page == null || page < 0) ? 0 : page);
return exportedLabelValues.values().stream().skip(toSkip).limit(limit).collect(Collectors.toList());
}

return new ArrayList<>(exportedLabelValues.values());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,20 @@
import static org.junit.jupiter.api.Assertions.assertNull;

import java.time.Instant;
import java.util.ArrayList;
import java.util.List;

import jakarta.inject.Inject;

import org.junit.jupiter.api.Test;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
import com.fasterxml.jackson.databind.node.JsonNodeType;
import com.fasterxml.jackson.databind.node.ObjectNode;

import io.hyperfoil.tools.horreum.api.data.ExportedLabelValues;
import io.hyperfoil.tools.horreum.test.HorreumTestProfile;
import io.quarkus.test.junit.QuarkusTest;
import io.quarkus.test.junit.TestProfile;
Expand All @@ -24,6 +28,9 @@
@TestProfile(HorreumTestProfile.class)
class LabelValuesServiceTest extends BaseServiceNoRestTest {

@Inject
ObjectMapper mapper;

@Inject
LabelValuesService labelValuesService;

Expand Down Expand Up @@ -238,4 +245,80 @@ void testGetFilterDefFromObjectWithMultiFilterAndBefore() {
assertNotNull(filterDef.simpleFilterObject().get("key1"));
assertEquals(3, filterDef.totalKeyChecks());
}

@org.junit.jupiter.api.Test
public void testLabelValuesParse() throws JsonProcessingException {
List<Object[]> toParse = new ArrayList<>();
toParse.add(
new Object[] { "job", mapper.readTree("\"quarkus-release-startup\""), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Max RSS", mapper.readTree("[]"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "build-id", mapper.readTree("null"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 1 CPU", mapper.readTree("null"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 2 CPU", mapper.readTree("null"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 4 CPU", mapper.readTree("null"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 8 CPU", mapper.readTree("null"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 32 CPU", mapper.readTree("null"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Quarkus - Kafka_tags", mapper.readTree("\"quarkus-release-startup\""), 10, 10,
Instant.now(), Instant.now() });
List<ExportedLabelValues> values = LabelValuesService.parse(toParse, null, 0);
assertEquals(1, values.size());
assertEquals(9, values.get(0).values.size());
assertEquals("quarkus-release-startup", values.get(0).values.get("job").asText());
assertEquals("null", values.get(0).values.get("Throughput 32 CPU").asText());

toParse.add(
new Object[] { "job", mapper.readTree("\"quarkus-release-startup\""), 10, 11, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Max RSS", mapper.readTree("[]"), 10, 11, Instant.now(), Instant.now() });
toParse.add(new Object[] { "build-id", mapper.readTree("null"), 10, 11, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 1 CPU", mapper.readTree("17570.30"), 10, 11, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 2 CPU", mapper.readTree("43105.62"), 10, 11, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 4 CPU", mapper.readTree("84895.13"), 10, 11, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 8 CPU", mapper.readTree("141086.29"), 10, 11, Instant.now(), Instant.now() });
values = LabelValuesService.parse(toParse, null, 0);
assertEquals(2, values.size());
assertEquals(9, values.get(0).values.size());
assertEquals(7, values.get(1).values.size());
assertEquals(84895.13d, values.get(1).values.get("Throughput 4 CPU").asDouble());
}

@org.junit.jupiter.api.Test
public void testLabelValuesParseWithLimit() throws JsonProcessingException {
List<Object[]> toParse = new ArrayList<>();
toParse.add(
new Object[] { "job", mapper.readTree("\"quarkus-release-startup\""), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Max RSS", mapper.readTree("[]"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "build-id", mapper.readTree("null"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 1 CPU", mapper.readTree("null"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 2 CPU", mapper.readTree("null"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 4 CPU", mapper.readTree("null"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 8 CPU", mapper.readTree("null"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 32 CPU", mapper.readTree("null"), 10, 10, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Quarkus - Kafka_tags", mapper.readTree("\"quarkus-release-startup\""), 10, 10,
Instant.now(), Instant.now() });

List<ExportedLabelValues> values = LabelValuesService.parse(toParse, null, 0);
assertEquals(1, values.size());
assertEquals(9, values.get(0).values.size());
assertEquals("quarkus-release-startup", values.get(0).values.get("job").asText());
assertEquals("null", values.get(0).values.get("Throughput 32 CPU").asText());

toParse.add(
new Object[] { "job", mapper.readTree("\"quarkus-release-startup\""), 10, 11, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Max RSS", mapper.readTree("[]"), 10, 11, Instant.now(), Instant.now() });
toParse.add(new Object[] { "build-id", mapper.readTree("null"), 10, 11, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 1 CPU", mapper.readTree("17570.30"), 10, 11, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 2 CPU", mapper.readTree("43105.62"), 10, 11, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 4 CPU", mapper.readTree("84895.13"), 10, 11, Instant.now(), Instant.now() });
toParse.add(new Object[] { "Throughput 8 CPU", mapper.readTree("141086.29"), 10, 11, Instant.now(), Instant.now() });
// limit the results to 1 record per page
values = LabelValuesService.parse(toParse, 1, 0);
assertEquals(1, values.size());
assertEquals(9, values.get(0).values.size());
assertEquals("quarkus-release-startup", values.get(0).values.get("job").asText());
assertEquals("null", values.get(0).values.get("Throughput 32 CPU").asText());
values = LabelValuesService.parse(toParse, 1, 1);
assertEquals(1, values.size());
assertEquals(7, values.get(0).values.size());
assertEquals(84895.13d, values.get(0).values.get("Throughput 4 CPU").asDouble());
}
}
Loading

0 comments on commit d208817

Please sign in to comment.