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

[Entity Analytics] [Entity Store] Show errors on entity store enablement #198263

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

tiansivive
Copy link
Contributor

Summary

This PR adds user feedback for errors that happen when enabling the entity store.
Any errors during the async setup of store resources will show up as toasts, whist initial INIT request failures will appear as an error callout.

Screenshot 2024-10-29 at 16 48 03
Screenshot 2024-10-29 at 16 47 19

@tiansivive tiansivive added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 Theme: entity_analytics Team:Entity Analytics Security Entity Analytics Team v8.16.0 backport:version Backport to applied version labels labels Oct 30, 2024
@tiansivive tiansivive requested a review from a team as a code owner October 30, 2024 09:05
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-entity-analytics (Team:Entity Analytics)

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Hey @tiansivive ! Overall looks good. Focussed review on SO parts and left some qs.

},
{
type: 'data_backfill',
backfillFn: (document) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are adding a new default field to the schema that is optional we don't need to run this backfill operation, leaving it undefined should be fine. Or does null mean something specific in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, it was meant to clearly state there was no value as its optional. I wasn't sure whether just leaving it undefined would work. I'll update

Comment on lines 33 to 35
error: {
type: 'object',
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we searching over this value? If not then we don't need to include it in mappings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so I dont need to add a migration for it? I can just "silently" add it to the saved object?

Copy link
Contributor Author

@tiansivive tiansivive Oct 30, 2024

Choose a reason for hiding this comment

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

I ended up removing the migration, seems like that is the correct way to do this since we never want to search on errors. Please do let me know if there are other concerns wrt the SO @jloleysens

@elasticmachine
Copy link
Contributor

⏳ Build in-progress, with failures

Failed CI Steps

History

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:Entity Analytics Security Entity Analytics Team Theme: entity_analytics v8.16.0 v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants