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

[Exploratory View] Added step level filtering/breakdowns #115182

Merged
merged 16 commits into from
Oct 18, 2021

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Oct 15, 2021

Summary

Fix elastic/uptime#372

User can filter step level metrics using step selection.

User can also do breakdowns by step name

image

@shahzad31 shahzad31 self-assigned this Oct 15, 2021
@shahzad31 shahzad31 added v7.16.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability labels Oct 15, 2021
@shahzad31 shahzad31 marked this pull request as ready for review October 15, 2021 12:53
@shahzad31 shahzad31 requested a review from a team as a code owner October 15, 2021 12:53
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

Copy link

@drewpost drewpost left a comment

Choose a reason for hiding this comment

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

Did this make the Monitor and Step selections in the definition of the series single select? We really shouldn't have multi-select on these fields

Copy link

@drewpost drewpost left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks @shahzad31 !!

Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

Can we hide the step dropdown if ALL_VALUES is selected.

Screen Shot 2021-10-16 at 8 20 13 AM

I was unable to break down by step name, even when selecting a specific browser monitor. Edit: this happened, even after I fixed my mappings, but I am unable to recreate now. Also, we may need a popover to explain why breakdown by step name is disabled when unavailable.
Screen Shot 2021-10-15 at 2 12 20 PM

We should consider moving fields onto new lines. It's getting crowded, and the flexible layout means that the inputs change size as the user is interacting with them.

Screen Shot 2021-10-16 at 7 38 09 AM

When breaking down by step name, we see the step names for all monitors, not just the one selected.
Screen Shot 2021-10-16 at 8 27 12 AM

@shahzad31
Copy link
Contributor Author

Can we hide the step dropdown if ALL_VALUES is selected.

ALL_VALUES isn't even possible to select anymore in monitor name.

I was unable to break down by step name, even when selecting a specific browser monitor. Edit: this happened, even after I fixed my mappings, but I am unable to recreate now. Also, we may need a popover to explain why breakdown by step name is disabled when unavailable.

I have added the tool tooltip in case where it's not available.

We should consider moving fields onto new lines. It's getting crowded, and the flexible layout means that the inputs change size as the user is interacting with them.

Have tried to handle this

When breaking down by step name, we see the step names for all monitors, not just the one selected. Yes this is a broad issue with filering and breakdown, i am planing to handle this by moving layer level filters to global query, i am having few discussions with lens team to handle this.

cc @dominiqueclarke

@dominiqueclarke dominiqueclarke self-requested a review October 18, 2021 12:50
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

Should the step name dropdown be available for up pings, down pings, and monitor duration duration metric? When selecting it in those metrics, there is no data data displayed.

Screen Shot 2021-10-18 at 9 18 18 AM

When navigating to exploratory view from Uptime, ALL_VALUES is auto selected. Since we removed ALL_VALUES from monitor name, how should we handle navigating from Uptime using the Analyze data button?
Screen Shot 2021-10-18 at 9 17 58 AM

@shahzad31
Copy link
Contributor Author

Hidden step name filter for non step level metrics like monitor duration/up pings/down pings

@dominiqueclarke dominiqueclarke self-requested a review October 18, 2021 14:45
@shahzad31 shahzad31 added the auto-backport Deprecated - use backport:version if exact versions are needed label Oct 18, 2021
@shahzad31 shahzad31 requested a review from drewpost October 18, 2021 17:21
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

LGTM


const STEP_METRIC_FILTER: ColumnFilter = {
language: 'kuery',
query: `synthetics.type: step/metrics`,
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for backticks here or below, but really not that important.

@@ -51,6 +53,18 @@ export function Breakdowns({ seriesConfig, seriesId, series }: Props) {
}
};

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not including a dependencies array is a great way to make sure this only fires on the first render. I'll need to remember to use this because I'm often so keyed into wanting to map the effect back to changes in the dependencies. 💯

@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
observability 379.1KB 381.6KB +2.5KB
uptime 571.0KB 571.0KB +14.0B
total +2.5KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
observability 38.1KB 38.4KB +344.0B

History

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

cc @shahzad31

@shahzad31 shahzad31 merged commit 7e1acab into elastic:master Oct 18, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Oct 18, 2021
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Oct 18, 2021
…115456)

Co-authored-by: Dominique Clarke <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Shahzad <[email protected]>
Co-authored-by: Dominique Clarke <[email protected]>
@shahzad31 shahzad31 deleted the step-level-filtering branch November 14, 2021 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exploratory View - Synthetics Step Level Capabilities
5 participants