Add community_id, fingerprint, and network_direction processors #3011

andrewkroh · 2024-10-10T23:31:53Z

Add ingest processor definitions for community_id, fingerprint, network_direction, and registered_domain processors. All parameters and descriptions were scraped from the 8.16 docs.

Add ip to the list of types supported by the convert processor.

Add ecs_compatibility to grok processor.

Add output_format to date processor.

Related issues

Match the ConvertType enum to the values and order defined in Elasticsearch's ConverProcessor. https://github.com/elastic/elasticsearch/blob/a0cd389b43f4a155ae849e7730a8bcb6a9deb35f/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ConvertProcessor.java#L31 Fixes #2309

Fixes #2553

Fixes #2593

Fixes #2617

github-actions · 2024-10-10T23:33:24Z

Following you can find the validation results for the APIs you have changed.

API	Status	Request	Response
`ingest.delete_geoip_database`	🟢	1/1	1/1
`ingest.delete_pipeline`	🟢	15/15	15/15
`ingest.geo_ip_stats`	🟢	1/1	1/1
`ingest.get_geoip_database`	🟢	6/6	6/6
`ingest.get_pipeline`	🟢	22/22	22/22
`ingest.processor_grok`	🟢	1/1	1/1
`ingest.put_geoip_database`	🟢	3/3	3/3
`ingest.put_pipeline`	🟢	59/59	59/59
`ingest.simulate`	🟢	10/10	10/10

You can validate these APIs yourself by using the make validate target.

flobernd

Thank you for contributing to the spec 🙂

I left some comments.

specification/ingest/_types/Processors.ts

flobernd · 2024-10-11T14:36:31Z

I just noticed that we have a Ip (_types.Ip) alias defined which we should probably as well use instead of just string for all fields that represent an IP address.

Based on values from: https://github.com/elastic/elasticsearch/blob/c5da25754007af51029a676a8784b16be16d8995/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/FingerprintProcessor.java#L218

Based on: https://github.com/elastic/elasticsearch/blob/c5da25754007af51029a676a8784b16be16d8995/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/CommunityIdProcessor.java#L418-L428

internal_networks and internal_networks_field are mutually exclusive so they must both be optional.

andrewkroh · 2024-10-11T17:50:05Z

specification/ingest/_types/Processors.ts

+  /**
+   * Object field containing extracted domain components. If an empty string,
+   * the processor adds components to the document’s root.
+   * @server-default <empty string>


@flobernd, What should I do here? The docs say the default is an empty string. Should I remove this entirely, change it to "", something else?

I think we don't have a way to represent an empty string at the moment. This is something that we should probably improve in the future. cc @swallez @pquentin @l-trotta

We can remove the @server_default annotation for now.

andrewkroh · 2024-10-11T18:13:13Z

specification/ingest/_types/Processors.ts

+   * constructed with template snippets. Must specify only one of
+   * internal_networks or internal_networks_field.
+   */
+  internal_networks: string[]


This is the one field that is almost an IP address where would could use the Ip[] type. But that would be inaccurate because the values can be a IP or a CIDR. Should I create a type alias for CIDR somewhere such that the type can be (Ip | CIDR)[]?

Please refresh my memory. Is xxx.xxx.xxx.xxx without a slash a valid CIDR notation that defaults to /32?

In any way, I think it's fine keeping just string[] in this case.

Please refresh my memory. Is xxx.xxx.xxx.xxx without a slash a valid CIDR notation that defaults to /32?

Yes, that's how the processor is behaving for ipv4. I didn't check it, but for ipv6 I would expect similar behavior just using /128 to indicate that it's matching one address.

github-actions · 2024-10-11T18:22:22Z

Following you can find the validation results for the APIs you have changed.

API	Status	Request	Response
`ingest.delete_geoip_database`	🟢	1/1	1/1
`ingest.delete_pipeline`	🟢	15/15	15/15
`ingest.geo_ip_stats`	🟢	1/1	1/1
`ingest.get_geoip_database`	🟢	6/6	6/6
`ingest.get_pipeline`	🟢	22/22	22/22
`ingest.processor_grok`	🟢	1/1	1/1
`ingest.put_geoip_database`	🟢	3/3	3/3
`ingest.put_pipeline`	🟢	59/59	59/59
`ingest.simulate`	🟢	10/10	10/10

You can validate these APIs yourself by using the make validate target.

andrewkroh · 2024-10-11T18:31:03Z

With this change, all of the processors and parameters used across elastic/integrations ingest pipelines are accounted for in the spec.

flobernd · 2024-10-14T13:48:36Z

Hi @andrewkroh, thanks for updating the PR. Looks good now 🙂

Let's just remove the @server_default <empty string> tag and we should be good to go. cc @pquentin @l-trotta if you want to have a second look.

github-actions · 2024-10-14T14:12:01Z

Following you can find the validation results for the APIs you have changed.

API	Status	Request	Response
`ingest.delete_geoip_database`	🟢	1/1	1/1
`ingest.delete_pipeline`	🟢	15/15	15/15
`ingest.geo_ip_stats`	🟢	1/1	1/1
`ingest.get_geoip_database`	🟢	6/6	6/6
`ingest.get_pipeline`	🟢	22/22	22/22
`ingest.processor_grok`	🟢	1/1	1/1
`ingest.put_geoip_database`	🟢	3/3	3/3
`ingest.put_pipeline`	🟢	59/59	59/59
`ingest.simulate`	🟢	10/10	10/10

You can validate these APIs yourself by using the make validate target.

l-trotta

LGTM! thank you

…rocessors (#3011) Add ingest processor definitions for `community_id`, `fingerprint`, `network_direction`, and `registered_domain` processors. All parameters and descriptions were scraped from the 8.16 docs. Add `ip` to the list of types supported by the `convert` processor. Add `ecs_compatibility` to grok processor. Add `output_format` to date processor. Related issues - Fixes #2309 - Fixes #2553 - Fixes #2593 - Fixes #2617 (cherry picked from commit d1857ea)

…rocessors (#3011) (#3018) Add ingest processor definitions for `community_id`, `fingerprint`, `network_direction`, and `registered_domain` processors. All parameters and descriptions were scraped from the 8.16 docs. Add `ip` to the list of types supported by the `convert` processor. Add `ecs_compatibility` to grok processor. Add `output_format` to date processor. Related issues - Fixes #2309 - Fixes #2553 - Fixes #2593 - Fixes #2617 (cherry picked from commit d1857ea) Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>

andrewkroh added 5 commits October 10, 2024 18:44

Processors.ts - add community_id

a24055b

Fixes #2553

Processors.ts - add fingerprint

e0cfc17

Fixes #2593

Processors.ts - add network_direction

0ded54f

Fixes #2617

update doc links table.csv

51f7537

github-actions bot added the specification label Oct 10, 2024

andrewkroh mentioned this pull request Oct 10, 2024

Add community_id, fingerprint, and network_direction processors #3010

Closed

andrewkroh added the backport 8.19 label Oct 10, 2024

andrewkroh marked this pull request as ready for review October 10, 2024 23:34

flobernd reviewed Oct 11, 2024

View reviewed changes

andrewkroh added 7 commits October 11, 2024 13:25

review - add @server-default, use Field/Fields

56d8aa3

review - add FingerprintDigest enum to represent fingerprint method

98ff04e

Based on values from: https://github.com/elastic/elasticsearch/blob/c5da25754007af51029a676a8784b16be16d8995/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/FingerprintProcessor.java#L218

Processors.ts - add registered_domain processor

f24dc11

Processors.ts - add output_format to DateProcessor

80f4a95

Processors.ts - add ecs_compatibility to GrokProcessor

77e5e34

Processors.ts - update transports listed for community_id

6b83892

Based on: https://github.com/elastic/elasticsearch/blob/c5da25754007af51029a676a8784b16be16d8995/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/CommunityIdProcessor.java#L418-L428

network_direction processor - internal_networks needs to be optional

c420d53

internal_networks and internal_networks_field are mutually exclusive so they must both be optional.

andrewkroh commented Oct 11, 2024

View reviewed changes

fix server-default -> server_default

4982c5f

andrewkroh force-pushed the bugfix/missing-processors branch from e853e2f to 4982c5f Compare October 11, 2024 18:20

remove server_default for empty string

9439259

andrewkroh requested a review from flobernd October 14, 2024 14:15

l-trotta approved these changes Oct 14, 2024

View reviewed changes

andrewkroh merged commit d1857ea into main Oct 14, 2024
6 checks passed

andrewkroh deleted the bugfix/missing-processors branch October 14, 2024 15:01

github-actions bot mentioned this pull request Oct 14, 2024

[Backport 8.x] Add community_id, fingerprint, and network_direction processors #3018

Merged

Add community_id, fingerprint, and network_direction processors #3011

Add community_id, fingerprint, and network_direction processors #3011

Uh oh!

Conversation

andrewkroh commented Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 10, 2024

Uh oh!

flobernd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

flobernd commented Oct 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewkroh Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

flobernd Oct 14, 2024

Choose a reason for hiding this comment

Uh oh!

andrewkroh Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

flobernd Oct 14, 2024

Choose a reason for hiding this comment

Uh oh!

andrewkroh Oct 14, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 11, 2024

Uh oh!

andrewkroh commented Oct 11, 2024

Uh oh!

flobernd commented Oct 14, 2024

Uh oh!

github-actions bot commented Oct 14, 2024

Uh oh!

l-trotta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andrewkroh commented Oct 10, 2024 •

edited

Loading

flobernd commented Oct 11, 2024 •

edited

Loading