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.

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

Add os.full, to allow capturing full OS name string, including version #259

Merged
merged 6 commits into from
Dec 11, 2018

Conversation

webmat
Copy link
Contributor

@webmat webmat commented Dec 7, 2018

I think having a field with the full OS name (including version) is a useful concept, in addition to the broken up name + version fields. It makes some types of analysis much simpler.

I was inspired to add this when I realized that our user agent parser gives us such a string, and ECS doesn't have a home for it.

@@ -24,7 +24,14 @@
type: keyword
example: "Mac OS X"
description: >
Operating system name.
Operating system name, without the version.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think an OS name is always without the version? I see the context you are coming from but I don't think we should mention that here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The user agent parser gives us the following. Check out os:

          "user_agent" : {
            "patch" : "3538",
            "major" : "70",
            "minor" : "0",
            "os" : "Mac OS X 10.14.0",
            "os_minor" : "14",
            "os_major" : "10",
            "name" : "Chrome",
            "os_name" : "Mac OS X",
            "device" : "Other"
          }

I do think that's useful to have this value to compare overall OS usage. The broken down os name and os version in two fields is useful for broader analysis (os name) and then filtering down one OS at a time to see the version breakdowns. But it's not obvious how to use these two fields to get a sense of the overall usage of each specific OS version, all at the same time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be that there is first a filter on the OS and then an aggregation on the version to get a correct result?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you want to look at the breakdown within one OS, yes, filtering first works, and then the versions you're looking at are consistent and make sense within the context of that OS.

If you want to look at the overall "population" however (e.g. looking at all the OSes accessing your website), then having the full name with version is helpful.

Then you can see a ranking like:

  • Mac OS X 10.12: 43345
  • Windows 10: 40300
  • Mac OS X 10.11: 20000
  • Windows 7: 12000

and so on...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think an OS name is always without the version? I see the context you are coming from but I don't think we should mention that here.

Actually this brings up an interesting case to mind. Do people consider "Windows 7" to be the OS name, or is it "Windows"? I think if we don't keep this clarification, we'll get "Windows 7" and "Windows 10" in this field (just without the minor & patch versions).

schemas/os.yml Outdated
Operating system name.
Operating system name, without the version.

- name: full_name
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that really the full_name. This looks to strongly inspired by user.full_name but I don't think it's the same. Otherwise it would be ruflin 18 ;-)

Do we need this in ECS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open to suggestions for other names. Could be os.description, perhaps. Or just os.full. I want the concept, but I have no strong attachment to the name full_name, I agree it's not perfect.

Copy link
Contributor

Choose a reason for hiding this comment

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

I definitively prefer os.full over os.full_name. os.description would be alternative but full still sounds better here. @andrewkroh perhaps some ideas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to .full. I'm good with that.

I think full_name was a bit more clear on what it was, but I can see your point vs user.full_name. There's some patterns we can use in many places (like the customizable .name), but I wouldn't have put .full_name in those patterns, so I didn't really see a conflict there :-)

@webmat webmat self-assigned this Dec 10, 2018
Copy link
Contributor

@MikePaquette MikePaquette left a comment

Choose a reason for hiding this comment

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

I am OK with either os.full or os.full_name

@webmat webmat changed the title Add os.full_name, to allow capturing full OS name string. Add os.full, to allow capturing full OS name string. Dec 11, 2018
@webmat webmat changed the title Add os.full, to allow capturing full OS name string. Add os.full, to allow capturing full OS name string, including version Dec 11, 2018
- Update example with a 'code name' instead
- Use the real Mojave version in the `os.version` example, to keep the examples consistent
@webmat webmat merged commit e50891b into elastic:master Dec 11, 2018
@webmat webmat deleted the os-full-name branch December 11, 2018 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants