Fix flint skipping index syntax issues #1846

YANG-DB · 2024-05-22T22:44:55Z

Description

update vpc flow logs & cloud trail flint related integration content changes

Issues Resolved

VPC

Minor fix for the VPC Flow Log Integration for Flint Version 1.1.0
Fix skipping index related issue
Fix table definition from json to parquet to match the VPC log based producer protocol

CloudTrail

Minor fix for the Amazon Log Integration for Flint Version 1.1.0
Fix skipping index related issue
Update table creation statement according to Athena DDL Statement
See related Athena S3 setup tutorial

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

- vpc flow - cloud trail Signed-off-by: YANGDB <[email protected]>

- vpc flow - cloud trail - multiple records protocol support Signed-off-by: YANGDB <[email protected]>

amsiglan · 2024-05-22T23:08:14Z

server/adaptors/integrations/__data__/repository/aws_cloudtrail/aws_cloudtrail-1.0.0.json

+    },
+    {
+      "name": "dashboards-flint-records",
+      "label": "Dashboards & Visualizations adapted to Flint",


Same label as above, will this be confusing to the user?

RyanL1997

In generally LGTM. Just left some minor thoughts.

RyanL1997 · 2024-05-22T23:03:38Z

server/adaptors/integrations/__data__/repository/aws_cloudtrail/aws_cloudtrail-1.0.0.json

      "version": "1.0.0",
      "extension": "sql",
      "type": "query",
      "workflows": ["dashboards-flint"]
    },
    {
-      "name": "create_mv_cloud-trail",
+      "name": "create_mv_cloud-trail-records",


Just for my knowledge, what is the record referencing to?

extended format for multiple records shown here

Do I understand right that we only have acceleration for single-record, and multi-record has queries without acceleration? LGTM from a technical standpoint, but I wonder how common multi-record is compared to single-record/what the impact of that is.

yes - this is due to the existing limitation caused by the skipping index creation statement

RyanL1997 · 2024-05-22T23:08:42Z

...s/integrations/__data__/repository/amazon_vpc_flow/assets/create_table_parquet_vpc-1.0.0.sql

@@ -58,5 +58,5 @@ CREATE EXTERNAL TABLE IF NOT EXISTS {table_name} (
  accountid STRING,
  eventday STRING
 )
-USING json
+USING parquet


Nice. Unrelated to this change, but I do notice that there is an empty file called "create_mv_vpc-1.0.0.sql" in the vpc asset directory. Should we also remove that?

- vpc flow - cloud trail - multiple records protocol support Signed-off-by: YANGDB <[email protected]>

Swiddis · 2024-05-22T23:45:34Z

...tegrations/__data__/repository/aws_cloudtrail/assets/create_mv_cloud-trail-records-1.0.0.sql

+  rec.tlsDetails.clientProvidedHostHeader AS `aws.cloudtrail.tlsDetailsclient_provided_host_header`
+FROM
+  {table_name}
+  LATERAL VIEW explode(Records) myTable AS rec


Are there issues with having the table alias default to myTable? Looks like it probably won't cause issues since it's just a virtual table but I'm not sure where the virtual table actually gets stored or if this should be more descriptively named.

Swiddis

I know we discussed at one point enabling the create table query by default so it doesn't cause an error if running with only queries selected and no other flows -- would now be a good time to implement that? OTOH I don't want to rock the boat since it's not in-scope for the immediate issue.

Aside from that, there's one unescaped field still -- did a full diff with the known-working version we made yesterday and that's the only delta so I can approve after that.

Swiddis · 2024-05-22T23:58:14Z

...tors/integrations/__data__/repository/amazon_vpc_flow/assets/create_skipping_index-1.0.0.sql

+    `src_endpoint.ip` BLOOM_FILTER,
+    `dst_endpoint.ip` BLOOM_FILTER,
+    `src_endpoint.svc_name` VALUE_SET,
+    `dst_endpoint.svc_name` VALUE_SET,
    traffic.bytes MIN_MAX


`traffic.bytes` ?

good catch - thanks

- vpc flow - cloud trail - multiple records protocol support Signed-off-by: YANGDB <[email protected]>

Signed-off-by: YANGDB <[email protected]>

ps48 · 2024-05-23T17:37:39Z

...tors/integrations/__data__/repository/amazon_vpc_flow/assets/create_skipping_index-1.0.0.sql

+    `src_endpoint.ip` BLOOM_FILTER,
+    `dst_endpoint.ip` BLOOM_FILTER,
+    `src_endpoint.svc_name` VALUE_SET,
+    `dst_endpoint.svc_name` VALUE_SET,
+    `traffic.bytes` MIN_MAX


Are we removing the field request_processing_time MIN_MAX,?

It doesn't exist in the data schema, so it wasn't supposed to be there anyways

ps48 · 2024-05-23T17:41:39Z

...rations/__data__/repository/aws_cloudtrail/assets/create_table_cloud-trail-records-1.0.0.sql

+OPTIONS (
+   compression='gzip',
+   recursivefilelookup='true',
+   multiline 'true'


Question? Should this be multiLine='true' .

ps48 · 2024-05-23T17:44:34Z

...rs/integrations/__data__/repository/aws_cloudtrail/assets/create_table_cloud-trail-1.0.0.sql

-  recursivefilelookup='true'
+   PATH '{s3_bucket_location}',
+   recursivefilelookup='true',
+   multiline 'true'


Is multiline part case sensitive. I've usually seen commands with multiLine L captialised.

PATH='{s3_bucket_location}', recursivefilelookup='true', multiLine='true'

I think both multiline and multiLine are be accepted

* update flint related issues for - vpc flow - cloud trail - multiple records protocol support Signed-off-by: YANGDB <[email protected]> * update flint vega ip sankey visualization query Signed-off-by: YANGDB <[email protected]> * update flint vega ip sankey visualization query Signed-off-by: YANGDB <[email protected]> --------- Signed-off-by: YANGDB <[email protected]> (cherry picked from commit 0d2a1c7) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

opensearch-trigger-bot · 2024-05-28T23:25:22Z

The backport to 2.13 failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/dashboards-observability/backport-2.13 2.13
# Navigate to the new working tree
pushd ../.worktrees/dashboards-observability/backport-2.13
# Create a new branch
git switch --create backport/backport-1846-to-2.13
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 0d2a1c7361520e52a4f5a014e19ee3d38bb91eeb
# Push it to GitHub
git push --set-upstream origin backport/backport-1846-to-2.13
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/dashboards-observability/backport-2.13

Then, create a pull request where the base branch is 2.13 and the compare/head branch is backport/backport-1846-to-2.13.

* update flint related issues for - vpc flow - cloud trail - multiple records protocol support * update flint vega ip sankey visualization query --------- (cherry picked from commit 0d2a1c7) Signed-off-by: YANGDB <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

YANG-DB added 6 commits May 22, 2024 11:32

update flint related issues for

b61d5f9

- vpc flow - cloud trail Signed-off-by: YANGDB <[email protected]>

update flint related issues for

df37903

- vpc flow - cloud trail Signed-off-by: YANGDB <[email protected]>

update flint related issues for

468c3b5

- vpc flow - cloud trail Signed-off-by: YANGDB <[email protected]>

update flint related issues for

1be3f9d

- vpc flow - cloud trail Signed-off-by: YANGDB <[email protected]>

update flint related issues for

1f85628

- vpc flow - cloud trail Signed-off-by: YANGDB <[email protected]>

update flint related issues for

6388bbe

- vpc flow - cloud trail - multiple records protocol support Signed-off-by: YANGDB <[email protected]>

YANG-DB requested review from ps48, kavithacm, derek-ho, joshuali925, dai-chen, rupal-bq, mengweieric, vamsimanohar, Swiddis, penghuo, seankao-az, anirudha, paulstn and sumukhswamy as code owners May 22, 2024 22:44

YANG-DB added backport 2.x backport 2.13 labels May 22, 2024

RyanL1997 mentioned this pull request May 22, 2024

[Integration] Fix the create table query for vpc parquet #1845

Closed

6 tasks

amsiglan reviewed May 22, 2024

View reviewed changes

RyanL1997 reviewed May 22, 2024

View reviewed changes

update flint related issues for

ed82366

- vpc flow - cloud trail - multiple records protocol support Signed-off-by: YANGDB <[email protected]>

YANG-DB added integrations Used to denote items related to the Integrations project ux-integration ux related integration issues labels May 22, 2024

update flint related issues for

f313d27

- vpc flow - cloud trail - multiple records protocol support Signed-off-by: YANGDB <[email protected]>

Swiddis reviewed May 22, 2024

View reviewed changes

Swiddis requested changes May 23, 2024

View reviewed changes

Swiddis added the bug Something isn't working label May 23, 2024

update flint related issues for

82c30fb

- vpc flow - cloud trail - multiple records protocol support Signed-off-by: YANGDB <[email protected]>

Swiddis approved these changes May 23, 2024

View reviewed changes

YANG-DB added 2 commits May 22, 2024 18:51

update flint vega ip sankey visualization query

ca5e2b8

Signed-off-by: YANGDB <[email protected]>

update flint vega ip sankey visualization query

893cea6

Signed-off-by: YANGDB <[email protected]>

YANG-DB requested review from amsiglan and RyanL1997 May 23, 2024 02:38

Swiddis approved these changes May 23, 2024

View reviewed changes

ps48 reviewed May 23, 2024

View reviewed changes

Merge branch 'main' into fix-flint-skipping-index-syntax-issues

602314b

RyanL1997 approved these changes May 26, 2024

View reviewed changes

ps48 approved these changes May 28, 2024

View reviewed changes

YANG-DB merged commit 0d2a1c7 into opensearch-project:main May 28, 2024
13 of 19 checks passed

opensearch-trigger-bot bot mentioned this pull request May 28, 2024

[Backport 2.x] Fix flint skipping index syntax issues #1852

Merged

opensearch-trigger-bot bot added the backport-failed label May 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix flint skipping index syntax issues #1846

Fix flint skipping index syntax issues #1846

YANG-DB commented May 22, 2024

amsiglan May 22, 2024

RyanL1997 left a comment

RyanL1997 May 22, 2024

YANG-DB May 22, 2024

Swiddis May 22, 2024 •

edited

Loading

YANG-DB May 23, 2024

RyanL1997 May 22, 2024

Swiddis May 22, 2024

Swiddis left a comment

Swiddis May 22, 2024

YANG-DB May 23, 2024

ps48 May 23, 2024

Swiddis May 24, 2024

ps48 May 23, 2024 •

edited

Loading

ps48 May 23, 2024

ps48 May 23, 2024

YANG-DB May 24, 2024

opensearch-trigger-bot bot commented May 28, 2024

Fix flint skipping index syntax issues #1846

Fix flint skipping index syntax issues #1846

Conversation

YANG-DB commented May 22, 2024

Description

Issues Resolved

VPC

CloudTrail

Check List

Choose a reason for hiding this comment

RyanL1997 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Swiddis May 22, 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

Swiddis left a 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

ps48 May 23, 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

opensearch-trigger-bot bot commented May 28, 2024

Swiddis May 22, 2024 •

edited

Loading

ps48 May 23, 2024 •

edited

Loading