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

[SIEM] [Detections] Renames Signals to Alerts #67731

Merged
merged 7 commits into from
Jun 3, 2020
Merged

[SIEM] [Detections] Renames Signals to Alerts #67731

merged 7 commits into from
Jun 3, 2020

Conversation

spong
Copy link
Member

@spong spong commented May 29, 2020 •
edited
Loading

Summary

Resolves #65944

Renames Signals -> Alerts on the main Detection Engine page. Including:

  • Timeline Event Selector
  • Alerts Histogram
  • Alerts Table

Does not include:

  • Detections -> Alerts navigation rename
  • SignalsByCategory rename as there already exists an AlertsByCategory, verify changing to ExternalAlertsByCategory
  • Anything server-side or related to siemSignalsIndex

Checklist

@spong spong requested review from a team as code owners May 29, 2020 04:23
@spong spong self-assigned this May 29, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@spong spong mentioned this pull request Jun 3, 2020
Merged
2 tasks
rylnd
rylnd approved these changes Jun 3, 2020
View reviewed changes
Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

One comment about some potentially missed translations, but: LGTM! Very minimal logical changes so review was a breeze.

Checked it out locally and verified:

  • creating rules and generating signals alerts
  • creating/saving timelines
  • route redirects
  • general clicking around and trying to break things

x-pack/plugins/siem/public/alerts/containers/detection_engine/alerts/translations.ts
}
);

export const SIGNAL_GET_NAME_FAILURE = i18n.translate(
'xpack.siem.containers.detectionEngine.signals.errorGetSignalDescription',
'xpack.siem.containers.detectionEngine.alerts.errorGetAlertDescription',
{
defaultMessage: 'Failed to get signal index name',
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this (and the one below it) be changed? I saw your note about not touching signalsIndex but I see other similar strings being changed so 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it was tough to draw the line with some of these, but I left this and the one below as is since it was referencing the signals index and we're not quite sure what that's going to be named yet.

x-pack/plugins/siem/public/alerts/containers/detection_engine/alerts/translations.ts
{
defaultMessage: 'Failed to get signal index name',
}
);

export const SIGNAL_POST_FAILURE = i18n.translate(
'xpack.siem.containers.detectionEngine.signals.errorPostSignalDescription',
'xpack.siem.containers.detectionEngine.alerts.errorPostAlertDescription',
{
defaultMessage: 'Failed to create signal index',
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto ^^

x-pack/plugins/siem/public/alerts/containers/detection_engine/alerts/types.ts
@@ -7,17 +7,17 @@
export interface BasicSignals {
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 this one should technically be BasicAlerts. Buttt we could also replace it with an inline { signal: AbortSignal } in the few places it's used, too. 🤷‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was 🤷‍♂️ on that as well as I couldn't tell if BasicSignal was meant to refer to the AbortSignal or SignalSignal. I agree going inline might just be for the best here.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@spong spong merged commit 4c30d91 into elastic:master Jun 3, 2020
@spong spong deleted the signals-2-alerts branch June 3, 2020 03:50
@spong spong mentioned this pull request Jun 3, 2020
Merged
spong added a commit to spong/kibana that referenced this pull request Jun 3, 2020
@spong
[SIEM] [Detections] Renames Signals to Alerts (elastic#67731)
1c9be09 
## Summary

Resolves elastic#65944

Renames `Signals` -> `Alerts` on the main Detection Engine page. Including:
* Timeline Event Selector
* Alerts Histogram
* Alerts Table

Does not include:
* `Detections` -> `Alerts` navigation rename
* `SignalsByCategory` rename as there already exists an `AlertsByCategory`, verify changing to `ExternalAlertsByCategory`
* Anything server-side or related to `siemSignalsIndex`


### Checklist

- [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [X] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
spong added a commit that referenced this pull request Jun 3, 2020
@spong
[SIEM] [Detections] Renames Signals to Alerts (#67731) (#68123)
fb85d66 
## Summary

Resolves #65944

Renames `Signals` -> `Alerts` on the main Detection Engine page. Including:
* Timeline Event Selector
* Alerts Histogram
* Alerts Table

Does not include:
* `Detections` -> `Alerts` navigation rename
* `SignalsByCategory` rename as there already exists an `AlertsByCategory`, verify changing to `ExternalAlertsByCategory`
* Anything server-side or related to `siemSignalsIndex`


### Checklist

- [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [X] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
@spong spong mentioned this pull request Jun 4, 2020
Closed
@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rylnd rylnd rylnd approved these changes

Assignees

@spong spong

Labels
Feature:Detection Rules Security Solution rules and Detection Engine release_note:enhancement Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM v7.9.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[SIEM][Timeline] Rename Signals table to Alerts
5 participants
@spong @elasticmachine @kibanamachine @rylnd @MindyRS