-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Ingest pipelines] add community id processor #103863
[Ingest pipelines] add community id processor #103863
Conversation
@elasticmachine merge upstream |
@elasticmachine merge upstream |
merge conflict between base and head |
Pinging @elastic/kibana-stack-management (Team:Stack Management) |
...application/components/pipeline_editor/components/processor_form/processors/community_id.tsx
Outdated
Show resolved
Hide resolved
...application/components/pipeline_editor/components/processor_form/processors/community_id.tsx
Outdated
Show resolved
Hide resolved
...blic/application/components/pipeline_editor/components/shared/map_processor_type_to_form.tsx
Outdated
Show resolved
Hide resolved
...blic/application/components/pipeline_editor/components/shared/map_processor_type_to_form.tsx
Outdated
Show resolved
Hide resolved
...blic/application/components/pipeline_editor/components/shared/map_processor_type_to_form.tsx
Outdated
Show resolved
Hide resolved
...ngest_pipelines/public/application/components/pipeline_editor/context/processors_context.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @sabarasaba,
thank you so much for working on this! Code changes LGTM, but I'd like to suggest getting a review from a docs writer, for example to check that id is spelled ID everywhere (following how it's spelled in the docs). Also I think it would be useful to add default values to the field's help text, same as you did for the target field.
…pipeline_editor/components/shared/map_processor_type_to_form.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com>
…pipeline_editor/components/shared/map_processor_type_to_form.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com>
…pipeline_editor/components/shared/map_processor_type_to_form.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com>
…pipeline_editor/components/processor_form/processors/community_id.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com>
…pipeline_editor/components/processor_form/processors/community_id.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com>
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @sabarasaba. I left a few comments related to defaults, field name references, and the docs link. I'll take another after those are addressed.
...application/components/pipeline_editor/components/processor_form/processors/community_id.tsx
Outdated
Show resolved
Hide resolved
...application/components/pipeline_editor/components/processor_form/processors/community_id.tsx
Outdated
Show resolved
Hide resolved
...application/components/pipeline_editor/components/processor_form/processors/community_id.tsx
Outdated
Show resolved
Hide resolved
...blic/application/components/pipeline_editor/components/shared/map_processor_type_to_form.tsx
Show resolved
Hide resolved
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for docs review. Thanks @sabarasaba.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, @sabarasaba!
I re-tested locally and everything looks great 👍 Thank you so much for addressing my comments in the initial review!
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: cc @sabarasaba |
* wip: community_id processor * fix up validation conditions for fields * add tests * Fix layout and tests * Fix copy and validations * Remove unused prop * fix test matchers * lowercase copy * Convert hardcoded values to variables * Remove extra dollar sign * fix copy variable * Update x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/shared/map_processor_type_to_form.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com> * Update x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/shared/map_processor_type_to_form.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com> * Update x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/shared/map_processor_type_to_form.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com> * Update x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/processor_form/processors/community_id.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com> * Update x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/processor_form/processors/community_id.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com> * No need to whitelist known fields * CR changes Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com>
…y-show-migrate-to-authzd-users * 'master' of github.com:elastic/kibana: (48 commits) [Canvas] Expression shape (elastic#103219) [FTR] Skips Vega tests [Sample data] Use Lens in ecommerce data (elastic#106039) [APM] Backends inventory & overview page routes (elastic#106223) [TSVB] Add more functional tests for Gauge and TopN (elastic#105361) Add toggle to enable/disable rule install from SOs (elastic#106189) Improve unit test coverage of FS API calls (elastic#106242) Remove recursive plugin status in meta field (elastic#106286) [Ingest pipelines] add community id processor (elastic#103863) [XY axis] Fixes the values inside bar charts (elastic#106198) [data.search] Set default expiration to 1m if search sessions are disabled (elastic#105329) set the doc title when navigating to reporting and unset when navigating away (elastic#106253) [Lens] Display legend inside chart (elastic#105571) [RAC] [TGrid] Migrate the TGrid's rendering to `EuiDataGrid` (elastic#106199) [Security Solutions] Removes the elastic legacy client from lists and security_solution plugins (elastic#106130) [Enterprise Search] Require security plugin in 8.0 (elastic#106307) [DOCS] Updates screenshots in Dev Tools docs (elastic#105859) [DOCS] Updates text and screenshots in tags doc (elastic#105853) [Alerting] Allow rule types to extract/inject saved object references on rule CRU (elastic#101896) Jest and Storybook fixes (elastic#104991) ... # Conflicts: # x-pack/plugins/reporting/public/plugin.ts
* wip: community_id processor * fix up validation conditions for fields * add tests * Fix layout and tests * Fix copy and validations * Remove unused prop * fix test matchers * lowercase copy * Convert hardcoded values to variables * Remove extra dollar sign * fix copy variable * Update x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/shared/map_processor_type_to_form.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com> * Update x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/shared/map_processor_type_to_form.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com> * Update x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/shared/map_processor_type_to_form.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com> * Update x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/processor_form/processors/community_id.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com> * Update x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/processor_form/processors/community_id.tsx Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com> * No need to whitelist known fields * CR changes Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Yulia Čech <6585477+yuliacech@users.noreply.github.com>
Fixes: #86321
As discussed with @dborodyansky, the two fields that depend on each other (
iana_number
andtransport
) are disabled accordingly depending on which one was set instead of being hidden. I've left all the fields on a single row except for the ip and port variants since it seemed to me that it made more sense that way given their correlation.Release Note
The Ingest Node Pipelines UI added support to configure a community id processor. This processor computes the Community ID for network flow data as defined in the Community ID Specification.
Default description
New processor fields