Cleanup Harvester and Input parts of Filebeat #1896
Conversation
ruflin
commented
Jun 22, 2016
ruflin
commented
- Cleanup defaults by moving them to config
- Move readers and sources into their own packages
- Add some minor comments
- Move state to its own package
- Move multiline and json to its own file
- Rename FileState to State as now package name is included
ruflin
force-pushed
the
harvester-cleanup
branch
from
c368735 to
daf409b
Compare
| DefaultCloseOlder = 1 * time.Hour | ||
| DefaultForceCloseFiles = false | ||
| DefaultMaxBytes = 10 * (1 << 20) // 10MB | ||
| ) |
There was a problem hiding this comment.
we still need the consts if defaultConfig already contains all defaults?
ruflin
force-pushed
the
harvester-cleanup
branch
from
daf409b to
9f44c13
Compare
| // through a decoder converting the reader it's encoding to utf-8. | ||
| type LineSource struct { | ||
| reader *encoding.LineReader | ||
| } |
There was a problem hiding this comment.
LineSource is basically an adapter for converting an io.Reader into a LineProcessor producing lines. Alternative names LinesAdapter or LineProducer. Perfectly valid to move LineSource into separate go file and only define interfaces in line.go.
There was a problem hiding this comment.
For getting proper multiline json support one has to implement JsonProducer adapter wrapping io.Reader with json.Decoder. dealing with encoding might still be required, as golang requires UTF-8, but json can be UTF-16 as well.
There was a problem hiding this comment.
WDYT about LineEncoder as this is what it actually does?
ruflin
force-pushed
the
harvester-cleanup
branch
from
9f44c13 to
3d76bef
Compare
* Cleanup defaults by moving them to config * Move readers and sources into their own packages * Add some minor comments * Move state to its own package * Move multiline and json to its own file * Rename FileState to State as now package name is included * Adjust test cases * Move multiline config to processor * Move json to processor * Separate limit and line processor * Improve error handling in registrar
ruflin
force-pushed
the
harvester-cleanup
branch
from
3d76bef to
6ce744e
Compare
|
jenkins, retest it |
1 similar comment
|
jenkins, retest it |
| "github.com/elastic/beats/libbeat/common" | ||
| ) | ||
|
|
||
| var ( | ||
| defaultConfig = harvesterConfig{ | ||
| BufferSize: cfg.DefaultHarvesterBufferSize, | ||
| DocumentType: cfg.DefaultDocumentType, | ||
| BufferSize: 16 << 10, // 16384 |
There was a problem hiding this comment.
While I can read it, I like 16 * (1 << 10). Or give 1<<10 a name like, sz1K = (1<<10)
There was a problem hiding this comment.
something like time.Second would be nice. e.g. sizes.K or sizes.M.
There was a problem hiding this comment.
I think the easiest solution here is to just write 16384. I don't think it 16 << 10 or any of the other variants brings a benefit.
There was a problem hiding this comment.
it's easier to get the number wrong when using 16384 over 16 * units.Kibi for example.
