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.

Sign up for GitHub

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

Jump to bottom

Collect Events.* fields for telemetry #107976

Merged
merged 5 commits into from
Aug 17, 2021
Merged

Collect Events.* fields for telemetry #107976

merged 5 commits into from
Aug 17, 2021

Conversation

rw-access
Copy link
Contributor

Summary

Collect Events.* fields alongside events.*.

We're switching this field for 7.15, because it's an Endpoint field that's not ECS, so this is historically our way of handling that to avoid potentially breaking issues down the line.

Checklist

Delete any items that are not applicable to this PR.

@rw-access rw-access added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.15.0 v7.16.0 labels Aug 9, 2021
@rw-access rw-access requested a review from a team as a code owner August 9, 2021 21:54
@MikePaquette
Copy link

@rw-access while this approach (using capital E Events.*) is technically consistent with the ECS Custom Field guidance on Modeling to Reduce the Chance of Conflict - Capitalization, it could cause confusion in the future for users who are not aware of this distinction, and mistakenly think the Event.* fields are ECS fields.

Is there another custom fieldset name we could use that would cause less potential confusion?

@rw-access
Copy link
Contributor Author

Hey @MikePaquette in my opinion, I think a new field name would actually cause more confusion. I'd rather avoid making up a new word, and right now since these alerts are EQL-based under the hood, we have consistency with the Elasticsearch EQL API response (see events under sequences).

jeska
jeska reviewed Aug 16, 2021
View reviewed changes
x-pack/plugins/security_solution/server/lib/telemetry/sender.ts
@@ -464,6 +464,7 @@ const allowlistProcessFields: AllowlistFields = {
args: true,
name: true,
executable: true,
code_signature: true,
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding this!

jeska
jeska approved these changes Aug 16, 2021
View reviewed changes
Copy link
Member

@jeska jeska left a comment

Choose a reason for hiding this comment

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

🚢 :shipit: ⚓ 🚀 📦

@rw-access rw-access enabled auto-merge (squash) August 16, 2021 18:57
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

pjhampton
pjhampton reviewed Aug 17, 2021
View reviewed changes
x-pack/plugins/security_solution/server/lib/telemetry/sender.ts
events: allowlistBaseEventFields,
// behavioral protection re-nests some field sets under Events.* (>=7.15)
Events: allowlistBaseEventFields,
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good. Please help out with data engineering by keeping the data dictionary up to date in our pro cloud repo.

pjhampton
pjhampton approved these changes Aug 17, 2021
View reviewed changes
Copy link
Contributor

@pjhampton pjhampton left a comment

Choose a reason for hiding this comment

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

🌔 🚀 ✨ LGTM ✨ 🚀 🌔

@rw-access rw-access merged commit 0eae57b into elastic:master Aug 17, 2021
@pjhampton pjhampton added the auto-backport Deprecated - use backport:version if exact versions are needed label Aug 17, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 17, 2021
@rw-access @kibanamachine
Collect Events.* fields for telemetry (elastic#107976)
dc0f54c 
* Collect Events.* fields for telemetry
* Add process.code_signature
@kibanamachine kibanamachine mentioned this pull request Aug 17, 2021
Merged
@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
✅ 7.x
❌ 7.16 The branch "7.16" is invalid or doesn't exist

Successful backport PRs will be merged automatically after passing CI.

To backport manually run:
node scripts/backport --pr 107976

@pjhampton
Copy link
Contributor

That's fine RE 7.16 branch - it hasn't been cut yet.

kibanamachine added a commit that referenced this pull request Aug 17, 2021
@kibanamachine @rw-access
Collect Events.* fields for telemetry (#107976) (#108861)
55db4df 
* Collect Events.* fields for telemetry
* Add process.code_signature

Co-authored-by: Ross Wolf <31489089+rw-access@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeska jeska jeska approved these changes

@pjhampton pjhampton pjhampton approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.15.0 v7.16.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rw-access @MikePaquette @kibanamachine @pjhampton @jeska