Metering connector actions request body bytes #186804

ersin-erdal · 2024-06-24T10:43:21Z

Towards: https://github.com/elastic/response-ops-team/issues/209

This PR collects body-bytes from the requests made by the connectors to the 3rd parties and saves them in the event-log.

There is a new metric collector: ConnectorMetricsCollector
Action TaskRunner, creates a new instance of it on each action execution and passes it to the actionType.executor.
Then the actionType.executor passes it to the request function provided by the actions plugin.
Request function passes the response (or the error) from axios to addRequestBodyBytes method of the ConnectorMetricsCollector.
Since axios always returns request.headers['Content-Length'] either in success result or error, metric collector uses its value to get the request body bytes.

In case there is no Content-Length header, addRequestBodyBytes method fallbacks to the body object that we pass as the second param. So It calculates the body bytes by using Buffer.byteLength(body, 'utf8');, which is also used by axios to populate request.headers['Content-Length']

For the connectors or the subActions that we don't use the request function or axios:
addRequestBodyBytes method is called just before making the request only with the body param in order to force it to use the fallback.

Note: If there are more than one requests in an execution, the bytes are summed.

To verify:

Create a rule with a connector that you would like to test.
Let the rule run and check the event-log of your connector, request body bytes should be saved in:
kibana.action.execution.metrics.request_body_bytes

Alternatively:
You can create a connector and run it on its test tab.

You can use the below query to check the event-log:

 {
    "query": {
      "bool": { 
        "must": [
          { "match": { "event.provider":"actions"}},
          { "match": { "kibana.action.type_id":"{**your-action-type-id**}"}} 
        ],
        "filter": [ 
          { "term":  { "event.action": "execute" }}
        ]
      }
    },
    "size" : 100
}

ersin-erdal · 2024-07-09T21:07:50Z

/ci

elasticmachine · 2024-07-09T23:34:51Z

Pinging @elastic/response-ops (Team:ResponseOps)

paul-tavares · 2024-07-10T13:18:13Z

x-pack/plugins/stack_connectors/server/connector_types/sentinelone/sentinelone.ts

-    zipPassCode,
-  }: SentinelOneFetchAgentFilesParams) {
-    const agent = await this.getAgents({ uuid: agentUUID });
+  public async fetchAgentFiles(


@ersin-erdal ,
I read the PR description and just want to make sure that the changes done to these public methods have no impact on consumers of the COnnector's sub-actions - correct? The second argument (ConnectorMetricsCollector) is something that the Action task runner will take care of internally?

Also,
Is there a way you can trigger the Security Solution tests (unit) to run on PR? They only currently run if files from the Security Solution plugin are changed, thus they did not run on this one. If needed, you can make a change (a JS comment would do, I think) to one of our files - perhaps one these:

https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/server/endpoint/services/actions/clients/sentinelone/sentinel_one_actions_client.ts

https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/server/endpoint/services/actions/clients/crowdstrike/crowdstrike_actions_client.ts

That should trigger CI to run our full test suite.

cc/ @tomsonpl - just FYI

Correct, I intentionally didn't touch any params of the connector types, just passed the ConnectorMetricsCollector as a second param.

Connector types just passes it to the request function, so it can collect the metrics there.
Then the Actions plugin will save the data collected by it, in event log.

There shouldn't be any change or impact on the connectors, they should work as they were.

I will add a comment in Security Solution as you suggest. Thanks.

pmuellr

I didn't have a chance to do a complete review, but looked at connector_metrics_collector.ts to see how we were implementing the core logic. Basic idea is sound, but I think we need to re-arrange the logic and add a try/catch on the stringify ...

pmuellr · 2024-07-12T21:43:39Z

x-pack/plugins/actions/server/lib/connector_metrics_collector.ts

+  };
+
+  public addRequestBodyBytes(result?: AxiosError | AxiosResponse, body: string | object = '') {
+    const sBody = typeof body === 'string' ? body : JSON.stringify(body);


I don't have a great feeling about depending on axios to send a string | object. Or, I'm afarid of using JSON.stringify() without a try/catch - it can throw :-). I'd be ok with using a ${body} version of the content length in case of an error.

Since this could be expensive, I think we should move this AFTER the Content-Length check, since that seems highly likely to be the common case. So just check if Content-Length is there and return that, otherwise do the other calculations (stringify, byteLength, etc).

It's not axios returning string | object, it's us passing the body as a fallback here, and we always pass it either as string or object. And breaking JSON.stringify is really hard, it just stringify what ever you pass :)

But you are right putting it after Content-Length check is better.
Moved it and wrapped with try-catch just in case.

pmuellr · 2024-07-12T21:45:41Z

x-pack/plugins/actions/server/lib/create_action_event_log_record_object.test.ts

@@ -345,6 +356,7 @@ describe('createActionEventLogRecordObject', () => {
        ],
        action: {
          name: 'test name',
+          type_id: '.slack',


this must have been the source of the question on snake- vs camel- case hahahaha

This type_id seems out of place to me. Is this the way we do it with rule id's?

No it was SO name :) Like, action or alert.
This is a field name, it is the same for the rule, just the field name there is rule_type_id... but rule.rule_type_id looks weird to me...

…9-meter-request-bytes

elasticmachine · 2024-07-13T20:19:49Z

⏳ Build in-progress, with failures

Buildkite Build
Commit: 7ae62280ee013860d72652127fcbd75e18423b6b

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #47 / apis ESQL sync error messages Using string field type: text and number field type: integer Checking error messages
[job] [logs] FTR Configs #47 / apis ESQL sync error messages Using string field type: text and number field type: integer Checking error messages

History

💔 Build #221292 failed 52f35db308f19366bf80ae52a516f17374fd9149
💔 Build #221288 failed 72cc5af6616bc54f817edd1f4676e644d3901163
💔 Build #221285 failed 6c4e455
💚 Build #220816 succeeded ce5158c

cc @ersin-erdal

pmuellr

LGTM, thx for the changes and explanations, sorry for the delay!

…9-meter-request-bytes

kibana-ci · 2024-08-26T23:37:56Z

💚 Build Succeeded

Buildkite Build
Commit: 23b4921
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-186804-23b4921310d9

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`actions`	301	308	+7

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`actions`	32	33	+1

Unknown metric groups

API count

id	before	after	diff
`actions`	307	314	+7

History

💚 Build #229951 succeeded 5cc8053
💔 Build #229919 failed c6fe41a
💛 Build #229695 was flaky 56c8e95
💛 Build #228809 was flaky f1ab8c8
💛 Build #228182 was flaky c3e053c

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @ersin-erdal

tomsonpl

Defend Workflows code review 👍 and testes crowdstrike connector manually.

Thanks!

kibanamachine · 2024-08-27T09:53:01Z

💔 All backports failed

Status	Branch	Result
❌	8.15	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 186804

Questions ?

Please refer to the Backport tool documentation

ashokaditya

I noticed this while testing for alerts for SentinelOne and Elastic Defend endpoints.

API error

error response

ashokaditya · 2024-08-27T08:21:39Z

x-pack/plugins/stack_connectors/server/connector_types/sentinelone/sentinelone.ts

+    queryParams?: SentinelOneGetActivitiesParams,
+    connectorUsageCollector?: ConnectorUsageCollector


Suggested change

queryParams?: SentinelOneGetActivitiesParams,

connectorUsageCollector?: ConnectorUsageCollector

queryParams: SentinelOneGetActivitiesParams | undefined,

connectorUsageCollector: ConnectorUsageCollector

ashokaditya · 2024-08-27T08:23:22Z

x-pack/plugins/stack_connectors/server/connector_types/sentinelone/sentinelone.ts

+        params: queryParams,
+        responseSchema: SentinelOneGetActivitiesResponseSchema,
+      },
+      connectorUsageCollector!


if you apply the above suggestion then:

Suggested change

connectorUsageCollector!

connectorUsageCollector

Resolves: elastic/response-ops-team#209 This PR is a follow-on of #186804. Creates a new task that runs every 1 hour to push the total connector-request-body-bytes that have been saved in the event log to usage-api.

Resolves: elastic/response-ops-team#209 This PR is a follow-on of elastic#186804. Creates a new task that runs every 1 hour to push the total connector-request-body-bytes that have been saved in the event log to usage-api. (cherry picked from commit 216f899)

Resolves: elastic/response-ops-team#209 This PR is a follow-on of elastic#186804. Creates a new task that runs every 1 hour to push the total connector-request-body-bytes that have been saved in the event log to usage-api.

Metering connector actions request body bytes

2234e3a

ersin-erdal added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.15.0 labels Jun 24, 2024

ersin-erdal self-assigned this Jun 24, 2024

WIP

1981342

ersin-erdal force-pushed the 209-meter-request-bytes branch from 0d8dd42 to 1981342 Compare July 7, 2024 18:56

ersin-erdal added 2 commits July 7, 2024 21:07

merge main

da28c23

Collect request body bytes of the outgoing requests from the connectors

4d180d6

ersin-erdal force-pushed the 209-meter-request-bytes branch from e898038 to 4d180d6 Compare July 9, 2024 17:27

ersin-erdal added 3 commits July 9, 2024 19:44

merge main

ffcda46

fix unit test

c6dff6c

fix bedrock functional test

8fdd8fc

ersin-erdal marked this pull request as ready for review July 9, 2024 23:34

ersin-erdal requested review from a team as code owners July 9, 2024 23:34

ersin-erdal requested review from ashokaditya and paul-tavares July 9, 2024 23:34

ersin-erdal added the release_note:skip Skip the PR/issue when compiling release notes label Jul 9, 2024

paul-tavares reviewed Jul 10, 2024

View reviewed changes

ersin-erdal and others added 2 commits July 11, 2024 11:14

Add comments to trigger security solutions unit tests

ce5158c

Merge branch 'main' into 209-meter-request-bytes

f0c8def

pmuellr requested changes Jul 12, 2024

View reviewed changes

ersin-erdal added 2 commits July 13, 2024 14:15

try-catch stringify in collector

7bd6af0

Merge remote-tracking branch 'origin/209-meter-request-bytes' into 20…

6c4e455

…9-meter-request-bytes

Add logger to tests

a50e49e

ersin-erdal and others added 2 commits August 14, 2024 16:18

remove snapshot

7023229

Merge branch 'main' into 209-meter-request-bytes

c3e053c

pmuellr approved these changes Aug 19, 2024

View reviewed changes

ersin-erdal removed the request for review from a team August 20, 2024 10:36

sorenlouv approved these changes Aug 20, 2024

View reviewed changes

ersin-erdal added 2 commits August 21, 2024 14:05

merge main

8ab0f53

Merge remote-tracking branch 'origin/209-meter-request-bytes' into 20…

050e9a9

…9-meter-request-bytes

ersin-erdal requested a review from a team as a code owner August 21, 2024 11:09

revert change on security solutions

f1ab8c8

ersin-erdal removed the request for review from a team August 21, 2024 11:22

Merge branch 'main' into 209-meter-request-bytes

56c8e95

ersin-erdal mentioned this pull request Aug 26, 2024

Push the request-body-bytes collected by the ConnectorUsageCollector to usage-api #191259

Closed

stephmilovic approved these changes Aug 26, 2024

View reviewed changes

ersin-erdal and others added 4 commits August 26, 2024 22:56

Merge branch 'main' into 209-meter-request-bytes

c6fe41a

remove temporary comments

a1152cf

Merge remote-tracking branch 'origin/209-meter-request-bytes' into 20…

5cc8053

…9-meter-request-bytes

fix executor test

23b4921

tomsonpl approved these changes Aug 27, 2024

View reviewed changes

ersin-erdal merged commit 9372027 into elastic:main Aug 27, 2024
21 checks passed

ersin-erdal deleted the 209-meter-request-bytes branch August 27, 2024 09:48

kibanamachine added the v8.16.0 label Aug 27, 2024

ashokaditya reviewed Aug 27, 2024

View reviewed changes

ersin-erdal removed the v8.15.0 label Aug 27, 2024

kibanamachine added the backport:skip This commit does not require backporting label Aug 27, 2024

ersin-erdal mentioned this pull request Sep 30, 2024

Push the total request-body-bytes to usage-api #194429

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metering connector actions request body bytes #186804

Metering connector actions request body bytes #186804

ersin-erdal commented Jun 24, 2024 •

edited by kibanamachine

Loading

ersin-erdal commented Jul 9, 2024

elasticmachine commented Jul 9, 2024

paul-tavares Jul 10, 2024

ersin-erdal Jul 10, 2024

pmuellr left a comment

pmuellr Jul 12, 2024

ersin-erdal Jul 13, 2024

pmuellr Jul 12, 2024

ersin-erdal Jul 13, 2024 •

edited

Loading

elasticmachine commented Jul 13, 2024 •

edited

Loading

pmuellr left a comment

kibana-ci commented Aug 26, 2024 •

edited

Loading

API count

tomsonpl left a comment

kibanamachine commented Aug 27, 2024

ashokaditya left a comment

ashokaditya Aug 27, 2024

ashokaditya Aug 27, 2024

		queryParams?: SentinelOneGetActivitiesParams,
		connectorUsageCollector?: ConnectorUsageCollector

Metering connector actions request body bytes #186804

Metering connector actions request body bytes #186804

Conversation

ersin-erdal commented Jun 24, 2024 • edited by kibanamachine Loading

To verify:

ersin-erdal commented Jul 9, 2024

elasticmachine commented Jul 9, 2024

paul-tavares Jul 10, 2024

Choose a reason for hiding this comment

ersin-erdal Jul 10, 2024

Choose a reason for hiding this comment

pmuellr left a comment

Choose a reason for hiding this comment

pmuellr Jul 12, 2024

Choose a reason for hiding this comment

ersin-erdal Jul 13, 2024

Choose a reason for hiding this comment

pmuellr Jul 12, 2024

Choose a reason for hiding this comment

ersin-erdal Jul 13, 2024 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Jul 13, 2024 • edited Loading

⏳ Build in-progress, with failures

Failed CI Steps

Test Failures

History

pmuellr left a comment

Choose a reason for hiding this comment

kibana-ci commented Aug 26, 2024 • edited Loading

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Public APIs missing exports

API count

History

tomsonpl left a comment

Choose a reason for hiding this comment

kibanamachine commented Aug 27, 2024

💔 All backports failed

Manual backport

Questions ?

ashokaditya left a comment

Choose a reason for hiding this comment

ashokaditya Aug 27, 2024

Choose a reason for hiding this comment

ashokaditya Aug 27, 2024

Choose a reason for hiding this comment

ersin-erdal commented Jun 24, 2024 •

edited by kibanamachine

Loading

ersin-erdal Jul 13, 2024 •

edited

Loading

elasticmachine commented Jul 13, 2024 •

edited

Loading

kibana-ci commented Aug 26, 2024 •

edited

Loading