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

Migrate add_docker_metadata to ECS #9412

Merged
merged 1 commit into from
Dec 27, 2018
Merged

Migrate add_docker_metadata to ECS #9412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Rename beat.timezone to event.timezone. {pull}9458[9458]
- Use _doc as document type. {pull}9056[9056]{pull}9573[9573]
- Update to Golang 1.11.3. {pull}9560[9560]
- Migrate add_docker_metadata fields to ECS.

*Auditbeat*

Expand Down
15 changes: 6 additions & 9 deletions auditbeat/docs/fields.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2670,30 +2670,27 @@ Docker stats collected from Docker.
*`docker.container.id`*::
+
--
type: keyword

Unique container id.
type: alias

alias to: container.id

--

*`docker.container.image`*::
+
--
type: keyword

Name of the image the container was built on.
type: alias

alias to: container.image.name

--

*`docker.container.name`*::
+
--
type: keyword

Container name.
type: alias

alias to: container.name

--

Expand Down
2 changes: 1 addition & 1 deletion auditbeat/include/fields.go
View file
Open in desktop

Large diffs are not rendered by default.

22 changes: 22 additions & 0 deletions dev-tools/ecs-migration.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,28 @@
alias6: true
alias: true

# Docker processor
- from: docker.container.id
to: container.id
alias: true
alias6: true

- from: docker.container.image
to: container.image.name
alias6: true
alias: true

- from: docker.container.name
to: container.name
alias6: true
alias: true

- from: docker.container.labels # TODO: How to map these?
Copy link
Member Author

Choose a reason for hiding this comment

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

@graphaelli Have you found a good way to migrate such fields?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we can. I'm assuming this is user-populated, including key names? If that's the case, then the list of fields to migrate is unknowable.

Perhaps if some fields are often present by default, we can create aliases for those, though.

Copy link
Member

@graphaelli graphaelli Dec 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I have not found a good way. Can the migration assistant create those aliases dynamically? That is, iterate over the source labels and create aliases on the old indices for each one? Another option is to inform users to include both fields with an or clause in searches and aggregations.

APM has the same issue where context.tags are moving to labels. It may be possible there to introduce an APM UI-only concept of "tags" "labels" and make the context.tags.$key=$value or labels.$key=value query on behalf of the user - we have not discussed this possibility but I'd be curious what you think.

Copy link
Member Author

Choose a reason for hiding this comment

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

@graphaelli Indeed that is something that the migration assistant could potentially do. I think in this case it's more important for apm-server as you probably need it for the 6.x compatiblity layer and in our case it's nice to have as we can also break it in 7 and inform the user about this one. If we can use then the same feature to, that is great.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think the migration assistant will be our best bet.

Copy link
Member Author

Choose a reason for hiding this comment

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

@graphaelli Can you take this up with the Kibana team working on the migration assistant to see what they think?

Copy link
Member

Choose a reason for hiding this comment

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

They've added it to their use cases for the migration assistant, there will be hooks for us to handle it there.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great news. Thanks for following up.

to: container.labels
alias6: false
alias: true


# Filebeat modules

## Suricata module
Expand Down
15 changes: 6 additions & 9 deletions filebeat/docs/fields.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -696,30 +696,27 @@ Docker stats collected from Docker.
*`docker.container.id`*::
+
--
type: keyword

Unique container id.
type: alias

alias to: container.id

--

*`docker.container.image`*::
+
--
type: keyword

Name of the image the container was built on.
type: alias

alias to: container.image.name

--

*`docker.container.name`*::
+
--
type: keyword

Container name.
type: alias

alias to: container.name

--

Expand Down
2 changes: 1 addition & 1 deletion filebeat/include/fields.go
View file
Open in desktop

Large diffs are not rendered by default.

15 changes: 6 additions & 9 deletions heartbeat/docs/fields.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,30 +261,27 @@ Docker stats collected from Docker.
*`docker.container.id`*::
+
--
type: keyword

Unique container id.
type: alias

alias to: container.id

--

*`docker.container.image`*::
+
--
type: keyword

Name of the image the container was built on.
type: alias

alias to: container.image.name

--

*`docker.container.name`*::
+
--
type: keyword

Container name.
type: alias

alias to: container.name

--

Expand Down
2 changes: 1 addition & 1 deletion heartbeat/include/fields.go
View file
Open in desktop

Large diffs are not rendered by default.

15 changes: 6 additions & 9 deletions journalbeat/docs/fields.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,30 +542,27 @@ Docker stats collected from Docker.
*`docker.container.id`*::
+
--
type: keyword

Unique container id.
type: alias

alias to: container.id

--

*`docker.container.image`*::
+
--
type: keyword

Name of the image the container was built on.
type: alias

alias to: container.image.name

--

*`docker.container.name`*::
+
--
type: keyword

Container name.
type: alias

alias to: container.name

--

Expand Down
2 changes: 1 addition & 1 deletion journalbeat/include/fields.go
View file
Open in desktop

Large diffs are not rendered by default.

23 changes: 13 additions & 10 deletions libbeat/processors/add_docker_metadata/_meta/fields.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,21 @@
type: group
fields:
- name: container.id
type: keyword
description: >
Unique container id.
type: alias
path: container.id
migration: true

- name: container.image
type: keyword
description: >
Name of the image the container was built on.
type: alias
path: container.image.name
migration: true
webmat marked this conversation as resolved.
Show resolved Hide resolved

- name: container.name
type: keyword
description: >
Container name.
- name: container.labels
type: alias
path: container.name
migration: true

- name: container.labels # TODO: How to map these?
type: object
object_type: keyword
description: >
Expand Down
12 changes: 4 additions & 8 deletions libbeat/processors/add_docker_metadata/add_docker_metadata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import (

const (
processorName = "add_docker_metadata"
dockerContainerIDKey = "docker.container.id"
dockerContainerIDKey = "container.id"
cgroupCacheExpiration = 5 * time.Minute
)

Expand Down Expand Up @@ -89,7 +89,7 @@ func buildDockerMetadataProcessor(cfg *common.Config, watcherConstructor docker.
"field": "source",
"separator": string(os.PathSeparator),
"index": config.SourceIndex,
"target": "docker.container.id",
"target": dockerContainerIDKey,
})
sourceProcessor, err = actions.NewExtractField(procConf)
if err != nil {
Expand Down Expand Up @@ -168,10 +168,6 @@ func (d *addDockerMetadata) Run(event *beat.Event) (*beat.Event, error) {
container := d.watcher.Container(cid)
if container != nil {
meta := common.MapStr{}
metaIface, ok := event.Fields["docker"]
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove this one as I felt it's not needed anymore when using Put with the DeepUpdate. Please double check if that is correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

while that's true, I think that will create more unneeded allocations? this is the kind of code that runs lots of times

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I can fully follow here. You are mostly worried about the DeepUpdate part?

Copy link
Member Author

Choose a reason for hiding this comment

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

Going to merge for now. @exekias Let's revisit this next year to make sure we have the correct optimisations in place.

if ok {
meta = metaIface.(common.MapStr)
}

if len(container.Labels) > 0 {
labels := common.MapStr{}
Expand All @@ -187,9 +183,9 @@ func (d *addDockerMetadata) Run(event *beat.Event) (*beat.Event, error) {
}

meta.Put("container.id", container.ID)
meta.Put("container.image", container.Image)
meta.Put("container.image.name", container.Image)
meta.Put("container.name", container.Name)
event.Fields["docker"] = meta.Clone()
event.Fields.DeepUpdate(meta.Clone())
} else {
d.log.Debugf("Container not found: cid=%s", cid)
}
Expand Down
82 changes: 41 additions & 41 deletions libbeat/processors/add_docker_metadata/add_docker_metadata_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,21 +129,21 @@ func TestMatchContainer(t *testing.T) {
assert.NoError(t, err, "processing an event")

assert.EqualValues(t, common.MapStr{
"docker": common.MapStr{
"container": common.MapStr{
"id": "container_id",
"image": "image",
"labels": common.MapStr{
"a": common.MapStr{
"x": "1",
},
"b": common.MapStr{
"value": "2",
"foo": "3",
},
"container": common.MapStr{
"id": "container_id",
"image": common.MapStr{
"name": "image",
},
"labels": common.MapStr{
"a": common.MapStr{
"x": "1",
},
"b": common.MapStr{
"value": "2",
"foo": "3",
},
"name": "name",
},
"name": "name",
},
"foo": "container_id",
}, result.Fields)
Expand Down Expand Up @@ -178,17 +178,17 @@ func TestMatchContainerWithDedot(t *testing.T) {
assert.NoError(t, err, "processing an event")

assert.EqualValues(t, common.MapStr{
"docker": common.MapStr{
"container": common.MapStr{
"id": "container_id",
"image": "image",
"labels": common.MapStr{
"a_x": "1",
"b": "2",
"b_foo": "3",
},
"name": "name",
"container": common.MapStr{
"id": "container_id",
"image": common.MapStr{
"name": "image",
},
"labels": common.MapStr{
"a_x": "1",
"b": "2",
"b_foo": "3",
},
"name": "name",
},
"foo": "container_id",
}, result.Fields)
Expand Down Expand Up @@ -228,16 +228,16 @@ func TestMatchSource(t *testing.T) {
assert.NoError(t, err, "processing an event")

assert.EqualValues(t, common.MapStr{
"docker": common.MapStr{
"container": common.MapStr{
"id": "FABADA",
"image": "image",
"labels": common.MapStr{
"a": "1",
"b": "2",
},
"name": "name",
"container": common.MapStr{
"id": "FABADA",
"image": common.MapStr{
"name": "image",
},
"labels": common.MapStr{
"a": "1",
"b": "2",
},
"name": "name",
},
"source": inputSource,
}, result.Fields)
Expand Down Expand Up @@ -291,16 +291,16 @@ func TestMatchPIDs(t *testing.T) {
assert.NoError(t, err, "initializing add_docker_metadata processor")

dockerMetadata := common.MapStr{
"docker": common.MapStr{
"container": common.MapStr{
"id": "FABADA",
"image": "image",
"labels": common.MapStr{
"a": "1",
"b": "2",
},
"name": "name",
"container": common.MapStr{
"id": "FABADA",
"image": common.MapStr{
"name": "image",
},
"labels": common.MapStr{
"a": "1",
"b": "2",
},
"name": "name",
},
}

Expand Down
Loading