-
Notifications
You must be signed in to change notification settings - Fork 267
Refocus of os types #1257 #2423
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
base: main
Are you sure you want to change the base?
Refocus of os types #1257 #2423
Conversation
efadeb6
to
014141a
Compare
014141a
to
44f6636
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking as to this is not simply adding more OS types. It is changing the behavior of the attribute entirely.
The links you added to the issue description, for example from ECS https://www.elastic.co/docs/reference/ecs/ecs-os#field-os-type also have a os.type
which is pretty much the same as what we have today.
61a508e
to
59ff97c
Compare
@joaopgrassi can you review and unblock as implementation is following what was discussed in the semconv. |
Unblocking, but we still need to get the system semconv group onboard with this change.
model/os/registry.yaml
Outdated
- id: zos | ||
value: 'zos' | ||
brief: "IBM z/OS" | ||
deprecated: "Use `os.name` instead" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zos
is a real unique operating system type, I don't think we should be removing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my research it is a Unix os.
brief: "Linux" | ||
deprecated: Should use `unix` instead' | ||
stability: development | ||
- id: unix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sold on putting all unixes together. I think there is still value in separating linux
, darwin
, and bsd
at least. While it's true they are all unix-likes, they are unique enough and are treated differently enough in the minds of most users to deserve their own attributes. I think moving all the *bsd
attributes to os.name
is good so we can keep that, but I think this should restore linux
, darwin
, and bsd
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So looking at implementations and language api's the common functionality is being able to determine the OS type which is what is being expressed by this attribute. You can then look at the os.name &/or os.family to get that specific info, you are after.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the previous state is more useful than just a 'unix' value. It may be too specific today, but I don't think we should jump to the other extreme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have os.name/os.type to be more specific. And in fact this is what I was told to implement in the sem conv meeting as I had originally extended type to include alot of other common os but that was not wanted due to it becoming a never ending list. At the same time we also needing to migrate the other properties from ecs, hence the request was to focus type on the minimal items and provide the other properties.
Also this approach here would nicely complement the solution I have proposed in #66 as if type is Unix, then you can also provide details of the Unix.kernel which could be Linux, Darwin etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To further support the simplification proposed here #2641 has now been created to express the kernel type ie linux, Darwin.
That pr provides another way to maintain the same granularity while introducing consistent attribute usage and further migration of ecs.
brief: "Linux" | ||
deprecated: Should use `unix` instead' | ||
stability: development | ||
- id: unix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the previous state is more useful than just a 'unix' value. It may be too specific today, but I don't think we should jump to the other extreme.
de5172f
to
71dc443
Compare
Signed-off-by: James Thompson <[email protected]>
Signed-off-by: James Thompson <[email protected]>
71dc443
to
bdb46dc
Compare
2ccdf52
to
cf0a252
Compare
169e5b6
to
8023a56
Compare
Closes #1257
Changes
Deprecate all existing os types and instead use the field to describe the type of the OS ie windows or Unix. The existing types should be the
os.name
and/orunix.kernel.name
with the later handled via #2641Note: if the PR is touching an area that is not listed in the existing areas, or the area does not have sufficient domain experts coverage, the PR might be tagged as experts needed and move slowly until experts are identified.
Merge requirement checklist
[chore]