Skip to content

[Logs+] Add specialized locators#163148

Closed
mohamedhamed-ahmed wants to merge 13 commits intoelastic:mainfrom
mohamedhamed-ahmed:158382-logs-explorer-add-specialized-locators
Closed

[Logs+] Add specialized locators#163148
mohamedhamed-ahmed wants to merge 13 commits intoelastic:mainfrom
mohamedhamed-ahmed:158382-logs-explorer-add-specialized-locators

Conversation

@mohamedhamed-ahmed
Copy link
Contributor

@mohamedhamed-ahmed mohamedhamed-ahmed commented Aug 4, 2023

closes #158382

📝 Summary

This PR adds new customized locators to the log explorer profile. At the moment we only implemented a single dataset selector locator, but we are planning to have more when we start implementing the multiselect in the dataset selector.
The PR replaces the temp navigation to the default discover we implemented for 8.10 here.

✅ Testing

  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 discover in the log explorer profile with the integration and dataset preselected.

🎥 Demo

Screen.Recording.2023-08-11.at.21.29.57.mov

@mohamedhamed-ahmed mohamedhamed-ahmed added the WIP Work in progress label Aug 4, 2023
@ghost
Copy link

ghost commented Aug 4, 2023

🤖 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!)

@mohamedhamed-ahmed
Copy link
Contributor Author

I implemented the locators in the common folder, and as a result, had to move the public/utils into common

@mohamedhamed-ahmed mohamedhamed-ahmed added Team:APM - DEPRECATED Use Team:obs-ux-infra_services. 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 backport:skip This PR does not require backporting labels Aug 12, 2023
const datasetsService = new DatasetsService().start({
http: core.http,
});
const [{ initializeLogExplorerProfileStateService, waitForState }] = await Promise.all([
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can maybe remove the Prmise.all as its not needed anymore

const { initializeLogExplorerProfileStateService, waitForState } =
      await logExplorerMachineModuleLoadable;
      

@mohamedhamed-ahmed mohamedhamed-ahmed marked this pull request as ready for review August 19, 2023 10:58
@mohamedhamed-ahmed mohamedhamed-ahmed requested a review from a team August 19, 2023 10:58
@mohamedhamed-ahmed mohamedhamed-ahmed requested a review from a team as a code owner August 19, 2023 10:58
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@mohamedhamed-ahmed mohamedhamed-ahmed removed the WIP Work in progress label Aug 19, 2023
@yngrdyn
Copy link
Contributor

yngrdyn commented Aug 21, 2023

What happen if users cannot create the system integration? where they will land?

image

@mohamedhamed-ahmed
Copy link
Contributor Author

What happen if users cannot create the system integration? where they will land?

Are they going to be allowed to navigate? or should the Explore logs button be disabled in this case?

@yngrdyn
Copy link
Contributor

yngrdyn commented Aug 22, 2023

Are they going to be allowed to navigate? or should the Explore logs button be disabled in this case?

AFAIK they will be allowed to continue. Maybe @ruflin, @isaclfreire or @grabowskit can shed a light regarding this.
If allowing them is the case, what about redirecting them to the data view as we are doing now if the installation didn't happened?

@mohamedhamed-ahmed
Copy link
Contributor Author

mohamedhamed-ahmed commented Aug 22, 2023

Are they going to be allowed to navigate? or should the Explore logs button be disabled in this case?

AFAIK they will be allowed to continue. Maybe @ruflin, @isaclfreire or @grabowskit can shed a light regarding this. If allowing them is the case, what about redirecting them to the data view as we are doing now if the installation didn't happened?

@yngrdyn Adding to what you mentioned.
As I can see in the code, we have 3 states once the user clicks on system logs

Screenshot 2023-08-22 at 12 32 34
  1. Pending
Screenshot 2023-08-22 at 12 33 17
  1. Successful
Screenshot 2023-08-22 at 12 33 40
  1. Failed
Screenshot 2023-08-22 at 12 33 57

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.

@kibana-ci
Copy link

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
discoverLogExplorer 317 321 +4
observabilityOnboarding 120 85 -35
total -31

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
discoverLogExplorer 0 2 +2

Async chunks

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

id before after diff
discoverLogExplorer 221.1KB 184.3KB -36.8KB
observabilityOnboarding 292.0KB 121.5KB -170.6KB
total -207.4KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
discoverLogExplorer 0 1 +1

Page load bundle

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

id before after diff
discoverLogExplorer 5.9KB 35.3KB +29.4KB
observabilityOnboarding 4.6KB 4.6KB -54.0B
total +29.3KB
Unknown metric groups

API count

id before after diff
discoverLogExplorer 0 2 +2

async chunk count

id before after diff
discoverLogExplorer 8 6 -2

History

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

@weltenwort
Copy link
Member

@mohamedhamed-ahmed as discussed, let's redirect this PR to target #164995. I reduced the scope and fields a bit in comparison to the original ACs.

@mohamedhamed-ahmed
Copy link
Contributor Author

This PR will be closed as it has been implemented as part of this PR

#165962

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.11.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Log Explorer] Add specialized locators

7 participants