Kibana: [Logs UI] view log line in context

[katrin-freihofner]

We defined a way for the Logs UI to allow users to quickly see a log message in context.

Find more background information and related discussions in the design issues

40808, elastic/kibana#50304

1. Select

When selecting a logline, a (popup) menu should appear that lets users select the Logline in context action.

Additionally, the popup menu should show a link to view the details of a log message.

2. Context

The applied context is defined by host.name:[host.name] and log.file.path:[file.path] or [container.id].

The lookup hierarchy for these fields should be as follows:

1. Use container.id if that value exists
2. Use host.name + log.file.path if _both_ values exist
3. If neither of these two criteria are met, don't show the option in the menu The option will always be visible, but disabled if we don't have the data (elastic/logs/issues/13)

3. Highlight

The selected log line stays within the view and is highlighted as a reference.

---

Design work

Design issue
Figma prototype

---

Changes in the APIs

• [x] Add a context property to the logEntryRT type. (https://github.com/elastic/kibana/pull/60759)

export const logEntryRT = rt.type({
  id: rt.string,
  cursor: logEntriesCursorRT,
  columns: rt.array(logColumnRT),
  context: rt.partial({
    'container.id': rt.string,
    'host.name': rt.string,
    'log.file.path': rt.string,
  })
});

The API will return whatever fields are available. The UI will then make the decision of which fields to use for the context.

Changes in the UI

• [x] Adjust colors in the highlighted row lines to match the new design (https://github.com/elastic/kibana/pull/60773)
• [x] Replace "expand" icon with contextual menu (https://github.com/elastic/kibana/pull/60773)
• [ ] Create useViewLogInContext hook

interface Props {
  // If we go for a `-Infinity` to `Infinity` range this is not necessary
  startTimestamp: number;
  endTimestamp: number;
}

interface State {
  centerEntry: LogEntry | null;
  entries: LogEntry[];
  isVisible: boolean;
}

interface Callbacks {
  showContextModal: (entry: LogEntry) => void;
  hideContextModal: () => void;

  // If we don't do pagination this is not necessary
  fetchPreviousPage: () => void;
  fetchNextPage: () => void;
}

const useViewLogInContext(props: Props): [State, Callbacks] {
  // use `props.centerEntry.context` to fetch the entries around the given line, with the context that makes sense.
}

• [ ] Place the Provider for the hook
• [ ] Create the modal component
• [ ] Add an entry to the context menu to show/hide the modal component

[elasticmachine]

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

[jasonrhodes]

@roncohen what was your suggested hierarchy of fields for the context lookup?

A

1. host.name + log.file.path if both values exist
2. container.id if that value exists
3. don't show option

B

1. container.id if that value exists
2. host.name + log.file.path if both values exist
3. don't show option

C
something else?

[jasonrhodes]

@katrin-freihofner thank you for all the work on this!!! Now that we're aligned on a direction for the first iteration, what do you and @mukeshelastic think about softening some of the language in the context pop up? Instead of showing the host/path values as applied filters, maybe something like "Now viewing logs from [/var/log/a...b.log] on host [filebeat-2]", or "Now viewing logs from container [whatever-id]"?

[katrin-freihofner]

@katrin-freihofner thank you for all the work on this!!! Now that we're aligned on a direction for the first iteration, what do you and @mukeshelastic think about softening some of the language in the context pop up? Instead of showing the host/path values as applied filters, maybe something like "Now viewing logs from [/var/log/a...b.log] on host [filebeat-2]", or "Now viewing logs from container [whatever-id]"?

I like that - I will give it a try. @mukeshelastic what is your opinion?

[roncohen]

@jasonrhodes lets go for B as a start

btw. i filed this: https://github.com/elastic/ecs/issues/770

[jasonrhodes]

@roncohen thanks, I updated the ticket description with option B.

[katrin-freihofner]

@katrin-freihofner thank you for all the work on this!!! Now that we're aligned on a direction for the first iteration, what do you and @mukeshelastic think about softening some of the language in the context pop up? Instead of showing the host/path values as applied filters, maybe something like "Now viewing logs from [/var/log/a...b.log] on host [filebeat-2]", or "Now viewing logs from container [whatever-id]"?

Like this:

[roncohen]

personally, I prefer the previous design that shows the filter in square boxes as opposed to the more verbose ones here. @jasonrhodes is your concern that people will not understand the filter without the more text-full description?

[jasonrhodes]

@roncohen I actually think they look more like applied filters that can be modified and interacted with, rather than just being information about the fixed context that you're looking at. But I don't feel super strongly about it, I think they both work.

[mukeshelastic]

Sorry for the delay in responding to this thread. Katrin and I discussed and shared the same concern that Jason mentioned above. Showing it as filter implies that it is filter bar ( similar to stream filter bar exp) and that isn't our intention here.

[roncohen]

OK, i don't think we need to add text to convey that these are static filters. There are lots of places we have label/value style information shown without text. How about <key>: value, key: <value> on a plain white background?

[mukeshelastic]

As long as we can convey that these are non-editable filters, it should work. If we follow the pattern you suggested in other place to convey the non-edit-ability, then makes sense to follow it here as well.

[afgomez]

I'm looking into the work necessary for this task. I have two questions:

Since we have now merged the SuperDatePicker and everything is fenced by a date range, I wonder if we should apply the date range in this view, or if we should consider all the data we have for the context.

So, if the user selects a range of now-15m to now, should the View Log in Context UI show lines only in that range or should it not limit itself?

The second question is: should this view have infinite scroll? How much context is enough context? If we think this is a "nice to have" we can do it as a separate task to get the feature merged earlier.

[afgomez]

Here's a list of what I think needs to be done. Many of these changes can be done in separate PRs.

Changes in the APIs

• [ ] Add a context property to the logEntryRT type.

export const logEntryRT = rt.type({
  id: rt.string,
  cursor: logEntriesCursorRT,
  columns: rt.array(logColumnRT),
  context: rt.partial({
    'container.id': rt.string,
    'host.name': rt.string,
    'log.file.path': rt.string,
  })
});

The API will return whatever fields are available. The UI will then make the decision of which fields to use for the context.

Changes in the UI

• [ ] Adjust colors in the highlighted row lines to match the new design.
• [ ] Replace "expand" icon with contextual menu.
• [ ] Create useViewLogInContext hook

interface Props {
  centerEntry: LogEntry | null;
  // If we go for a `-Infinity` to `Infinity` range this is not necessary
  startTimestamp: number;
  endTimestamp: number;
}

interface State {
  centerEntry: LogEntry | null;
  entries: LogEntry[];
  isVisible: boolean;
}

interface Callbacks {
  showModal: () => void;
  hideModal: () => void;

  // If we don't do pagination this is not necessary
  fetchPreviousPage: () => void;
  fetchNextPage: () => void;
}

const useViewLogInContext(props: Props): [State, Callbacks] {
  // use `props.centerEntry.context` to fetch the entries around the given line, with the context that makes sense.
}

• [ ] Place the Provider for the hook
• [ ] Create the modal component
• [ ] Add an entry to the context menu to show/hide the modal component

[weltenwort]

It's great to see that plan up front. Looks sensible to me with only a few questions coming to mind:

Place the Provider for the hook

We might not even need a provider. Unless the prop drilling becomes too burdensome the hook might just live locally in the modal component.

Add a context property to the logEntryRT type.

Thinking out loud: I wonder if the context fields should be hard-coded in the API or whether the client should include a list of context fields in the request that it is prepared to handle. That would make it more extensible in the future, but also more complicated now. So hard-coding it might be appropriate for now. :thought_balloon:

Do you already have an idea about how to split it up into PRs?

[afgomez]

Do you already have an idea about how to split it up into PRs?

One for the API changes, one for the visible changes in the stream (color + menu, initially with only one item), and a bigger one for the hook + modal.

The first two can be done and reviewed in parallel. The last one builds on top of them.

[weltenwort]

Sounds reasonable :+1: Depending on the answers to your questions about the context size and scrolling above a static size might be good enough to merge initially with refinements being added in follow-ups.

A few more thoughts about the hook arguments, state and return values that you outlined:

• The centerEntry should probably either be an argument or state, but not both. Intuitively I would say it is externally tracked state (e.g. in the url using https://github.com/elastic/kibana/blob/f18c571ed64e0669bd7283e08273d4f95f0d47e7/x-pack/plugins/infra/public/utils/use_url_state.ts#L15). A small wrapper hook could then expose showContextModal(centerEntry: Cursor) and hideContextModal.
• Is an explicit isVisible flag needed? Couldn't centerEntry === null be used to hide the modal? That would make a state where the overlay is visible but no center is set unrepresentable.

[afgomez]

Yeah, good points. I would then keep it only in the state. The center point needs to be set from outside (from the menu). We could set that with the callback.

I'm going to start doing the backend bit :)

[roncohen]

I caught up with @mukeshelastic and we're aligned on the following:

Since we have now merged the SuperDatePicker and everything is fenced by a date range, I wonder if we should apply the date range in this view, or if we should consider all the data we have for the context.
So, if the user selects a range of now-15m to now, should the View Log in Context UI show lines only in that range or should it not limit itself?

Let’s disregard the time picker when clicking “View in context”. If the user has selected a very narrow window in order to find that specific error log, and then clicks “View in context” it’s not useful to limit it to the time window they have selected. We have a limit on the number of lines above and below and we should “just” query enough time range to fill it out.

The second question is: should this view have infinite scroll? How much context is enough context? If we think this is a "nice to have" we can do it as a separate task to get the feature merged earlier.

yes, infinite scroll can definitely wait.

in short, we should aim to show the user a specific number of lines before and after the selected line, disregarding the time window selected

[weltenwort]

Querying ES without a date range filter can seriously impact the cluster. That was one of the main reasons for introducing the date range picker. Previously I had implemented an exponential widening of the interval until the desired number of entries had been found to work around this. @afgomez that functionality has disappeared, right?

[mukeshelastic]

@weltenwort If I understand your concern correctly, If we hard code the number of log lines to be shown as total of 1000 and if the logs are sparsely distributed over a large time range for the applied filter of index, filepath and hostname then it will have significant performance impact on the cluster?

[weltenwort]

Yes, exactly. If we don't specify a date range Elasticsearch will have to query all shards. With a date range it will rewrite the query to ignore most shards for the recommended date- or size-based index life cycle setup.

When I initially wrote the context view feature (without a date filter) for discover I got feedback from large-scale users that the queries stressed their clusters beyond reason (see #15143 for one such report).

[mukeshelastic]

Thanks for the confirmation @weltenwort.

@afgomez My suggestion would be to apply date filter with value set from date picker.
Does this additional scope of adding date time filter put this feature at 7.8 release risk?

For the infinite scroll, let's implement it in another issue. In this issue, let's restrict the scope to:

1. apply date time filter
2. After applying filter, restrict the number of log lines below a fixed number. Say total lines as 5000 with 2500 above and below? I don't know what would make a good number so this is a pure assumption.

Thoughts? @roncohen @weltenwort @afgomez

[mukeshelastic]

Created a separate issue for infinite scrolling https://github.com/elastic/kibana/issues/61848

[afgomez]

My concern is that if the date range specified is too narrow, the "view log in context" is not going to show a lot of context and make the feature less useful.

But maybe users will never run into that problem 🤷‍♂️

To move forward, I will use the existing date range, and if we see it is a problem we can add a minimum date range before and after the line.