-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[UA] ES deprecation logs in its own page #118659
Conversation
…_logs page (#118688) * Remove step completion logic and move fix_deprecation_logs folder to es_deprecation_logs * commit using @elastic.co
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few suggestions.
...upgrade_assistant/public/application/components/overview/fix_issues_step/fix_issues_step.tsx
Outdated
Show resolved
Hide resolved
...upgrade_assistant/public/application/components/overview/fix_issues_step/fix_issues_step.tsx
Outdated
Show resolved
Hide resolved
.../upgrade_assistant/public/application/components/es_deprecation_logs/es_deprecation_logs.tsx
Outdated
Show resolved
Hide resolved
👍 👍 👍 I haven't looked at the code, only tested locally. This UX feels so much better to me! The logs are accessible but not presented as blockers. Great work @sebelga and @sabarasaba. Let's wait a bit to ensure stakeholders are onboard with this change before merging. |
Co-authored-by: debadair <debadair@elastic.co>
@sebelga @sabarasaba We have approval to move forward with this change, so feel free to merge when ready. |
Pinging @elastic/kibana-stack-management (Team:Stack Management) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code changes lgtm, tested locally 🚀
Thanks for the review @sabarasaba ! Let's wait for @debadair input and then we'll merge it 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the suggestion, but LGTM either way.
Co-authored-by: debadair <debadair@elastic.co>
Thanks for the review @debadair ! 👍 |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
@sebelga I think we'll need to backport this to the |
Do we need to backport if UA is not enabled in |
@sebelga The only reason I can imagine we will need it is if there’s a security flaw in a common dependency that will require making changes throughout all plugins. In a situation like that, a backport would be painful if plugins are inconsistent between branches. The chance of that seems low so we could probably get away with not backporting it. OTOH the effort of a backport seems low. Do you see a downside to backporting? |
#119134 was not a straightforward process so I was wondering why we'd need to backport on a branch where we'll only apply security patches or bug fixes. Security patches seem to be more likely to be done server side so would not affect the changes here on UA. The rest are tests and doc links. |
Ah, thanks for explaining Seb! I didn't know that you encountered a large number of merge conflicts when you forwardported the change to |
With that said I did create the backport as indeed it is little effort and aligned branches are a very good thing to do for our future selves. 😊 (#119345) The conflicts with the previous backport came mainly because of translation being updated for the 7.16 release. And I think translation go |
The deprecation logs coming from ES have some UX flaws. Many of the deprecation logs (some noted as “Critical”) that we expose in Observability are coming from Kibana itself and the user can’t act on them. There isn’t really anything that they can do to solve those deprecations. Those logs create noise and could be a barrier to the upgrade process.
We want to reduce the visibility of the “use of deprecated API” section in Upgrade assistant (step 4 of the wizard) as we can’t confidently rely on them to help the user fix possible use of deprecated ES APIs.
In this PR we have removed the ES deprecation logs UI from the UA overview and moved it to its own page.
TODO
Screenshots