Jamf Pro: fix flaky system tests

[chemamartinez]

Proposed commit message

After this pagination fix, the condition to ask for more data is size(body.results) >= state.page_size (more context of this change here). This condition was always met in the system tests with page_size == 1 so the test keeps constantly requesting more data.

• Tests have been adapted to don't request more data when receiving fewer events than the page size.
• It has been added the limitation of the page_size minimum value.
• Removed the unused variable max_executions.
• Store prev_last_report_date to compare with current last_report_date and avoid repeated API requests when last report filter is the same as the previous request.

Checklist

• I have reviewed tips for building integrations and this pull request is aligned with them.
• I have verified that all data streams collect metrics or logs.
• I have added an entry to my package's changelog.yml file.
• I have verified that Kibana version constraints are current according to guidelines.
• I have verified that any added dashboard complies with Kibana's Dashboard good practices

Related issues

• Closes [Stack 9.0.0-SNAPSHOT] [jamf_pro] Failing test daily: system test: (elastic-agent logs - inventory) in jamf_pro.inventory #12314
• Closes [LogsDB] [Stack 8.18.0-SNAPSHOT] [jamf_pro] Failing test daily: system test: (elastic-agent logs - inventory) in jamf_pro.inventory #12278
• Closes [Stack 8.18.0-SNAPSHOT] [jamf_pro] Failing test daily: system test: (elastic-agent logs - inventory) in jamf_pro.inventory #12280

[elasticmachine]

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

[elastic-vault-github-plugin-prod]

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

[efd6]

Does this mean that if a user sets the page size to unity, they will have a problem?

[chemamartinez]

Does this mean that if a user sets the page size to unity, they will have a problem?

@efd6 I think so, unless the API returns a last empty response which I think is not probable.

[efd6]

I think in that case we need to document the limitation or fix it.

[chemamartinez]

Looking at the PR that changed the logic, I think the fix here would be going back to the previous condition

"want_more": body.totalCount > size(body.results),

as the bug that Chris mentioned was fixed in the logic of populating cursor.last_report_date. I don't see any issues in getting back to the old logic of the want_more check.

What do you think about it?

Otherwise, I can just add the limitation to the page_size description.

[efd6]

I'd be OK with that provided it was checked against a real endpoint.

[chemamartinez]

@efd6, I have tested both ways of defining want_more and both have the same limitation when page_size == 1.

In that case, due to the API design, it gets stuck in an infinite loop asking for the first event on the list. That's because the filter filter=general.reportDate is always populated from the only event received and when making the request, the same event is returned as we can only get one.

I'll leave things as they are and document that limitation.

[efd6]

Can we add a check for the last-used last_report_date matching the current value? If they match, this would indicate non-progression. Suggest something like

diff --git a/packages/jamf_pro/data_stream/inventory/agent/stream/cel.yml.hbs b/packages/jamf_pro/data_stream/inventory/agent/stream/cel.yml.hbs
index b6f2131ed4..e0da25d578 100644
--- a/packages/jamf_pro/data_stream/inventory/agent/stream/cel.yml.hbs
+++ b/packages/jamf_pro/data_stream/inventory/agent/stream/cel.yml.hbs
@@ -62,9 +62,9 @@ program: |-
                                                ?"event.original": state.?preserve_original_event.orValue(false) ? optional.of(e.encode_json()) : optional.none(),
                                        }
                                ),
-                               "want_more": size(body.results) >= state.page_size,
                                "cursor": state.?cursor.orValue({}).with(
                                        {
+                                               ?"prev_last_report_date": state.?cursor.last_report_date,
                                                ?"last_report_date": (size(body.results) > 0) ?
                                                        optional.of(
                                                                body.results[size(body.results) - 1].general.reportDate.as(d,
@@ -76,7 +76,9 @@ program: |-
                                        }
                                ),
                        }
-               )
+               ).as(state, state.with({
+                       "want_more": size(body.results) >= state.page_size && (state.?cursor.prev_last_report_date != state.?cursor.last_report_date)
+               }))
        )
   )
 state:

(untested)

[chemamartinez]

Good point, added at bb83c61.

It has been tested against the real API, even though we still have the limitation of page_size == 1, it helps with unnecessary requests every interval. Now, we just see one request per interval when the last_report_date filter cannot advance.

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 55d1b3a

History

• 💔 Build #22248 failed bb83c61
• 💚 Build #22211 succeeded 6e62591
• 💚 Build #22049 succeeded 6798f88

cc @chemamartinez

[elastic-sonarqube]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[efd6]

Is this change still required?

[chemamartinez]

I think so, the difference between before and after the last commit when page_size = 1 is that before, want_more was true when size(body.results) >= state.page_size (one result was returned, and one is expected), so it was constantly asking for more data in the same interval. Now, even though want_more is false because of the new condition state.?cursor.prev_last_report_date != state.?cursor.last_report_date, in the next interval the filter last_report_date is the same and will return the same event every time.

I checked that behaviour with page_size = 1 and the latest changes with the API.

[efd6]

OK, so the fix we have is just for the cpu overuse. It would be nice to fix the whole issue, but this will do for now.

[efd6]

Do we still need to change the tests?

[chemamartinez]

Same reason as for the page size limitation.

[elastic-vault-github-plugin-prod]

Package jamf_pro - 0.2.4 containing this change is available at https://epr.elastic.co/package/jamf_pro/0.2.4/