-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Prefer DataView client over SavedObjects client when possible #136694
Conversation
isCachable: true, | ||
}); | ||
|
||
return services.dataViews.create({ title: res.apmDataViewTitle }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously the endpoint returned a fake data view. Now it only returns the data view index pattern (aka title) which is used to build a real (non-persisted) data view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great if we can just have one hook / endpoint instead of having a static and a dynamic data view. Now that we use the dataViews
services it is more straightforward to do so.
} | ||
) | ||
); | ||
name: 'APM UI', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
// if the existing data view does not matches the new one, force an update | ||
return existingDataView.attributes.title !== apmDataViewTitle; | ||
const existingDataView = await dataViewService.get(APM_STATIC_DATA_VIEW_ID); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaces savedObjectsClient.get
with dataViewService.get
defaultMessage: | ||
'An APM data view is required for some features in the APM UI.', | ||
} | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the tutorials it is currently possible to create a data view directly by clicking a button. I've removed this feature because:
- It is only supported via the legacy saved object calls
- it no longer adds any value, since the data view is automatically generated when opening APM UI
Pinging @elastic/apm-ui (Team:apm) |
savedObjectsClient.create( | ||
'index-pattern', | ||
getApmDataViewAttributes(apmDataViewTitle), | ||
await withApmSpan('create_index_pattern_saved_object', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
await withApmSpan('create_index_pattern_saved_object', async () => { | |
await withApmSpan('create_data_view', async () => { |
Pinging @elastic/uptime (Team:uptime) |
4eea34d
to
a1adc93
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UX changes LGTM, tested this locally running as an admin and it seems fine. We've also uncovered that the dashboard seems broken for readonly users, that's tracked in a separate ticket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! It looks cleaner now as well!
a1adc93
to
2bcdc41
Compare
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
Closes #135972
Closes #131368
This PR removes usages of
savedObjectsClient
where dataViews client can be used instead. Custom logic is also replaced with built-in methods provided by the dataViews client.