Additions to "file" #441

webmat · 2019-04-24T18:58:29Z

This PR addresses/improves a few things in the the "file" field set.

If any of these turns out controversial / complicated, I'll be happy to extract them to a separate PR. The goal here is to get the simple things in.

Added file.name (See add file.name #322)
Added file.directory
Added a lot of examples
Adjusted a few field definitions

Closes #322

neu5ron · 2019-04-24T19:02:48Z

BOOM!

ruflin · 2019-04-26T09:20:34Z

schemas/file.yml

+      Only relevant when `file.type` is "file".
+    example: 16384
+
+  - name: crtime


This is pretty cryptic for ECS standard I would say? What about created?

It could also be creationtime to keep in line with the *time naming schema we have going on.

The stat output calls it Birth (though always empty on Linux since it does not expose the creation time) - but I don't think that's a good name.

It's maybe unfortunate we already have ctime and mtime which are UNIX names that Windows does not have. ctime could be confused as the creation time. If we ever want to make a change we could go for more descriptive names, e.g. created|modified|changed|accessed.

Yes, I was specifically pondering this, as I was working on this PR.

The file field set didn't get much attention before 1.0.0, unfortunately. But I'm not a fan of the existing field names ctime and mtime, because they go against one of the principles we're aiming for with ECS. We try to avoid contractions and acronyms, unless the convention is overwhelmingly in favour of them (e.g. ip, pid).

Seeing that we already had both of these fields, I went with the other 2 standard field names. I went for internal consistency in file. But this is getting on my nerves even more, as we discuss this 😂

As @cwurm points out, ctime vs crtime are confusing. Any article about Linux filesystem timestamps (debugfs, stat) mention this common confusion.

So I'm very open to revisiting these field names. It's too late for now to change ctime and mtime, although we could create 4 new fields with less ambiguous names and mark the first two as deprecated for the life of ECS 1.x, and remove them in 2.0.

Proposed names:

file.created: creation time

file.accessed: last access time

file.modified (deprecates file.mtime): last modification time for the file content

file.metadata_modified (deprecates file.ctime): last modification time for the file metadata.

I'm not a big fan of this last one. Ideas welcome.

For now we can push this naming decision to a bit later, focus on the two new fields, and make a note to deprecate mtime/ctime.

ruflin · 2019-04-26T09:22:20Z

schemas/file.yml


  - name: device
    level: extended
    type: keyword
    description: Device that is the source of the file.
+    example: sda
+
+  - name: system


I'm wondering if this belongs here. file is information about a specific file. I see how it could fit it in but stumbled over it.

The other part I would not call it system as I expect us to have a system object in the future related to system metrics. system already exists today in Metricbeat. Alternative names?

file.filesystem?

I'm pretty strongly in favour of adding this field even though it's not information directly about a given file. It's still useful metadata about the place where a file lives.

I wanted to point to host as very general metadata about an event, but host is a field set, not a field inside one specific field set.

Perhaps we could do the same with filesystem information, and introduce a volume field set?

Volume fields:

name

filesystem

size (bytes)

used (bytes)

letter (Windows drive letter)

kind (ssd, spinning rust)

type (platform specific types, such as AWS' gp2, io1 etc)

id

serial_number
...

ruflin · 2019-04-26T09:23:02Z

schemas/file.yml

+    description: >
+      Last time the file attributes or metadata changed.
+
+  - name: atime


I guess these are the "standard" names that is why we use them here?

webmat · 2019-04-29T18:18:46Z

For now I'll take out file.system and the timestamps, so we can move forward with this PR.

I'll open a PR right away for the 2 timestamps with clearer names. I'd like to introduce the other 2 timestamps sooner than later and deprecate the two original ones. But this is less pressing. We just need to introduce the deprecation well in advance of ECS 2.0, which is quite a ways off in the future :-)

I'll make a note for the volume field set. We haven't had a pressing need for it yet, so it may take a bit longer before I get around to adding it as a field set. Although the more I think about it, the more I think it makes sense.

webmat · 2019-04-29T20:45:15Z

Thanks for the feedback!

I've taken out the new timestamps as well as file.system. Only non-controversial changes are left here, IMO.

Please have a quick look, to see if this PR is ready.

New, more readable timestamps proposed in #445, and #444 opens the discussion for volume (including volume.filesystem).

cwurm · 2019-04-30T17:11:21Z

schemas/file.yml


  - name: ctime
    level: extended
    type: date
-    description: Last time file metadata changed.
+    description: Last time the file attributes or metadata changed.


On Linux and macOS, ctime will also be updated when the file contents change (together with mtime).

I've pushed a documentation tweak about this. I find it my edit a bit clumsy, and I'm not 100% convinced this belongs in ECS.

In a sense this helps explain the relationship between two ECS fields, so I'm ok with it going in.

On the other hand, I feel like we're getting too close to documenting OS quirks & the like, which can be different from one platform to the next.

But in this case I'm ok with this addition. I'll merge tomorrow, unless there's disagreement.

@cwurm Any other objections about this PR? Does this documentation update work for you?

It's not the best prose, but we can always tweak improve it later if needed :-)

@cwurm Any other objections about this PR? Does this documentation update work for you?

Yes, LGTM

On the other hand, I feel like we're getting too close to documenting OS quirks & the like, which can be different from one platform to the next.

True, though I'm not sure we can avoid it entirely. Only UNIX has atime/mtime/ctime/crtime, Windows has only lpCreationTime/lpLastAccessTime/lpLastWriteTime (and lpLastAccessTime is not tracked by default).

So for Windows then, is lpLastWriteTime == mtime (modified in a future version?), with ctime empty (later changed?)? Or is lpLastWriteTime == mtime == ctime? The Auditbeat FIM module must be implementing this in some way, but I don't know how off the top of my head.

We had a situation where - mtime was the Content change, and - ctime was the Metadata change. This adjustment's goal is to avoid having people remember these fields with an incorrect mnemonic, where mtime=Metadata & ctime=Content. This is why I'm downplaying the usage of the word "metadata".

This reverts commit 117691c.

This reverts commit dc6b3c5.

This reverts commit 1022e82.

This reverts commit f122f6b.

…riptions

MikePaquette

LGTM.

ruflin

LGTM

@webmat General recommendation: Don't mix different things into PR which will get them merged much more quickly. The additions to file belong together, but all the changes to the docs I would consider not related. If there would have been 2 PR's the additional fields could have gone in much more quickly as an example.

webmat · 2019-05-10T13:42:41Z

@ruflin Agreed. I was just trying to streamline my time, before the month of May, where I have 3/4 weeks not on ECS :-)

webmat self-assigned this Apr 24, 2019

webmat requested review from ruflin and cwurm April 24, 2019 18:58

This was referenced Apr 24, 2019

add file.name #322

Closed

file creation and access time #422

Closed

File System #440

Open

webmat force-pushed the file-additions branch from d23bca4 to e80a20b Compare April 25, 2019 15:57

ruflin reviewed Apr 26, 2019

View reviewed changes

This was referenced Apr 29, 2019

Introduce a "volume" field set #444

Open

Add 2 missing timestamps to file #445

Merged

ruflin approved these changes Apr 30, 2019

View reviewed changes

cwurm reviewed Apr 30, 2019

View reviewed changes

Mathieu Martin added 15 commits May 1, 2019 10:58

Small description tweaks in file

bcd9d37

Add file.name 😅

9533f18

Add missing atime and crtime to file

6f4cbdf

Tweak file timestamp descriptions further

a39a71e

Add a bunch of examples to file fields

e8a0dae

Add file.system

61c7370

Add file.directory.

b1eec12

Changelog

f741983

Revert "Adjust the file.ctime description."

83aafc3

This reverts commit 117691c.

Revert "Add file.system"

5c41bca

This reverts commit dc6b3c5.

Revert "Tweak file timestamp descriptions further"

180d758

This reverts commit 1022e82.

Revert "Add missing atime and crtime to file"

2d558ab

This reverts commit f122f6b.

Update changelog for trimmed down PR

6b4f591

Re-introduce minor description tweaks to existing file timestamp desc…

ab78346

…riptions

Tweak ctime description to document a common OS quirk

b312872

webmat force-pushed the file-additions branch from 12aa9fc to b312872 Compare May 1, 2019 15:02

MikePaquette approved these changes May 1, 2019

View reviewed changes

cwurm approved these changes May 1, 2019

View reviewed changes

ruflin approved these changes May 2, 2019

View reviewed changes

webmat merged commit c185061 into elastic:master May 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Additions to "file" #441

Additions to "file" #441

webmat commented Apr 24, 2019 •

edited

Loading

neu5ron commented Apr 24, 2019

ruflin Apr 26, 2019

cwurm Apr 26, 2019

webmat Apr 29, 2019

ruflin Apr 26, 2019

cwurm Apr 26, 2019

webmat Apr 29, 2019 •

edited

Loading

ruflin Apr 26, 2019

webmat commented Apr 29, 2019 •

edited

Loading

webmat commented Apr 29, 2019 •

edited

Loading

cwurm Apr 30, 2019

webmat Apr 30, 2019 •

edited

Loading

webmat May 1, 2019

cwurm May 1, 2019

MikePaquette left a comment

ruflin left a comment

webmat commented May 10, 2019

Additions to "file" #441

Additions to "file" #441

Conversation

webmat commented Apr 24, 2019 • edited Loading

neu5ron commented Apr 24, 2019

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

webmat Apr 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webmat commented Apr 29, 2019 • edited Loading

webmat commented Apr 29, 2019 • edited Loading

Choose a reason for hiding this comment

webmat Apr 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikePaquette left a comment

Choose a reason for hiding this comment

ruflin left a comment

Choose a reason for hiding this comment

webmat commented May 10, 2019

webmat commented Apr 24, 2019 •

edited

Loading

webmat Apr 29, 2019 •

edited

Loading

webmat commented Apr 29, 2019 •

edited

Loading

webmat commented Apr 29, 2019 •

edited

Loading

webmat Apr 30, 2019 •

edited

Loading