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

[Explorer] Add Explorer app locator #164995

Closed
weltenwort opened this issue Aug 28, 2023 · 13 comments · Fixed by #165962
Closed

[Explorer] Add Explorer app locator #164995

weltenwort opened this issue Aug 28, 2023 · 13 comments · Fixed by #165962
Assignees
Labels
Feature:LogsExplorer Logs Explorer feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services

Comments

@weltenwort
Copy link
Member

weltenwort commented Aug 28, 2023

📓 Summary

In order to programmatically navigate to the new observability log explorer app, we want to add a locator, that supports passing data specific in a type-safe way.

✔️ Acceptance criteria

  • There is a locator in the plugin x-pack/plugins/observability_log_explorer, that navigates to the observability log explorer app, which...
    • accepts properties to specify the integration name and the data stream name
    • optionally accepts a sub-set of properties of the Discover locator:
      • timeRange
      • refreshInterval
      • filters
      • query
      • columns
      • sort
  • If the observability_log_explorer plugin is enabled, the on-boarding flows for system logs and application logs use the locator to navigate to the observability log explorer app via the "Explore Logs" Button at the end.
  • Otherwise it navigates to Discover with an appropriate data view and filters. (which is the case for the serverless security project type)
  • If the infra plugin is enabled, the log links in APM use the infra locators as before.
  • If the infra plugin is disabled, the log links in APM use the new observability_log_explorer locator (instead of completely hiding the link as implemented in [Infra] Disable infra plugin in serverless projects #165289).
@weltenwort weltenwort 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 labels Aug 28, 2023
@elasticmachine
Copy link
Contributor

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

@isaclfreire
Copy link

To answer @mohamedhamed-ahmed comment in the previous issue:

The problem is in Pending, and Failed states.
I guess it would make sense that the button is disabled while we are at the Pending state and only enable it once we land on either one of the other 2 states.
The question is Where should we actually navigate to if the status is Failed, if we should allow navigation at all.

I agree with the first proposal: the button is disabled while we are at pending - it's a transitional state - and then it becomes active in both cases: if the install is successful or not. In my opinion, although the integration install failed, users should not be prevented to get to Logs Explorer, to see other logs they might have or even just to see an empty state, but browse the app.

@mohamedhamed-ahmed
Copy link
Contributor

although the integration install failed, users should not be prevented to get to Logs Explorer

@isaclfreire What should we navigate to in this case? should we select the logs-* in this case or something else?

@isaclfreire
Copy link

logs-* LGTM. @ruflin wdyt? do you have any concerns?

To be more specific, whenever the installation fails, we could change the CTA of the button from 'Explore logs' to something like 'Open Logs Explorer' with the primary blue color. Or just anything really that still takes them to their logs, but is not strictly related to the success/failure of the process

@ruflin
Copy link
Member

ruflin commented Sep 5, 2023

logs-*-* SGTM. I assume logs-*-* is our default and not logs-*.

@weltenwort
Copy link
Member Author

We mean the "all logs" selection mode, right? The log explorer app locator shouldn't have to know about the internals.

@formgeist formgeist changed the title [Log Explorer] Add log explorer app locator [Explorer] Add log explorer app locator Sep 6, 2023
@formgeist formgeist changed the title [Explorer] Add log explorer app locator [Explorer] Add Explorer app locator Sep 6, 2023
@yngrdyn
Copy link
Contributor

yngrdyn commented Sep 7, 2023

What should we navigate to in this case?

Could we just go to an specific dataset? In this case we know what are the system integration datasets, even though user couldn't install the integration probably the logs are arriving there

@mohamedhamed-ahmed
Copy link
Contributor

mohamedhamed-ahmed commented Sep 7, 2023

We mean the "all logs" selection mode, right? The log explorer app locator shouldn't have to know about the internals.

Yes this is what I meant, using the all datasets locator which selects All Logs option from the dataset selector.

Could we just go to an specific dataset? In this case we know what are the system integration datasets, even though user couldn't install the integration probably the logs are arriving there

I think in case of failures we will be navigating to the above mentioned selection.

@yngrdyn we might need to update the Explore Logs button in this case, so that its aware of the integration installation status, so that we are able to modify the navigation in case of failures, or disabling the button in case of pending state.

@yngrdyn
Copy link
Contributor

yngrdyn commented Sep 7, 2023

@yngrdyn we might need to update the Explore Logs button in this case, so that its aware of the integration installation status, so that we are able to modify the navigation in case of failures, or disabling the button in case of pending state.

Should we create a new issue? or just fix this in the context of this one?

@mohamedhamed-ahmed
Copy link
Contributor

Should we create a new issue? or just fix this in the context of this one?

I would suggest maybe creating a separate issue, the scope of this PR has many modifications already which will be pushed soon. But I am fine if you think we should do it here.

@yngrdyn
Copy link
Contributor

yngrdyn commented Sep 7, 2023

Let's tackle this in a separate issue then.

Feel free to comment there or complete the description @isaclfreire or @mohamedhamed-ahmed

@ruflin
Copy link
Member

ruflin commented Sep 11, 2023

accepts properties to specify the integration name and the data stream name

Assuming we expand the selection to hosts/services in the futere, I assume we will need to expand this eventually?

@weltenwort
Copy link
Member Author

Yes, we've decided to rather have several small and focused locators for different cases. For now we'll have locators for single selection and "all" selection, but can easily add more later for further selection modes.

mohamedhamed-ahmed added a commit that referenced this issue Sep 20, 2023
closes #164995
closes #165618
closes #166596

## 📝  Summary

### Observability Log Explorer Locators:

This PR adds 2 new customized locators to the Observability log explorer
profile. At the moment we implemented:

       1- Single dataset selector locator
       2- All dataset selector locator

With more locators to come in the future depending on the use cases.

### Log Explorer Locators:

We also added a log explorer locator that navigates to discover, this
can be used in case the **Observability Log Explorer** plugin is
disabled.

### Logs Onboarding:

The PR also replaces the temp navigation to the default discover we
implemented for[ 8.10
here](#163218) with the above new
Observability Log Explorer locators.

### APM:

After [disabling infra plugin in serverless
projects](#165289), APM links to
infra locators in serverless have been replaced to use the above
locators.

### Observability Landing Page:

The landing page now redirects to the Log Explorer if `logs-*-*` has
data in it, otherwise the flow continues as before.

### Necessary Refactoring:

To avoid the circular dependency between `ObservabilityLogExplorer` &
`ObservabilityOnboarding` after each one using the other's locator and
importing the necessary types, I moved the type definition for all
locators in the `deeplinks` package.

## ✅  Testing

- Onboarding Wizard in Serverless and Stateful

    1. Navigate to the onboarding flow `/app/observabilityOnboarding/`
    2. Choose either System logs or Stream log files
    3. Go through the onboarding wizard
    4. Click the Explore logs button at the end
5. You should be redirected to observability log explorer with the
integration and dataset preselected.

- APM links in Serverless

1. Navigate to APM and click on the logs links as shown in the Demos
below
2. All links should navigate to Observability Log Explorer with the
queries set in the search bar.

## 🎥 Demos

- APM Serverless


https://github.com/elastic/kibana/assets/11225826/7161364e-333f-4ac4-87d5-7f1ffec699b3


- APM Stateful


https://github.com/elastic/kibana/assets/11225826/058c9587-b766-4d4f-a73d-50fd381be4bb


- Onboarding Serverless



https://github.com/elastic/kibana/assets/11225826/ee1cab42-f91c-4558-aa5f-4fa7e8963427

- Onboarding Stateful



https://github.com/elastic/kibana/assets/11225826/a376a12b-499b-4488-a75a-d06e81f8e21d

- Observability Landing Page 



https://github.com/elastic/kibana/assets/11225826/c1c084ca-b1b1-4c4b-a4e6-ae8e157dcf57

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co>
Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani01@gmail.com>
gergoabraham pushed a commit to gergoabraham/kibana that referenced this issue Sep 21, 2023
closes elastic#164995
closes elastic#165618
closes elastic#166596

## 📝  Summary

### Observability Log Explorer Locators:

This PR adds 2 new customized locators to the Observability log explorer
profile. At the moment we implemented:

       1- Single dataset selector locator
       2- All dataset selector locator

With more locators to come in the future depending on the use cases.

### Log Explorer Locators:

We also added a log explorer locator that navigates to discover, this
can be used in case the **Observability Log Explorer** plugin is
disabled.

### Logs Onboarding:

The PR also replaces the temp navigation to the default discover we
implemented for[ 8.10
here](elastic#163218) with the above new
Observability Log Explorer locators.

### APM:

After [disabling infra plugin in serverless
projects](elastic#165289), APM links to
infra locators in serverless have been replaced to use the above
locators.

### Observability Landing Page:

The landing page now redirects to the Log Explorer if `logs-*-*` has
data in it, otherwise the flow continues as before.

### Necessary Refactoring:

To avoid the circular dependency between `ObservabilityLogExplorer` &
`ObservabilityOnboarding` after each one using the other's locator and
importing the necessary types, I moved the type definition for all
locators in the `deeplinks` package.

## ✅  Testing

- Onboarding Wizard in Serverless and Stateful

    1. Navigate to the onboarding flow `/app/observabilityOnboarding/`
    2. Choose either System logs or Stream log files
    3. Go through the onboarding wizard
    4. Click the Explore logs button at the end
5. You should be redirected to observability log explorer with the
integration and dataset preselected.

- APM links in Serverless

1. Navigate to APM and click on the logs links as shown in the Demos
below
2. All links should navigate to Observability Log Explorer with the
queries set in the search bar.

## 🎥 Demos

- APM Serverless


https://github.com/elastic/kibana/assets/11225826/7161364e-333f-4ac4-87d5-7f1ffec699b3


- APM Stateful


https://github.com/elastic/kibana/assets/11225826/058c9587-b766-4d4f-a73d-50fd381be4bb


- Onboarding Serverless



https://github.com/elastic/kibana/assets/11225826/ee1cab42-f91c-4558-aa5f-4fa7e8963427

- Onboarding Stateful



https://github.com/elastic/kibana/assets/11225826/a376a12b-499b-4488-a75a-d06e81f8e21d

- Observability Landing Page 



https://github.com/elastic/kibana/assets/11225826/c1c084ca-b1b1-4c4b-a4e6-ae8e157dcf57

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co>
Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani01@gmail.com>
@gbamparop gbamparop added Feature:LogsExplorer Logs Explorer feature and removed Feature:Logs UI Logs UI feature labels Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:LogsExplorer Logs Explorer feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants