Skip to content
This repository has been archived by the owner on May 25, 2022. It is now read-only.

File Input Cleanup - Step 1 #155

Merged
merged 3 commits into from
May 25, 2021
Merged

File Input Cleanup - Step 1 #155

merged 3 commits into from
May 25, 2021

Conversation

djaglowski
Copy link
Member

The codebase within the file_input operator is complex enough that functional changes are difficult to confidently implement. This PR is minor step towards untangling the code base.

  • Open files in the makeReaders method
  • Add Reader.Close
  • Remove unnecessary copying of file handles

@djaglowski djaglowski marked this pull request as ready for review May 20, 2021 14:22
@djaglowski djaglowski requested review from a team and jsirianni May 20, 2021 14:22
@djaglowski
Copy link
Member Author

Manually benchmarking these changes:

collector-contrib:main:

Log10kDPS/filelog                       |PASS  |     15s|    28.4|    30.2|         43|         50|    150000|        150000|
Log10kDPS/filelog_checkpoints           |PASS  |     15s|    28.6|    30.0|         42|         50|    150000|        150000|
Log10kDPS/kubernetes_containers         |PASS  |     16s|    56.2|    56.7|         47|         55|    150000|        150000|

With changes:

Log10kDPS/filelog                       |PASS  |     15s|    28.9|    29.5|         43|         50|    150000|        150000|
Log10kDPS/filelog_checkpoints           |PASS  |     15s|    28.3|    30.0|         42|         49|    149600|        149600|
Log10kDPS/kubernetes_containers         |PASS  |     15s|    57.0|    58.0|         45|         54|    150000|        150000|```

@tigrannajaryan tigrannajaryan merged commit 6184b76 into open-telemetry:main May 25, 2021
@djaglowski djaglowski deleted the cleanup-file-input-1 branch May 25, 2021 20:36
jsirianni added a commit to observIQ/stanza that referenced this pull request May 25, 2021
@jsirianni
Cleanup file input open-telemetry/opentelemetry-log-collection#155
0b3d95c
@jsirianni jsirianni mentioned this pull request May 25, 2021
Merged
4 tasks
jsirianni pushed a commit to observIQ/stanza that referenced this pull request May 26, 2021
Cleanup file input open-telemetry/opentelemetry-log-collection#155 (#316
34a85da 
)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tigrannajaryan tigrannajaryan tigrannajaryan approved these changes

@jsirianni jsirianni Awaiting requested review from jsirianni

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@djaglowski @tigrannajaryan