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

Kpi host #34769

Closed
wants to merge 12 commits into from
Closed

Kpi host #34769

wants to merge 12 commits into from

Conversation

angorayc
Copy link
Contributor

@angorayc angorayc commented Apr 9, 2019

This PR is to add kpi matrix on hosts page: https://github.com/elastic/ingest-dev/issues/352
Screenshot 2019-04-15 at 17 12 48

Questions:

  1. Would we want to combine some of the matrix? e.g. Auditbeat FIM|Auditd Events to jsut show Auditbeat Events as we probably have more numbers to show in the future.
  2. The query for 3. Number of user matrix seems to be only valid for Auditbeat, would we want that?
  • 1. Number of hosts, based on the host.name field - general_query.dsl
  • 2. Number of installed packages, based on the system.audit.package.name field - general_query.dsl
  • 3. Number of users - TBD
  • 4. Number of processes - query_process.dsl
  • 5. Number of authentication attempts (failed / successful) - query_auth.dsl
  • 6. File events: simple count of Auditbeat FIM events - query_event.dsl
  • 7. Audit events: simple count of Auditbeat Auditd events - query_event.dsl
  • 8. Windows events: simple count of Winlogbeat events - query_event.dsl
  • 9. Log events: simple count of Filebeat events - query_event.dsl
  • 10. And maybe something around network/sockets, maybe just number of sockets (cardinality of socket.entity_id), or maybe number of unique IP addresses (cardinality of source.ip/destination.ip). - general_query.dsl

@elasticmachine
Copy link
Contributor

Pinging @elastic/secops

@spong
Copy link
Member

spong commented Apr 9, 2019

@angorayc, yeah, we'll want to calculate the host count using host.name as mentioned in elastic/ingest-dev#352. @XavierM is in the process of refactoring this right now as part of the firstSeen/lastSeen work on the Host Details page, so you should be able to use the query from his upcoming PR and be good to go!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@angorayc angorayc marked this pull request as ready for review April 12, 2019 01:13
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@MichaelMarcialis
Copy link
Contributor

Hey, @angorayc (cc @tsg). Great job putting this together. Here are my questions and suggestions regarding these KPI additions:

  1. Based on the wireframes (both original and the ones I'm working on), it appears that there have been a number of additions and a few omissions in the KPIs. Number of user names is missing. Monitored log files appears to have been expanded out into individual Beat categories. Unique sources and destinations is a new addition. What was the impetus for these changes, and do they improve the user experience of a security analyst looking at hosts? My fear is that the KPI area is becoming a dumping ground for any and all metrics we can throw at the user, but it should really be a place for carefully curated content that describes what's happening on the page at a high level with a few short metrics/visualizations. That said, if there's any KPIs here that are extraneous or not useful to a security analyst's tasks, I would say cut them.

  2. Specifically regarding the hosts KPI, I chose to omit that from my wireframes as the full hosts table and count appears immediately below it. However your hosts KPI (1,888) doesn't appear to match the host table's count (15). Is that a bug? If not, are these counts referencing two different metrics?

  3. I would like us to start moving away from using the EuiCard components and instead use a combination of the EuiStat, EuiPanel and EuiIcon components in order to render KPIs across the application. Doing so will yield a look similar to what I am proposing within my wireframes. Let me know if that's something you feel comfortable doing, or if you'd prefer I make such changes.

  4. After cutting any extraneous KPI metrics, I would combine KPI boxes where it makes sense to do so (such as authentication successes and failures, as suggested in my wireframes). This should be able to be done by nesting an EuiFlexGroup in an EuiPanel, in addition to the other components suggested above.

  5. Depending on how many KPI metrics in total we plan to show after cutting any unnecessary ones, assuming the count is more than four or five, I would break the KPIs out into two rows to cut down on how much horizontal space is required (rather than jamming them all into a single row).

All that said, please let me know if you'd like to have a chat about this, or if you'd like me to mock something up based on the answers to the above, and I'd be happy to accommodate.

},
};

export const getKpiHostsQueryMock = (logger: Logger) => ({
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this doesn't have any usages, is this intended as a helper for a future schema test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I will have to make a unit test for schema in this case 🤣

}

extend type Source {
KpiHosts(id: String, timerange: TimerangeInput!, filterQuery: String): KpiHostsData
Copy link
Member

Choose a reason for hiding this comment

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

Do you want a ! at the end of KpiHostsData? Without the ! the source resolver generated will accept a null:

export namespace SourceResolvers {
  export interface Resolvers<Context = SiemContext, TypeParent = Source> {
    KpiHosts?: KpiHostsResolver<KpiHostsData | null, TypeParent, Context>;
}

Looking at the other resolvers under SourceResolvers in /server/graphql/types.ts, it seems most most only accept their explicit type (so with a ! above).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks! It makes no sense if KpiHosts is null and accepted.

@angorayc
Copy link
Contributor Author

angorayc commented Apr 15, 2019

Hey @MichaelMarcialis, thanks for the feedback :) There's an issue regarding to what matrix we are going to show on this page. I'll merge some of the KPIs, split them into two rolls if there are still more than four or five left, and double check with the team and see if all the KPIs are useful. In my next PR I'll align the KPI widget on both hosts and network page with the latest design. Please feel free to let me know if there's other thing I should cover in this PR. Thank you so much!

  1. Based on the wireframes (both original and the ones I'm working on), it appears that there have been a number of additions and a few omissions in the KPIs. Number of user names is missing. Monitored log files appears to have been expanded out into individual Beat categories. Unique sources and destinations is a new addition. What was the impetus for these changes, and do they improve the user experience of a security analyst looking at hosts? My fear is that the KPI area is becoming a dumping ground for any and all metrics we can throw at the user, but it should really be a place for carefully curated content that describes what's happening on the page at a high level with a few short metrics/visualizations. That said, if there's any KPIs here that are extraneous or not useful to a security analyst's tasks, I would say cut them.

I agree that we should make the most out of the space and keep only something super useful.

  • Number of user names - We couldn't get the precise number for this field beside Auditbeat, therefor we can think about if we still want this KPI.
  • Number of authentication attempts - Displayed individually for success and failure at the moment, will combine them into a single field.
  • Auditbeat FIM events and Auditbeat auditd events - Displayed individually at the moment, will combine them and rename it to Auditbeat events.
  • Unique sources and destinations IPs - Will double check if it is worthwhile to show put source and destination IPs here.
  1. Specifically regarding the hosts KPI, I chose to omit that from my wireframes as the full hosts table and count appears immediately below it. However your hosts KPI (1,888) doesn't appear to match the host table's count (15). Is that a bug? If not, are these counts referencing two different metrics?
  • Thanks for pointing this out. The query for this KPI is different from what we have on the page, I'll check it again what we really want to show. If the hosts KPI means the same as what we've got in the host table, I'll remove that.
  1. I would like us to start moving away from using the EuiCard components and instead use a combination of the EuiStat, EuiPanel and EuiIcon components in order to render KPIs across the application. Doing so will yield a look similar to what I am proposing within my wireframes. Let me know if that's something you feel comfortable doing, or if you'd prefer I make such changes.
  1. After cutting any extraneous KPI metrics, I would combine KPI boxes where it makes sense to do so (such as authentication successes and failures, as suggested in my wireframes). This should be able to be done by nesting an EuiFlexGroup in an EuiPanel, in addition to the other components suggested above.
  • Sure, I'm happy with moving away from EuiCard. It just I've made KPIs on network page with EuiCard already, I'm thinking of implement this and the chart for authentication attempt in the next PR, so they can always have the same look.
  1. Depending on how many KPI metrics in total we plan to show after cutting any unnecessary ones, assuming the count is more than four or five, I would break the KPIs out into two rows to cut down on how much horizontal space is required (rather than jamming them all into a single row).

All that said, please let me know if you'd like to have a chat about this, or if you'd like me to mock something up based on the answers to the above, and I'd be happy to accommodate.

  • Okay! This will do. I'll break them into two rows if it is more than four or five.

@angorayc angorayc closed this Apr 15, 2019
@angorayc
Copy link
Contributor Author

Screenshot 2019-04-15 at 17 12 48

Hey Michael, I updated it accordingly, any suggestions are welcome.

  • Number of user names - Check with the team and see if we still want it if it only works for Auditbeat.
  • Host Kpi aligns with the one below after I sync up with the latest codebase but I keep it so far to make it ten matrix in total, so it has five in each row.
  • Filebeat Events - Checking the query and see why it's not there
  • Source / Destination IPs - Double check with the team if we can combine or show single direction.

@angorayc
Copy link
Contributor Author

Can't reopen this PR, so please refer to this one instead.
#35065

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants