feat: add default multiline autodetection #215
Conversation
![orca-security-us[bot]](https://avatars.githubusercontent.com/in/276250?s=60&v=4){kind=small}
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
 Passed |
Infrastructure as Code |  0  0  0  0 |
View in Orca |
| Passed |
SAST | 0 0 0 0 |
View in Orca |
| Passed |
Secrets | 0 0 0 0 |
View in Orca |
| Passed |
Vulnerabilities | 0 0 0 0 |
View in Orca |
| - id: multiline-recombine | ||
| type: recombine | ||
| combine_field: body | ||
| is_first_entry: body matches "^(\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}|\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}(\\.\\d+)?Z?|[A-Za-z]+ \\d{1,2} \\d{2}:\\d{2}:\\d{2}|\\d{4}\\/\\d{2}\\/\\d{2} \\d{2}:\\d{2}:\\d{2})" |
There was a problem hiding this comment.
A few comments on the regex:
- can we document what this is and where it came from?
- is
\\/right for forward slash? I'm surprised it needs to be escaped at all. - is the space vs
Timportant? Can we combine those? - is
-vs/important for dates? can we combine them?- ex for my last two combination requests:
\d{4}[/-]\d{2}[/-]\d{2}[T ]\d{2}:\d{2}:\d{2}(\.\d+)?Z?and then it could be OR'ed with[A-Za-z]+ \d{1,2} \d{2}:\d{2}:\d{2}like you have it.
- ex for my last two combination requests:
There was a problem hiding this comment.
these are just based on the 4 timestamps that datadog supports https://docs.datadoghq.com/agent/logs/auto_multiline_detection/?tab=configurationfile#enable-multi-line-aggregation-per-integration and they're just OR'd together
There was a problem hiding this comment.
and the escaping is weird because the recombine is another layer of parsing, it was throwing errors without
There was a problem hiding this comment.
i've merged the regexs for slashes versus dashes, the other ones I think its clearer to leave them separate since they're less close
| @@ -23,6 +23,12 @@ receivers: | |||
| operators: | |||
| - type: filter | |||
| expr: 'body matches "otel-contrib"' | |||
| {{- if .HostMonitoring.Logs.AutoMultilineDetection }} | |||
| - id: multiline-recombine | |||
There was a problem hiding this comment.
Does this operator differ from the top level multiline config option?
https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/v0.124.0/receiver/filelogreceiver#example---multiline-logs-parsing
If they work the same, I think it's better to use the multiline option since it can be overridden by users more easily than a field in an array.
There was a problem hiding this comment.
so i think specifically the order of operations between the multiline top level config and then the operators pipelines means that we need to define this as another operator after the container operator for kubernetes specifically. I dont actually know if its necessary in this case but figured it might be best to do it the same way everywhere so that the behavior is most likely to be similar.
Description
OB-XXX Please explain the changes you made here.
Checklist