Skip to content
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] Support inline log views in the UI components #142840

Closed
Tracked by #134412
Kerry350 opened this issue Oct 6, 2022 · 1 comment · Fixed by #152933
Closed
Tracked by #134412

[Logs UI] Support inline log views in the UI components #142840

Kerry350 opened this issue Oct 6, 2022 · 1 comment · Fixed by #152933
Assignees
Labels
Feature:Discover Discover Application Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services

Comments

@Kerry350
Copy link
Contributor

Kerry350 commented Oct 6, 2022

📓 Summary

ℹ️ Part of #134412.
🔗 depends on #151489

We want to allow the user and other parts of Kibana to specify the Logs UI configuration in an ad-hoc manner. Currently the Logs UI is configured using a LogView, which is fetched using its logViewId. In order to support transient and unpersisted state the log view management code must be able to initialize and change log views in memory without loading them from or persisting them to saved objects.

✔️ Acceptance criteria

  • The LogViewStateMachine owns the current logViewId instead of just reacting to its change.
  • The LogViewStateMachine can...
    • load a log view based on a logViewId.
    • create a log view in its context (based on an id and a set of attributes) without persisting.
    • update the log view held in its context without persisting.
    • persist the log view held in its context.
  • The useLogView hook and provider's logViewId prop is replaced with an initialLogViewReference prop, which initializes the state machine.
  • When using an inline log view, the alert creation link is disabled.
  • When using an inline log view, the ML pages (anomalies and categories) display a splash page warning the user inline log views are unsupported
  • The URL contains a logView parameter that can contain both a reference to a log view but also an inline log view instead of the previously used sourceId (which is upgraded to a logView parameter upon initialization).
  • Any changes to the inline log view are written back to the URL so it persists across page reloads.
  • When using an inline log view, the settings page contains a warning callout above the "apply" button, that indicates the use of an inline log view and contains a button to revert to the persisted default log view.

Background information

The Logs UI supports the concept of LogViews (previously source configurations). A LogViewId (sourceId in older references) can be sourced from multiple locations including a LogView (saved object), internal source configuration, or a source configuration (also a saved object). The LogViewsClient handles the heavy lifting when it comes to determining how to load the raw attributes, and then finally how to resolve the attributes to return a fully resolved LogView.

To support inline LogViews we should amend relevant code to allow an inline LogView to be used. This will be the equivalent of a fully resolved LogView. E.g.

{
    indices: dataView.title,
    timestampField: dataView.timeFieldName ?? TIMESTAMP_FIELD,
    tiebreakerField: TIEBREAKER_FIELD,
    messageField: ['message'],
    fields: dataView.fields,
    runtimeMappings: resolveRuntimeMappings(dataView),
    columns: logViewAttributes.logColumns,
    name: logViewAttributes.name,
    description: logViewAttributes.description,
  };

When users switch to a new DataView from the selector we can create an inline LogView.

ℹ️ Some consideration might need to be given to columns in the first stages.

@Kerry350 Kerry350 added Feature:Discover Discover Application Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels Oct 6, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@weltenwort weltenwort added the needs-refinement A reason and acceptance criteria need to be defined for this issue label Jan 23, 2023
@weltenwort weltenwort self-assigned this Jan 23, 2023
@weltenwort weltenwort changed the title [Logs UI] Support transient / inline / non-persisted settings [Logs UI] Support inline log views in the UI components Feb 16, 2023
@weltenwort weltenwort removed their assignment Feb 20, 2023
@weltenwort weltenwort removed the needs-refinement A reason and acceptance criteria need to be defined for this issue label Feb 20, 2023
@Kerry350 Kerry350 self-assigned this Feb 22, 2023
Kerry350 added a commit that referenced this issue Mar 2, 2023
## Summary

Closes #151489

This is the server side portion of support for transient / inline Log
Views. Alerting and ML based functionality is scoped to the persisted
type only, as we won't be supporting inline Log Views in those contexts.

In terms of UI, changes have been made as close to the edge / boundary
to the server (e.g. where we actually make network requests) as
possible. This is because the bulk of the UI changes will come in
#142840.

## Testing

This is predominantly a type-driven refactoring (the best kind), so we
should be relatively safe here (especially since inline views aren't in
the UI yet). But, a quick test of each page and a high level functional
test would still be good.
bmorelli25 pushed a commit to bmorelli25/kibana that referenced this issue Mar 10, 2023
## Summary

Closes elastic#151489

This is the server side portion of support for transient / inline Log
Views. Alerting and ML based functionality is scoped to the persisted
type only, as we won't be supporting inline Log Views in those contexts.

In terms of UI, changes have been made as close to the edge / boundary
to the server (e.g. where we actually make network requests) as
possible. This is because the bulk of the UI changes will come in
elastic#142840.

## Testing

This is predominantly a type-driven refactoring (the best kind), so we
should be relatively safe here (especially since inline views aren't in
the UI yet). But, a quick test of each page and a high level functional
test would still be good.
Kerry350 added a commit that referenced this issue Mar 29, 2023
## Summary

This closes #142840. It is the UI portion of support for inline Log
Views.

## Visible changes to the UI

### ML warning

![Screenshot 2023-03-10 at 14 55
34](https://user-images.githubusercontent.com/471693/224348959-8db70d91-dc8b-4f4e-926b-ec05e7481b78.png)

### Alert dropdown warning

![Screenshot 2023-03-10 at 14 55
43](https://user-images.githubusercontent.com/471693/224349026-cdf17767-225a-4ecd-af8a-b90e2a21816f.png)


### Settings page warning

![Screenshot 2023-03-10 at 14 56
02](https://user-images.githubusercontent.com/471693/224349066-bcb63ba8-fee8-4b7a-b41b-7d89e09f002a.png)


## Reviewer hints / notes

- The ACs on the issue are quite extensive and should provide a good
number of things to test.
  - Make use of the "playground" page (see below) to make this easier
 
- The `AlertDropdown` has been made lazy as the page load bundle
increased by 100kb otherwise.

- Our `link-to` functionality is scoped to persisted Log Views only at
the moment as historically they've only accepted a path segment, not
full query parameters. We can look to extend this in the future once we
have concrete linking needs.

## Questions

- I have allowed the Log View client to handle both the inline and
persisted Log Views. I wonder if the function names become confusing?
(e.g. are the RESTful prefixes misleading now?).

- The ML warning splash page links to settings to revert to a persisted
Log View. It could also be done in place on the page. I went back and
forth over whether we should keep the reverting in one place?


## Testing

There is now a "state machine playground" available at the following
URL: `/app/logs/state-machine-playground`, it is enabled in developer
mode only. It's not fancy or pretty it just serves to make testing
things easier. There are various buttons, e.g. `Switch to inline Log
View`, to facilitate easier testing whilst a Log View switcher is not in
the UI itself. You can utilise these buttons, and then head to other
pages to ensure things are working correctly, e.g. warning callouts and
disabled buttons etc. If you'd like to play with the options used, e.g.
for `update`, amend the code within `state_machine_playground.tsx`. It's
also useful just to sit on this page, try various things, and verify
what happens in the developer tools (does the context update correctly
etc).

## Known issues

- When saving on the settings page we actually revert to a "Loading data
sources" state. This is also the case on `main`. The reason for this is
the check within settings looks like so:

```ts
if ((isLoading || isUninitialized) && !resolvedLogView) {
    return <SourceLoadingPage />;
}
```

but the `resolvedLogView` state matching looks like so:

```ts
const resolvedLogView = useSelector(logViewStateService, (state) =>
    state.matches('checkingStatus') ||
    state.matches('resolvedPersistedLogView') ||
    state.matches('resolvedInlineLogView')
      ? state.context.resolvedLogView
      : undefined
  );
```

so even though we have resolved a Log View previously the state matching
overrides this. I'd prefer to follow this up in a separate issue as I'd
like to think through the ramifications a bit more. It's not a bug, but
it is jarring UX.

---------

Co-authored-by: Kibana Machine <[email protected]>
jgowdyelastic pushed a commit to jgowdyelastic/kibana that referenced this issue Mar 30, 2023
## Summary

This closes elastic#142840. It is the UI portion of support for inline Log
Views.

## Visible changes to the UI

### ML warning

![Screenshot 2023-03-10 at 14 55
34](https://user-images.githubusercontent.com/471693/224348959-8db70d91-dc8b-4f4e-926b-ec05e7481b78.png)

### Alert dropdown warning

![Screenshot 2023-03-10 at 14 55
43](https://user-images.githubusercontent.com/471693/224349026-cdf17767-225a-4ecd-af8a-b90e2a21816f.png)


### Settings page warning

![Screenshot 2023-03-10 at 14 56
02](https://user-images.githubusercontent.com/471693/224349066-bcb63ba8-fee8-4b7a-b41b-7d89e09f002a.png)


## Reviewer hints / notes

- The ACs on the issue are quite extensive and should provide a good
number of things to test.
  - Make use of the "playground" page (see below) to make this easier
 
- The `AlertDropdown` has been made lazy as the page load bundle
increased by 100kb otherwise.

- Our `link-to` functionality is scoped to persisted Log Views only at
the moment as historically they've only accepted a path segment, not
full query parameters. We can look to extend this in the future once we
have concrete linking needs.

## Questions

- I have allowed the Log View client to handle both the inline and
persisted Log Views. I wonder if the function names become confusing?
(e.g. are the RESTful prefixes misleading now?).

- The ML warning splash page links to settings to revert to a persisted
Log View. It could also be done in place on the page. I went back and
forth over whether we should keep the reverting in one place?


## Testing

There is now a "state machine playground" available at the following
URL: `/app/logs/state-machine-playground`, it is enabled in developer
mode only. It's not fancy or pretty it just serves to make testing
things easier. There are various buttons, e.g. `Switch to inline Log
View`, to facilitate easier testing whilst a Log View switcher is not in
the UI itself. You can utilise these buttons, and then head to other
pages to ensure things are working correctly, e.g. warning callouts and
disabled buttons etc. If you'd like to play with the options used, e.g.
for `update`, amend the code within `state_machine_playground.tsx`. It's
also useful just to sit on this page, try various things, and verify
what happens in the developer tools (does the context update correctly
etc).

## Known issues

- When saving on the settings page we actually revert to a "Loading data
sources" state. This is also the case on `main`. The reason for this is
the check within settings looks like so:

```ts
if ((isLoading || isUninitialized) && !resolvedLogView) {
    return <SourceLoadingPage />;
}
```

but the `resolvedLogView` state matching looks like so:

```ts
const resolvedLogView = useSelector(logViewStateService, (state) =>
    state.matches('checkingStatus') ||
    state.matches('resolvedPersistedLogView') ||
    state.matches('resolvedInlineLogView')
      ? state.context.resolvedLogView
      : undefined
  );
```

so even though we have resolved a Log View previously the state matching
overrides this. I'd prefer to follow this up in a separate issue as I'd
like to think through the ramifications a bit more. It's not a bug, but
it is jarring UX.

---------

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants