Migrate system process metricset fields to ECS by jsoriano · Pull Request #10332 · elastic/beats

jsoriano · 2019-01-24T19:24:11Z

No description provided.

jsoriano · 2019-01-24T19:25:24Z

metricbeat/module/system/process/_meta/data.json

-            "ppid": 40547,
-            "state": "running",
-            "username": "ruflin"
+            "pgid": 2080,


It can be weird to find pgid here but the rest of ids in process.*, should we add this to ECS?

+1. even if it's not in ECS, we can push it under process.*.

@webmat FYI

To what fields file should I add this field? To the ECS one even if it is not in ECS yet?

I have added it by now to field.ecs.yml and opened a PR in ECS elastic/ecs#311

jsoriano · 2019-01-24T23:26:13Z

CI failure not related, reproduced locally and it seems related to latest ES snapshot:

{"type":"mapper_parsing_exception","reason":"failed to parse field [suricata.eve.flow.start] of type [date]","caused_by":{"type":"illegal_argument_exception","reason":"failed to parse date field [2018-10-03T14:42:44.613469+0000] with format [strict_date_optional_time||epoch_millis]","caused_by":{"type":"date_time_parse_exception","reason":"Text '2018-10-03T14:42:44.613469+0000' could not be parsed, unparsed text found at index 26"}}

ruflin · 2019-01-25T14:28:37Z

metricbeat/module/system/process/_meta/data.json

Nice data.json update.

metricbeat/module/system/process/process.go

libbeat/metric/system/process/process.go

webmat

Mostly commented about the group ID

webmat · 2019-01-25T20:49:32Z

auditbeat/docs/fields.asciidoc


+--
+
+*`process.pgid`*::


As proposed in your ECS PR, I would go for process.group.id here (and process.user.id if you have that)

Actually seeing the full context, I wonder if we shouldn't simply map to the top level fields, like you did for user.name. So:

system.process.username => user.name

system.process.pgid => group.id

And equivalent mappings, if you have user id and group name.

WDYT @ruflin ?

I'd say unless the events can contain more than one user or group, we shouldn't nest them.

As commented in elastic/ecs#311, the pgid identifies a group of processes, not a group of users. It is a value similar to ppid, I think it belongs to process.

webmat · 2019-01-25T20:54:30Z

metricbeat/module/system/process/_meta/data.json

+        ],
+        "executable": "/usr/bin/dockerd-ce",
+        "name": "dockerd",
+        "pgid": 2080,


And don't forget to leave group ID as text (wherever it ends up)

By text I meant keyword datatype, here

I agree that these ids could be keyword, but this is a value of the same kind as pid or ppid and they are defined as long in ECS 🤔

Gotcha, I was not familiar with pgid. If the semantics are like PID, this can be left as numeric, then.

So recap: I'm good with the field process.pgid, and I'm good with it remaining numeric 👍

libbeat/_meta/fields.ecs.yml

ruflin · 2019-01-28T09:07:52Z

libbeat/metric/system/process/process.go

-		Cwd:      exe.Cwd,
-		Env:      env,
+		Pid:        pid,
+		Ppid:       state.Ppid,


Ah I see, we also have it as part of the monitoring reporting?

Yes, I think this is also used for beats monitoring.

jsoriano · 2019-01-29T13:27:31Z

pgid moved to metricbeat only fields

webmat

I'm good with the pgid as it is now. I wasn't familiar with the concept.

Noticed that cwd migration is missing an alias in fields.yml and ecs-migration.yml. After that, I think we're good :-)

webmat · 2019-01-29T15:54:46Z

metricbeat/module/system/process/_meta/data.json

+        ],
+        "executable": "/usr/bin/dockerd-ce",
+        "name": "dockerd",
+        "pgid": 2080,


Gotcha, I was not familiar with pgid. If the semantics are like PID, this can be left as numeric, then.

So recap: I'm good with the field process.pgid, and I'm good with it remaining numeric 👍

webmat · 2019-01-29T15:55:57Z

metricbeat/module/system/process/_meta/fields.yml

+      type: alias
+      path: user.name
+      migration: true
    - name: cwd


cwd should be migrated to process.working_directory

Oh, good catch, thanks.

webmat · 2019-01-29T15:57:04Z

metricbeat/tests/system/test_processors.py

+            "process.pid",
        ]:
-            assert key in output
+            assert key in output, "'%s' not found" % key


❤️ more verbose failures ;-)

webmat

LGTM

jsoriano added 2 commits January 24, 2019 20:21

Migrate fields from process system module to ECS

02e7714

pgid not in ECS

d1baaa6

jsoriano added module review Metricbeat Metricbeat ecs breaking change labels Jan 24, 2019

jsoriano self-assigned this Jan 24, 2019

jsoriano requested review from ruflin and webmat January 24, 2019 19:24

jsoriano requested review from a team as code owners January 24, 2019 19:24

jsoriano commented Jan 24, 2019

View reviewed changes

jsoriano added 3 commits January 24, 2019 20:49

Get args from gosigar instead of splitting command line

467db0c

Collect executable too

40648de

Delete duplicated args

81d275d

jsoriano force-pushed the mb-system-process-ecs branch from 5f8dea6 to 81d275d Compare January 24, 2019 19:52

Revert leftover

f005d88

ruflin mentioned this pull request Jan 24, 2019

Transition Beats to ECS #8655

Closed

jsoriano added 2 commits January 24, 2019 23:21

Fix test

8f6289e

Merge remote-tracking branch 'origin/master' into mb-system-process-ecs

264609c

jsoriano added the Team:Integrations Label for the Integrations team label Jan 24, 2019

Merge remote-tracking branch 'origin/master' into mb-system-process-ecs

7cca1f8

ruflin reviewed Jan 25, 2019

View reviewed changes

jsoriano added 5 commits January 25, 2019 19:08

Add note about new fields to changelog

5ba4d06

More explicit get and remove from process events

e2ab4d8

Move system.process.pgid to process.pgid

70bbb65

Merge remote-tracking branch 'origin/master' into mb-system-process-ecs

99038de

Fix test

95eb36c

jsoriano force-pushed the mb-system-process-ecs branch from 5490e3a to 95eb36c Compare January 25, 2019 18:18

Update all beats

5dc80c1

jsoriano requested a review from a team as a code owner January 25, 2019 18:58

webmat reviewed Jan 25, 2019

View reviewed changes

ruflin reviewed Jan 28, 2019

View reviewed changes

ruflin mentioned this pull request Jan 28, 2019

Move Metricbeat module fields to ECS #10218

Closed

jsoriano added 4 commits January 29, 2019 14:18

Move process.pgid to metricbeat by now

afde078

Merge remote-tracking branch 'origin/master' into mb-system-process-ecs

1776690

Add pgid to ecs migration file

040d2db

Remove unneeded change

7253946

webmat suggested changes Jan 29, 2019

View reviewed changes

jsoriano added 2 commits January 29, 2019 17:39

Added missing migration aliases for process working directory

18e3625

Reorder fields

30d4933

webmat approved these changes Jan 30, 2019

View reviewed changes

jsoriano merged commit 5f90c08 into elastic:master Jan 31, 2019

jsoriano deleted the mb-system-process-ecs branch January 31, 2019 00:50


		--

		`process.pgid`::

Conversation

jsoriano commented Jan 24, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsoriano commented Jan 24, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

webmat left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsoriano commented Jan 29, 2019

Uh oh!

webmat left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webmat left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants