[Logs+] Add timed feedback toast #167682

Kerry350 · 2023-09-29T16:45:24Z

Summary

Adds a toast message asking for (optional) feedback when navigating from Observability Onboarding > Observability Log Explorer (via the Explore Logs button) after three minutes. The origin is attached to the history location state as part of the Locator.

State machine

A lightweight state machine handles the origin interpreting. We will very likely have more origins in the future.

Reviewer hints

Start the flow at /app/observabilityOnboarding/customLogs, continue to the last step of the wizard, click the "Explore logs" button to navigate to the Observability Log Explorer.
You can alter the FEEDBACK_DELAY constant for easier testing.
Only the onboarding origin should trigger the feedback toast.
Moves the feedback link to Observability shared to avoid cyclic dependency issues.

Screenshot

apmmachine · 2023-09-29T16:45:39Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
/oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2023-10-02T16:40:36Z

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

Kerry350 · 2023-10-02T16:45:37Z

.../plugins/observability_log_explorer/public/state_machines/origin_interpreter/src/defaults.ts

+ * 2.0.
+ */
+
+export const DEFAULT_CONTEXT = undefined;


The machine is setup in a way that it could easily support future context so that we have parity with our other state machines.

Kerry350 · 2023-10-02T17:03:13Z

I need to reduce the observabilityLogExplorer page load bundle size, but otherwise this is ready for review.

…fix'

tonyghiani

Hey Kerry, I did a first review on the feature and works well, I left a couple of minor suggestion to align with the design.

Another thing I noticed is that it works only for custom logs, is that the expected behavior?
Don't we collect any feedback for the System logs onboarding?

I'll now start digging into the code 👌

...ns/observability_log_explorer/public/state_machines/origin_interpreter/src/notifications.tsx

tonyghiani · 2023-10-03T09:49:48Z

Playing more with the feature I noticed a couple more things:

3 minutes is a really long time to wait before the toast appears on the screen, I'm afraid we won't be able to collect many feedback with this delay, even if I'm aware is a temporary workaround, shall we consider reducing the wait? Maybe something like 20-30 seconds is enough? Wdyt?
Once the toast appear, it stays on screen for only 5 seconds, which might not be enough time for the user to notice, read the CTA and click on it.
I would leave it on the screen until the user interacts with it, they might want to play around on the page before taking the survey or they simply won't notice immediately.
hint: setting the toastLifeTimeMs property on the toast notification should be enough, maybe 10mins (600000 ms) is reasonable maximum time on screen?

packages/deeplinks/observability/locators/observability_log_explorer.ts

...ins/observability_log_explorer/public/state_machines/origin_interpreter/src/state_machine.ts

tonyghiani · 2023-10-03T10:30:03Z

Left another couple of minor suggestions (non-blocking), code looks good 👏

Kerry350 · 2023-10-03T11:36:38Z

@tonyghiani

Another thing I noticed is that it works only for custom logs, is that the expected behavior?
Don't we collect any feedback for the System logs onboarding?

Good point, I'd assumed this was for custom only. But reading back through the ACs I think it is for both. The Onboarding wizard also shows the feedback button for both. I'll add it for system logs too.

…s/origin_interpreter/src/notifications.tsx Co-authored-by: Marco Antonio Ghiani <[email protected]>

Kerry350 · 2023-10-03T11:57:17Z

@tonyghiani

3 minutes is a really long time to wait before the toast appears on the screen, I'm afraid we won't be able to collect many >feedback with this delay, even if I'm aware is a temporary workaround, shall we consider reducing the wait? Maybe >something like 20-30 seconds is enough? Wdyt?

Yeah, I agree with this. 30 seconds seems like a fair compromise for now. I think that's long enough to not feel harassed once landing on the new page.

Once the toast appear, it stays on screen for only 5 seconds, which might not be enough time for the user to notice, read >the CTA and click on it.
I would leave it on the screen until the user interacts with it, they might want to play around on the page before taking the >survey or they simply won't notice immediately.

Yeah, according to UX guidelines 3-5 seconds is around average for a two line toast. However, because we're requesting an action I'm inclined to agree. I think 10 minutes is a bit too long (even if they can dismiss it manually). I'll change it to 1 minute, compared to a few seconds this is still a long time.

…kibana into 166968-add-timed-toast-message

Kerry350 · 2023-10-03T13:57:28Z

@tonyghiani Thanks for the speedy review as always 🙌 All feedback has been responded to.

…fix'

kibana-ci · 2023-10-03T16:49:43Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 39139c3

Failed CI Steps

FTR Configs #42

Test Failures

[job] [logs] FTR Configs #42 / serverless observability UI Observability Log Explorer Dataset Selector with installed integrations and uncategorized data streams when open on the data views tab should display a list of available data views

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`observabilityLogExplorer`	53	112	+59

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/deeplinks-observability`	14	16	+2
`observabilityShared`	287	288	+1
total			+3

Async chunks

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

id	before	after	diff
`observabilityLogExplorer`	0.0B	89.3KB	+89.3KB
`observabilityOnboarding`	240.2KB	240.4KB	+121.0B
total			+89.5KB

Page load bundle

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

id	before	after	diff
`observabilityLogExplorer`	17.0KB	18.4KB	+1.4KB
`observabilityOnboarding`	5.6KB	5.7KB	+149.0B
`observabilityShared`	47.0KB	47.1KB	+108.0B
total			+1.7KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/deeplinks-observability`	24	26	+2
`observabilityShared`	290	291	+1
total			+3

async chunk count

id	before	after	diff
`observabilityLogExplorer`	0	1	+1

History

💔 Build #164877 failed 599f196
💔 Build #164836 failed 2589634
💔 Build #164636 failed 8a1c588
💔 Build #164109 failed df789c78ebb0882b31ec7d847dcb0d267c08d58c

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

cc @Kerry350

tonyghiani · 2023-10-04T07:56:45Z

...s/observability_log_explorer/public/state_machines/origin_interpreter/src/lazy_component.tsx

note: There is a dynamic utility in the log-explorer plugin which allows, similarly to withSuspense, to lazy load and wrap with suspense any component.
This is out of scope for this PR, but we could move it into the @kbn/shared-ux-utility package and use it here to since it starts to get duplicated (also implemented in the logs-shared plugin).

Just saying for reference in case you need it again, it would allow to skip creating this extra file :)

Thanks 👌 Yeah, I considered the dynamic utility (it was still somewhat fresh in my mind from merging #164102), but like you say it wasn't in a convenient location yet. I didn't actually realise it'd hit the "rule of 3" with log explorer (not a strict rule by any means, just loosely something I've been using with so many recent PRs shifting packages around). I won't change it now in the interest of getting this merged, but could definitely be a followup.

tonyghiani

Code and functionalities LGTM, well done 👏

crespocarlos

LGTM.

gbamparop

Observability onboarding changes LGTM!

achyutjhunjhunwala

Obs Onboarding Plugin changes looks good 👍🏼

## Summary Closes elastic#166968 Adds a toast message asking for (optional) feedback when navigating from Observability Onboarding > Observability Log Explorer (via the Explore Logs button) after three minutes. The origin is attached to the history location state as part of the Locator. ## State machine A lightweight state machine handles the origin interpreting. We will very likely have more origins in the future. ![Screenshot 2023-10-02 at 17 33 41](https://github.com/elastic/kibana/assets/471693/b0f9ba81-b857-4185-a2dd-8049fae43932) ## Reviewer hints - Start the flow at `/app/observabilityOnboarding/customLogs`, continue to the last step of the wizard, click the "Explore logs" button to navigate to the Observability Log Explorer. - You can alter the `FEEDBACK_DELAY` constant for easier testing. - **Only** the onboarding origin should trigger the feedback toast. - Moves the feedback link to Observability shared to avoid cyclic dependency issues. ## Screenshot ![Screenshot 2023-10-03 at 14 45 21](https://github.com/elastic/kibana/assets/471693/8c5f0ac0-43a5-44f7-a361-4ea2f66e42b8) --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Marco Antonio Ghiani <[email protected]>

Kerry350 force-pushed the 166968-add-timed-toast-message branch from df789c7 to 8ae5cd5 Compare October 2, 2023 16:25

Kerry350 self-assigned this Oct 2, 2023

Kerry350 added Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services release_note:skip Skip the PR/issue when compiling release notes labels Oct 2, 2023

Kerry350 marked this pull request as ready for review October 2, 2023 16:40

Kerry350 requested review from a team as code owners October 2, 2023 16:40

Kerry350 commented Oct 2, 2023

View reviewed changes

Kerry350 force-pushed the 166968-add-timed-toast-message branch from ea35209 to 03e7517 Compare October 2, 2023 16:51

Add feedback toast for the onboarding origin

f1869fe

Kerry350 force-pushed the 166968-add-timed-toast-message branch from c0088ed to f1869fe Compare October 2, 2023 17:01

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

8a1c588

…fix'

tonyghiani self-requested a review October 3, 2023 07:47

tonyghiani reviewed Oct 3, 2023

View reviewed changes

packages/deeplinks/observability/locators/observability_log_explorer.ts Outdated Show resolved Hide resolved

tonyghiani reviewed Oct 3, 2023

View reviewed changes

...ins/observability_log_explorer/public/state_machines/origin_interpreter/src/state_machine.ts Show resolved Hide resolved

Kerry350 and others added 2 commits October 3, 2023 12:51

Update x-pack/plugins/observability_log_explorer/public/state_machine…

d1c0b8f

…s/origin_interpreter/src/notifications.tsx Co-authored-by: Marco Antonio Ghiani <[email protected]>

Update x-pack/plugins/observability_log_explorer/public/state_machine…

2589634

…s/origin_interpreter/src/notifications.tsx Co-authored-by: Marco Antonio Ghiani <[email protected]>

Kerry350 added 3 commits October 3, 2023 13:02

Review tweaks

4efe952

Merge branch '166968-add-timed-toast-message' of github.com:Kerry350/…

4559a5f

…kibana into 166968-add-timed-toast-message

Alter delays

1eccd36

Add to system logs onboarding

599f196

Kerry350 and others added 2 commits October 3, 2023 16:32

Lazily load origin interpreter

993e02c

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

39139c3

…fix'

tonyghiani reviewed Oct 4, 2023

View reviewed changes

tonyghiani approved these changes Oct 4, 2023

View reviewed changes

crespocarlos requested review from crespocarlos and removed request for crespocarlos October 4, 2023 12:03

crespocarlos approved these changes Oct 4, 2023

View reviewed changes

gbamparop approved these changes Oct 5, 2023

View reviewed changes

achyutjhunjhunwala approved these changes Oct 5, 2023

View reviewed changes

Kerry350 merged commit 825ef56 into elastic:main Oct 5, 2023

kibanamachine added v8.12.0 backport:skip This commit does not require backporting labels Oct 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Logs+] Add timed feedback toast #167682

[Logs+] Add timed feedback toast #167682

Kerry350 commented Sep 29, 2023 •

edited

Loading

apmmachine commented Sep 29, 2023

elasticmachine commented Oct 2, 2023

Kerry350 Oct 2, 2023

Kerry350 commented Oct 2, 2023

tonyghiani left a comment

tonyghiani commented Oct 3, 2023

tonyghiani commented Oct 3, 2023

Kerry350 commented Oct 3, 2023 •

edited

Loading

Kerry350 commented Oct 3, 2023 •

edited

Loading

Kerry350 commented Oct 3, 2023

kibana-ci commented Oct 3, 2023

API count

async chunk count

tonyghiani Oct 4, 2023

Kerry350 Oct 4, 2023

tonyghiani left a comment

crespocarlos left a comment

gbamparop left a comment

achyutjhunjhunwala left a comment

[Logs+] Add timed feedback toast #167682

[Logs+] Add timed feedback toast #167682

Conversation

Kerry350 commented Sep 29, 2023 • edited Loading

Summary

State machine

Reviewer hints

Screenshot

apmmachine commented Sep 29, 2023

🤖 GitHub comments

elasticmachine commented Oct 2, 2023

Kerry350 Oct 2, 2023

Choose a reason for hiding this comment

Kerry350 commented Oct 2, 2023

tonyghiani left a comment

Choose a reason for hiding this comment

tonyghiani commented Oct 3, 2023

tonyghiani commented Oct 3, 2023

Kerry350 commented Oct 3, 2023 • edited Loading

Kerry350 commented Oct 3, 2023 • edited Loading

Kerry350 commented Oct 3, 2023

kibana-ci commented Oct 3, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

async chunk count

History

tonyghiani Oct 4, 2023

Choose a reason for hiding this comment

Kerry350 Oct 4, 2023

Choose a reason for hiding this comment

tonyghiani left a comment

Choose a reason for hiding this comment

crespocarlos left a comment

Choose a reason for hiding this comment

gbamparop left a comment

Choose a reason for hiding this comment

achyutjhunjhunwala left a comment

Choose a reason for hiding this comment

Kerry350 commented Sep 29, 2023 •

edited

Loading

Kerry350 commented Oct 3, 2023 •

edited

Loading

Kerry350 commented Oct 3, 2023 •

edited

Loading