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

[Infra UI] Serverless 'Open in Logs' link within Host fly-out is broken #171082

Closed
roshan-elastic opened this issue Nov 13, 2023 · 11 comments · Fixed by #172137
Closed

[Infra UI] Serverless 'Open in Logs' link within Host fly-out is broken #171082

roshan-elastic opened this issue Nov 13, 2023 · 11 comments · Fixed by #172137
Assignees
@awahab07
Labels
bug Fixes for quality problems that affect the customer experience Feature:ObsHosts Hosts feature within Observability Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team Team:obs-ux-logs Observability Logs User Experience Team

Comments

@roshan-elastic
Copy link

Description

This link in serverless is broken:

Opening Logs in serverless
image

The link takes you through to this page

image
@roshan-elastic roshan-elastic added the bug Fixes for quality problems that affect the customer experience label Nov 13, 2023
@botelastic botelastic bot added the needs-team Issues missing a team label label Nov 13, 2023
@roshan-elastic
Copy link
Author

Hey @felixbarny,

I think you mentioned to me that this 'open in logs' functionality needs to be disabled in the logs embeddable? Is that right?

Raising this issue as in production it is broken.

@smith @jennypavlova FYI probably part of the epic on serverless Infra UI

@kuisathaverat
Copy link
Contributor

In the navigation from Inventory in the list of pods, the link points to app/logs
image
image

@roshan-elastic
Copy link
Author

Thanks @kuisathaverat

@roshan-elastic
Copy link
Author

Same here:

image

@crespocarlos
Copy link
Contributor

@roshan-elastic we should probably ping @elastic/obs-ux-logs-team . The aforementioned places are using the node logs locator to create the link URL

@smith smith added the Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team label Nov 15, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Nov 15, 2023
@smith smith added the Feature:ObsHosts Hosts feature within Observability label Nov 15, 2023
@mohamedhamed-ahmed
Copy link
Contributor

@crespocarlos We lately enabled infra in serverless, but at the same time the stream UI is disabled there. Which is being used by the locators.

The logic in the locator was to navigate to Stream UI if infra is enabled since it was assumed that Stream UI is going to be there, otherwise navigate to Log explorer. But now with having Infra enabled but Stream disabled its failing as it can't find the route.

@crespocarlos
Copy link
Contributor

Thanks, @mohamedhamed-ahmed. In serverless, shouldn't the locator navigate to the new Logs Explorer? Please correct me if I'm wrong, but I think the Logs Stream UI won't join serverless.

@mohamedhamed-ahmed
Copy link
Contributor

@crespocarlos IIRC the logic was a bit different.
As per the locator ticket we had the logic as below:

  • 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

Will double check this with the team again.

@smith smith added the Team:obs-ux-logs Observability Logs User Experience Team label Nov 16, 2023
@awahab07 awahab07 self-assigned this Nov 20, 2023
@crespocarlos
Copy link
Contributor

@mohamedhamed-ahmed did you have the chance to check the logic?

@mohamedhamed-ahmed
Copy link
Contributor

@crespocarlos Yes, the issue is mainly because the Stream UI is now disabled in serverless, but at the same time the locator registration is still happening here.
This allows consumers of these locators to still use them and try navigating to the stream UI which is actually not available.

The quickest solution would be to conditionally register these locators depending on the logsUIEnabled flag as well.
But we decided to do this in a more generic way by extracting the locator logic to observability-logs-shared so that not each consumer would have to do these conditions and @awahab07 is picking this up.

@mohamedhamed-ahmed mohamedhamed-ahmed mentioned this issue Nov 27, 2023
Closed
@awahab07 awahab07 mentioned this issue Nov 29, 2023
Merged
awahab07 added a commit that referenced this issue Jan 10, 2024
@awahab07
[Infra UI] Fix Open in Logs links in Infra and APM when in Serverle…
2f2083b 
…ss (#172137)

Fixes #171082

## Summary

The PR wraps the
[LogsLocator](https://github.com/elastic/kibana/blob/f59ac2916d02d643310dd44a8f80b7a9cc61f608/x-pack/plugins/infra/common/locators/logs_locator.ts#L18C27-L18C27)
and
[NodeLogsLocator](https://github.com/elastic/kibana/blob/f59ac2916d02d643310dd44a8f80b7a9cc61f608/x-pack/plugins/infra/common/locators/node_logs_locator.ts#L16)
of **infra** plugin inside corresponding locators in **logs_shared**
plugin while including the fallback logic to navigate to Logs Explorer
when Steam UI isn't available.

Previously, it was assumed that Steam UI will always be available as
long as Infra UI is available, but **infra** plugin introduced a new
feature flag `logsUIEnabled` which when `false` won't enable the
`/stream/` route in Serverless.

The added locators in **logs_shared** will now check whether locators
redirecting to `/steam/` are available, otherwise they'll redirect to
Logs Explorer, thus the new locators are abstracting this decision in
their definition. This abstraction was already being done in **apm**
plugin, which has also been refactored to use the newly added
**logs_shared** locators.

Links in Serverless:


https://github.com/elastic/kibana/assets/2748376/16e5747a-546e-44b3-87e3-95428945cf63
delanni pushed a commit to delanni/kibana that referenced this issue Jan 11, 2024
@awahab07 @delanni
[Infra UI] Fix Open in Logs links in Infra and APM when in Serverle…
8c14f93 
…ss (elastic#172137)

Fixes elastic#171082

## Summary

The PR wraps the
[LogsLocator](https://github.com/elastic/kibana/blob/f59ac2916d02d643310dd44a8f80b7a9cc61f608/x-pack/plugins/infra/common/locators/logs_locator.ts#L18C27-L18C27)
and
[NodeLogsLocator](https://github.com/elastic/kibana/blob/f59ac2916d02d643310dd44a8f80b7a9cc61f608/x-pack/plugins/infra/common/locators/node_logs_locator.ts#L16)
of **infra** plugin inside corresponding locators in **logs_shared**
plugin while including the fallback logic to navigate to Logs Explorer
when Steam UI isn't available.

Previously, it was assumed that Steam UI will always be available as
long as Infra UI is available, but **infra** plugin introduced a new
feature flag `logsUIEnabled` which when `false` won't enable the
`/stream/` route in Serverless.

The added locators in **logs_shared** will now check whether locators
redirecting to `/steam/` are available, otherwise they'll redirect to
Logs Explorer, thus the new locators are abstracting this decision in
their definition. This abstraction was already being done in **apm**
plugin, which has also been refactored to use the newly added
**logs_shared** locators.

Links in Serverless:


https://github.com/elastic/kibana/assets/2748376/16e5747a-546e-44b3-87e3-95428945cf63
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@awahab07 awahab07

Labels
bug Fixes for quality problems that affect the customer experience Feature:ObsHosts Hosts feature within Observability Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team Team:obs-ux-logs Observability Logs User Experience Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Infra UI] Fix Open in Logs links in Infra and APM when in Serverless awahab07/kibana
7 participants
@smith @awahab07 @crespocarlos @kuisathaverat @mohamedhamed-ahmed @elasticmachine @roshan-elastic