Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pkg/stanza] Always recombine if possible, even if incomplete #30797

Merged

Conversation

djaglowski
Copy link
Member

Previously, certain circumstances could result in partial logs being emitted without any recombiniation. This could occur when using is_first_entry, if the first partial log from a source was emitted before a matching "start of log" indicator was found. This could also occur when the collector was shutting down.

@djaglowski djaglowski force-pushed the pkg-stanza-recombine-clean-state branch from 0dcbf76 to a54b297 Compare January 26, 2024 18:45
@djaglowski djaglowski force-pushed the pkg-stanza-recombine-clean-state branch from a54b297 to 0612ff0 Compare January 26, 2024 19:15
@djaglowski djaglowski marked this pull request as ready for review January 26, 2024 20:27
@djaglowski djaglowski requested review from a team and Aneurysm9 January 26, 2024 20:27
Comment on lines -185 to -197
entryWithBodyAttr(t1, "file1", map[string]string{"file.path": "file1"}),
entryWithBodyAttr(t1, "file3", map[string]string{"file.path": "file1"}),
entryWithBodyAttr(t1, "file1", map[string]string{"file.path": "file1"}),
entryWithBodyAttr(t2, "file2", map[string]string{"file.path": "file1"}),
entryWithBodyAttr(t1, "file1", map[string]string{"file.path": "file1"}),
entryWithBodyAttr(t2, "file2", map[string]string{"file.path": "file2"}),
entryWithBodyAttr(t2, "file3", map[string]string{"file.path": "file2"}),
entryWithBodyAttr(t1, "start", map[string]string{"file.path": "file1"}),
entryWithBodyAttr(t1, "more1a", map[string]string{"file.path": "file1"}),
entryWithBodyAttr(t1, "start", map[string]string{"file.path": "file1"}),
entryWithBodyAttr(t2, "more1b", map[string]string{"file.path": "file1"}),
entryWithBodyAttr(t2, "start", map[string]string{"file.path": "file1"}),
entryWithBodyAttr(t2, "more2a", map[string]string{"file.path": "file2"}),
entryWithBodyAttr(t2, "more2b", map[string]string{"file.path": "file2"}),
},
[]*entry.Entry{
entryWithBodyAttr(t1, "file1\nfile3", map[string]string{"file.path": "file1"}),
entryWithBodyAttr(t2, "file1\nfile2", map[string]string{"file.path": "file1"}),
entryWithBodyAttr(t2, "file2", map[string]string{"file.path": "file2"}),
entryWithBodyAttr(t2, "file3", map[string]string{"file.path": "file2"}),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of these changes are just to clarify the test. (Use meaningful content instead of "file1", etc). The primary change is that the last two tokens are emitted as one after the timeout, rather than each being emitted immediately. A complementary change below adds ExpectEntries because the final token from file1 and the other tokens are not emitted in a deterministic order.

@djaglowski
Copy link
Member Author

@open-telemetry/collector-contrib-approvers, this PR is just waiting for a review if anyone has a moment.

@djaglowski djaglowski merged commit 3f48044 into open-telemetry:main Jan 31, 2024
87 checks passed
@djaglowski djaglowski deleted the pkg-stanza-recombine-clean-state branch January 31, 2024 20:50
@github-actions github-actions bot added this to the next release milestone Jan 31, 2024
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Feb 1, 2024
…elemetry#30797)

Previously, certain circumstances could result in partial logs being
emitted without any recombiniation. This could occur when using
`is_first_entry`, if the first partial log from a source was emitted
before a matching "start of log" indicator was found. This could also
occur when the collector was shutting down.
@sumo-drosiek
Copy link
Member

@djaglowski Previous behavior was intentional: open-telemetry/opentelemetry-log-collection#415

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants