Fix fluent-bit newline and tab characters #2286
Conversation
cmd/fluent-bit/loki.go
Outdated
| func replaceNewLineAndTab(line string) string{ | ||
| replacer := strings.NewReplacer(`\n`, "\n", `\t`, "\t") | ||
| return replacer.Replace(line) | ||
| } | ||
|
|
There was a problem hiding this comment.
This is a hacky solution. Somewhere the encoding by Json and logfmt is treating \n and \t as escaped. So, we have to replace those with actual \n and \t.
Any better ways?
There was a problem hiding this comment.
well I guess the replacer can be global for sure.
There was a problem hiding this comment.
And so you probably don't need this function.
There was a problem hiding this comment.
Yeah looks like a fine solution to me. Basically unescaping those characters.
There was a problem hiding this comment.
Have you tried to see what fluentd send for those ? I'm curious to see if they do the same.
There was a problem hiding this comment.
Fluent-bit records are fine. I just checked the content before Marshaling the record here
Line 203 in 7399bef
\n and \t looks fine here.
But after the marshaling, result from jsoniter.ConfigCompatibleWithStandardLibrary.Marshal(records) doesn't respect \n and \t. Looks like Marshaling is escaping these characters.
Same applies to logfmt encoding as well
Line 217 in 7399bef
There was a problem hiding this comment.
Which is correct everything is enclosed by "" and \ must be escaped. May be this is more a Grafana issue ? when reading \\n it should read\n ? cc @aocenas WDYT ?
There was a problem hiding this comment.
Yeah, this can be handled from Grafana side as well. However, line would be this Hello\nworld and not Hello\\nworld. So, on Grafana they can replace \n with actual newline.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2286 +/- ##
==========================================
- Coverage 62.45% 62.45% -0.01%
==========================================
Files 158 158
Lines 12762 12761 -1
==========================================
- Hits 7971 7970 -1
Misses 4177 4177
Partials 614 614
|
f483135 to
e1f9c6e
Compare
e1f9c6e to
63d0aaf
Compare
cmd/fluent-bit/loki.go
Outdated
| @@ -115,12 +118,13 @@ func autoLabels(records map[string]interface{}, kuberneteslbs model.LabelSet) er | |||
| return errors.New("kubernetes labels not found, no labels will be added") | |||
| } | |||
|
|
|||
| replacer := strings.NewReplacer("/", "_", ".", "_", "-", "_") | |||
| keyReplacer := strings.NewReplacer("/", "_", ".", "_", "-", "_") | |||
There was a problem hiding this comment.
that could be global too while we are at it.
There was a problem hiding this comment.
Yeah initially made that global but we will get this error https://cloud.drone.io/grafana/loki/3437/1/2.
If the line contains . or / that would be replaced as well. So, just kept it local
There was a problem hiding this comment.
not sure what you mean moving this to a new global variable, should be fine ?
There was a problem hiding this comment.
For example if the log message has 12.2 foo bar as value, doing replace with this global replacer
strings.NewReplacer("/", "_", ".", "_", "-", "_", `\n`,, "\n", `\t`, "\t")
will make the log message as 12_2 foo bar which is not desired.
There was a problem hiding this comment.
that is not what I was really asking to use a single replace, but two globals, one for key one for line.
There was a problem hiding this comment.
you mean we can have 2 separate global variables?
63d0aaf to
ced3439
Compare
|
Hi guys, we're running into the same issue. Can you please help to understand at what version this fix included? We use grafana 8.3.3, loki 2.3.0 and fluent bit 1.8.11 |
What this PR does / why we need it:
Fixes fluent-bit newline and tab characters
Which issue(s) this PR fixes:
Fixes #2246