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

Adding risk.* as beta #2051

Merged
merged 5 commits into from
Sep 20, 2022
Merged

Adding risk.* as beta #2051

merged 5 commits into from
Sep 20, 2022

Conversation

kgeller
Copy link
Contributor

@kgeller kgeller commented Sep 15, 2022

No description provided.

@kgeller kgeller marked this pull request as ready for review September 15, 2022 17:54
@kgeller kgeller requested a review from a team as a code owner September 15, 2022 17:54
@ajosh0504
Copy link
Contributor

Overall LGTM- I admittedly don't understand enough about the process. Some high-level q's:

  • Any reason why the fields should still be included under experimental/generated if they will no longer be experimental?
  • I also didn't see the risk fields being added for host in the experimental directory- was this intentional?

@kgeller
Copy link
Contributor Author

kgeller commented Sep 19, 2022

@ajosh0504

  • Any reason why the fields should still be included under experimental/generated if they will no longer be experimental?

The experimental section is the actual experimental fields + all the GA & beta fields.

  • I also didn't see the risk fields being added for host in the experimental directory- was this intentional?

I think this is just Github not recognizing it, I see them there

8.6.0-dev+exp,true,host,host.risk.calculated_level,keyword,extended,,High,A risk classification level calculated by an internal system as part of entity analytics and entity risk scoring.

@ajosh0504
Copy link
Contributor

@kgeller In that case, this LGTM!

@MikePaquette
Copy link
Contributor

Thanks @kgeller for creating this Pull request.

Could we enhance the description a bit to help users avoid confusion with the event.risk_score and event.risk_score_norm fields?
Maybe by updating the title, short, and description as follows:

  1. title: Risk information
  2. short: Fields for describing risk score and level.
  3. description: > Fields for describing risk score and risk level of entities such as hosts and users. These fields are not allowed to be nested under event.*. Please continue to use event.risk_score and event.risk_score_norm for event risk.

@kgeller
Copy link
Contributor Author

kgeller commented Sep 20, 2022

@MikePaquette updated!

@ajosh0504
Copy link
Contributor

@kgeller I will also clarify this in the Stage 3 PR as per your request.

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.

LGTM, thanks!

@kgeller kgeller merged commit 5ac642b into elastic:main Sep 20, 2022
@kgeller kgeller deleted the risk-beta branch September 20, 2022 15:36
kgeller added a commit to kgeller/ecs that referenced this pull request Sep 20, 2022
# Conflicts:
#	experimental/generated/csv/fields.csv
#	generated/csv/fields.csv
kgeller added a commit that referenced this pull request Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants