search

grafana / explore-logs

Repo for the Loki log exploration app
GNU Affero General Public License v3.0
310 stars 14 forks source link

Logs panel: add button to copy link to log line #855

Closed matyax closed 4 weeks ago

matyax commented 1 month ago

Requires https://github.com/grafana/grafana/pull/95330

This PR adds support for linking to log lines from the logs viz.

Closes #772

matyax commented 4 weeks ago

Testing coverage is pending, but I'd appreciate if you give it a try in the mean time.

gtk-grafana commented 4 weeks ago

Looks pretty good, a couple of nits:

  1. When copying a link in the logs panel there's no feedback that the link was copied, in classic Explore we show a toast on copy, in Explore Logs Table we show a tooltip image image

I prefer the tooltip, but if it's not possible with #95330 the toast will do for now, but we should create an issue to sync up the UIs if this is the case.

  1. Interoperability with the table link: When switching back and forth between panels the links do not behave as expected, each panel has an independent line link. Ideally if you have a link to a log line in one viz, and a user switches to the other viz, that line should still be highlighted in the other viz. However this can be addressed in a new issue/PR and should not block release of this feature (feel free to assign to me).

Otherwise LGTM, and thanks for cleaning things up!

matyax commented 4 weeks ago

Thanks for the feedback! For point 2, I was thinking that we should update the table to use the panelState param that the viz, Dashboards, and Explore use. https://github.com/grafana/grafana/blob/main/public/app/plugins/panel/logs/LogsPanel.tsx#L448-L450

Created https://github.com/grafana/explore-logs/issues/857

matyax commented 4 weeks ago

Addressed point 1:

imagen

There's no success variant for IconButton but it does the trick.

gtk-grafana commented 4 weeks ago

Oh, except I didn't check that this works with older Grafana versions, will this fallback gracefully? I guess it should as logRowMenuIconsAfter option won't do anything

xenartro commented 4 weeks ago

Yeah, it would just not render. On the bright side, whenever permalink support was added to LogsPanel, any URL that contains the parameter should at least scroll to the linked log line for free.