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.

Sign up for GitHub

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

Set log.offset to the start of the reported line in filestream #30445

Merged
merged 2 commits into from
Feb 17, 2022
Merged

Set log.offset to the start of the reported line in filestream #30445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c1d32be
Set log.offset to the offset of the line start
kvch Feb 17, 2022
6df2650
add changelog entry
kvch Feb 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...main[Check the HEAD dif
- Fix using log_group_name_prefix in aws-cloudwatch input. {pull}29695[29695]
- Fix multiple instances of the same module configured within `filebeat.modules` in filebeat.yml. {issue}29649[29649] {pull}29952[29952]
- aws-s3: fix race condition in states used by s3-poller. {issue}30123[30123] {pull}30131[30131]
- Report the starting offset of the line in `log.offset` when using `filestream` instead of the end to be ECS compliant. {pull}30445[30445]

*Filebeat*
- Fix broken Kafka input {issue}29746[29746] {pull}30277[30277]
Expand Down
6 changes: 4 additions & 2 deletions libbeat/reader/readfile/metafields.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,9 @@ func NewFilemeta(r reader.Reader, path string, offset int64) reader.Reader {
func (r *FileMetaReader) Next() (reader.Message, error) {
message, err := r.reader.Next()

r.offset += int64(message.Bytes)
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment in here noting that log.offset should be the start of the reported line, and not the end? Let's document what the behaviour needs to be and why in the code itself.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like I was too late :) The commit message will explain it at least.


// if the message is empty, there is no need to enrich it with file metadata
if message.IsEmpty() {
r.offset += int64(message.Bytes)
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand the difference in this change. There are only 2 returns in this function and having r.offset += int64(message.Bytes) before this if covered both and now they are just copied before each return. Why?

Copy link
Member

Choose a reason for hiding this comment

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

ah, I think I get it now, that's because the value is used in DeepUpdate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly.

return message, err
}

Expand All @@ -56,6 +55,9 @@ func (r *FileMetaReader) Next() (reader.Message, error) {
},
},
})

r.offset += int64(message.Bytes)

return message, err
}

Expand Down
3 changes: 2 additions & 1 deletion libbeat/reader/readfile/metafields_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ func TestMetaFieldsOffset(t *testing.T) {
if err == io.EOF {
break
}
offset += int64(msg.Bytes)

expectedFields := common.MapStr{}
if len(msg.Content) != 0 {
Expand All @@ -67,6 +66,8 @@ func TestMetaFieldsOffset(t *testing.T) {
},
}
}
offset += int64(msg.Bytes)

require.Equal(t, expectedFields, msg.Fields)
require.Equal(t, offset, in.offset)
}
Expand Down