Skip to content

Commit 438b20e

Browse files
authored
[extension/text_encoding] Avoid spurious marshalling separators at end of lines (#45274)
As a companion to #45272, this partially fixes #42797 by avoiding spurious newlines between blocks of log records. This change modifies existing unit tests to demonstrate the new behavior. Since tests already existed for this behavior, I recognize that the existing behavior may already be working as intended. Even so, I offer this in the spirit of fully addressing the concerns raise in #42797.
1 parent 7367f5d commit 438b20e

File tree

3 files changed

+38
-3
lines changed

3 files changed

+38
-3
lines changed
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog)
7+
component: extension/text_encoding
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Avoid spurious marshalling separators at end of lines
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [42797]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: |
19+
Previously, text_encoding would append the marshalling separator to the end of
20+
each log record, potentially resulting in double-newlines between blocks of
21+
records.
22+
23+
# If your change doesn't affect end users or the exported elements of any package,
24+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
25+
# Optional: The change log or logs in which this entry should be included.
26+
# e.g. '[user]' or '[user, api]'
27+
# Include 'user' if the change is relevant to end users.
28+
# Include 'api' if there is a change to a library API.
29+
# Default: '[user]'
30+
change_logs: [user]

extension/encoding/textencodingextension/text.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,19 @@ func (r *textLogCodec) UnmarshalLogs(buf []byte) (plog.Logs, error) {
6363

6464
func (r *textLogCodec) MarshalLogs(ld plog.Logs) ([]byte, error) {
6565
var b []byte
66+
appendedLogRecord := false
67+
6668
for i := 0; i < ld.ResourceLogs().Len(); i++ {
6769
rl := ld.ResourceLogs().At(i)
6870
for j := 0; j < rl.ScopeLogs().Len(); j++ {
6971
sl := rl.ScopeLogs().At(j)
7072
for k := 0; k < sl.LogRecords().Len(); k++ {
7173
lr := sl.LogRecords().At(k)
74+
if appendedLogRecord {
75+
b = append(b, []byte(r.marshalingSeparator)...)
76+
}
7277
b = append(b, []byte(lr.Body().AsString())...)
73-
b = append(b, []byte(r.marshalingSeparator)...)
78+
appendedLogRecord = true
7479
}
7580
}
7681
}

extension/encoding/textencodingextension/text_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func TestTextRoundtrip(t *testing.T) {
2424
assert.Equal(t, 2, ld.LogRecordCount())
2525
b, err := codec.MarshalLogs(ld)
2626
require.NoError(t, err)
27-
require.Equal(t, "foo\nbar\n", string(b))
27+
require.Equal(t, "foo\nbar", string(b))
2828
}
2929

3030
func TestTextRoundtripMissingNewline(t *testing.T) {
@@ -38,7 +38,7 @@ func TestTextRoundtripMissingNewline(t *testing.T) {
3838
assert.Equal(t, 2, ld.LogRecordCount())
3939
b, err := codec.MarshalLogs(ld)
4040
require.NoError(t, err)
41-
require.Equal(t, "foo\nbar\n", string(b))
41+
require.Equal(t, "foo\nbar", string(b))
4242
}
4343

4444
func TestNoSeparator(t *testing.T) {

0 commit comments

Comments
 (0)