-
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
[Logs UI] Allow for plugins to inject internal source configurations #36066
[Logs UI] Allow for plugins to inject internal source configurations #36066
Conversation
Pinging @elastic/infrastructure-ui |
💚 Build Succeeded |
adbc04c
to
3887f6d
Compare
💔 Build Failed |
💚 Build Succeeded |
c2c8a2a
to
4a7039b
Compare
💔 Build Failed |
19f6c19
to
f351d81
Compare
💚 Build Succeeded |
💚 Build Succeeded |
Sorry to any reviewers who might have already started taking a look at this. Just after ending the draft state of this PR I noticed that I should really improve and simplify the "link-to" urls. I switched them around from The change was local to the |
💔 Build Failed |
jenkins, test this |
💔 Build Failed |
ccd58b4
to
1e25978
Compare
💚 Build Succeeded |
The CI failure seems to have been unrelated and a rebase on |
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... @weltenwort and I discussed adding a follow up PR to add some tests around useUrlState
hook. I also set up a custom source and used it with in the app and everything worked as expected. Good Job!
…lastic#36066) This exposes an API on the Kibana server object (old platform style) to define internal source configurations, which can not be edited by the user and take precedence above any stored configurations.
…nfiguration code (#169430) Resolves #168240 ### Changes - Removes `fields.message` from the `infrastructure-ui-source` saved object who's value was being populated by `xpack.infra.sources.default.fields.message` from config. See https://www.elastic.co/guide/en/kibana/master/logs-ui-settings-kb.html - Removes `getInternalSourceConfiguration` and `defineInternalSourceConfiguration` functions introduced in #36066 as I cannot see them being used anywhere. Stops exposing `defineInternalSourceConfiguration` as part of server plugin interface. - Removes `getStaticDefaultSourceConfiguration` from InfraSources class as we aren't using `sources` from kibana config in source configuration anymore. - Removes deprecations warning of removal in 8.0 for other fields belonging to config xpack.infra.sources.* introduced in #115103 - Removes `getAllSourceConfigurations` used only in removed deprecations file f427278#diff-081721894fc437938eb652beae0a0640ddeee532ec5e48af1f3093c8074f1eecL195 - Removed `getAllSavedSourceConfigurations` only used in `getAllSourceConfigurations` ### How to test - `infrastructure-ui-source` saved object no longer has `fields` attributes - in kibana.yml set `xpack.infra.sources.default.fields.message: ['testmessage', '@testmessage']` - go to Infra -> Settings - Change the name and save the form - view the `infrastructure-ui-source` saved object and no `fields` attribute should exist (unless it previously existed in an already existing `infrastructure-ui-source` saved object - changes do not affect`fields.message` is used in logs_shared plugin - i'm not sure how to easily test this and relying on someone with more Logs knowledge to help out, but from what I can see logs accesses the config directly (`xpack.infra.sources.default.fields.message`) and never uses the saved object field being removed.
Summary
This exposes an API on the Kibana server object (old platform style) to define internal source configurations, which can not be edited by the user and take precedence above any stored configurations.
The API consists of the
server.plugins.infra.defineInternalSourceConfiguration(sourceId, sourceProperties)
function, which is supposed to be called from theinit
orpostInit
callback of plugins to define the source configuration with a unique source id. This source id can then be used when linking to the Logs UI to enforce usage of the internal source configuration without interference by other user-defined configurations. If it is not specified, the usual "default" source configuration will be used.More specifically, the link-to log routes now support specifying a source id in the url as in
/link-to/:sourceId/logs
and/link-to/:sourceId/(host|pod|container)-logs
. For these log links, this is turned into the appropriate url params. It is implemented for the snapshot and metrics redirects until these pages fully support referencing non-default sources.closes #30792
Example Usage
In a plugin definition, the
postInit
callback could look something like this:Then on the front-end, the plugin's UI could link to that source:
The user would not be able to change the configuration through the configuration UI, because it would be fully controlled by the plugin.
Review Notes
use_url_state
hook, which is essentially a port of the previous<UrlStateContainer>
component to hooks, but simpler. It behaves similar to asetState
hook backed by the rison-encoded url. It doesn't provide any imperativeonChange
callback anymore, because that can be easily be imitated usinguseEffect
. Both the hook and the old container component use the same encoding and can be used simultaneously.HistoryContext.Provider
.sourceId
state is managed via hooks, I added a bridge component on the logs page to inject thesourceId
into redux land via a simplesetSourceId
action.origin
, which is one ofinternal
,fallback
orstored,
to indicate where the configuration comes from. If it isinternal
, then saving through the configuration flyout is disabled.Testing
One possibility to test whether using an internal source actually works is to add something like shown in the example usage above to one of the other x-pack plugins and directly visit the corresponding
link-to
route in the browser.It's also important to ensure that not specifying any source id defaults to the "default" source id and preserves the previous behaviour.
Checklist
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsThis was checked for keyboard-only and screenreader accessibilityFor maintainers
This was checked for breaking API changes and was labeled appropriatelyThis includes a feature addition or change that requires a release note and was labeled appropriately