Set `log.offset` to the start of the reported line in filestream by kvch · Pull Request #30445 · elastic/beats

kvch · 2022-02-17T10:20:16Z

What does this PR do?

This PR fixes the offset reported in log.offset field. Previously, it reported the offset of the end of the line. However, the field documentation says that it should be the start offset of the line. Ref: https://www.elastic.co/guide/en/beats/filebeat/current/exported-fields-log.html

Why is it important?

Filebeat should comply with ECS.

Checklist

My code follows the style guidelines of this project
~~- [ ] I have commented my code, particularly in hard-to-understand areas~~
~~- [ ] I have made corresponding changes to the documentation~~
~~- [ ] I have made corresponding change to the default configuration files~~
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

elasticmachine · 2022-02-17T10:20:25Z

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

mergify · 2022-02-17T10:20:25Z

This pull request does not have a backport label. Could you fix it @kvch? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

elasticmachine · 2022-02-17T10:30:54Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-02-17T10:23:25.705+0000
Duration: 118 min 33 sec

Test stats 🧪

Test	Results
Failed	0
Passed	38335
Skipped	3297
Total	41632

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

rdner · 2022-02-17T13:11:58Z

libbeat/reader/readfile/metafields.go

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


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?

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

) (cherry picked from commit 8aca673)

cmacknz · 2022-02-17T13:52:20Z

libbeat/reader/readfile/metafields.go

 func (r *FileMetaReader) Next() (reader.Message, error) {
 	message, err := r.reader.Next()

-	r.offset += int64(message.Bytes)


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.

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

…nd-k8s-env * upstream/main: fix typos and improve sentences (elastic#30432) Add drop and explicit tests to avoid duplicate ingest of elasticsearch logs (elastic#30440) {,x-pack/}auditbeat: replace uses of github.com/pkg/errors with stdlib equivalents (elastic#30321) Spelling fix (elastic#30439) packetbeat/beater: make sure Npcap installation runs before interfaces are needed in all cases (elastic#30438) Add BC about Homebrew no longer being available in 8.0 (elastic#30419) Install gawk as a replacement for mawk in Docker containers. (elastic#30452) Clean up python-related system tests (elastic#30415) Fix TestNewModuleRegistry flakiness (elastic#30453) [Filebeat] [auditd]: Support EXECVE events with truncated argument list (elastic#30382) Set `log.offset` to the start of the reported line in filestream (elastic#30445) clarify SelectedPackageTypes meaning and improve its usage (elastic#30142) [elasticsearch module] serialize shards properties (elastic#30408) Add docs about hints and templates autodiscovery priority (elastic#30343)

…ckaging-docker * upstream/main: (26 commits) Update docker/distribution to 2.8.0 (elastic#30462) Add `parsers` examples to `filestream` reference configuration (elastic#30529) extend documentation about setting orchestrator.cluster fields (elastic#30518) Forward-port 8.0.1 changelog to main (elastic#30522) Switch skip to use `CI` (elastic#30512) packetbeat/beater: don't attempt to install npcap when already installed (elastic#30509) Fix Docker module: rename fields on dashboards (elastic#30500) fix typos and improve sentences (elastic#30432) Add drop and explicit tests to avoid duplicate ingest of elasticsearch logs (elastic#30440) {,x-pack/}auditbeat: replace uses of github.com/pkg/errors with stdlib equivalents (elastic#30321) Spelling fix (elastic#30439) packetbeat/beater: make sure Npcap installation runs before interfaces are needed in all cases (elastic#30438) Add BC about Homebrew no longer being available in 8.0 (elastic#30419) Install gawk as a replacement for mawk in Docker containers. (elastic#30452) Clean up python-related system tests (elastic#30415) Fix TestNewModuleRegistry flakiness (elastic#30453) [Filebeat] [auditd]: Support EXECVE events with truncated argument list (elastic#30382) Set `log.offset` to the start of the reported line in filestream (elastic#30445) clarify SelectedPackageTypes meaning and improve its usage (elastic#30142) [elasticsearch module] serialize shards properties (elastic#30408) ...

) (#30449) (cherry picked from commit 8aca673) Co-authored-by: Noémi Ványi <kvch@users.noreply.github.com>

) (#30450) (cherry picked from commit 8aca673) Co-authored-by: Noémi Ványi <kvch@users.noreply.github.com>

) (#30448) (cherry picked from commit 8aca673) Co-authored-by: Noémi Ványi <kvch@users.noreply.github.com> Co-authored-by: Noémi Ványi <sitbackandwait@gmail.com>

Set log.offset to the offset of the line start

c1d32be

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Feb 17, 2022

kvch added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Feb 17, 2022

mergify bot assigned kvch Feb 17, 2022

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Feb 17, 2022

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Feb 17, 2022

kvch added backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.0.0 Automated backport with mergify backport-v8.1.0 Automated backport with mergify labels Feb 17, 2022

mergify bot removed the backport-skip Skip notification from the automated backport with mergify label Feb 17, 2022

add changelog entry

6df2650

rdner reviewed Feb 17, 2022

View reviewed changes

rdner approved these changes Feb 17, 2022

View reviewed changes

kvch merged commit 8aca673 into elastic:main Feb 17, 2022

mergify bot pushed a commit that referenced this pull request Feb 17, 2022

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

05f3672

) (cherry picked from commit 8aca673)

mergify bot mentioned this pull request Feb 17, 2022

[7.17](backport #30445) Set log.offset to the start of the reported line in filestream #30448

Merged

mergify bot pushed a commit that referenced this pull request Feb 17, 2022

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

22ddcfe

) (cherry picked from commit 8aca673)

mergify bot mentioned this pull request Feb 17, 2022

[8.0](backport #30445) Set log.offset to the start of the reported line in filestream #30449

Merged

mergify bot pushed a commit that referenced this pull request Feb 17, 2022

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

2bb62e7

) (cherry picked from commit 8aca673)

mergify bot mentioned this pull request Feb 17, 2022

[8.1](backport #30445) Set log.offset to the start of the reported line in filestream #30450

Merged

cmacknz reviewed Feb 17, 2022

View reviewed changes

kvch added a commit that referenced this pull request Mar 8, 2022

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

03e6f42

) (#30449) (cherry picked from commit 8aca673) Co-authored-by: Noémi Ványi <kvch@users.noreply.github.com>

kvch added a commit that referenced this pull request Mar 8, 2022

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

082b73a

) (#30450) (cherry picked from commit 8aca673) Co-authored-by: Noémi Ványi <kvch@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

Set `log.offset` to the start of the reported line in filestream#30445
kvch merged 2 commits intoelastic:mainfrom
kvch:fix-filebeat-report-the-start-of-file

kvch commented Feb 17, 2022 •

edited

Loading

Uh oh!

elasticmachine commented Feb 17, 2022

Uh oh!

mergify bot commented Feb 17, 2022

Uh oh!

elasticmachine commented Feb 17, 2022 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

Uh oh!

rdner Feb 17, 2022

Uh oh!

rdner Feb 17, 2022

Uh oh!

kvch Feb 17, 2022

Uh oh!

cmacknz Feb 17, 2022

Uh oh!

cmacknz Feb 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

kvch commented Feb 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Why is it important?

Checklist

Uh oh!

elasticmachine commented Feb 17, 2022

Uh oh!

mergify bot commented Feb 17, 2022

Uh oh!

elasticmachine commented Feb 17, 2022 • edited by jenkins-beats-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

Uh oh!

rdner Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

rdner Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

kvch Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

cmacknz Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

cmacknz Feb 17, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kvch commented Feb 17, 2022 •

edited

Loading

elasticmachine commented Feb 17, 2022 •

edited by jenkins-beats-ci bot

Loading