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

Define all the error cases and build UI for it #156529

Closed
isaclfreire opened this issue May 3, 2023 · 9 comments · Fixed by #162241
Closed

Define all the error cases and build UI for it #156529

isaclfreire opened this issue May 3, 2023 · 9 comments · Fixed by #162241
Assignees
@yngrdyn
Labels
8.10 candidate apm:test-plan-done Pull request that was successfully tested during the test plan Feature:Logs Onboarding Logs Onboarding feature Feature: Observability Onboarding Team:obs-ux-logs Observability Logs User Experience Team

Comments

@isaclfreire
Copy link

Think through all the potential error cases that can happen during onboarding and figure out what we show users.
@botelastic botelastic bot added the needs-team Issues missing a team label label May 3, 2023
@isaclfreire
Copy link
Author

Designs will be added to this issue

@ogupte
Copy link
Contributor

ogupte commented May 25, 2023

Error cases should include validation on the configure step, API key / yml generation failure in the install shipper step, and progress errors in the Collect Logs step (status page).

@yngrdyn
Copy link
Contributor

yngrdyn commented Jun 1, 2023

Hey @isaclfreire,

I have a case where our dataset generator fails and users don't have a hint on why they can not move to the following step, here is a video containing such case

Screen.Recording.2023-06-01.at.17.08.24.mov

How can we give users some hint on what is happening? I'm afraid they can get frustrated if our generator fails and they don't have anything in the dataset name. I think this is also aplicable to other advanced settings with required fields.

@isaclfreire
Copy link
Author

isaclfreire commented Jun 9, 2023 •
edited
Loading

So if I understand correctly the error is that we weren't able to generale a dataset name because the log file path is wrong, right? Since we are moving the dataset name input out of the advanced settings, we can think of a callout message to be displayed. Example:
image

@gbamparop
Copy link
Contributor

So if I understand correctly the error is that we weren't able to generale a dataset name because the log file path is wrong, right? Since we are moving the dataset name input out of the advanced settings, we can think of a callout message to be displayed. Example:

I think that we should either be able to create a dataset name at all times or have a default one, or both. In this use case, it seems that the user didn't have enough information about what is valid and they get a warning / error because something went wrong on our end.

We could also mark the dataset name as required, with the default Eui states instead of a callout.

@yngrdyn
Copy link
Contributor

yngrdyn commented Jun 12, 2023

@isaclfreire now the we have moved dataset name input out of advanced settings we could go with @gbamparop approach that is letting know users visually that they are missing a required field. IMO all the required inputs that are hidden in advanced settings should have a default value (like the namespace) rather than a autogenerated value (because things could fail in the way and it won't be clear why they are failing or why they cannot continue in the flow)

@isaclfreire
Copy link
Author

Agreed!

@davismcphee davismcphee removed the needs-team Issues missing a team label label Jun 14, 2023
@botelastic botelastic bot added the needs-team Issues missing a team label label Jun 14, 2023
@davismcphee davismcphee added Team:APM - DEPRECATED Use Team:obs-ux-infra_services. and removed needs-team Issues missing a team label labels Jun 14, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:APM)

@gbamparop gbamparop assigned yngrdyn and unassigned grabowskit Jul 14, 2023
@isaclfreire
Copy link
Author

Another error case @yngrdyn was able to identify is when users clear the 'dataset name' field in the first step of custom logs onboarding. They shouldn't be able to move forward until they define a name, since the field is required. Here's how the error message should look like:

image

@yngrdyn yngrdyn mentioned this issue Jul 19, 2023
Merged
yngrdyn added a commit that referenced this issue Jul 19, 2023
@yngrdyn
[Logs onboarding] Error handling in configureLogs step (#162241)
0fd8a11 
Closes #156529.

`Dataset name` is a field that is auto populated based on the `Log file
path`, if the user deletes the autogenerated value they cannot continue
with the next step but before this changes there were no indicative of
why they cannot continue.

### Changes
- Added validation function to datasetName field.
- Added i18n error.

#### Before
<img width="2199" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/24cc166e-c143-4b35-9372-0fcdb4e78f4a">

#### After
<img width="2200" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/72302aa6-1597-439b-8d80-88e7debf1010">
Ruhshan pushed a commit to Ruhshan/kibana that referenced this issue Jul 19, 2023
@yngrdyn @Ruhshan
[Logs onboarding] Error handling in configureLogs step (elastic#162241)
e0476d0 
Closes elastic#156529.

`Dataset name` is a field that is auto populated based on the `Log file
path`, if the user deletes the autogenerated value they cannot continue
with the next step but before this changes there were no indicative of
why they cannot continue.

### Changes
- Added validation function to datasetName field.
- Added i18n error.

#### Before
<img width="2199" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/24cc166e-c143-4b35-9372-0fcdb4e78f4a">

#### After
<img width="2200" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/72302aa6-1597-439b-8d80-88e7debf1010">
ThomThomson pushed a commit to ThomThomson/kibana that referenced this issue Aug 1, 2023
@yngrdyn
[Logs onboarding] Error handling in configureLogs step (elastic#162241)
e635e1c 
Closes elastic#156529.

`Dataset name` is a field that is auto populated based on the `Log file
path`, if the user deletes the autogenerated value they cannot continue
with the next step but before this changes there were no indicative of
why they cannot continue.

### Changes
- Added validation function to datasetName field.
- Added i18n error.

#### Before
<img width="2199" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/24cc166e-c143-4b35-9372-0fcdb4e78f4a">

#### After
<img width="2200" alt="image"
src="https://github.com/elastic/kibana/assets/1313018/72302aa6-1597-439b-8d80-88e7debf1010">
@yngrdyn yngrdyn added the apm:test-plan-done Pull request that was successfully tested during the test plan label Sep 6, 2023
@gbamparop gbamparop added Team:obs-ux-logs Observability Logs User Experience Team Feature:Logs Onboarding Logs Onboarding feature and removed Team:APM - DEPRECATED Use Team:obs-ux-infra_services. apm:logs-onboarding labels Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yngrdyn yngrdyn

Labels
8.10 candidate apm:test-plan-done Pull request that was successfully tested during the test plan Feature:Logs Onboarding Logs Onboarding feature Feature: Observability Onboarding Team:obs-ux-logs Observability Logs User Experience Team
Projects
None yet
Milestone
No milestone
7 participants
@yngrdyn @ogupte @gbamparop @elasticmachine @davismcphee @grabowskit @isaclfreire