Add autodetection mode for add_docker_metadata #13374

ChrsMark · 2019-08-28T14:17:30Z

This PR is a proposal for enabling autodetection mode for add_docker_metadata.

Currently we configure add_cloud_metadata and add_host_metadata by default in all Beats.

add_cloud_metadata does autodetection, and it will only enrich events if the beat is running on a known public cloud.

It would be useful to have the same for Docker (and Kubernetes), so if Beats is started in a scenario where Docker is available, it automatically enables metadata for it. If Docker is not present nothing will happen.

In this PR only the Docker part is covered. After we conclude to a final approach, a follow-up PR will come for the Kubernetes part.

How to test it

With docker available
a. Run metricbeat or filebeat in an environement where Docker is present.
b. Verify that processor is being automatically enabled.
With docker unavailable
a. Run metricbeat or filebeat in an environement where Docker is not present.
b. Verify that processor is not being automatically enabled.

libbeat/processors/add_docker_metadata/add_docker_metadata.go

ChrsMark · 2019-08-29T14:27:56Z

@jsoriano thanks for reviewing!

Your suggestion have been addressed, however the checks are failing due to the addition in filebeat.yml. Any ideas on how I could resolve this?

jsoriano · 2019-08-29T15:52:28Z

Your suggestion has been addressed, however the checks are failing due to the addition in filebeat.yml. Any ideas on how I could resolve this?

filebeat.yml is generated from files in libbeat/_meta, you will need to make the change there and then run make update from the beats directory to regenerate all configuration files.

libbeat/processors/add_docker_metadata/add_docker_metadata.go

jsoriano

LGTM, I have added a couple of extra suggestions, let me know what you think.

libbeat/_meta/config.yml.tmpl

jsoriano · 2019-09-02T13:14:08Z

libbeat/processors/add_docker_metadata/add_docker_metadata.go

+	cli, err := client.NewEnvClient()
+	errorMsg := fmt.Sprintf("%v: docker environment not detected.", processorName)
+	if err != nil {
+		logp.Info(errorMsg)


Log this error at debug level, and with the error too?

Suggested change

logp.Info(errorMsg)

logp.Debugf("%s: %v", errorMsg, err)

And same thing for the other error some lines below.

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark · 2019-09-03T11:40:33Z

Changed the implementation a little bit so as to perform the connectivity check for Docker with the proper configuration.
Also added a test for the case of Docker is unreachable.

jsoriano

Ok, I think we can go on with this 👍 I have added some comments about some minor details.

jsoriano · 2019-09-03T14:46:45Z

libbeat/processors/add_docker_metadata/add_docker_metadata.go

+		dockerAvailable = true
+		logp.Debug("add_docker_metadata", "%v: docker environment detected", processorName)
+		if err = watcher.Start(); err != nil {
+			return nil, err


You can use errors.Wrap to give a bit more of context here:

Suggested change

return nil, err

return nil, errors.Wrap(err, "failed to start watcher")

jsoriano · 2019-09-03T14:47:11Z

libbeat/processors/add_docker_metadata/add_docker_metadata_test.go

+	assert.NoError(t, err, "processing an event")
+
+	assert.Equal(t, common.MapStr{}, result.Fields)
+}


Thanks for adding a test for this

jsoriano · 2019-09-03T14:48:13Z

libbeat/common/docker/watcher.go

@@ -132,6 +132,12 @@ func NewWatcher(host string, tls *TLSConfig, storeShortID bool) (Watcher, error)
 		return nil, err
 	}

+	// extra check to confirm that Docker is available


Nit.

Suggested change

// extra check to confirm that Docker is available

// Extra check to confirm that Docker is available

jsoriano · 2019-09-03T15:00:39Z

libbeat/processors/add_docker_metadata/add_docker_metadata.go

@@ -19,6 +19,7 @@ package add_docker_metadata

 import (
 	"fmt"
+


Please remove this extra new line.

Suggested change

jsoriano · 2019-09-03T15:01:13Z

libbeat/processors/add_docker_metadata/add_docker_metadata.go

@@ -123,6 +132,9 @@ func lazyCgroupCacheInit(d *addDockerMetadata) {
 func (d *addDockerMetadata) Run(event *beat.Event) (*beat.Event, error) {
 	var cid string
 	var err error
+	if !d.dockerAvailable {
+		return event, nil
+	}


Nit. You could move this code to the very end of the function, before these declarations.

jsoriano · 2019-09-03T15:03:07Z

CHANGELOG.next.asciidoc

@@ -244,6 +244,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - add_host_metadata is no GA. {pull}13148[13148]
 - Add `registered_domain` processor for deriving the registered domain from a given FQDN. {pull}13326[13326]
 - Add support for RFC3339 time zone offsets in JSON output. {pull}13227[13227]
+- Add autodetection mode for add_docker_metadata {pull}13374[13374]


Maybe we should mention something about enabling it by default.

Suggested change

- Add autodetection mode for add_docker_metadata {pull}13374[13374]

- Add autodetection mode for add_docker_metadata and enable it by default in included configuration files {pull}13374[13374]

jsoriano · 2019-09-03T15:04:38Z

libbeat/common/docker/watcher.go

+	_, err = client.Info(context.Background())
+	if err != nil {
+		return nil, err
+	}


I wonder if this can affect other users of NewWatcher, it seems safe though.

Yes it could affect others (AutodiscoverBuilder) in case of a connectivity error, but it wont be a problem I think, since an error will be returned for a case that is indeed problematic.

Signed-off-by: chrismark <chrismarkou92@gmail.com>

The PR #13374 adds `add_docker_metadata` for all Beats. However, this results in the following error in Functionbeat when deployed to AWS: ``` Exiting: error initializing processors: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running? ``` Thus, the function fails to start and process messages. This PR removes the processor from the configuration of Functionbeat. So the function is able to start with the default processor configuration.

…4424) The PR elastic#13374 adds `add_docker_metadata` for all Beats. However, this results in the following error in Functionbeat when deployed to AWS: ``` Exiting: error initializing processors: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running? ``` Thus, the function fails to start and process messages. This PR removes the processor from the configuration of Functionbeat. So the function is able to start with the default processor configuration. (cherry picked from commit d9a4c9c)

…14480) The PR #13374 adds `add_docker_metadata` for all Beats. However, this results in the following error in Functionbeat when deployed to AWS: ``` Exiting: error initializing processors: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running? ``` Thus, the function fails to start and process messages. This PR removes the processor from the configuration of Functionbeat. So the function is able to start with the default processor configuration. (cherry picked from commit d9a4c9c)

…14479) The PR #13374 adds `add_docker_metadata` for all Beats. However, this results in the following error in Functionbeat when deployed to AWS: ``` Exiting: error initializing processors: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running? ``` Thus, the function fails to start and process messages. This PR removes the processor from the configuration of Functionbeat. So the function is able to start with the default processor configuration. (cherry picked from commit d9a4c9c)

…4424) (elastic#14480) The PR elastic#13374 adds `add_docker_metadata` for all Beats. However, this results in the following error in Functionbeat when deployed to AWS: ``` Exiting: error initializing processors: Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running? ``` Thus, the function fails to start and process messages. This PR removes the processor from the configuration of Functionbeat. So the function is able to start with the default processor configuration. (cherry picked from commit c25f71a)

ChrsMark self-assigned this Aug 28, 2019

ChrsMark added Team:Integrations Label for the Integrations team containers Related to containers use case in progress Pull request is currently in progress. labels Aug 28, 2019

ChrsMark mentioned this pull request Aug 28, 2019

Automatically enable add_docker_metadata or add_kubernetes_metadata #13068

Closed

ChrsMark added the [zube]: In Progress label Aug 28, 2019

ChrsMark force-pushed the add_docker_metadata_autodetection branch 2 times, most recently from 93b280c to e95b724 Compare August 28, 2019 14:39

jsoriano reviewed Aug 28, 2019

View reviewed changes

libbeat/processors/add_docker_metadata/add_docker_metadata.go Outdated Show resolved Hide resolved

ChrsMark force-pushed the add_docker_metadata_autodetection branch from 52ca9f6 to fc97617 Compare August 29, 2019 14:04

jsoriano reviewed Aug 29, 2019

View reviewed changes

libbeat/processors/add_docker_metadata/add_docker_metadata.go Outdated Show resolved Hide resolved

ChrsMark requested review from a team as code owners August 30, 2019 10:09

ChrsMark requested a review from jsoriano August 30, 2019 10:56

jsoriano reviewed Aug 30, 2019

View reviewed changes

libbeat/processors/add_docker_metadata/add_docker_metadata.go Outdated Show resolved Hide resolved

libbeat/processors/add_docker_metadata/add_docker_metadata.go Outdated Show resolved Hide resolved

ChrsMark force-pushed the add_docker_metadata_autodetection branch from 099ff3a to ca1d202 Compare September 2, 2019 13:05

jsoriano added the test-plan Add this PR to be manual test plan label Sep 2, 2019

jsoriano reviewed Sep 2, 2019

View reviewed changes

ChrsMark force-pushed the add_docker_metadata_autodetection branch 2 times, most recently from 6314ecf to 9a36f81 Compare September 3, 2019 07:25

ChrsMark added the [zube]: In Review label Sep 3, 2019

zube bot removed the [zube]: In Progress label Sep 3, 2019

ChrsMark force-pushed the add_docker_metadata_autodetection branch from 9a36f81 to 5434efa Compare September 3, 2019 09:49

ChrsMark added 4 commits September 3, 2019 12:49

Enable autodetection mode for add_docker_metadata

fa07b9f

Signed-off-by: chrismark <chrismarkou92@gmail.com>

Move docker autodetection check in contructor

3a4bcd8

Signed-off-by: chrismark <chrismarkou92@gmail.com>

Fix configs & move the check to seperate function

d123be0

Signed-off-by: chrismark <chrismarkou92@gmail.com>

Return silent processor in case of not detected docker

0e087ac

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark force-pushed the add_docker_metadata_autodetection branch from 5434efa to 6f679bb Compare September 3, 2019 09:50

ChrsMark changed the title ~~Enable autodetection mode for add_docker_metadata~~ Add autodetection mode for add_docker_metadata Sep 3, 2019

ChrsMark force-pushed the add_docker_metadata_autodetection branch from 6f679bb to ecb905d Compare September 3, 2019 09:53

Use proper config for docker env check

abfc865

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark force-pushed the add_docker_metadata_autodetection branch from ecb905d to abfc865 Compare September 3, 2019 10:11

ChrsMark mentioned this pull request Sep 3, 2019

Add k8s metadata autodetection #13473

Merged

jsoriano added the v7.5.0 label Sep 3, 2019

jsoriano reviewed Sep 3, 2019

View reviewed changes

Add minor improvements

5d3c11e

Signed-off-by: chrismark <chrismarkou92@gmail.com>

jsoriano approved these changes Sep 4, 2019

View reviewed changes

ChrsMark merged commit 8e5bdc3 into elastic:master Sep 4, 2019

zube bot added [zube]: Done and removed [zube]: In Review labels Sep 4, 2019

ChrsMark removed the in progress Pull request is currently in progress. label Sep 4, 2019

andresrc removed the [zube]: Done label Sep 9, 2019

ChrsMark mentioned this pull request Sep 12, 2019

Update docs for metadata_processors #13650

Merged

ChrsMark added the [zube]: (7.5) Planned for release label Oct 2, 2019

andresrc added the needs testing notes label Oct 10, 2019

ChrsMark removed the needs testing notes label Oct 11, 2019

kvch mentioned this pull request Nov 11, 2019

Remove add_docker_metadata from Functionbeat configuration #14424

Merged

This was referenced Nov 12, 2019

Cherry-pick #14424 to 7.x: Remove add_docker_metadata from Functionbeat configuration #14479

Merged

Cherry-pick #14424 to 7.5: Remove add_docker_metadata from Functionbeat configuration #14480

Merged

ChrsMark mentioned this pull request Nov 26, 2019

Beats fail to start if add_docker_metadata initialization fails #12461

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add autodetection mode for add_docker_metadata #13374

Add autodetection mode for add_docker_metadata #13374

ChrsMark commented Aug 28, 2019 •

edited

Loading

ChrsMark commented Aug 29, 2019 •

edited

Loading

jsoriano commented Aug 29, 2019

jsoriano left a comment

jsoriano Sep 2, 2019

ChrsMark commented Sep 3, 2019

jsoriano left a comment

jsoriano Sep 3, 2019

jsoriano Sep 3, 2019

jsoriano Sep 3, 2019

jsoriano Sep 3, 2019

jsoriano Sep 3, 2019

jsoriano Sep 3, 2019

jsoriano Sep 3, 2019

ChrsMark Sep 4, 2019

	return nil, err
	return nil, errors.Wrap(err, "failed to start watcher")

	// extra check to confirm that Docker is available
	// Extra check to confirm that Docker is available

	- Add autodetection mode for add_docker_metadata {pull}13374[13374]
	- Add autodetection mode for add_docker_metadata and enable it by default in included configuration files {pull}13374[13374]

Add autodetection mode for add_docker_metadata #13374

Add autodetection mode for add_docker_metadata #13374

Conversation

ChrsMark commented Aug 28, 2019 • edited Loading

How to test it

ChrsMark commented Aug 29, 2019 • edited Loading

jsoriano commented Aug 29, 2019

jsoriano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrsMark commented Sep 3, 2019

jsoriano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrsMark commented Aug 28, 2019 •

edited

Loading

ChrsMark commented Aug 29, 2019 •

edited

Loading