Conversation
|
Pinging @elastic/siem (Team:SIEM) |
💔 Build Failed |
|
@angorayc This looks pretty good to me already. Two small comments:
Not sure I understand this one, in which sense to align it. |
Given that our DNS table is group by IP: therefor I am wondering would user expect to find IP as the x-axis for histogram or not (this is what we are doing)?
In some existing histogram, we put x-axis as timestamp and split series
From users' perspective: From implementation's perspective: Therefore I think I need to rephrase my question, as in the end I'd like to know whether we should use timestamp or IP as x-axis. |
|
@angorayc I see, but you seem to have copied the Anomalies table when this one should go in the DNS tab, right? That one looks like this, it's grouped by domain:
I see what you are saying with the x-axis. I am good either way, but I kind of like the histogram as you currently have it in your PR, I think it's nice that not all charts look the same. |
| setQuery, | ||
| setAbsoluteRangeDatePicker, | ||
| }: NetworkRoutesProps) => { | ||
| const narrowDateRange = useCallback( |
There was a problem hiding this comment.
why have you decided to remove useCallback?
There was a problem hiding this comment.
Nice catch, I was playing around with it and failed to add it back.
| dataKey="histogram" | ||
| endDate={to} | ||
| startDate={from} | ||
| title="DNS bytes out by domain" |
| value, | ||
| }); | ||
|
|
||
| const getCustomChartData = ( |
There was a problem hiding this comment.
maybe we could extract logic to ./utils.ts?
There was a problem hiding this comment.
Good call, isolated utils functions and types to separate files.
Apologies for the wrong example, but the idea remains the same. It's nice to find that current implementation is ok. |
💚 Build Succeeded |
💔 Build Failed |
💚 Build Succeeded |
💔 Build Failed |
💚 Build Succeeded |
💔 Build Failed |
|
jenkins, test this |
💚 Build Succeeded |
💚 Build Succeeded |
* add histogram * fix types error * rename matrix histogram component * clean up * add dns histogram container * wrap passed in function with useCallback * isolate utils functions * extract types * disable chart legend for dns histogram * add dns bytes in
* add histogram * fix types error * rename matrix histogram component * clean up * add dns histogram container * wrap passed in function with useCallback * isolate utils functions * extract types * disable chart legend for dns histogram * add dns bytes in
Summary
Implementation of #42461
Add DNS histogram on network page:
Summary for Code Change:
matrix_histogram(as the X-axis may be other types aside timestamp)Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[ ] Documentation was added for features that require explanation or tutorials[ ] Unit or functional tests were updated or added to match the most common scenarios[ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
[ ] This was checked for breaking API changes and was labeled appropriately[ ] This includes a feature addition or change that requires a release note and was labeled appropriately