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

feat: trim whitechars for file with multiline #212

Merged
merged 3 commits into from
Jul 9, 2021

Conversation

sumo-drosiek
Copy link
Member

@sumo-drosiek sumo-drosiek commented Jul 6, 2021

Strip whitechars if using multiline

@sumo-drosiek sumo-drosiek requested a review from a team July 6, 2021 12:25
@codecov
Copy link

codecov bot commented Jul 6, 2021

Codecov Report

Merging #212 (29c08f0) into main (8b99134) will decrease coverage by 0.0%.
The diff coverage is 64.7%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #212     +/-   ##
=======================================
- Coverage   75.6%   75.5%   -0.1%     
=======================================
  Files         95      95             
  Lines       4399    4408      +9     
=======================================
+ Hits        3327    3332      +5     
- Misses       745     748      +3     
- Partials     327     328      +1     
Impacted Files Coverage Δ
operator/helper/multiline.go 84.2% <64.7%> (-3.3%) ⬇️
operator/builtin/input/tcp/tcp.go 72.9% <0.0%> (-1.7%) ⬇️
operator/builtin/input/file/reader.go 64.5% <0.0%> (+2.1%) ⬆️

@sumo-drosiek sumo-drosiek force-pushed the drosiek-trim-whitechars branch from 864c23a to fd4dbb8 Compare July 6, 2021 12:45
Comment on lines 82 to 83
token = trimWhitespaces(data)
advance = len(token)
Copy link
Member

Choose a reason for hiding this comment

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

advance is used to set the starting point for the next read. By advancing after trimming, I think we may be setting up the value such that we will reread the whitespace, which doesn't seem ideal. Unless I'm misunderstanding some part of this, then I would expect that we do:

Suggested change
token = trimWhitespaces(data)
advance = len(token)
token = trimWhitespaces(data)
advance = len(data)

@sumo-drosiek sumo-drosiek force-pushed the drosiek-trim-whitechars branch from 9fd0858 to cceeaeb Compare July 7, 2021 06:43
@sumo-drosiek sumo-drosiek force-pushed the drosiek-trim-whitechars branch from cceeaeb to e67e78e Compare July 7, 2021 06:43
@djaglowski djaglowski merged commit 2ffe032 into open-telemetry:main Jul 9, 2021
@sumo-drosiek sumo-drosiek deleted the drosiek-trim-whitechars branch August 2, 2021 12:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants