Explore/Logs: Escaping of incorrectly escaped log lines #31352

ivanahuckova · 2021-02-19T13:16:09Z

Over the time, we've had a decent amount of reported issues related to escaping of newlines. Wast majority of those was due to the fact, that users had incorrectly escaped log lines. In that case, there is not much we can do at client side - at Grafana, as we don't know which characters are actual newlines and which have just the \n pattern.

@ethirolle came up with and documented a good idea to give users a chance to replace their incorrectly escaped newlines and tabs with simply replacing \t, \n, \r, with actual newline or tab.

❗️What is very important - we should communicate to users that the lines aren't correctly escaped and that the first step should be fixing escaping. However, for some logs it is very hard/almost impossible to correctly escape them. And for those cases, we would provide them with a feature that would replace incorrectly escapes newlines with actual newlines. Of course, it might always produce incorrect results, as we don't know which characters are actual newlines and which have just the \n pattern, but I think that if we communicate it clearly, users will understand.

In this PR, I am showing the Escaping button only when we detect \t, \n, \r in any of the log lines. It has also experimental feature tooltip. We are also escaping and re-rendering only those lines, that are incorrectly escaped.

escaping.mov

When no incorrectly escaped lines - no button:

Which issue(s) this PR fixes:
Fixes #27964

Special notes for your reviewer:

This is the POC/my hack day project.

public/app/features/explore/Logs.tsx

aocenas · 2021-02-23T13:12:47Z

I like this, seems like a useful thing and I like that it's visible only with needed with good copy explaining why it's there.

@davkal @jessover9000 what do you think?

…ectly-escaped-logs

Elfo404

I played with it a bit and seems to work like a charm! 🚀 🔥

I only have some doubts regarding some phrasing/naming for which I added a few comments.

I also couldn't see any major perf issue, did you run some tests as well?

EDIT: Also, I'm not sure if the button can be instead a switch placed beside all the other options, it would make things a bit more consistent but it's starting to get crowded in there, so I'd wait for more UI/UX feedback.

Elfo404 · 2021-03-03T13:37:20Z

packages/grafana-ui/src/components/Logs/LogRow.tsx

@@ -42,14 +43,15 @@ interface Props extends Themeable {
  timeZone: TimeZone;
  allowDetails?: boolean;
  logsSortOrder?: LogsSortOrder | null;
+  escapedNewlines?: boolean;


what about forceEscape instead?
or something else in an "active" form, like showEscaped, would it make it clearer?

Elfo404 · 2021-03-03T13:38:20Z

public/app/features/explore/Logs.tsx

+          <MetaInfoText
+            metaItems={[
+              {
+                label: 'Your logs might have incorrectly escaped newlines',


maybe, as it's also about tabs:

Suggested change

label: 'Your logs might have incorrectly escaped newlines',

label: 'Your logs might have incorrectly escaped content',

Elfo404 · 2021-03-03T13:39:15Z

packages/grafana-data/src/types/logs.ts

@@ -62,6 +62,7 @@ export interface LogRowModel {
  // Actual log line
  entry: string;
  hasAnsi: boolean;
+  hasUnescapedNewlines: boolean;


what about hasUnescapedContent (we are also applying this to \t )

Found some issues

Elfo404 · 2021-03-03T14:14:01Z

I found an issue when using this log line: "Lorem \u001B[31mipsum \\n et\u001B[0m dolor":

Screen.Recording.2021-03-03.at.14.04.01.mov

which also happens if the unescaped \n is outside of the ANSI codes:

but works fine when there's no ANSI code in the string:

Screen.Recording.2021-03-03.at.14.07.15.mov

Also, which is a minor thing probably, as It's a very super-edge case, if I change the time range to only show a subset of logs that don't need escaping, the button will still be there. the only way to make it go away is to refresh the page. I don't know if this was intended, but it might be confusing:

Screen.Recording.2021-03-03.at.14.10.52.mov

* POC: Escaping of incorrectly escaped log lines * Remove unused import * Fix test, change copy * Make escapedNewlines optional * Fix typechecks * Remove loading state from the escaping button * Update namings

Rewrite the tooltip for the smart feature in PR grafana#31352 that replaces incorrectly escaped newlines in log lines. - Clarify the functionality of the feature to emphasize its interactivity. - Remove language suggesting that the feature is experimental, when we intended to suggest reviewing the results manually for correctness.

Describe and provide steps for using the "Escape newlines" feature in Explore added in PR grafana#31352.

…es feature (#46709) * Explore/Logs: Clarify phrasing of newline escape fix tooltip Rewrite the tooltip for the smart feature in PR #31352 that replaces incorrectly escaped newlines in log lines. - Clarify the functionality of the feature to emphasize its interactivity. - Remove language suggesting that the feature is experimental, when we intended to suggest reviewing the results manually for correctness. * Docs: Document escape newlines feature Describe and provide steps for using the "Escape newlines" feature in Explore added in PR #31352. * Rewrite topic lead to clarify conditional behavior * Describe reversion of replacements as a standalone task Reverting the replacements is a separate action, so it should have its own task. This also describes the option's transformation and tightens the task language. * Remove Grafana 7 version qualifier * Clarify escape sequence detection lede on task Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Clarify "Remove escaping" state change Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Clarify confidence of tooltip content Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>

…es feature (#46709) * Explore/Logs: Clarify phrasing of newline escape fix tooltip Rewrite the tooltip for the smart feature in PR #31352 that replaces incorrectly escaped newlines in log lines. - Clarify the functionality of the feature to emphasize its interactivity. - Remove language suggesting that the feature is experimental, when we intended to suggest reviewing the results manually for correctness. * Docs: Document escape newlines feature Describe and provide steps for using the "Escape newlines" feature in Explore added in PR #31352. * Rewrite topic lead to clarify conditional behavior * Describe reversion of replacements as a standalone task Reverting the replacements is a separate action, so it should have its own task. This also describes the option's transformation and tightens the task language. * Remove Grafana 7 version qualifier * Clarify escape sequence detection lede on task Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Clarify "Remove escaping" state change Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Clarify confidence of tooltip content Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> (cherry picked from commit 42431e6)

…es feature (#46709) (#47606) * Explore/Logs: Clarify phrasing of newline escape fix tooltip Rewrite the tooltip for the smart feature in PR #31352 that replaces incorrectly escaped newlines in log lines. - Clarify the functionality of the feature to emphasize its interactivity. - Remove language suggesting that the feature is experimental, when we intended to suggest reviewing the results manually for correctness. * Docs: Document escape newlines feature Describe and provide steps for using the "Escape newlines" feature in Explore added in PR #31352. * Rewrite topic lead to clarify conditional behavior * Describe reversion of replacements as a standalone task Reverting the replacements is a separate action, so it should have its own task. This also describes the option's transformation and tightens the task language. * Remove Grafana 7 version qualifier * Clarify escape sequence detection lede on task Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Clarify "Remove escaping" state change Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Clarify confidence of tooltip content Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> (cherry picked from commit 42431e6) Co-authored-by: Garrett Guillotte <100453168+gguillotte-grafana@users.noreply.github.com>

…es feature (#46709) * Explore/Logs: Clarify phrasing of newline escape fix tooltip Rewrite the tooltip for the smart feature in PR #31352 that replaces incorrectly escaped newlines in log lines. - Clarify the functionality of the feature to emphasize its interactivity. - Remove language suggesting that the feature is experimental, when we intended to suggest reviewing the results manually for correctness. * Docs: Document escape newlines feature Describe and provide steps for using the "Escape newlines" feature in Explore added in PR #31352. * Rewrite topic lead to clarify conditional behavior * Describe reversion of replacements as a standalone task Reverting the replacements is a separate action, so it should have its own task. This also describes the option's transformation and tightens the task language. * Remove Grafana 7 version qualifier * Clarify escape sequence detection lede on task Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Clarify "Remove escaping" state change Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Clarify confidence of tooltip content Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com>

ivanahuckova added 2 commits February 19, 2021 13:45

POC: Escaping of incorrectly escaped log lines

2bf79ea

Remove unused import

af2f4b0

ivanahuckova self-assigned this Feb 19, 2021

ivanahuckova added area/panel/logs type/feature-request labels Feb 19, 2021

ivanahuckova added this to Under review in Observability (deprecated, use Observability Squad) via automation Feb 19, 2021

ivanahuckova added 3 commits February 19, 2021 14:46

Fix test, change copy

09a5cd5

Make escapedNewlines optional

9d6c606

Fix typechecks

47b647b

ivanahuckova requested review from zoltanbedi, aocenas, davkal, Elfo404 and gabor February 22, 2021 13:51

aocenas reviewed Feb 23, 2021

View reviewed changes

public/app/features/explore/Logs.tsx Outdated Show resolved Hide resolved

ivanahuckova changed the title ~~Explore: POC - Escaping of incorrectly escaped log lines~~ Explore/Logs: Escaping of incorrectly escaped log lines Feb 26, 2021

ivanahuckova added 2 commits February 26, 2021 16:07

Merge remote-tracking branch 'origin/master' into ivana/escape-incorr…

1afeb9b

…ectly-escaped-logs

Remove loading state from the escaping button

4dc0025

ivanahuckova marked this pull request as ready for review February 26, 2021 15:19

ivanahuckova requested a review from a team February 26, 2021 15:19

ivanahuckova requested a review from a team as a code owner February 26, 2021 15:19

ivanahuckova requested review from torkelo and dprokop and removed request for a team, torkelo and dprokop February 26, 2021 15:19

Elfo404 previously approved these changes Mar 3, 2021

View reviewed changes

Elfo404 approved these changes Mar 3, 2021

View reviewed changes

Update namings

a9854d0

ivanahuckova merged commit 4c2e5fc into master Mar 3, 2021

Observability (deprecated, use Observability Squad) automation moved this from Under review to Done Mar 3, 2021

ivanahuckova deleted the ivana/escape-incorrectly-escaped-logs branch March 3, 2021 17:32

ivanahuckova added this to the 7.5.0-beta1 milestone Mar 3, 2021

ivanahuckova mentioned this pull request Mar 5, 2021

Explore/Logs: Fix escaping in ANSI logs #31731

Merged

addisonElliott mentioned this pull request Apr 26, 2021

Newlines are escaped improperly in fluentd plugin grafana/loki#3656

Closed

ivanahuckova mentioned this pull request Feb 23, 2022

Docs: Improve wording for tooltip in "Add escaping feature" in Explore #45770

Closed

This was referenced Feb 23, 2022

Unescape JSON logs in Grafana for better UX #45502

Closed

valueString webhook body is empty #45459

Closed

gguillotte-grafana added a commit to gguillotte-grafana/grafana that referenced this pull request Mar 17, 2022

Docs: Document escape newlines feature

e6f0b44

Describe and provide steps for using the "Escape newlines" feature in Explore added in PR grafana#31352.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explore/Logs: Escaping of incorrectly escaped log lines #31352

Explore/Logs: Escaping of incorrectly escaped log lines #31352

ivanahuckova commented Feb 19, 2021 •

edited

aocenas commented Feb 23, 2021

Elfo404 left a comment •

edited

Elfo404 Mar 3, 2021

Elfo404 Mar 3, 2021

Elfo404 Mar 3, 2021

Elfo404 commented Mar 3, 2021

	label: 'Your logs might have incorrectly escaped newlines',
	label: 'Your logs might have incorrectly escaped content',

Explore/Logs: Escaping of incorrectly escaped log lines #31352

Explore/Logs: Escaping of incorrectly escaped log lines #31352

Conversation

ivanahuckova commented Feb 19, 2021 • edited

aocenas commented Feb 23, 2021

Elfo404 left a comment • edited

Choose a reason for hiding this comment

Elfo404 Mar 3, 2021

Choose a reason for hiding this comment

Elfo404 Mar 3, 2021

Choose a reason for hiding this comment

Elfo404 Mar 3, 2021

Choose a reason for hiding this comment

Elfo404 commented Mar 3, 2021

ivanahuckova commented Feb 19, 2021 •

edited

Elfo404 left a comment •

edited