Prospector and Harvester Cleanup #1144
Conversation
| "github.com/elastic/beats/filebeat/config" | ||
| "github.com/elastic/beats/filebeat/harvester/encoding" | ||
| "github.com/elastic/beats/filebeat/input" | ||
| ) | ||
|
|
||
| type Harvester struct { | ||
| Id uuid.UUID |
There was a problem hiding this comment.
Why does a havester need a UUID? As far as I can tell, this value is transient, right?
There was a problem hiding this comment.
The goal is to be able to track harvesters and also properly shut them down. So in case an error appears it is possible to better tell which harvester had problems. This is related to the PR here: https://github.com/elastic/beats/pull/964/files#diff-72613c7cf09182c67339cf99ed741eccR25
There was a problem hiding this comment.
The UUID only exists for the lifetime of a harvester.
There was a problem hiding this comment.
If we need a ID to track an ephemeral harvester then why not just just assign ID's based on one up counter? There's no requirement for global uniqueness here as far as I can tell.
There was a problem hiding this comment.
That would also work, but we the would need a central place to generate the ids for all harvester. UUID is convenient as I don't have to think about all this. Is there a disadvantage to using a UUID here?
There was a problem hiding this comment.
If we write the Id to logs and to stats, it is somehow persisted. Assuming logs and stats are stored in ES, it will make finding a specific harvester easier if we go with UUID I think as only one filter has to be applied. WDYT?
There was a problem hiding this comment.
If the haverster ID is {{beat.UUID}}-hv{{havester ID}} then you can find logs and stats associated with a specific harvester, and you can also implicitly associate those logs and stats to the specific beat without having to maintain a mapping of havester ID to owning beat UUID.
There was a problem hiding this comment.
The thing I don't like about the above is that currently beat.UUID is not accessible from the harvester. Perhaps it is easier to remove the UUID addition from this PR and continue the discussion in #964 (where it is coming from initially) as there it in more context?
There was a problem hiding this comment.
Yeah, I think it makes sense to introduce the Havester ID in #964. We can discuss more there. If we need the beat UUID we can (preferable) inject it or have a singleton that makes it available.
There was a problem hiding this comment.
@andrewkroh UUID removed.
This PR simplifies elastic#964 by already applying the non related parts. * Remove unnecessary for loop in readline
Prospector and Harvester Cleanup
This PR simplifies #964 by already applying the non related parts.