Optimise ESF #148

aspacca · 2022-08-03T06:52:05Z

Enhancement

What does this PR do?

Add benchmark for covering expand_event_list_from_field and json dumper
Fix json_content_type not passed as argument to storage factories
Set custom json parser and dumper in es client and for storage decorator and event expander
Add disabled value for json_content_type in order to totally skip json auto discovery

Why is it important?

We need to optimise as much as possible handling json content, since the most performance impacting code in the forwarder.
Having benchmarks available for the different use cases will help decide which json library to use and in case even switching to different libraries according to the matrix of expand_event_list_from_field and json_content_type setting provided by the user

I've run the new benchmark and identified ujson` as the most performing json package

Here the outcome of several iterations of optimisation:

320 MB, 10mins timeout, batch_max_actions: 4000, batch_max_bytes: 104857600
1.3.0-rc20
Sent Events: 618500, Duration: 480secs, Max Memory Used: 97MB
Sent Events: 445500, Duration: 341secs, Max Memory Used: 100MB

1.3.0-rc13
Sent Events: 580000, Duration: 481secs, Max Memory Used: 135MB
Sent Events: 484000, Duration: 404secs, Max Memory Used: 138MB

1.3.0-rc8
Sent Events: 364000, Duration: 482secs, Max Memory Used: 131MB
Sent Events: 364000, Duration: 481secs, Max Memory Used: 131MB
Sent Events: 336000, Duration: 445secs, Max Memory Used: 132MB

1.3.0-rc6
Sent Events: 356000, Duration: 480secs, Max Memory Used: 128MB
Sent Events: 356000, Duration: 483secs, Max Memory Used: 131MB
Sent Events: 352000, Duration: 477secs, Max Memory Used: 131MB

1.2.1
Sent Events: 340000, Duration: 483secs, Max Memory Used: 126MB
Sent Events: 340000, Duration: 483secs, Max Memory Used: 127MB
Sent Events: 340000, Duration: 483secs, Max Memory Used: 127MB
Sent Events:  44000, Duration: 285secs, Max Memory Used: 127MB

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.md

Author's Checklist

[ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

…r decorator

…ist_from_field

elasticmachine · 2022-08-03T07:21:11Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-08-17T05:46:20.283+0000
Duration: 42 min 24 sec

Test stats 🧪

Test	Results
Failed	0
Passed	418
Skipped	0
Total	418

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

…tent

tommyers-elastic

nice work ! couple of comments, no blockers.

tommyers-elastic · 2022-08-15T15:17:18Z

docs/README-AWS.md

 **Notes:**

 `inputs.[].json_content_type` can be defined as a string with on the of the following values:
 - *single*: indicates that the content of a single entry in the input payload is a single JSON object. The content can either be on a single line or spanning multiple lines. In this case the whole content of the payload is decoded as JSON object, with no limit on the number of lines the JSON object is spanning on.
 - *ndjson*: indicates that the content of a single entry in the input payload is a valid NDJSON format. In NDJSON format multiple single JSON objects formatted on a single line each are separated by a newline delimiter. In this case each line will be decoded as JSON object, improving the parsing performance.
+- *disabled*: instructs the Elastic Server Forwarder to not attempt any JSON content automatic discovery and threat the content as plain text, improving the performance.


couple of small typos here: Server = Serverless, threat = treat.

tommyers-elastic · 2022-08-15T15:18:12Z

docs/README-AWS.md

@@ -470,11 +470,13 @@ In case of JSON objects spanning multiple lines a limit of 1000 lines is applied

 Sometimes relaying on the Elastic Serverless Forwarder JSON content auto-discovery feature might have a huge impact on performance, or you have a known payload content of a single JSON object spanning more than 1000 lines. In this case you can provide in the input configuration and hint on the nature of the JSON content: this will change the parsing logic applied and improve performance or overcome the 1000 lines limit.

+This setting allows also to disable at all any attempt of JSON content automatic discovery, in case of known plain text content.


i think it would good to move the reason you might do this up to this section. currently we mention it lower down - "improving performance", but i think we could expand on it a little here.

handlers/aws/cloudwatch_logs_trigger.py

handlers/aws/handler.py

tommyers-elastic · 2022-08-15T16:21:06Z

share/secretsmanager.py

 from typing import Any, Union

 import boto3
 from botocore.client import BaseClient as BotoBaseClient
+from ujson import JSONDecodeError


it would be good if we could swap out the implementation library at will without any additional code changes here (see above comment)

share/json.py

storage/payload.py

storage/storage.py

aspacca · 2022-08-17T14:57:27Z

fixes #151

Andrea Spacca added 26 commits July 28, 2022 22:44

Move expand event lisst from field to storage, refactor json collecto…

de2a95a

…r decorator

introduce json_content_type config

d019b37

temporary TestLambdaHandlerSuccessS3SQS fix

e4b0cc1

Update docs

42c57d8

rename "json" value to "single" value

143c103

refactor benchmark

34b45ee

lint

9c64a05

use continuing queue for kinesis input

e885c6e

adapt test and fix kinesis input continuation

9b83d34

fix docs

cd480d7

reduce kinesis trigger batch size, remove ReportBatchItemFailures

0523c1e

add kinesis-data-stream case for _handle_sqs_event

e62cd50

last_event_expanded_offset will be optional

7f63b26

last_event_expanded_offset in handler

c16e44d

last_event_expanded_offset in cloudwatch logs

fc19a86

last_event_expanded_offset in kinesis data stream

88c7cc4

last_event_expanded_offset in s3 notifcation

ce65ca3

last_event_expanded_offset in sqs

daecf7d

handle continuation from last_event_expanded_offset in expand_event_l…

30db89e

…ist_from_field

handle optional expanded_ending_offset in decorator

fdf034f

linting and deprecation

d86403a

address @todo in integration tests

79fc846

increase coverage

58b5baf

fix test

37cb3f4

add benchnmark for expand and dumper

d45e18d

increase rounds in benchmark

032051e

aspacca requested a review from tommyers-elastic August 3, 2022 06:52

aspacca self-assigned this Aug 3, 2022

aspacca changed the base branch from main to handle-offset-in-expand_event_list_from_field August 3, 2022 07:09

Andrea Spacca added 5 commits August 8, 2022 12:58

linting

a3309a3

deepcopy is expansive

05c3cfc

add disabled value for json_content_type

61be493

json serialiser in es, json most performing dumper and parser elsewhere

0830a6f

fix json_parser in storage decorator

60a05d8

aspacca changed the title ~~Benchmark for expand and dumper~~ Optimise ESF Aug 9, 2022

Andrea Spacca added 2 commits August 9, 2022 21:17

events_list = events_list[offset_skew:] pysimdjson parse the list con…

345d4b1

…tent

fix tests

fe579f8

aspacca requested a review from a team August 9, 2022 14:58

Andrea Spacca added 12 commits August 10, 2022 00:24

silence sensitive logging

b65cb67

increase coverage

030e10d

wider exception in parse and dumper for es

d6dfa1c

increase coverage, remove pysimdjson

e7c4afd

always use usjon

a97c436

fix tests

701a0b6

linting

02d53ef

fix tests

4d8ae1e

changelog

99d8c5c

typo in changelog

c9e06e5

a test file left behind

e491665

tests take longer

ad13910

aspacca changed the base branch from handle-offset-in-expand_event_list_from_field to main August 11, 2022 07:47

tommyers-elastic approved these changes Aug 15, 2022

View reviewed changes

Andrea Spacca added 5 commits August 16, 2022 17:39

cr fixes

3324d8b

fix #151

282fa88

strip message when checking for emoty event

d66a793

remove dumping possible sensitive data in the logs

53617fd

lint

188d04a

aspacca merged commit 216bd28 into main Aug 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimise ESF #148

Optimise ESF #148

aspacca commented Aug 3, 2022 •

edited

Loading

elasticmachine commented Aug 3, 2022 •

edited

Loading

Build stats

Test stats 🧪

tommyers-elastic left a comment

tommyers-elastic Aug 15, 2022

tommyers-elastic Aug 15, 2022

tommyers-elastic Aug 15, 2022

aspacca commented Aug 17, 2022

		@@ -470,11 +470,13 @@ In case of JSON objects spanning multiple lines a limit of 1000 lines is applied

		Sometimes relaying on the Elastic Serverless Forwarder JSON content auto-discovery feature might have a huge impact on performance, or you have a known payload content of a single JSON object spanning more than 1000 lines. In this case you can provide in the input configuration and hint on the nature of the JSON content: this will change the parsing logic applied and improve performance or overcome the 1000 lines limit.

		This setting allows also to disable at all any attempt of JSON content automatic discovery, in case of known plain text content.

Optimise ESF #148

Optimise ESF #148

Conversation

aspacca commented Aug 3, 2022 • edited Loading

What does this PR do?

Why is it important?

Checklist

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

elasticmachine commented Aug 3, 2022 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

tommyers-elastic left a comment

Choose a reason for hiding this comment

tommyers-elastic Aug 15, 2022

Choose a reason for hiding this comment

tommyers-elastic Aug 15, 2022

Choose a reason for hiding this comment

tommyers-elastic Aug 15, 2022

Choose a reason for hiding this comment

aspacca commented Aug 17, 2022

aspacca commented Aug 3, 2022 •

edited

Loading

elasticmachine commented Aug 3, 2022 •

edited

Loading