Fix otel log format issue #3123

[topikachu]

Covert KVList into a regular json string format.
This issue is fixed on branch 2.3.
Once merged, it can port to the main branch.

Or please tell me if the data-prepper project wants this fix on the main only.

Description

Fix the otel KVList

Issues Resolved

#3123

Check List

• New functionality includes testing.
• New functionality has been documented.
  • New functionality has javadoc added
• Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[dlvenable]

Thank you @topikachu for this contribution!

There is a comment above stating that the original authors did not find storing the data in objects to be OpenSearch friendly. As the current behavior is a string, this would be a breaking change.

We had a somewhat similar issue with metrics. Some situations call for flattening attributes and others not. The solution was to make this configurable in the otel_metrics processor. See #2163 for an example.

I think we could have a similar solution here. Perhaps make the otel_trace_source configurable. That is the code which performs the original parsing.

Thoughts?

@kmssap , Do you have anything to add or correct?

[topikachu]

There is a comment above stating that the original authors did not find storing the data in objects to be OpenSearch friendly. As the current behavior is a string, this would be a breaking change.

Hello @dlvenable

Thank you for taking the time to review my pull request and provide valuable feedback. I appreciate your insights and the opportunity to improve the project.

In response to your comment, I'd like to clarify the intent behind the change and how it addresses the issue.

Firstly, I'd like to emphasize that the changes I've introduced do not involve a redesign of the existing feature. Instead, the focus is solely on the output format related to handling nested objects.

The current state of the data-prepper produces a JSON string that encapsulates the inner data structures through recursive stringification. This can lead to a less readable and harder-to-parse format for complex nested data. The example I provided highlights this concern, where the data becomes obscured:
The origin otel data

  {
  	"outer1": {
  		"nested1": {
  			"nested2": "value"
  		}
  	}
  }

and the output is

'{"outer1":"{\\"nested1\\":\\"{\\\\\\"nested2\\\\\\":\\\\\\"value\\\\\\"}\\"}"}'

The proposed change aims to enhance this behavior by retaining the JSON string format while making it more accessible and coherent for complex nested data structures. By avoiding excessive stringification of inner levels, we can ensure that the resulting JSON remains both readable and parseable.

I've made sure that this adjustment does not negatively impact simple data formats like strings, integers, or booleans, and it maintains compatibility with non-nested key-value list formats, such as {key1: value1, key2: value2}.

Furthermore, the changes introduced here are meant to address the specific concern raised in issue #3123 and improve the readability of records collected from otel Kubernetes Objects Receiver.

Your guidance and expertise are invaluable to me as I work towards contributing effectively to this project. If you have any further suggestions or concerns, I'm eager to incorporate them into the pull request to ensure the best possible outcome for the project.

Thank you once again for your time and attention. I look forward to your continued feedback.

[topikachu]

@dlvenable and @kmssap
Do you have any comments?

[KarstenSchnitter]

The problem we originally tried to address with the escaping of KVLists was conflicts in OpenSearch. With nested fields it is very easy to create data structures, that lead to rejections in indexing. E.g. take these two objects:

[
    "outer": {
        "nested": {
            "inner": "some value"
        }
    },
    "outer": {
        "nested": "conflict"
    }
]

When data is created by different applications, these conflicts arise pretty fast. K8s attributes are quite notorious with regards to that. I believe, those conflicts are the reason, why DataPrepper chose to dedot attribute names by "@" in the generated JSON documents. Since OTel allows KVLists as attributes and Log Body, we decided to implement the escaping. I agree, that recursively applying the escaping is unnecessary and hinders readibility in OpenSearch. However, replacing the handling with nested objects will break on our deployments of DataPrepper and OpenSearch.

[KarstenSchnitter]

@dlvenable, if you want to reach @kmssap, try @kaimst.

[kaimst]

The change looks good to me, however I do share @dlvenable's opinion that the new behavior is potentially breaking existing setups and should be configurable.

[dlvenable]

@topikachu,

I agree with the improvement. But, other teams may be relying on the current behavior. I think it should be fairly straightforward to add a configuration. Would you be interested in adding that?

[dlvenable]

I think we'd need to make this configurable to merge it in.