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

Sidecar Failure Tracking View #14435

Merged
merged 26 commits into from Feb 22, 2023
Merged

Sidecar Failure Tracking View #14435

merged 26 commits into from Feb 22, 2023

Conversation

gally47
Copy link
Contributor

@gally47 gally47 commented Jan 13, 2023

Before, to know why your sidecar collectors are failing you have to go through 3 steps per sidecar/collector:

  1. go to /system/sidecars: Sidecars List Overview.
  2. click on the failing sidecar.
  3. click on Show Details link of the failing collector to open the Error Message Modal.

The previous solution is not optimal if you have hundreds of sidecars with many collectors failing.

In this PR we implemented a new view for Sidecar Failure Tracking to provide all the useful information about failure reasons in one view, that can help decrease troubleshooting time.

Screenshot 2023-01-27 at 15 48 38

Motivation and Context

fixes Graylog2/collector-sidecar#433

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@gally47 gally47 self-assigned this Jan 13, 2023
@gally47 gally47 force-pushed the sidecar-failure-tracking branch 5 times, most recently from 27b0d6f to b73fb1b Compare January 24, 2023 13:53
Copy link
Contributor

@grotlue grotlue left a comment

Choose a reason for hiding this comment

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

General approach looks good, but I have a few suggestions for changes.

I haven't tested it yet, will take some time for that tomorrow :)

@grotlue
Copy link
Contributor

grotlue commented Feb 7, 2023

I tested it locally and everything except for one point seems to work fine:

General testing

Monosnap.screencast.2023-02-07.16-47-07.mp4

Scrolling

Monosnap.screencast.2023-02-07.16-49-47.mp4

Test results

  • ✅ Page is showing a list of failed sidecars
  • ✅ Search is working including search syntax and reset button
  • ✅ List is sortable by name and last seen
  • ✅ Click on the sidecar name leads to the sidecar detail page
    • ✅ Detail modal of the error messages still works on that page as well
  • ✅ Error messages are scrollable
    • ✅ Click on more will open the error message in a modal
  • ✅ Results are paginated correctly
  • ✅ Inactive sidecars are hidden when clicking on "Hide Inactive Sidecars" button
  • ❌ When the search has no results the text doesn't show the filter correctly
    Screenshot 2023-02-07 at 16 51 42

ousmaneo and others added 2 commits February 10, 2023 09:04
Co-authored-by: Laura <grotlue@users.noreply.github.com>
Co-authored-by: Laura <grotlue@users.noreply.github.com>
@ousmaneo ousmaneo self-assigned this Feb 10, 2023
@ousmaneo ousmaneo requested review from grotlue and removed request for ousmaneo February 10, 2023 15:33
Copy link
Contributor

@grotlue grotlue left a comment

Choose a reason for hiding this comment

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

Code-wise it looks good now, thanks for all the changes!

While testing I saw that the issue with the text for a search without results still exists:
Screenshot 2023-02-16 at 10 49 28

Besides that I tested the following other features which all work as expected:

  • Pagination (3 pages, also checking different page size)
  • Filtering
  • Sort
  • Opening the error message
  • Link to detail view

@ousmaneo
Copy link
Contributor

Thank you for testing and reviewing. I fixed the issue.

Copy link
Contributor

@grotlue grotlue left a comment

Choose a reason for hiding this comment

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

The build is failing, but it's only due to unused vars. I check it and it looks good now!

Screenshot 2023-02-16 at 11 34 14

Copy link
Contributor

@grotlue grotlue left a comment

Choose a reason for hiding this comment

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

The text looks good now!

However, while checking it again I realised that something is off when changing the entries per page size.

With 52 test entries

  • ✅ it shows all on one page when the page is set to 100
  • ❌ it shows all on one page when the page size is set to 50 (expected: 2 pages)
  • ❌ it shows all on two pages when the page size is set to 25 (expected 3 pages)

@ousmaneo
Copy link
Contributor

Hi @grotlue, as we discussed, I removed the page size selector as in this case, it does not make sense since we are listing collectors state and not just sidecars.

Copy link
Contributor

@grotlue grotlue left a comment

Choose a reason for hiding this comment

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

Let's merge this! (once the build is green)

@ousmaneo ousmaneo merged commit 7931e32 into master Feb 22, 2023
@ousmaneo ousmaneo deleted the sidecar-failure-tracking branch February 22, 2023 08:05
@williamtrelawny
Copy link
Contributor

Hi @ousmaneo can you confirm for me which version (and backports if any) this was released to? I don't see anything about this in the Release Announcements

@ousmaneo
Copy link
Contributor

ousmaneo commented Jun 13, 2023

Hi @williamtrelawny this was merged and released with 5.1.0-beta.1. And does not have any backports.

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

Successfully merging this pull request may close these issues.

Include failure reason in Sidecar Collectors UI, improve failure tracking for large deployments
4 participants