Not expand Json Literal Strings in Grok processor

[SoniaSanzV]

Closes #175753

Summary

When a new pipeline was created and a Grok processor was selected, the pattern definitions were being parsed. This was only happening in the visualization and not in the data being saved. The reason was that it was using XJsonEditor as a component, which in turn was using useXJsonMode. This hooks contains a method that parses JSON-like strings which caused some characters not to be rendered because they were considered to be escaped. Something that this is not wanted in this specific editor.

So for this case I've created a component that uses the same editor option as XJsonEditor but uses the value without parsing so the processor pattern does not change.

Now, if we introduce this pattern, update the processor and edit again, the pattern displayed will look the same

{
    "ISSUE": "aaa\"bbb",
    "ISSUE2": "aaa\\(bbb"
}

Grok.mov

[elasticmachine]

Pinging @elastic/kibana-management (Team:Kibana Management)

[ElenaStoeva]

Thanks for working on this @SoniaSanzV! I tested locally and it works as expected now. I have a suggestion about reusing the XJsonEditor component instead of creating a new one.

[ElenaStoeva]

I'm wondering whether we really need to introduce this new component given that it reuses most of the logic in the XJsonEditor component - also, technically it also uses the xjson language so the new naming might be confusing. Wouldn't it be better to add some prop to the XJsonEditor component that tweaks the logic and determines whether we want to expand the strings or not?

[SoniaSanzV]

Yes, it is also a good idea. And you are so right about the name, so I will implement what you say.

[ElenaStoeva]

Thanks for refactoring the changes @SoniaSanzV! I tested again and realised that the current behavior in this PR may not be entirely correct. With the current logic, we don't allow triple-quote strings, but they are allowed by Es. For example, the following request works in Console:

PUT _ingest/pipeline/test-pipeline
{
  "processors": [
    {
      "grok": {
        "field": "test",
        "patterns": [
          "^%{ISSUE}$"
        ],
        "pattern_definitions": {
          "ISSUE": """aaa\"bbb""",
          "ISSUE2": "aaa\\(bbb"
        }
      }
    }
  ]
}

But if you type in this pattern definition in the UI, it says it's invalid:

Also, I found something else - Es seems to automatically expand the strings if they contain quotes inside. For example, if you run GET _ingest/pipeline/test-pipeline, the response is:

{
  "test-pipeline": {
    "processors": [
      {
        "grok": {
          "field": "test",
          "patterns": [
            "^%{ISSUE}$"
          ],
          "pattern_definitions": {
            "ISSUE": """aaa\"bbb""",
            "ISSUE2": """aaa\(bbb"""
          }
        }
      }
    ]
  }
}

(the second pattern definition now uses triple-quote even if we set it to use a single-quote).

I think we should still allow triple-quotes in the UI but make sure the value that is in the editor is not expanded/collapsed unnecessarily and is shown in the way the user typed it in/or Es returned it.

[SoniaSanzV]

I've been working around with that and I'm quite lost with which is the correct solution. I'm started to think that the original approach is the right one, since we are displaying the same pattern format that ES returns.
What I don't think it makes sense is to mix escaped values with unescaped values. I mean, if the user introduces the following pattern:

{
    "ISSUE": "aaa\"bbb",
    "ISSUE2": "aaa\\(bbb",
    "ISSUE3": """aaa\"bbb"""
} 

we can treat the JSON as escaped or unescaped, but not both. Actually, once the pipeline is created, the user click update processor and we fetch the data from ES, we don't have a way to know if the user original input was "ISSUE2": "aaa\\(bbb", or "ISSUE2": """aaa\(bbb"""
Whether we escape the quotation marks or not, I believe that what we show has to be consistent and cohesive.

[SoniaSanzV]

I have uploaded a new proposal (we can discuss it). In the end I have gone with the following: the user can enter any value they want as long as, once parsed, it is a valid JSON. Note that in the video, when “ISSUE3”: “”“aaa ‘bbb’”" is entered, the color of the editor is not red since the quotes have not yet been escaped and therefore it is not a proper JSON.
Once the processor is saved, it is always displayed with escaped values.

GROK.mov

The invalid JSON validator keeps working as expected

[elasticmachine]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: 564f8da

Failed CI Steps

• FTR Configs #77
• FTR Configs #88

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
ingestPipelines	405.2KB	405.3KB	+66.0B

History

• 💛 Build #246001 was flaky 22dbf1f69f5e256919e724f7bad82c3f1866ef58
• 💚 Build #245621 succeeded 501df3466fdae1ae05c474572d87a87e5b75610a

cc @SoniaSanzV

[ElenaStoeva]

Thanks for addressing my feedback @SoniaSanzV. I tested again and I noticed that now we have the opposite problem - triple quotes get transformed into single quotes, which is also something that users might complain about.

Before these changes:

Screen.Recording.2024-11-07.at.19.06.43.mov

With these Changes:

Screen.Recording.2024-11-07.at.19.04.53.mov

Overall, I don't really think that the initial behavior was really a bug since Kibana was simply transforming the strings into another format. Also, if you send the same request in Console, you will see that even Elasticsearch transforms single quotes with escape chars into triple quotes:

I think we should either do no transformation at all (i.e. the final request should be exactly as the user types it in) or we should leave it and let Kibana do the transformation as Elasticsearch does it.

[SoniaSanzV]

Yep, that's what I mentioned in my last comments. I don't think that the initial implementation was a bug either. My last implementation was just another option to have. But, as I said, maybe we can have an input that mixes escaped and unescaped jsons before the user saves the processor; but there is no way to know with the ES response what format they were in at the beginning, so we will always have to go with one of the two formats. And with that in mind, I think the one that matches the dev tools is better.