[BUG] Support duplicate index pattern name

[mengweieric]

Description

Before

After

After adding the id key, the duplicate index pattern is able to be selected, and seeing the index pattern id is reflected in search url.

Issues Resolved

Screenshot

Testing the changes

Check List

• All tests pass
  • yarn test:jest
  • yarn test:jest_integration
• New functionality includes testing.
• New functionality has been documented.
• Update CHANGELOG.md
• Commits are signed per the DCO using --signoff

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.00%. Comparing base (790c076) to head (e37822a).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5946   +/-   ##
=======================================
  Coverage   67.00%   67.00%           
=======================================
  Files        3307     3307           
  Lines       63614    63614           
  Branches    10163    10163           
=======================================
+ Hits        42625    42626    +1     
+ Misses      18519    18518    -1     
  Partials     2470     2470           

Flag	Coverage Δ
Linux_1	35.21% <ø> (ø)
Linux_2	55.09% <ø> (ø)
Linux_3	43.61% <ø> (+<0.01%)	⬆️
Linux_4	35.20% <ø> (ø)
Windows_1	35.24% <ø> (ø)
Windows_2	55.06% <ø> (ø)
Windows_3	43.63% <ø> (+<0.01%)	⬆️
Windows_4	35.20% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ashwin-pc]

Why are you only passing key for Index patterns? cant we have a key for every option?

[mengweieric]

I actually thought about this to also add key to non index pattern datasource, to do it generally where have 2 ways due to there's no such id in 'dataSet' for non-index-patterns datasources:

• use a some kind of id generator to add key few lines after this line
• define the id or leverage id like data in datasource fetched from backend in datasource class

The second approach is the one that I prefer as we should define it as part of the datasource itself either by generating the id there or leverage similar id like key in datasource data we fetched from backend. This approach, will require us to first align on what should be the unified and unique id other than index pattern id. This will require more work to modify interfaces and classes. Therefore, I'm not sure even if we proceed with this change, does it still qualify for a patch release?

The first approach, or other similar approaches I feel would require generating an id on the fly, which is more of a workaround way due to the current limitation, i don't feel we should go ahead with it in this case.

Therefore I've tested with key only for index patterns, it works with duplicated names across non-index-patterns and index patterns sections (see attached screenshot), without displaying and selecting issues, and also s3 datasource does not allow name duplications, plus the datasource registrations currently using the datasource name as key to register with datasource service which seems like a key for non-index-patterns datasource is not required for 2.11.

[ashwin-pc]

This isnt going in a patch. This should be going in the next minor, so we dont have to rush in a solution. Can the user have more than one s3 connection? or a similar name in spark? It looks like its a simple update to the datasource service to add an ID for every dataset. When creating the datasets, if it does not have a unique id, you can reuse the name as the ID if its unique, or make a combination such as connection.dataset_name as an id

[mengweieric]

This PR is created under the assumption that it could be considered as part of the patch release as users may raise issue in the case they import index pattern saved objects with the same name, this could mitigate the issue. But we may leave it if this is not the case.

It looks like its a simple update to the datasource service to add an ID for every dataset.

yes but at the meantime, the id has to be passed to the redirection url for log explorer, then the corresponding code there need to also be changed accordingly for duplicate name use cases and retest the entire flow. But if like you said we will handle it in next minor then for sure a long-term, id solution will be added anyway for all datasources without depending on if duplication is allowed or not to cover border scope of use cases.

[ashwin-pc]

Hmm, kayi see the value in this going in the patch. Since this component is due for a complete rewrite anyways, lets merge this in while you work on a proper rewrite so that the effort isnt duplicated.

[mengweieric]

sure sounds good.

[Flyingliuhub]

Now that it is selectable, how will the user know which one they should choose based on the name?

[mengweieric]

The user will be able to see the data loaded for that index pattern selected with corresponding id. And moving forward we could prefix the imported saved index patterns with cluster info.

[kavilla]

Sorry about the delay, @mengweieric could you provide context on this one?

In terms of index patterns, the current structure is that the name is the id for index patterns. I don't think it's impossible to get in a state because there is only client side validation when creating index patterns, but it was implemented assuming that you couldn't get easily create a duplicate index pattern through the UI. We currently have a different data source (if not local) being prepended to the index pattern so ideally that would solve the case where a non-local cluster's index pattern.

So if you could provide context on how we expect to see a duplicate index pattern in the way that we currently see in the playground and screen shot when it is created the expected way?

[mengweieric]

Sorry about the delay, @mengweieric could you provide context on this one?

In terms of index patterns, the current structure is that the name is the id for index patterns. I don't think it's impossible to get in a state because there is only client side validation when creating index patterns, but it was implemented assuming that you couldn't get easily create a duplicate index pattern through the UI. We currently have a different data source (if not local) being prepended to the index pattern so ideally that would solve the case where a non-local cluster's index pattern.

So if you could provide context on how we expect to see a duplicate index pattern in the way that we currently see in the playground and screen shot when it is created the expected way?

Like the issue currently exists in playground, when users import index pattern only from outside of the current cluster which contains any index pattern name same as the ones in current cluster, they will face this issue. These index patterns currently, after imported, are not prefixed with any additional datasource/cluster info therefore conflicts with the ones in the cluster.