Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add DeDot in add_docker_metadata processor #9505

Merged
merged 17 commits into from
Dec 17, 2018
Merged

Add DeDot in add_docker_metadata processor #9505

merged 17 commits into from
Dec 17, 2018

Conversation

kaiyan-sheng
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng commented Dec 12, 2018 •
edited
Loading

This pr is for issue: #9350
Currently metrics from docker container is after dedot, which look like this:

t docker.container.labels.com_docker_compose_service grafana
t docker.container.labels.com_docker_compose_version 1.22.0

and metrics from postgresql is not dedotted, which look like:

t docker.container.labels.com.docker.compose.service sentry-postgres
t docker.container.labels.com.docker.compose.version 1.22.0

This PR will add a config option in add_docker_metadata processor to enable/disable dedot.

closes #9350

ph and others added 5 commits November 27, 2018 10:51
@ph @kaiyan-sheng
Never default to a qualifier when none of them are set. (#9148)
aed6cc6 
Remove default version qualifier and rename the environment variable to set it from `BEAT_VERSION_QUALIFIER` to `VERSION_QUALIFIER` this will align with other parts of the stack.

**Tested with filebeat.**
```
 ❯ ./filebeat version                                                                                                                                                                                                                                                                                                                                          [08:39:01]
filebeat version 7.0.0 (amd64), libbeat 7.0.0 [0a0c267 built 2018-11-19 13:38:15 +0000 UTC]
```

**Without the patch**
```
 ❯ ./filebeat version                                                                                                                                                                                                                                                                                                                                          [08:40:07]
filebeat version 7.0.0-alpha1 (amd64), libbeat 7.0.0-alpha1 [b007837 built 2018-11-19 13:39:59 +0000 UTC]
```

Fixes: #8384
@kaiyan-sheng
Merge branch 'master' of github.com:kaiyan-sheng/beats
07c4fa5
@kaiyan-sheng
Merge branch 'master' of github.com:kaiyan-sheng/beats
ff52365
@kaiyan-sheng
Merge remote-tracking branch 'upstream/master'
5f9f575
@kaiyan-sheng
Merge remote-tracking branch 'upstream/master'
09d8b97
@ruflin
Copy link
Member

ruflin commented Dec 12, 2018

@kaiyan-sheng Thanks for the PR. Could you also in the PR describe what was changed so it can be compared to the actual code changes?

@ruflin ruflin added the Team:Integrations Label for the Integrations team label Dec 12, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/infrastructure

ruflin
ruflin reviewed Dec 12, 2018
View reviewed changes
Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

I would have expected this change to contain a config option so the user can turn it on / off especially for compatibility reasons with older versions.

libbeat/processors/add_docker_metadata/add_docker_metadata_test.go Show resolved Hide resolved
libbeat/processors/add_docker_metadata/add_docker_metadata.go Outdated Show resolved Hide resolved
ruflin
ruflin reviewed Dec 14, 2018
View reviewed changes
libbeat/docs/processors-using.asciidoc
@@ -847,6 +847,7 @@ processors:
#match_source_index: 4
#match_short_id: true
#cleanup_timeout: 60
#labels.dedot: false
Copy link
Member

Choose a reason for hiding this comment

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

Just wanted to comment that we so far use dedot.enabled but realised not even the naming of our config options are consistent :-(

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see dedot.enabled in http module. We can think about converging these options in a follow up.
The only thing is that for k8s we will have to think if we want two options, one for labels and another one for annotations.

Copy link
Member

Choose a reason for hiding this comment

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

Also not sure, both works for me.

@jsoriano
Copy link
Member

You will need to make update.

@kaiyan-sheng kaiyan-sheng requested a review from a team as a code owner December 14, 2018 16:43
@jsoriano
Copy link
Member

Something seems still pending, you may need to run make update fmt from the top-level directory.

ruflin
ruflin reviewed Dec 17, 2018
View reviewed changes
CHANGELOG.asciidoc Outdated Show resolved Hide resolved
ruflin
ruflin reviewed Dec 17, 2018
View reviewed changes
CHANGELOG.asciidoc Outdated Show resolved Hide resolved
@kaiyan-sheng kaiyan-sheng merged commit 58573a9 into elastic:master Dec 17, 2018
@kaiyan-sheng kaiyan-sheng deleted the add_dedot branch December 17, 2018 22:37
ruflin
ruflin reviewed Dec 18, 2018
View reviewed changes
auditbeat/docs/getting-started.asciidoc
@@ -97,6 +97,22 @@ endif::[]
[[docker]]
*docker:*

ifeval::["{release-state}"=="unreleased"]
Copy link
Member

Choose a reason for hiding this comment

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

I wonder how this was related to this PR?

kaiyan-sheng added a commit that referenced this pull request Dec 18, 2018
@kaiyan-sheng
Cherry-pick to 6.x: Add DeDot in add_docker_metadata processor (#9505) (
be89f4d 
#9602)

* Cherry-pick to 6.x: Add DeDot in add_docker_metadata processor (#9505)

* Add DeDot in add_docker_metadata processor

* Add dedot into config and default to be false

* Update changelog and documentation

* Add documentation into processors-using.asciidoc

* Run mage fmt update under x-pack filebeat

* Run mage fmt update on x-pack metricbeat

* Run update again

* Run make update fmt from top level dir

* Remove mistakes from rebase

* Remove repeated doc

(cherry picked from commit 58573a9)

* remove extra line in data.json
@exekias exekias mentioned this pull request Jan 3, 2019
Closed
3 tasks
DStape pushed a commit to DStape/beats that referenced this pull request Aug 20, 2019
@kaiyan-sheng
Add DeDot in add_docker_metadata processor (elastic#9505)
b7a0d79 
* Never default to a qualifier when none of them are set. (elastic#9148)

Remove default version qualifier and rename the environment variable to set it from `BEAT_VERSION_QUALIFIER` to `VERSION_QUALIFIER` this will align with other parts of the stack.

**Tested with filebeat.**
```
 ❯ ./filebeat version                                                                                                                                                                                                                                                                                                                                          [08:39:01]
filebeat version 7.0.0 (amd64), libbeat 7.0.0 [0a0c267 built 2018-11-19 13:38:15 +0000 UTC]
```

**Without the patch**
```
 ❯ ./filebeat version                                                                                                                                                                                                                                                                                                                                          [08:40:07]
filebeat version 7.0.0-alpha1 (amd64), libbeat 7.0.0-alpha1 [b007837 built 2018-11-19 13:39:59 +0000 UTC]
```

Fixes: elastic#8384

* Add DeDot in add_docker_metadata processor

* Add dedot into config and default to be false

* Update changelog and documentation

* Add documentation into processors-using.asciidoc

* Run mage fmt update under x-pack filebeat

* Run mage fmt update on x-pack metricbeat

* Run update again

* Run make update fmt from top level dir

* Remove mistakes from rebase

* Remove repeated doc
DStape pushed a commit to DStape/beats that referenced this pull request Aug 20, 2019
@kaiyan-sheng
Cherry-pick to 6.x: Add DeDot in add_docker_metadata processor (elast…
2411730 
…ic#9505) (elastic#9602)

* Cherry-pick to 6.x: Add DeDot in add_docker_metadata processor (elastic#9505)

* Add DeDot in add_docker_metadata processor

* Add dedot into config and default to be false

* Update changelog and documentation

* Add documentation into processors-using.asciidoc

* Run mage fmt update under x-pack filebeat

* Run mage fmt update on x-pack metricbeat

* Run update again

* Run make update fmt from top level dir

* Remove mistakes from rebase

* Remove repeated doc

(cherry picked from commit 58573a9)

* remove extra line in data.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsoriano jsoriano jsoriano approved these changes

@ruflin ruflin ruflin left review comments

Assignees

@kaiyan-sheng kaiyan-sheng

Labels
backport libbeat :Processors review Team:Integrations Label for the Integrations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Metricbeat Docker Container metrics uses different field name format
5 participants
@kaiyan-sheng @ruflin @elasticmachine @jsoriano @ph