Fix AWS cloudfront log parsing #10216

ltflb-bgdi · 2024-06-21T16:57:46Z

Bug

Proposed commit message

Fixed AWS cloudfront log parsing

Please explain:

WHAT:
- Completely refactored parsing by replacing grok with csv to better comply to the cloudfront standard log format.
- Fixed duration name in GROK csv parser, which was parsed into a _tmp field but never used.as not available.
- Fixed x-forwarded-for handling:
  Make sure only real ip addresses are used. All malformed ip-addresses are listed in error.message without raising a pipeline error.
- Moved cloudfront.content_type to http.response.mime_type, which is an ecs field.
- Added event.kind: pipeline_error to error handler.
WHY:
- The fixes above are needed to reliably process cloudfront logs. We have observed a lot of errors and grok timeouts, which lead to congestion and performance issues on our elastic clusters. With the new parser in place, parsing works without issues. Even though the pipeline contains much more processors than the one using grok, we do not see any performance impact.
- The current, csv based parser lays the foundation of adding support of cloudfront realtime logs in the future, since realtime log fields are configurable and thus using grok is way too complex.

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.

Author's Checklist

How to test this PR locally

The PR contains sample logs of the problematic events.
To be tested with elastic-package test pipeline --data-streams cloudfront_logs

Note: Please note that the log producing the error.message is not added to the sample logs since it would let the
tests fail. See: elastic/elastic-package#1392

Related issues

Relates [aws] Cloudfront logs Ingest pipeline faulty uri stem grok pattern #10507

cla-checker-service · 2024-06-21T16:57:52Z

💚 CLA has been signed

ltflb-bgdi · 2024-06-21T17:10:42Z

/test

ltflb-bgdi · 2024-08-05T13:14:13Z

/test

efd6 · 2024-08-06T02:35:44Z

packages/aws/changelog.yml

@@ -1,4 +1,9 @@
 # newer versions go on top
+- version: "2.23.0"


Suggested change

- version: "2.23.0"

- version: "2.22.1"

efd6 · 2024-08-06T02:38:14Z

packages/aws/manifest.yml

@@ -1,7 +1,7 @@
 format_version: 3.0.0
 name: aws
 title: AWS
-version: 2.22.0
+version: 2.23.0


Suggested change

version: 2.23.0

version: 2.22.1

Updated to 2.22.2 due to merge conflict

efd6 · 2024-08-06T02:40:11Z

packages/aws/data_stream/cloudfront_logs/fields/ecs.yml

Please remove this. It was intentionally removed in #10223.

efd6 · 2024-08-06T02:40:45Z

packages/aws/data_stream/cloudfront_logs/elasticsearch/ingest_pipeline/default.yml

-  - drop: 
-      if: "ctx.event.original.startsWith('#')"
+  - drop:
+      if: ctx?.event?.original?.startsWith('#')


Suggested change

if: ctx?.event?.original?.startsWith('#')

if: ctx.event?.original?.startsWith('#')

efd6 · 2024-08-06T02:43:23Z

packages/aws/data_stream/cloudfront_logs/_dev/test/pipeline/test-cloudfront.log

 #Fields: date time x-edge-location sc-bytes c-ip cs-method cs(Host) cs-uri-stem sc-status cs(Referer) cs(User-Agent) cs-uri-query cs(Cookie) x-edge-result-type x-edge-request-id x-host-header cs-protocol cs-bytes time-taken x-forwarded-for ssl-protocol ssl-cipher x-edge-response-result-type cs-protocol-version fle-status fle-encrypted-fields c-port time-to-first-byte x-edge-detailed-result-type sc-content-type sc-content-len sc-range-start sc-range-end
 2019-12-04	21:02:31	LAX1	392	89.160.20.112	GET	d111111abcdef8.cloudfront.net	/index.html	200	-	Mozilla/5.0%20(Windows%20NT%2010.0;%20Win64;%20x64)%20AppleWebKit/537.36%20(KHTML,%20like%20Gecko)%20Chrome/78.0.3904.108%20Safari/537.36	-	-	Hit	SOX4xwn4XV6Q4rgb7XiVGOHms_BGlTAC4KyHmureZmBNrjGdRLiNIQ==	d111111abcdef8.cloudfront.net	https	23	0.001	-	TLSv1.2	ECDHE-RSA-AES128-GCM-SHA256	Hit	HTTP/2.0	-	-	11040	0.001	Hit	text/html	78	-	-
 2019-12-04	21:02:31	LAX1	392	2a02:cf40:add:4002:91f2:a9b2:e09a:6fc6	GET	d111111abcdef8.cloudfront.net	/index.html	200	-	Mozilla/5.0%20(Windows%20NT%2010.0;%20Win64;%20x64)%20AppleWebKit/537.36%20(KHTML,%20like%20Gecko)%20Chrome/78.0.3904.108%20Safari/537.36	-	-	Hit	k6WGMNkEzR5BEM_SaF47gjtX9zBDO2m349OY2an0QPEaUum1ZOLrow==	d111111abcdef8.cloudfront.net	https	23	0.000	-	TLSv1.2	ECDHE-RSA-AES128-GCM-SHA256	Hit	HTTP/2.0	-	-	11040	0.000	Hit	text/html	78	-	-
-2019-12-04	21:02:31	LAX1	392	89.160.20.112	GET	d111111abcdef8.cloudfront.net	/index.html	200	-	Mozilla/5.0%20(Windows%20NT%2010.0;%20Win64;%20x64)%20AppleWebKit/537.36%20(KHTML,%20like%20Gecko)%20Chrome/78.0.3904.108%20Safari/537.36	-	-	Hit	f37nTMVvnKvV2ZSvEsivup_c2kZ7VXzYdjC-GUQZ5qNs-89BlWazbw==	d111111abcdef8.cloudfront.net	https	23	0.001	-	TLSv1.2	ECDHE-RSA-AES128-GCM-SHA256	Hit	HTTP/2.0	-	-	11040	0.001	Hit	text/html	78	-	-	
+2019-12-04	21:02:31	LAX1	392	89.160.20.112	GET	d111111abcdef8.cloudfront.net	/index.html	200	-	Mozilla/5.0%20(Windows%20NT%2010.0;%20Win64;%20x64)%20AppleWebKit/537.36%20(KHTML,%20like%20Gecko)%20Chrome/78.0.3904.108%20Safari/537.36	-	-	Hit	f37nTMVvnKvV2ZSvEsivup_c2kZ7VXzYdjC-GUQZ5qNs-89BlWazbw==	d111111abcdef8.cloudfront.net	https	23	0.001	-	TLSv1.2	ECDHE-RSA-AES128-GCM-SHA256	Hit	HTTP/2.0	-	-	11040	0.001	Hit	text/html	78	-	-


Does the change in this PR make us brittle to this?

Trailing tabs should not have an effect on the csv parsers. Still, according to the AWS specs we should not see this.

If this doesn't break the test, can we leave it in?

We could. But our vs-code setup is configured to automatically remove trailing spaces after editing.
I have done a test with trailing tabs by adding them with another editor and the parser did not fail.
Given that fact and that trailing tabs should not appear in real life, I vote for letting the trailing tab removed.

But our vs-code setup is configured to automatically remove trailing spaces after editing.

I don't think you should do that here; there may be tests in other packages that depend on trailing whitespace (note that we do not have an .editorconfig file in this repo so there is no guidance on the expectation of format).

While real data should not have the trailing tab, recent events have again demonstrated that testing with artifacts that match expectations can lead to bad outcomes. I would prefer that we show that we are robust to this case.

efd6 · 2024-08-06T02:52:38Z

packages/aws/data_stream/cloudfront_logs/_dev/test/pipeline/test-common-config.yml

+    - elastic_package_remove_error_message # Used to solve elastic-package test error on adding error message to event.
+dynamic_fields:
+  'event.ingested': "^.*$"


The dynamic_fields configuration is not needed since we are not setting it in the pipeline.

I don't like the testing-hack for error.message. Rather than doing this I'd suggest making a host:port parser to handle the cases, and map localhost to its IP (IPv4 should be fine).

I really don't like the testing-hack either! And to be honest, I really don't get it why there is no fix from elastic yet to allow the error.message field in the test, since is is just a common ecs field, which even can be part of the original message. This is a known problem with an existing issue waiting to be fixed for almost a year.

efd6 · 2024-08-06T03:09:44Z

packages/aws/data_stream/cloudfront_logs/elasticsearch/ingest_pipeline/default.yml

+      lang: painless
+      source: >-
+        ctx._tmp.timestamp = ctx._tmp.date + 'T' + ctx._tmp.time;
+      if: ctx?._tmp?.date != null && ctx?._tmp?.time != null


Suggested change

if: ctx?._tmp?.date != null && ctx?._tmp?.time != null

if: ctx._tmp?.date != null && ctx._tmp?.time != null

(similar change throughout: ctx is always non-null)

efd6 · 2024-08-06T03:10:08Z

packages/aws/data_stream/cloudfront_logs/elasticsearch/ingest_pipeline/default.yml

+      ignore_failure: true
+  - append:
+      field: related.ip
+      value: "{{ source.ip }}"


Suggested change

value: "{{ source.ip }}"

value: "{{{ source.ip }}}"

(throughout)

efd6 · 2024-08-06T03:11:15Z

packages/aws/data_stream/cloudfront_logs/elasticsearch/ingest_pipeline/default.yml

+      description: In rare cases, the field contains hex encoded whitspaces (\20). We replace them by whitespaces again to not break parsing.
+      field: _tmp.x_forwarded_for
+      target_field: _tmp.forwarded_ip
+      pattern: "\\\\x20"


Suggested change

pattern: "\\\\x20"

pattern: '\\x20'

Done. Note that AWS has analyzed this issue and considered it as a bug, with a fix expected to be rolled out within 4 weeks from now. We could think about removing this workaround already.

Removed gsub processor, since fix is on the way.

efd6 · 2024-08-06T03:13:40Z

packages/aws/data_stream/cloudfront_logs/elasticsearch/ingest_pipeline/default.yml

+      separator: ',\s*'
+      ignore_missing: true
+  - convert:
+      description: Creates a target field containing valid ip's if ALL values are valid ip's.


Losing all IPs if one is invalid seems unpleasant. This in conjunction with the special case you have in the pipeline test config suggest that a painless script over this array to clean it up would be a good thing to have. The only test failure that I saw was a case where the forwarded IP was localhost:8080, so a host port split and a mapping from "localhost" to "127.0.0.1" should do.

I completely agree that we should not loose valid addresses in case one them is invalid. I implemented it this way due to complexity.

Now I have rewritten it by using a script and it should almost do what you recommended. The only difference is that I ignore the host/port split and just look for a value starting with localhost. I guess this should do the job, since receiving host port is really an edge case.

As you can see, the current implementation is far from ideal. Since I did not find a simple way to check if a string is an ip, I used a regex with patterns copied from logstash. Please let me know if there is a better way of doing this, since I really don't like it at all.

I still add invalid addresses to error.message, but did not add a test log for it in order to not break the test.

efd6 · 2024-08-06T22:18:00Z

/test

efd6 · 2024-08-06T22:40:04Z

The system tests will need to be updated to use tabs as well. That file is here.

ltflb-bgdi · 2024-08-07T09:15:40Z

/test

ltflb-bgdi · 2024-08-07T09:19:52Z

/test

ltflb-bgdi · 2024-08-07T09:40:41Z

/test

andrewkroh · 2024-08-28T11:20:48Z

/test

andrewkroh · 2024-08-28T13:22:17Z

Failure message:

<testcase name="system test: default" classname="aws.cloudfront_logs" time="95.697515564">
<failure>
observed hit count 12 did not match expected hit count 11
</failure>
</testcase>

packages/aws/data_stream/cloudfront_logs/_dev/test/system/test-default-config.yml

andrewkroh · 2024-09-04T03:01:38Z

/test

elasticmachine · 2024-09-04T04:49:31Z

💚 Build Succeeded

Buildkite Build
Commit: 682261e

History

💔 Build #15117 failed c2db3bd
💔 Build #14803 failed f8b7aa8fc6df9125df5b5318f39edccd57636172
💚 Build #14678 succeeded ecf13e436218244cd5b43e3068b1972741007762
💚 Build #14579 succeeded 73b251d91adffe1d9fc6c1f0aecefed67c36ab9e
💔 Build #14527 failed 0202a5986b8a215469d21333719ff68a7087a016

elastic-sonarqube · 2024-09-04T04:49:37Z

Quality Gate failed

Failed conditions
64.5% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

andrewkroh · 2024-09-04T05:00:36Z

Everything looks in order to me now.

@elastic/obs-infraobs-integrations, as codeowner of cloudfront_logs, do you want to handle merging?

ltflb-bgdi · 2024-09-09T16:09:36Z

@elastic/obs-infraobs-integrations Any update?

elasticmachine · 2024-09-10T06:22:20Z

Package aws - 2.24.1 containing this change is available at https://epr.elastic.co/search?package=aws

* mimecast: add message release logs data stream (elastic#10732) * Fix AWS cloudfront log parsing * Refactored cloudfront parser * Updated aws cloudfront testlogs * Updated aws cloudfront logs test config * Moved cloudfront.content_type to http.response.mime_type * Added field mapping for aws.cloudfront range fields. * Fixed edge_detailed_result_type * Updated cloudfront.md * Implemented review recommendations * Updated cloudfront_logs system test log file * Fixed CI problems * Update sample_event.json * Update cloudfront.md * formatted and build with newest elastic-package version * Revert changes fixed by formatters * Update sample_event.json * Added review recommendations * Beautified some pipeline conditionals * Added additional cloudfront test case * Fixed test-default-config hit count --------- Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>

ltflb-bgdi requested review from a team as code owners June 21, 2024 16:57

ltflb-bgdi marked this pull request as draft June 21, 2024 16:58

ltflb-bgdi force-pushed the fix_pb-561_cloudfront_logs branch 3 times, most recently from 91f9492 to c31f466 Compare June 21, 2024 17:08

ltflb-bgdi marked this pull request as ready for review June 21, 2024 17:10

ltflb-bgdi changed the title ~~Fix pb 561 cloudfront logs~~ Fix AWS cloudfront log parsing Jun 21, 2024

ltflb-bgdi marked this pull request as draft July 19, 2024 14:18

andrewkroh added Integration:aws AWS Team:Obs-InfraObs Label for the Observability Infrastructure Monitoring team [elastic/obs-infraobs-integrations] labels Jul 19, 2024

ltflb-bgdi marked this pull request as ready for review July 26, 2024 14:21

ltflb-bgdi force-pushed the fix_pb-561_cloudfront_logs branch from 11fb5fc to 04579e2 Compare August 5, 2024 12:46

efd6 reviewed Aug 6, 2024

View reviewed changes

ltflb-bgdi force-pushed the fix_pb-561_cloudfront_logs branch from 3be4acd to ea0a6e7 Compare August 6, 2024 16:08

ltflb-bgdi force-pushed the fix_pb-561_cloudfront_logs branch from 94d1c5b to 7de3cf3 Compare August 7, 2024 09:11

ltflb-bgdi force-pushed the fix_pb-561_cloudfront_logs branch 2 times, most recently from 48c6f4a to d0f0010 Compare August 7, 2024 09:19

ltflb-bgdi force-pushed the fix_pb-561_cloudfront_logs branch from d0f0010 to dbbe9e3 Compare August 7, 2024 09:35

ltflb-bgdi force-pushed the fix_pb-561_cloudfront_logs branch from dbbe9e3 to 945386b Compare August 7, 2024 09:41

ltflb-bgdi and others added 13 commits August 22, 2024 09:22

Fixed edge_detailed_result_type

2bd26b6

Updated cloudfront.md

efa0733

Implemented review recommendations

b7ad8ae

Updated cloudfront_logs system test log file

6940e0e

Fixed CI problems

49192aa

Update sample_event.json

176b2e0

Update cloudfront.md

9da56d3

formatted and build with newest elastic-package version

9e7be47

Revert changes fixed by formatters

bbb1424

Update sample_event.json

ddf8720

Added review recommendations

5a7a8ea

Beautified some pipeline conditionals

82119f3

Added additional cloudfront test case

c2db3bd

ltflb-bgdi force-pushed the fix_pb-561_cloudfront_logs branch from f8b7aa8 to c2db3bd Compare August 22, 2024 07:24

anil-elastic approved these changes Aug 28, 2024

View reviewed changes

andrewkroh reviewed Aug 29, 2024

View reviewed changes

packages/aws/data_stream/cloudfront_logs/_dev/test/system/test-default-config.yml Outdated Show resolved Hide resolved

Fixed test-default-config hit count

682261e

andrewkroh removed the request for review from a team September 4, 2024 04:58

anil-elastic merged commit 7821fe3 into elastic:main Sep 10, 2024
4 of 5 checks passed

ltflb-bgdi deleted the fix_pb-561_cloudfront_logs branch February 17, 2025 15:10

		@@ -1,4 +1,9 @@
		# newer versions go on top
		- version: "2.23.0"

	if: ctx?.event?.original?.startsWith('#')
	if: ctx.event?.original?.startsWith('#')

	if: ctx?._tmp?.date != null && ctx?._tmp?.time != null
	if: ctx._tmp?.date != null && ctx._tmp?.time != null

Fix AWS cloudfront log parsing #10216

Fix AWS cloudfront log parsing #10216

Conversation

ltflb-bgdi commented Jun 21, 2024 • edited by harnish-elastic Loading

Proposed commit message

Checklist

Author's Checklist

How to test this PR locally

Related issues

cla-checker-service bot commented Jun 21, 2024 • edited Loading

ltflb-bgdi commented Jun 21, 2024

ltflb-bgdi commented Aug 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ltflb-bgdi Aug 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

efd6 commented Aug 6, 2024

efd6 commented Aug 6, 2024

ltflb-bgdi commented Aug 7, 2024

ltflb-bgdi commented Aug 7, 2024

ltflb-bgdi commented Aug 7, 2024

andrewkroh commented Aug 28, 2024

andrewkroh commented Aug 28, 2024

andrewkroh commented Sep 4, 2024

elasticmachine commented Sep 4, 2024

💚 Build Succeeded

History

elastic-sonarqube bot commented Sep 4, 2024

Quality Gate failed

andrewkroh commented Sep 4, 2024

ltflb-bgdi commented Sep 9, 2024

elasticmachine commented Sep 10, 2024

ltflb-bgdi commented Jun 21, 2024 •

edited by harnish-elastic

Loading

cla-checker-service bot commented Jun 21, 2024 •

edited

Loading

ltflb-bgdi Aug 7, 2024 •

edited

Loading