search

elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.77k stars 8.17k forks source link

Retry getInstallation promise on the CSP plugin setup #182268

Open afharo opened 5 months ago

afharo commented 5 months ago

The promise below might fail in case there's a glitch in ES connection and never retry.

https://github.com/elastic/kibana/blob/593d3911728cf68093925836ae2031746c7db6cd/x-pack/plugins/cloud_security_posture/server/plugin.ts#L106-L113

Is that OK? Should we apply any retry logic?

If we want to retry, p-retry could be a useful utility that's already present in the project.

elasticmachine commented 5 months ago

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

opauloh commented 4 months ago

Hey @afharo, thanks for bringing this to our attention, is it a general recommendation for the xpack plugins to retry?

Is there's any documentation or guidance around that, like how many retries should we do until consider it failed? any information would be appreciated!

afharo commented 4 months ago

is it a general recommendation for the xpack plugins to retry?

IMO, if there's any setup process involved, and Kibana will assume it succeeded later on, it should be retried.

I don't think we have any guidance about how many retries are necessary, sorry.

seanrathier commented 4 months ago

@afharo thanks for bringing this up. I did a quick scan to see if other plugins were doing this and could not find any. Can you reference a plugin that does this for our reference?

afharo commented 4 months ago

It is true that we have a very naive setup experience in Kibana (that's why I created all the issues tracked in https://github.com/elastic/kibana/issues/182291). And examples are scarce.

This is one example: https://github.com/elastic/kibana/blob/f442a0dd6b006ba3e0025517064aad117253bb0b/src/plugins/content_management/server/event_stream/es/init/es_event_stream_initializer.ts#L26-L35

Here's another one: https://github.com/elastic/kibana/blob/35b7b08595d02964c1931beb2f9d52634268b82c/x-pack/plugins/event_log/server/es/init.ts#L31-L46

Another one: https://github.com/elastic/kibana/blob/4c8d8ef596f71a9a1b82666ed2f6c3b72eea9ef8/x-pack/plugins/observability_solution/observability/server/utils/create_or_update_index.ts#L26-L61

seanrathier commented 4 months ago

Oh, I see now, thanks @afharo for the examples. 🚀

@kfirpeled, how do we want to proceed with this?