[Security Solution] only meter endpoints that are billable #186580
Conversation
joeypoon
added
release_note:skip
Flaky Test Runner Stats🟠 Some tests failed. - kibana-flaky-test-suite-runner#6363[❌] [Serverless] Security Solution Defend Workflows - Cypress: 0/25 tests passed. |
joeypoon
force-pushed
the
feature/endpoint-metering-billable-only
branch
from
65a2220
to
d0bb15d
Compare
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#6370[✅] [Serverless] Security Solution Defend Workflows - Cypress: 25/25 tests passed. |
|
Pinging @elastic/security-defend-workflows (Team:Defend Workflows) |
joeypoon
force-pushed
the
feature/endpoint-metering-billable-only
branch
from
d0bb15d
to
16bc156
Compare
Yeah, it's used here: No changes were needed to the test itself but this update to the indexing cypress task does get covered by that test. There was also a previous PR that dealt with adding the flag to the policy and adding associated tests. |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
one question: I see you updated the data loader to generate billable, not billable and even invalid docs - but do we have tests in place for this change?
Yeah, it's used here:
No changes were needed to the test itself but this update to the indexing cypress task does get covered by that test.
There was also a previous PR that dealt with adding the flag to the policy and adding associated tests in
metering.cy.ts
thanks for the details! i still don't understand how there's no need to update the tests in metering.cy.ts:
indexEndpointHeartbeats()indexesHEARTBEAT_COUNT = 2001number of valid and one invalid documents- then after collecting all chunks in the
recourse()function, the test expects the number of chunks to equalconst expectedChunks = Math.ceil(HEARTBEAT_COUNT / METERING_SERVICE_BATCH_SIZE);chunks <- here i'd expect it should be only half of this, asbillable: false | undefineddocs shouldn't be reported due to the current changes
or do I miss something?
(also i added another comment, sorry about that, it didn't occur to me on first review)
| }); | ||
|
|
||
| const operations = docs.flatMap((doc) => [ | ||
| // billable: false are not billed | ||
| const invalidDocs: EndpointHeartbeat[] = [ |
There was a problem hiding this comment.
this is now an additional doc to the number of count documents. i'd suggest to include it in count, or add a new option like countOfInvalidDocuments next to count
There was a problem hiding this comment.
This doesn't add to count since it is filtered out by the query. We want the count to not include it so we validate that invalid docs aren't incorrectly added to the count. The actual number of invalid docs does not matter here.
There was a problem hiding this comment.
i understand it doesn't matter, it's just violating the contract proposed by the function indexEndpointHeartbeat(count), it looks like an unwanted side effect of the loader
If billable doesn't exist then it is billed. So both |
thanks for the clarification. |
…licit (#187071) ## Summary Adds a new `unbilledCount` param to the `indexEndpointHeartbeats` cypress heartbeart indexing task to make the indexed `billable: false` docs functionality more explicit rather than a side effect as [suggested in the last PR](#186580 (comment)). ### Checklist - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Summary
Updates the endpoint metering task to only grab heartbeats if
billable: trueor ifbillabledoesn't exist (for backwards compatibility).Checklist
For maintainers