Skip to content

Comments

Fix entry duplication at beginning of file(s)#579

Merged
jsirianni merged 3 commits intomainfrom
fileinput-fix-duplication
Mar 8, 2022
Merged

Fix entry duplication at beginning of file(s)#579
jsirianni merged 3 commits intomainfrom
fileinput-fix-duplication

Conversation

@jsirianni
Copy link
Member

@jsirianni jsirianni commented Feb 25, 2022 •
edited
Loading

Description of Changes

Backporting a data duplication fix that was recently released in the OTEL repo. I don't have a great understanding of the complex file input operator, but I do know that the test being added (TestIgnoreEmptyFiles) is flaky (fails most of the time) until the actual fix is implemented. Once the fix is in place, the new test passes 100% of the time for me.

while :
do
go clean -testcache ./... && go test -run TestIgnoreEmptyFiles ./operator/builtin/input/file
done

For more context around the issue:

Please check that the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Add a changelog entry (for non-trivial bug fixes / features)
  • CI passes

@codecov
Copy link

codecov bot commented Feb 25, 2022 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (main@5198c73). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #579   +/-   ##
=======================================
  Coverage        ?   74.97%           
=======================================
  Files           ?      133           
  Lines           ?    10089           
  Branches        ?        0           
=======================================
  Hits            ?     7564           
  Misses          ?     2060           
  Partials        ?      465

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5198c73...d62640d. Read the comment docs.

@jsirianni jsirianni marked this pull request as ready for review February 28, 2022 13:58
@jsirianni jsirianni requested a review from a team as a code owner February 28, 2022 13:58
@rockb1017
Copy link

rockb1017 commented Mar 1, 2022

FYI, on the open telemetry log collection repo, it is not flaky.

 ~/go/src/github.com/rockb1017/opentelemetry-log-collection [main|⚑ 2] 
14:31 $ while :; do go clean -testcache ./... && go test -run TestIgnoreEmptyFiles ./operator/input/file; done

ok      github.com/open-telemetry/opentelemetry-log-collection/operator/input/file      6.787s
ok      github.com/open-telemetry/opentelemetry-log-collection/operator/input/file      6.197s
ok      github.com/open-telemetry/opentelemetry-log-collection/operator/input/file      6.222s
ok      github.com/open-telemetry/opentelemetry-log-collection/operator/input/file      6.196s
ok      github.com/open-telemetry/opentelemetry-log-collection/operator/input/file      6.218s
ok      github.com/open-telemetry/opentelemetry-log-collection/operator/input/file      6.232s
ok      github.com/open-telemetry/opentelemetry-log-collection/operator/input/file      6.214s
ok      github.com/open-telemetry/opentelemetry-log-collection/operator/input/file      6.184s
ok      github.com/open-telemetry/opentelemetry-log-collection/operator/input/file      6.245s
ok      github.com/open-telemetry/opentelemetry-log-collection/operator/input/file      6.173s
ok      github.com/open-telemetry/opentelemetry-log-collection/operator/input/file      6.193s
ok      github.com/open-telemetry/opentelemetry-log-collection/operator/input/file      6.207s

cpheps
cpheps approved these changes Mar 8, 2022
View reviewed changes
Copy link
Contributor

@cpheps cpheps left a comment

Choose a reason for hiding this comment

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

LGTM, Please update branch

@jsirianni jsirianni merged commit 33707f8 into main Mar 8, 2022
@jsirianni jsirianni deleted the fileinput-fix-duplication branch March 8, 2022 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@cpheps cpheps cpheps approved these changes

@jmwilliams89 jmwilliams89 jmwilliams89 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jsirianni @rockb1017 @cpheps @jmwilliams89