Fix how "all matches" extractor are handled by lampajr · Pull Request #2568 · Hyperfoil/Horreum

lampajr · 2025-10-29T10:09:58Z

Fixes Issue

The expectation is that "previewing" a label execution and its actual label value computation return the same result, which is not true a the moment when one of the extractor is marked as "all matches".

Changes proposed

Removed apparently wrong flatten((ArrayNode) row[3])
Added some more tests

Check List (Check all the applicable boxes)

My code follows the code style of this project.
My change requires changes to the documentation.
I have updated the documentation accordingly.
All new and existing tests passed.

lampajr · 2025-10-29T10:12:20Z

Commit 66e7c0b reproduces the problem raised by other users, it seems that when using "all matches" extractor (which should usually produce an array or arrays as result) is not working as expected when computing the corresponding label value.

In contrast, the "label preview" seems working as expected resulting in different results between the preview and the actual label value.

This seems to happen only if using JS function, in this case the input (array of arrays) is flattened when trying to compute the label value.

lampajr · 2025-10-29T10:15:25Z

Worth noting that the "label preview" is calling evaluateOnce under the hood, whereas the label values computation is calling evaluateWithCombinationFunction.

Signed-off-by: Andrea Lamparelli <a.lamparelli95@gmail.com>

lampajr · 2025-10-29T10:37:31Z

Found that the issue is happening only when there is one single extractor, and that extractor is "all matches" one.

Signed-off-by: Andrea Lamparelli <a.lamparelli95@gmail.com>

lampajr · 2025-10-29T11:00:20Z

Added some more tests covering first and all matches scenarios , especially when dealing with one single extractor.

lampajr · 2025-10-29T11:05:48Z

horreum-backend/src/main/java/io/hyperfoil/tools/horreum/svc/DatasetServiceImpl.java

        FingerprintDAO.deleteById(datasetId);
        Util.evaluateWithCombinationFunction(toCompute,
                (row) -> (String) row[2],
-                (row) -> (row[3] instanceof ArrayNode ? flatten((ArrayNode) row[3]) : (JsonNode) row[3]),


Don't know why this flatten was introduced, it seems forcing to do the "first match" in case of array of arrays..

Yes, idk if we have any data that would be affected if we remove it?

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/DatasetServiceTest.java

lampajr · 2025-10-29T12:24:10Z

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/DatasetServiceTest.java

    public void testDatasetLabelAsyncSingleWithReduceFunctionArray() {
        withExampleSchemas((schemas) -> {
-            int labelReduce = addLabel(schemas[0], "sum", "async (value) => { return value.reduce((a,b) => a+b); }",
+            int labelReduce = addLabel(schemas[0], "sum", "async (value) => { return value[0].reduce((a,b) => a+b); }",


It seems this test testDatasetLabelAsyncSingleWithReduceFunctionArray and testDatasetLabelSingleWithReduceFunctionArray were created to test exactly that use case when using "All Matches" and reduce function.. but in my opinion the expectation was wrong and did not make sense as it would change the results/expectation a user will have when dealing with such arrays as input.

IMHO the proper way is to consider the input as an array of arrays which is exactly what it would be in all other use cases

Yeah, I agree, but it would be good to understand why... From the git log it seems @willr3 created this. @willr3, any comments?

I created testDatasetLabelAsyncSingleWithReduceFunctionArray to test the use of async and based it off what already existed in testDatasetLabelSingleWithReduceFunctionArray so I cannot comment about the use of value[0] vs value.

stalep

Lgtm! Very nice with those tests added!!
It would be good to get some background info from @willr3 if he remembers why it's like this in the first place (if I'm correct that he wrote it :).

stalep · 2025-10-30T08:55:11Z

horreum-backend/src/main/java/io/hyperfoil/tools/horreum/svc/DatasetServiceImpl.java

        FingerprintDAO.deleteById(datasetId);
        Util.evaluateWithCombinationFunction(toCompute,
                (row) -> (String) row[2],
-                (row) -> (row[3] instanceof ArrayNode ? flatten((ArrayNode) row[3]) : (JsonNode) row[3]),


Yes, idk if we have any data that would be affected if we remove it?

stalep · 2025-10-30T09:01:34Z

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/DatasetServiceTest.java

    public void testDatasetLabelAsyncSingleWithReduceFunctionArray() {
        withExampleSchemas((schemas) -> {
-            int labelReduce = addLabel(schemas[0], "sum", "async (value) => { return value.reduce((a,b) => a+b); }",
+            int labelReduce = addLabel(schemas[0], "sum", "async (value) => { return value[0].reduce((a,b) => a+b); }",


Yeah, I agree, but it would be good to understand why... From the git log it seems @willr3 created this. @willr3, any comments?

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/DatasetServiceTest.java

willr3

Only made minor comments about readability and the background on the async test I added

lampajr · 2025-10-31T16:28:10Z

Only made minor comments about readability and the background on the async test I added

Applied your suggestions ✅

Signed-off-by: Andrea Lamparelli <a.lamparelli95@gmail.com>

Added all match extractor test

66e7c0b

Signed-off-by: Andrea Lamparelli <a.lamparelli95@gmail.com>

lampajr force-pushed the js_all_match_extractor branch from 1829377 to 66e7c0b Compare October 29, 2025 10:36

lampajr added 2 commits October 29, 2025 11:54

Added all match extractor on datasets

86376cb

Signed-off-by: Andrea Lamparelli <a.lamparelli95@gmail.com>

Added first match extractor on datasets and runs

563338d

Signed-off-by: Andrea Lamparelli <a.lamparelli95@gmail.com>

lampajr force-pushed the js_all_match_extractor branch from 615d685 to 131e5ca Compare October 29, 2025 11:04

lampajr commented Oct 29, 2025

View reviewed changes

lampajr force-pushed the js_all_match_extractor branch from 131e5ca to 530402e Compare October 29, 2025 12:10

lampajr marked this pull request as ready for review October 29, 2025 12:10

lampajr commented Oct 29, 2025

View reviewed changes

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/DatasetServiceTest.java Outdated Show resolved Hide resolved

lampajr force-pushed the js_all_match_extractor branch from 530402e to 5cb808f Compare October 29, 2025 12:22

lampajr commented Oct 29, 2025

View reviewed changes

lampajr requested review from stalep and willr3 October 29, 2025 12:24

lampajr added the backport label Oct 29, 2025

lampajr self-assigned this Oct 29, 2025

lampajr force-pushed the js_all_match_extractor branch from 5cb808f to c34a943 Compare October 29, 2025 12:38

stalep approved these changes Oct 30, 2025

View reviewed changes

willr3 reviewed Oct 31, 2025

View reviewed changes

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/DatasetServiceTest.java Outdated Show resolved Hide resolved

willr3 reviewed Oct 31, 2025

View reviewed changes

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/DatasetServiceTest.java Outdated Show resolved Hide resolved

willr3 reviewed Oct 31, 2025

View reviewed changes

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/DatasetServiceTest.java Outdated Show resolved Hide resolved

willr3 approved these changes Oct 31, 2025

View reviewed changes

lampajr force-pushed the js_all_match_extractor branch from c34a943 to 6edcc0c Compare October 31, 2025 16:27

Removed unnecessary and wrong flatten

8c6579a

Signed-off-by: Andrea Lamparelli <a.lamparelli95@gmail.com>

lampajr force-pushed the js_all_match_extractor branch from 6edcc0c to 8c6579a Compare October 31, 2025 16:29

lampajr merged commit ff1f77a into Hyperfoil:master Oct 31, 2025
2 checks passed

rh-appservices-perf mentioned this pull request Oct 31, 2025

[0.19] Fix how "all matches" extractor are handled #2573

Merged

6 tasks

lampajr deleted the js_all_match_extractor branch October 31, 2025 16:59

Conversation

lampajr commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixes Issue

Changes proposed

Check List (Check all the applicable boxes)

Uh oh!

lampajr commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lampajr commented Oct 29, 2025

Uh oh!

lampajr commented Oct 29, 2025

Uh oh!

lampajr commented Oct 29, 2025

Uh oh!

lampajr Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

stalep Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lampajr Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

stalep Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

willr3 Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

stalep left a comment

Choose a reason for hiding this comment

Uh oh!

stalep Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

stalep Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

willr3 left a comment

Choose a reason for hiding this comment

Uh oh!

lampajr commented Oct 31, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lampajr commented Oct 29, 2025 •

edited

Loading

lampajr commented Oct 29, 2025 •

edited

Loading