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

Improvements to Operating System Patch State 5004 #1088

Merged
merged 24 commits into from
Jun 19, 2024

Conversation

jasonbreimer
Copy link
Contributor

@jasonbreimer jasonbreimer commented May 20, 2024

###Updated 6/7

Description of changes: A number of attributes to improve Patch State. Mean time is a commonly used metric to evaluate patch efficacy and is captured in new attributes for duration_avg_, is_installed is a new attribute that describes the installation state, and is_installed_id the normalized installation state of the kb_article, and reboot_time to capture the reboot or true install time for reboot dependent patches.

New dictionary attributes
attributes: duration_avg_
Examples: duration_avg_days, duration_avg_hours, duration_avg_msecs, duration_avg_mins, duration_avg_months, duration_avg_secs, duration_avg_weeks, and duration_avg_years
description: This captures the average time. This will be used for "mean time to patch" or MTTP.
Object: kb_article
Note: this could be set per kb_article or per kb_article array for a total "MTTP"

attribute: install_state
description: the installation state
Object: kb_article
Note: This could be for a single kb_article or array of many kb_articles.

attribute: install_state_id
description: the normalized installation state
Object: kb_article
Note: Includes installed, not installed, installed pending reboot, other, and unknown. This could be for a single kb_article or array of many kb_articles.

attribute: boot_time
description: This captures the boot time of the system.
Object: device
Note: This is a neat one and very related to patching.

Partially verified

This commit is signed with the committer’s verified signature.
targos’s contribution has been verified via GPG key.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
Add is_installed, reboot_time, and mean_time

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
Add "is_installed" to allow data showing missing and/or installed KB's.

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
added reboot time to device object for use in Class 5004

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
fixed "." punctuation in reboot_time

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
Add mean time to patch

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
@jasonbreimer jasonbreimer added enhancement New feature or request discovery Issues related to Discovery Category labels May 20, 2024
@jasonbreimer
Copy link
Contributor Author

This has passed local elixir validation by author.

update mean_time description at Paul's suggestion

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
@jasonbreimer
Copy link
Contributor Author

I made a small modification to the mean_time description at Paul's request.

dictionary.json Outdated Show resolved Hide resolved
dictionary.json Outdated Show resolved Hide resolved
objects/kb_article.json Outdated Show resolved Hide resolved
@davemcatcisco
Copy link
Contributor

Just realised I've been commenting on PRs using my personal GitHub account @davemcincork. It's me. Honest!

update dictionary mean_time attribute description and type.

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
Update the attribute mean_time description for clarity.

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
@jasonbreimer
Copy link
Contributor Author

I have made all adjustments as recommended by @davemcincork. Thanks Dave!

dictionary.json Outdated Show resolved Hide resolved
@jasonbreimer
Copy link
Contributor Author

Group suggestion is to use duration rather than mean_time.

@jasonbreimer
Copy link
Contributor Author

Looking at the description:
The event duration or aggregate time, the amount of time the event covers from start_time to end_time in milliseconds.

We don't always have start_time or end_time.
This is also meant to an average calculated value.

It seems like a mean time value and a duration value are different?

Perhaps duration could be modified by I'm still in favor of keeping a new mean_time attribute as it seems a better fit.

update description for is_installed, "see specific usage".

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
@jasonbreimer
Copy link
Contributor Author

Another quick concern from the group regarded using integer_t for mean_time and limit to seconds only? Perhaps this would not allow usages when time value is milliseconds?

@jasonbreimer
Copy link
Contributor Author

jasonbreimer commented May 28, 2024

I think this may have been the concern regarding limits in duration time with RFC-3339:

note: nothing below dur-second.

Durations:

dur-second = 1DIGIT "S"
dur-minute = 1
DIGIT "M" [dur-second]
dur-hour = 1DIGIT "H" [dur-minute]
dur-time = "T" (dur-hour / dur-minute / dur-second)
dur-day = 1
DIGIT "D"
dur-week = 1DIGIT "W"
dur-month = 1
DIGIT "M" [dur-day]
dur-year = 1*DIGIT "Y" [dur-month]
dur-date = (dur-day / dur-month / dur-year) [dur-time]

duration = "P" (dur-date / dur-time / dur-week)

Replace mean_time with duration_avg_<time> attributes. There are new 8 attributes that capture average duration in milliseconds, seconds, minutes, hours, days, weeks, months, years. 

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
Replace mean_time with duration_avg_days, duration_avg_hours, and duration_avg_mins.

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
Edit the is_installed attribute to add is_installed_id to capture several options such as unknown, other, installed, not installed, installed pending reboot.

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
The dictionary item for is_installed has been modified. This change allows a normalized "id" to be used for the installation state. This allows more granularity. 

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
Change case sensitivity to align with other attributes. 

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
Add additional descriptions for is_installed_id values.

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
@jasonbreimer
Copy link
Contributor Author

I believe I have resolved all issues and requests!

This change now contains new dictionary items for Average Duration. There are 8 new attributes for duration_avg_days, duration_avg_hours, duration_avg_msecs, duration_avg_mins, duration_avg_months, duration_avg_secs, duration_avg_weeks, and duration_avg_years. This replaces the "average_time" or "mean_time" attribute. This should capture a number of duration unit types and allow these to be used more generally.

This change also includes is_installed_id a normalized enum that includes installed, not installed, installed pending reboot, other, and unknown. This is also intended to allow the is_installed attribute to be more granular.

@jasonbreimer
Copy link
Contributor Author

I have run this branch through local elixir validation without errors. No issues detected.

renamed is_installed and is_installed_id to installed_state and installed_state_id

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
Dictionary changes for is_installed to installed_state.

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
Small language changes to installed_state and installed_state_id.

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
Small language changes to installed_state and installed_state_id to match dictionary. 

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
@jasonbreimer
Copy link
Contributor Author

I've made changes discussed in 6/4 call for renaming is_installed and is_installed_id to installed_state and installed_state_id.

Changes include: Dictionary rename changes and kb_article object attribute rename language changes.

@jasonbreimer
Copy link
Contributor Author

I have re-run local elixir validation without issue.

dictionary.json Outdated Show resolved Hide resolved
dictionary.json Outdated Show resolved Hide resolved
objects/device.json Outdated Show resolved Hide resolved
@jasonbreimer
Copy link
Contributor Author

I'll make these changes but going to need to make them final so that I can make some progress. I actually started with install_state but was suggested to change to installed_state. I also started with boot_time and was suggested reboot_time.

Rename attributes install_state and install_state_id as well as boot_time.

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
Renamed attribute boot_time.

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
Renamed attributes install_state and install_state_id.

Signed-off-by: Jason Reimer <jason.reimer@tanium.com>
@jasonbreimer
Copy link
Contributor Author

Suggested changes regarding attribute naming have been completed.

Changes pass local elixir validation.

This PR is ready for review.

@pagbabian-splunk
Copy link
Contributor

Yes, let's get this in now, and if we decide the duration_xx set deserves an object with an enum discriminator, we can always update before 1.3.

Copy link
Contributor

@davemcatcisco davemcatcisco left a comment

Choose a reason for hiding this comment

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

Thanks for making those naming changes.

@davemcatcisco
Copy link
Contributor

I'll make these changes but going to need to make them final so that I can make some progress. I actually started with install_state but was suggested to change to installed_state. I also started with boot_time and was suggested reboot_time.

Oh sorry...I would have discussed with the person who requested those changes if I had known that. But thanks for making them.

@mikeradka mikeradka merged commit bb1faf7 into ocsf:main Jun 19, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discovery Issues related to Discovery Category enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants