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.

Sign up for GitHub

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

Jump to bottom

[Logs UI] View log in context modal #62198

Merged
merged 26 commits into from
Apr 24, 2020
Merged

[Logs UI] View log in context modal #62198

merged 26 commits into from
Apr 24, 2020

Conversation

afgomez
Copy link
Contributor

@afgomez afgomez commented Apr 1, 2020 •
edited by jasonrhodes
Loading

Summary

Closes #58487.

Implements a feature to show a certain log line within context. We define context as either:

  • Lines within the same host.name and log.file.path
  • Lines within the same container.id

Final view (we removed the line at the top of the modal that repeated the "selected log" and changed the selected log highlight to yellow so it stands out, with a tooltip as well)

Screenshot 2020-04-22 at 10 42 21

Screenshot 2020-04-22 at 10 42 10

Checklist

For maintainers

Alejandro Fernández Gómez and others added 15 commits April 1, 2020 19:02
Swap labels
168156c
Add onViewInContext callback to actions column
68cf1f8
Scaffold menu item for log entries with context
f38f93d
Add useViewLogInContext hook
5767fa4
Narrow down LogEntry[context] type
075495d
WIP: wire up menu item with modal
e4fe5b0
Tweak when the actions column should render in a row
bbe38d5 
If no callbacks are passed to the `LogEntryRow` component, the actions
column should not render. This will allow us to reuse the `LogEntryRow`
component in the modal for View log in context.
Render entry for context
8fd0782
@jasonrhodes
Fetches entries when modal is opened
b30ff3e
@jasonrhodes
Merge branch '58487-view-logs-in-context-modal' of github.com:afgomez…
8819f78 
…/kibana into 58487-view-logs-in-context-modal
Rename previously unused variable
25005b7
Expose loading state
1070433
WIP: render entries from the context
d0b7de8
Tweak dimensions of the modal
4432ee4
Fix tests
faae49b
@afgomez afgomez marked this pull request as ready for review April 16, 2020 13:16
@afgomez afgomez requested a review from a team as a code owner April 16, 2020 13:16
Alejandro Fernández Gómez added 2 commits April 16, 2020 15:19
Show the modal correctly on mobile
2bb5780 
The default props for `<EuiFlexGroup>` add a `flex-wrap` to the flex group that
makes things break on mobile.

To fix the layout on narrow screens and ensure the content doesn't wrap
we need to set `responsive={false}`, ironically.
Hide the actions menu for each line in the context view
6db6cab
@afgomez afgomez added Feature:Logs UI Logs UI feature release_note:enhancement Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.8.0 v8.0.0 labels Apr 16, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

@afgomez
Copy link
Contributor Author

afgomez commented Apr 16, 2020

@elasticmachine merge upstream

jasonrhodes
jasonrhodes requested changes Apr 21, 2020
View reviewed changes
Copy link
Member

@jasonrhodes jasonrhodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Based on conversation with @mukeshelastic, let's remove the "selected log line" at the top of the modal and this should be good to go.

@felixbarny
Copy link
Member

felixbarny commented Apr 22, 2020 •
edited
Loading

Lines within the same host.name and log.file.path

Question about log.file.path. That field comes from Filebeat, right? It is, however, not an official ECS field: https://www.elastic.co/guide/en/ecs/current/ecs-log.html. Do you know when and if Filebeat plans to move to file.path? Would it be feasible to consider both log.file.path and file.path for view context in line?

@roncohen
Copy link
Contributor

@felixbarny elastic/ecs#802

@jasonrhodes
Copy link
Member

@elasticmachine merge upstream

@afgomez
Copy link
Contributor Author

afgomez commented Apr 24, 2020

@elasticmachine merge upstream

@afgomez afgomez merged commit 53b33bb into elastic:master Apr 24, 2020
@afgomez afgomez deleted the 58487-view-logs-in-context-modal branch April 24, 2020 20:29
@afgomez afgomez mentioned this pull request Apr 24, 2020
Merged
@roncohen
Copy link
Contributor

👏

afgomez pushed a commit that referenced this pull request Apr 25, 2020
[7.x] [Logs UI] View log in context modal (#62198) (#64457)
0fbe1d1
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonrhodes jasonrhodes jasonrhodes approved these changes

Assignees
No one assigned
Labels
Feature:Logs UI Logs UI feature release_note:enhancement Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.8.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Logs UI] view log line in context
6 participants
@afgomez @elasticmachine @felixbarny @roncohen @jasonrhodes @kibanamachine