[ML] File upload serverless compatibility

[jgowdyelastic]

Fixes problem with the file data visualizer using "number_of_shards": 1 in the default index settings which is not compatible with serverless.

This PR adds a new default_index_settings endpoint to the file upload plugin which will not return { "number_of_shards": 1 } when running in serverless.

Serverless

Non serverless

[elasticmachine]

Pinging @elastic/ml-ui (:ml)

[nreese]

I have a question about adding setIsServerless method to file_upload plugin. Is this needed? Instead could something like initializerContext.env.packageInfo.buildFlavor === 'serverless' work? Seems messy to have all plugins register their own serverless flags. I think it might be better to use a global flag from kibana.

[jgowdyelastic]

I have a question about adding setIsServerless method to file_upload plugin. Is this needed? Instead could something like initializerContext.env.packageInfo.buildFlavor === 'serverless' work? Seems messy to have all plugins register their own serverless flags. I think it might be better to use a global flag from kibana.

I've changed the code to use buildFlavor. Originally we were told not to be making checks like this, and for features to be enabled/disabled by calls from the three serverless project plugins.
However it seems this is not what people are doing, and using buildFlavor is far simpler.

[sphilipse]

I have a question about adding setIsServerless method to file_upload plugin. Is this needed? Instead could something like initializerContext.env.packageInfo.buildFlavor === 'serverless' work? Seems messy to have all plugins register their own serverless flags. I think it might be better to use a global flag from kibana.

Butting in here because I saw this come across my github notifications, but the generally accepted method is to add config flags to enable/disable/modify features, rather than try to infer whether Kibana is running in Serverless mode. Those flags are then set in config/serverless.yml, but in theory these flags could be set in other contexts too. This approach allows for a little more decoupling and makes it easy to turn features on/off.

I'm also not sure if initializerContext.env.packageInfo.buildFlavor === 'serverless' is going to be a stable way to identify whether Kibana is running in serverless in the future.

[nreese]

I've changed the code to use buildFlavor. Originally we were told not to be making checks like this, and for features to be enabled/disabled by calls from the three serverless project plugins.

This is how I have been instructed to in past PRs, see #162354 (comment) for details.

I'm also not sure if initializerContext.env.packageInfo.buildFlavor === 'serverless' is going to be a stable way to identify whether Kibana is running in serverless in the future.

@azasypkin Can you add any context here. We are getting mixed signals about how to signal if kibana is running in serverless.

[nreese]

What was the reasoning behind moving this to a REST call? Seems like it complicates the UI and adds extra http traffic.

[jgowdyelastic]

because of the way the File Data Visualizer is embedded as a component in ML and Home, this seemed to be the simplest way for the plugin to determine which settings to use. This was based on my original implementation where the serverless plugins called the file upload plugin on the server side.
However, I was also unaware of being able to use buildFlavor which also works client side. So it looks like this PR can can simplified even more and this endpoint removed.

[sphilipse]

I've changed the code to use buildFlavor. Originally we were told not to be making checks like this, and for features to be enabled/disabled by calls from the three serverless project plugins.

This is how I have been instructed to in past PRs, see #162354 (comment) for details.

I'm also not sure if initializerContext.env.packageInfo.buildFlavor === 'serverless' is going to be a stable way to identify whether Kibana is running in serverless in the future.

@azasypkin Can you add any context here. We are getting mixed signals about how to signal if kibana is running in serverless.

That's an interesting discussion that would be well-served by a larger audience, IMO. Basically, the more centralized communication around Serverless development has focused on using config flags as the preferred method. I don't disagree with @azasypkin that a Serverless flag might be useful, but if there was any communication about that other than within teams and in Github PR comments I must have missed it.

Regardless, I won't hold up this PR whichever way you want to go. Just wanted to flag the (as far as I know) generally accepted way of handling this :)

[jgowdyelastic]

Closing this PR as the approach of using buildFlavor to determine serverlessness means every part of this PR needs to change. So it's easier to start again from scratch.

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 17183a0

Metrics [docs]

Async chunks

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

id	before	after	diff
dataVisualizer	606.3KB	606.5KB	+214.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
fileUpload	7.3KB	7.5KB	+210.0B

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

cc @jgowdyelastic

[azasypkin]

@azasypkin Can you add any context here. We are getting mixed signals about how to signal if kibana is running in serverless.
That's an interesting discussion that would be well-served by a larger audience, IMO. Basically, the more centralized communication around Serverless development has focused on using config flags as the preferred method. I don't disagree with @azasypkin that a Serverless flag might be useful, but if there was any communication about that other than within teams and in Github PR comments I must have missed it.

Sorry for any confusion here. The perspective I outlined in #162354 (comment) is what we've been discussing in our team (Kibana Security). While it still holds true for us, and I believe that "fake" feature flags aren't a panacea, it's not something that has been widely discussed.

I believe we've learned a lot since the initial proposal to use config feature flags for everything that needs to be disabled or change behavior in the Serverless offering. We've discovered different use cases we hadn't initially anticipated and introduced different APIs to deal with them. Unless I missed the existing guidance, I think it's a good time to summarize the options we have today (real/fake feature flags gated with offeringBasedSchema, buildFlavor provided by the plugin context on both the server and client side, ES capabilities API, and other implicit and explicit ES capabilities checks) and provide, perhaps, more prescriptive guidance on when to use which approach. I'm tagging @elastic/kibana-core here as the most authoritative source of truth on this topic 🙂

[sorenlouv]

I believe we've learned a lot since the initial proposal to use config feature flags for everything that needs to be disabled or change behavior in the Serverless offering
...
I'm tagging https://github.com/orgs/elastic/teams/kibana-core here as the most authoritative source of truth on this topic

Does that mean we can remove the fake feature flags and replace them with a single global flag? That would be nice. In hindsight (and hindsight is tricky!) I wish we could have taken this route earlier as proposed by solution teams. I wonder if there is a larger learning here about choosing the simple approach before enforcing the complex one.