Minor improvements to Journalbeat #8618

kvch · 2018-10-15T15:11:38Z

remove the exposed channel
convert field values to integer if possible
fix incorrect field name
tie github.com/coreos/go-systemd/sdjournal to v17

houndci-bot · 2018-10-15T15:11:51Z

journalbeat/reader/fields.go

@@ -19,69 +19,75 @@ package reader

 import "github.com/coreos/go-systemd/sdjournal"

+type FieldConversion struct {


exported type FieldConversion should have comment or be unexported

journalbeat/reader/fields.go

journalbeat/reader/journal.go

ph · 2018-10-16T13:43:43Z

journalbeat/reader/journal.go

@@ -275,8 +249,18 @@ func (r *Reader) toEvent(entry *sdjournal.JournalEntry) *beat.Event {
 			normalized := strings.ToLower(strings.TrimLeft(k, "_"))
 			custom.Put(normalized, v)
 		} else {
-			if isKept(kk) {
-				fields.Put(kk, v)
+			if !kk.dropped {


nit pick: can we rename kk to something more meaningful?

Consider moving the inner statements into a helper function. The nesting makes it a little hard to follow. E.g.

journalbeat/_meta/fields.common.yml

journalbeat/input/input.go

journalbeat/reader/fields.go

urso · 2018-10-16T13:57:42Z

journalbeat/reader/journal.go

@@ -275,8 +249,18 @@ func (r *Reader) toEvent(entry *sdjournal.JournalEntry) *beat.Event {
 			normalized := strings.ToLower(strings.TrimLeft(k, "_"))
 			custom.Put(normalized, v)
 		} else {
-			if isKept(kk) {
-				fields.Put(kk, v)
+			if !kk.dropped {


Consider moving the inner statements into a helper function. The nesting makes it a little hard to follow. E.g.

kvch · 2018-10-17T15:15:08Z

This should be ready for another round of review.

urso · 2018-10-17T16:12:14Z

journalbeat/reader/journal.go

+	j, logger, err := opener()
+	if err != nil {
+		return nil, err
+	}


This function looks weird. We pass a logger as parameters, so to replace it with the logger instantiated via 'opener'?

The first newReader does is calling 'opener'. Let's simplify this by not passing a 'factory' here. Also, common context should come first:

func newReader(log *logp.Logger, done chan struct{}, cfg Config, journal *sdjournal.Journal, state checkpoint.JournalState) (*Reader, error) { }

journalbeat/reader/journal.go

urso

LGTM

urso

make check is failing on python code (autopep8).

ruflin · 2018-10-18T15:22:40Z

Rebase should fix the autopep8 problem.

* refactoring of async API * correct name of field * tie vendored lib to latest release * convert string fields to int && drop if needed * do not expose internal type * do not block on out channel if beat is stopped * fix name of registry_file option * more refactoring * add missing notice * port uuid to new lib * address review notes * rm factory * index custom as nested object

* Initialize Journalbeat (#8277) This is the first PR to initialize Journalbeat with minimal functionality. The architecture is mimicing Filebeat so it can be merged into FB in the future. It means it has multiple inputs which can share configuration (`backoff`, `backoff_factor`, etc.). Inputs can have multiple readers, each reader reads from a journal specified in the list of `paths`. The readers are not going to implement the interface `Harverster` until it's merged into Filebeat, because it would overcomplicate event publishing unnecessarily and would need to duplicate too much Filebeat code. Checkpointing is copied from Winlogbeat. Once the new registry file is merged, it will be migrated. Example configuration to read from the beginning of the local journal ```yml journalbeat.inputs: - paths: [] seek: head ``` Features * read from local journal, journal file and directory * position tracking by using check-pointing as it's done in Winlogbeat * seek to "tail", "head", "cursor" * minimal E2E tests * fields.yml and documentation Vendored: * github.com/coreos/go-systemd/sdjournal * Journalbeat matches support && minor additions (#8324) ### Matching support From now on it's possible to match for journal entry fields in Journalbeat using the new option `matches`. This requires a list of key value pairs separated by "=". The key has to be a journalbeat event key (e.g systemd.unit) and the value is the exact value journal reader needs to find in the entries. Example configuration which returns NGINX and dhclient entries from the journal: ```yml include_matches: - "systemd.unit=nginx" - "process.name=dhclient" ``` ### Docker fields Added docker fields from: https://docs.docker.com/config/containers/logging/journald/ - `container.id` - `container.id_truncated` - `container.name` - `container.image.tag` - `container.partial` ### Parse timestamp of entries Journalbeat parses the timestamp of the entry and adds it to the event as `@timestamp`. The time of reading by Journalbeat is saved in `read_timestamp`. ### Save custom fields Custom fields by various sources are stored under `custom`. Field names are normalized, meaning `"_"` prefix is removed and every letter is lowercase. ### Fields && processors From now on it is possible to configure `processors` and `fields`, etc on `input` level. ### Metrics The size of each open reader is reporting in bytes: ``` { "journalbeat": { "journals": { "journal_1": { "path": "system.journal", "size_in_bytes": 123124214, } } } ``` * Minor improvements to Journalbeat (#8618) * Packaging of journalbeat (#8702) Journalbeat is going to be built using the new Debian 8 container, because systemd version in Debian 7 is too old (v44 instead of the required v187). Minor changes: * add missing X-Pack folder to journalbeat * do not crosscompile journalbeat due to missing dependencies locally * Add journalbeat docs (#8735) * Add journalbeat docs

* Initialize Journalbeat (elastic#8277) This is the first PR to initialize Journalbeat with minimal functionality. The architecture is mimicing Filebeat so it can be merged into FB in the future. It means it has multiple inputs which can share configuration (`backoff`, `backoff_factor`, etc.). Inputs can have multiple readers, each reader reads from a journal specified in the list of `paths`. The readers are not going to implement the interface `Harverster` until it's merged into Filebeat, because it would overcomplicate event publishing unnecessarily and would need to duplicate too much Filebeat code. Checkpointing is copied from Winlogbeat. Once the new registry file is merged, it will be migrated. Example configuration to read from the beginning of the local journal ```yml journalbeat.inputs: - paths: [] seek: head ``` Features * read from local journal, journal file and directory * position tracking by using check-pointing as it's done in Winlogbeat * seek to "tail", "head", "cursor" * minimal E2E tests * fields.yml and documentation Vendored: * github.com/coreos/go-systemd/sdjournal * Journalbeat matches support && minor additions (elastic#8324) From now on it's possible to match for journal entry fields in Journalbeat using the new option `matches`. This requires a list of key value pairs separated by "=". The key has to be a journalbeat event key (e.g systemd.unit) and the value is the exact value journal reader needs to find in the entries. Example configuration which returns NGINX and dhclient entries from the journal: ```yml include_matches: - "systemd.unit=nginx" - "process.name=dhclient" ``` Added docker fields from: https://docs.docker.com/config/containers/logging/journald/ - `container.id` - `container.id_truncated` - `container.name` - `container.image.tag` - `container.partial` Journalbeat parses the timestamp of the entry and adds it to the event as `@timestamp`. The time of reading by Journalbeat is saved in `read_timestamp`. Custom fields by various sources are stored under `custom`. Field names are normalized, meaning `"_"` prefix is removed and every letter is lowercase. From now on it is possible to configure `processors` and `fields`, etc on `input` level. The size of each open reader is reporting in bytes: ``` { "journalbeat": { "journals": { "journal_1": { "path": "system.journal", "size_in_bytes": 123124214, } } } ``` * Minor improvements to Journalbeat (elastic#8618) * Packaging of journalbeat (elastic#8702) Journalbeat is going to be built using the new Debian 8 container, because systemd version in Debian 7 is too old (v44 instead of the required v187). Minor changes: * add missing X-Pack folder to journalbeat * do not crosscompile journalbeat due to missing dependencies locally * Add journalbeat docs (elastic#8735) * Add journalbeat docs (cherry picked from commit 24d0e08)

* Add Journalbeat (#8703) * Initialize Journalbeat (#8277) This is the first PR to initialize Journalbeat with minimal functionality. The architecture is mimicing Filebeat so it can be merged into FB in the future. It means it has multiple inputs which can share configuration (`backoff`, `backoff_factor`, etc.). Inputs can have multiple readers, each reader reads from a journal specified in the list of `paths`. The readers are not going to implement the interface `Harverster` until it's merged into Filebeat, because it would overcomplicate event publishing unnecessarily and would need to duplicate too much Filebeat code. Checkpointing is copied from Winlogbeat. Once the new registry file is merged, it will be migrated. Example configuration to read from the beginning of the local journal ```yml journalbeat.inputs: - paths: [] seek: head ``` Features * read from local journal, journal file and directory * position tracking by using check-pointing as it's done in Winlogbeat * seek to "tail", "head", "cursor" * minimal E2E tests * fields.yml and documentation Vendored: * github.com/coreos/go-systemd/sdjournal * Journalbeat matches support && minor additions (#8324) From now on it's possible to match for journal entry fields in Journalbeat using the new option `matches`. This requires a list of key value pairs separated by "=". The key has to be a journalbeat event key (e.g systemd.unit) and the value is the exact value journal reader needs to find in the entries. Example configuration which returns NGINX and dhclient entries from the journal: ```yml include_matches: - "systemd.unit=nginx" - "process.name=dhclient" ``` Added docker fields from: https://docs.docker.com/config/containers/logging/journald/ - `container.id` - `container.id_truncated` - `container.name` - `container.image.tag` - `container.partial` Journalbeat parses the timestamp of the entry and adds it to the event as `@timestamp`. The time of reading by Journalbeat is saved in `read_timestamp`. Custom fields by various sources are stored under `custom`. Field names are normalized, meaning `"_"` prefix is removed and every letter is lowercase. From now on it is possible to configure `processors` and `fields`, etc on `input` level. The size of each open reader is reporting in bytes: ``` { "journalbeat": { "journals": { "journal_1": { "path": "system.journal", "size_in_bytes": 123124214, } } } ``` * Minor improvements to Journalbeat (#8618) * Packaging of journalbeat (#8702) Journalbeat is going to be built using the new Debian 8 container, because systemd version in Debian 7 is too old (v44 instead of the required v187). Minor changes: * add missing X-Pack folder to journalbeat * do not crosscompile journalbeat due to missing dependencies locally * Add journalbeat docs (#8735) * Add journalbeat docs (cherry picked from commit 24d0e08)

* Initialize Journalbeat (elastic#8277) This is the first PR to initialize Journalbeat with minimal functionality. The architecture is mimicing Filebeat so it can be merged into FB in the future. It means it has multiple inputs which can share configuration (`backoff`, `backoff_factor`, etc.). Inputs can have multiple readers, each reader reads from a journal specified in the list of `paths`. The readers are not going to implement the interface `Harverster` until it's merged into Filebeat, because it would overcomplicate event publishing unnecessarily and would need to duplicate too much Filebeat code. Checkpointing is copied from Winlogbeat. Once the new registry file is merged, it will be migrated. Example configuration to read from the beginning of the local journal ```yml journalbeat.inputs: - paths: [] seek: head ``` Features * read from local journal, journal file and directory * position tracking by using check-pointing as it's done in Winlogbeat * seek to "tail", "head", "cursor" * minimal E2E tests * fields.yml and documentation Vendored: * github.com/coreos/go-systemd/sdjournal * Journalbeat matches support && minor additions (elastic#8324) ### Matching support From now on it's possible to match for journal entry fields in Journalbeat using the new option `matches`. This requires a list of key value pairs separated by "=". The key has to be a journalbeat event key (e.g systemd.unit) and the value is the exact value journal reader needs to find in the entries. Example configuration which returns NGINX and dhclient entries from the journal: ```yml include_matches: - "systemd.unit=nginx" - "process.name=dhclient" ``` ### Docker fields Added docker fields from: https://docs.docker.com/config/containers/logging/journald/ - `container.id` - `container.id_truncated` - `container.name` - `container.image.tag` - `container.partial` ### Parse timestamp of entries Journalbeat parses the timestamp of the entry and adds it to the event as `@timestamp`. The time of reading by Journalbeat is saved in `read_timestamp`. ### Save custom fields Custom fields by various sources are stored under `custom`. Field names are normalized, meaning `"_"` prefix is removed and every letter is lowercase. ### Fields && processors From now on it is possible to configure `processors` and `fields`, etc on `input` level. ### Metrics The size of each open reader is reporting in bytes: ``` { "journalbeat": { "journals": { "journal_1": { "path": "system.journal", "size_in_bytes": 123124214, } } } ``` * Minor improvements to Journalbeat (elastic#8618) * Packaging of journalbeat (elastic#8702) Journalbeat is going to be built using the new Debian 8 container, because systemd version in Debian 7 is too old (v44 instead of the required v187). Minor changes: * add missing X-Pack folder to journalbeat * do not crosscompile journalbeat due to missing dependencies locally * Add journalbeat docs (elastic#8735) * Add journalbeat docs

* Add Journalbeat (elastic#8703) * Initialize Journalbeat (elastic#8277) This is the first PR to initialize Journalbeat with minimal functionality. The architecture is mimicing Filebeat so it can be merged into FB in the future. It means it has multiple inputs which can share configuration (`backoff`, `backoff_factor`, etc.). Inputs can have multiple readers, each reader reads from a journal specified in the list of `paths`. The readers are not going to implement the interface `Harverster` until it's merged into Filebeat, because it would overcomplicate event publishing unnecessarily and would need to duplicate too much Filebeat code. Checkpointing is copied from Winlogbeat. Once the new registry file is merged, it will be migrated. Example configuration to read from the beginning of the local journal ```yml journalbeat.inputs: - paths: [] seek: head ``` Features * read from local journal, journal file and directory * position tracking by using check-pointing as it's done in Winlogbeat * seek to "tail", "head", "cursor" * minimal E2E tests * fields.yml and documentation Vendored: * github.com/coreos/go-systemd/sdjournal * Journalbeat matches support && minor additions (elastic#8324) From now on it's possible to match for journal entry fields in Journalbeat using the new option `matches`. This requires a list of key value pairs separated by "=". The key has to be a journalbeat event key (e.g systemd.unit) and the value is the exact value journal reader needs to find in the entries. Example configuration which returns NGINX and dhclient entries from the journal: ```yml include_matches: - "systemd.unit=nginx" - "process.name=dhclient" ``` Added docker fields from: https://docs.docker.com/config/containers/logging/journald/ - `container.id` - `container.id_truncated` - `container.name` - `container.image.tag` - `container.partial` Journalbeat parses the timestamp of the entry and adds it to the event as `@timestamp`. The time of reading by Journalbeat is saved in `read_timestamp`. Custom fields by various sources are stored under `custom`. Field names are normalized, meaning `"_"` prefix is removed and every letter is lowercase. From now on it is possible to configure `processors` and `fields`, etc on `input` level. The size of each open reader is reporting in bytes: ``` { "journalbeat": { "journals": { "journal_1": { "path": "system.journal", "size_in_bytes": 123124214, } } } ``` * Minor improvements to Journalbeat (elastic#8618) * Packaging of journalbeat (elastic#8702) Journalbeat is going to be built using the new Debian 8 container, because systemd version in Debian 7 is too old (v44 instead of the required v187). Minor changes: * add missing X-Pack folder to journalbeat * do not crosscompile journalbeat due to missing dependencies locally * Add journalbeat docs (elastic#8735) * Add journalbeat docs (cherry picked from commit 24d0e08)

* refactoring of async API * correct name of field * tie vendored lib to latest release * convert string fields to int && drop if needed * do not expose internal type * do not block on out channel if beat is stopped * fix name of registry_file option * more refactoring * add missing notice * port uuid to new lib * address review notes * rm factory * index custom as nested object

kvch added in progress Pull request is currently in progress. review labels Oct 15, 2018

kvch requested review from ph and urso October 15, 2018 15:11

houndci-bot reviewed Oct 15, 2018

View reviewed changes

kvch mentioned this pull request Oct 15, 2018

Journalbeat #8323

Closed

23 tasks

kvch force-pushed the feature-journalbeat branch from c66a084 to 395171c Compare October 16, 2018 09:57

kvch force-pushed the fix-async-api branch from 3a30c9b to dacef17 Compare October 16, 2018 09:58

kvch removed the in progress Pull request is currently in progress. label Oct 16, 2018

ph suggested changes Oct 16, 2018

View reviewed changes

urso suggested changes Oct 16, 2018

View reviewed changes

urso reviewed Oct 17, 2018

View reviewed changes

journalbeat/reader/journal.go Show resolved Hide resolved

ph approved these changes Oct 17, 2018

View reviewed changes

urso approved these changes Oct 18, 2018

View reviewed changes

urso suggested changes Oct 18, 2018

View reviewed changes

kvch force-pushed the feature-journalbeat branch from 395171c to d6d12ae Compare October 18, 2018 17:34

kvch added 11 commits October 18, 2018 19:35

refactoring of async API

37eeb1e

correct name of field

ea8a2bc

tie vendored lib to latest release

6ecc5c5

convert string fields to int && drop if needed

86df662

do not expose internal type

ee7c420

do not block on out channel if beat is stopped

910be5b

fix name of registry_file option

7e59cb6

more refactoring

317be6d

add missing notice

6a24f82

port uuid to new lib

d0da2c8

address review notes

c024d11

kvch added 2 commits October 18, 2018 19:35

rm factory

44d2084

index custom as nested object

14e8b45

kvch force-pushed the fix-async-api branch from fd1a4d5 to 14e8b45 Compare October 18, 2018 17:35

urso approved these changes Oct 18, 2018

View reviewed changes

kvch merged commit b6a2e5f into elastic:feature-journalbeat Oct 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minor improvements to Journalbeat #8618

Minor improvements to Journalbeat #8618

kvch commented Oct 15, 2018

houndci-bot Oct 15, 2018

ph Oct 16, 2018

urso Oct 16, 2018

urso Oct 16, 2018

kvch commented Oct 17, 2018

urso Oct 17, 2018

urso left a comment

urso left a comment

ruflin commented Oct 18, 2018

		@@ -19,69 +19,75 @@ package reader

		import "github.com/coreos/go-systemd/sdjournal"

		type FieldConversion struct {

Minor improvements to Journalbeat #8618

Minor improvements to Journalbeat #8618

Conversation

kvch commented Oct 15, 2018

houndci-bot Oct 15, 2018

Choose a reason for hiding this comment

ph Oct 16, 2018

Choose a reason for hiding this comment

urso Oct 16, 2018

Choose a reason for hiding this comment

urso Oct 16, 2018

Choose a reason for hiding this comment

kvch commented Oct 17, 2018

urso Oct 17, 2018

Choose a reason for hiding this comment

urso left a comment

Choose a reason for hiding this comment

urso left a comment

Choose a reason for hiding this comment

ruflin commented Oct 18, 2018