Skip to content
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

[Osquery] Refactor ECS editor field #130582

Merged

Conversation

patrykkopycinski
Copy link
Contributor

@patrykkopycinski patrykkopycinski commented Apr 19, 2022

Refactored ECS editor field to leverage updated UseArray field and form.updateFieldValues() from:

This PR is also removing steps from the Live Query form

image

@patrykkopycinski patrykkopycinski self-assigned this Apr 19, 2022
@patrykkopycinski patrykkopycinski force-pushed the chore/ecs-form-refactor branch from b0c0cd3 to 58a180d Compare April 26, 2022 19:56
@patrykkopycinski patrykkopycinski marked this pull request as ready for review April 26, 2022 19:56
@patrykkopycinski patrykkopycinski requested review from a team as code owners April 26, 2022 19:56
@patrykkopycinski patrykkopycinski added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:Asset Management Security Asset Management Team Feature:Osquery Security Solution Osquery feature v8.3.0 labels Apr 26, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-asset-management (Team:Asset Management)

@patrykkopycinski
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@tomsonpl tomsonpl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of changes, but seems solid, thanks! previous ECS code was confusing for me

grouper.setTotalAgents(totalNumAgents);
grouper.updateGroup(AGENT_GROUP_KEY.Platform, groups.platforms);
grouper.updateGroup(AGENT_GROUP_KEY.Policy, groups.policies);
grouper.setTotalAgents(agentGroupsData?.totalCount);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that we need optional chaining? There is a check for agentGroupsData in line 154

import { SavedQueriesDropdown } from '../../saved_queries/saved_queries_dropdown';

const FORM_ID = 'liveQueryForm';

const StyledEuiAccordion = styled(EuiAccordion)`
${({ isDisabled }: { isDisabled: boolean }) => isDisabled && 'display: none;'}
${({ isDisabled }: { isDisabled?: boolean }) => isDisabled && 'display: none;'}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what would you say about typing isDisabled as 'isDisabled?: true' ? Since it may not exist, the value we want is true, never false.

x-pack/plugins/osquery/public/results/results_table.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@miltonhultgren miltonhultgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Infra change looks good to me 👍🏼

…refactor

# Conflicts:
#	x-pack/plugins/osquery/cypress/integration/all/live_query.spec.ts
#	x-pack/plugins/osquery/cypress/tasks/live_query.ts
#	x-pack/plugins/osquery/public/live_queries/form/index.tsx
#	x-pack/plugins/osquery/public/live_queries/index.tsx
#	x-pack/plugins/osquery/public/results/results_table.tsx
#	x-pack/plugins/osquery/public/routes/saved_queries/edit/tabs.tsx
#	x-pack/plugins/osquery/public/saved_queries/saved_query_flyout.tsx
#	x-pack/plugins/osquery/public/shared_components/osquery_action/index.tsx
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
osquery 261 262 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
infra 1002.7KB 1002.7KB +19.0B
osquery 998.9KB 997.4KB -1.5KB
total -1.5KB
Unknown metric groups

ESLint disabled line counts

id before after diff
osquery 123 118 -5

Total ESLint disabled count

id before after diff
osquery 128 123 -5

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @patrykkopycinski

@patrykkopycinski patrykkopycinski merged commit e1bd7da into elastic:main Apr 29, 2022
@patrykkopycinski patrykkopycinski deleted the chore/ecs-form-refactor branch April 29, 2022 12:53
kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Osquery Security Solution Osquery feature release_note:skip Skip the PR/issue when compiling release notes Team:Asset Management Security Asset Management Team v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants