[Uptime] Use dynamic index pattern in Uptime

[shahzad31]

Summary

Fix: #53616

Use Index Pattern service to create fake Index pattern.

Will only fetch index pattern once unlike current implementation.

Testing:

1. make sure kuery bar is working as expected in Uptime

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

• This was checked for cross-browser compatibility, including a check against IE11
• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• This was checked for keyboard-only and screenreader accessibility

For maintainers

• This was checked for breaking API changes and was labeled appropriately
• This includes a feature addition or change that requires a release note and was labeled appropriately

[elasticmachine]

Pinging @elastic/uptime (Team:uptime)

[andrewvc]

We probably shouldn't merge this because we probably shouldn't be creating an index pattern in the first place.

I did some digging, we only use the index pattern data for the Kuery Bar, but nothing about it requires it to be an actual index pattern saved object. We should instead just directly serve the JSON from a REST endpoint as a file based resource since it's shipped with Kibana. We never really needed to use saved objects here in the first place.

I'll add this note to the issue.

We should probably close this PR since it doesn't fix the root cause.

[shahzad31]

We probably shouldn't merge this because we probably shouldn't be creating an index pattern in the first place.

I did some digging, we only use the index pattern data for the Kuery Bar, but nothing about it requires it to be an actual index pattern saved object. We should instead just directly serve the JSON from a REST endpoint as a file based resource since it's shipped with Kibana. We never really needed to use saved objects here in the first place.

I'll add this note to the issue.

We should probably close this PR since it doesn't fix the root cause.

@andrewvc problem with Json file is that it is hard to maintain. If we add a field in mapping, we will have to manually update the file. Generating Index Pattern from Mapping and using that seems like a good approach to me.

[shahzad31]

@elasticmachine merge upstream

[andrewvc]

Functionality is so much better, and it's so much cleaner with Redux!

[andrewvc]

I realize that this is not new code, but it may be worth addressing while we're working on this.

In general swallowing errors is something we should avoid because it makes debugging issues (esp. in production) very difficult. Tracing back a genuine error to this code would be tricky, and then we wouldn't know if the issue was in the stringify part, the map instantiation, or the JSON parsing. If we have to do it we should limit it to a single statement. If it's to catch the JSON parse failure then we should do something like:

let parsed;
let parseError;
try {
  parsed = JSON.parse(urlFilters);
} catch (e) {
  parseError = e;
}

// We can safely ignore parse errors here because <<insert explanation>>
if (parseError) {
  kueryString = stringify(kueries)
} else {
  ...the happy path...
}

Then, we could execute conditional logic later depending on whether there's an error or not.

Note that we should document the reason why we're swallowing the error here. Is there a situation where the JSON is expected to be invalid? If so, why? Why is it better to ignore it rather than surfacing an error?

My suspicion is that we can possibly even remove the error handling here. @justinkambic may know more here.

[shahzad31]

i think we can't have control over user input into url or kuery bar, i have improve the code to make it more readable.

[shahzad31]

@andrewvc i took care of PR feedback, regarding those try/catch, i think we need them, since user can enter any value into url, so it will be hard to predict, if it will parse.

[andrewvc]

LGTM. I still have some concerns around our error handling, but since this isn't new code I'm fine merging this. I'll open a follow-up PR with reworked error handling since that might be a better way to show what I'm talking about.

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: a6c6842

History

• 💚 Build #23284 succeeded 9ff8744
• 💚 Build #22609 succeeded 61c2c8d
• 💔 Build #22420 failed 8f449dd
• 💚 Build #21671 succeeded 266fa99

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