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

Fix empty node stats pipelines #17185

Merged
merged 2 commits into from
Feb 28, 2025
Merged

Fix empty node stats pipelines #17185

merged 2 commits into from
Feb 28, 2025

Conversation

kaisecheng
Copy link
Contributor

@kaisecheng kaisecheng commented Feb 28, 2025 •
edited
Loading

Release notes

Fixed an issue where the /_node/stats API displayed empty pipeline metrics when X-Pack monitoring was enabled

What does this PR do?

When monitoring is enabled, the metrics store does not initialise data for monitoring pipeline. The api has null pointer when fetching the pipeline config like workers, batch_size.

Why is it important/What is the impact to the user?

/_node/stats gives empty pipelines: {}

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@mergify Mergify
Copy link

mergify bot commented Feb 28, 2025

This pull request does not have a backport label. Could you fix it @kaisecheng? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit.
  • backport-8.x is the label to automatically backport to the 8.x branch.

@mergify Mergify
Copy link

mergify bot commented Feb 28, 2025

backport-8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label.

@mergify mergify bot added the backport-8.x Automated backport to the 8.x branch with mergify label Feb 28, 2025
@kaisecheng kaisecheng added backport-8.17 Automated backport with mergify backport-9.0 Automated backport to the 9.0 branch with mergify backport-8.18 Automated backport with mergify labels Feb 28, 2025
@kaisecheng kaisecheng force-pushed the fix_node_stats_pipelines branch from 2f3c03c to c0b4cdb Compare February 28, 2025 18:38
@kaisecheng kaisecheng marked this pull request as ready for review February 28, 2025 18:47
@elastic-sonarqube Elastic SonarQube
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

donoghuc
donoghuc reviewed Feb 28, 2025
View reviewed changes
logstash-core/lib/logstash/api/commands/stats.rb
:workers => stats[:config][:workers],
:batch_size => stats[:config][:batch_size],
:batch_delay => stats[:config][:batch_delay],
:workers => stats.dig(:config, :workers),
Copy link
Member

Choose a reason for hiding this comment

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

So the bug before was indexing a nil class? In the case we dont have data here should we remove nils from the map? Should we default to an empty string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it was nil. All other fields in this pipeline are not showing any meaningful data, it only gives empty array or null. This monitoring feature is deprecated for years. I think we have good enough reason to not set a default value here and just show null

@elasticmachine
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

History

donoghuc
donoghuc approved these changes Feb 28, 2025
View reviewed changes
Copy link
Member

@donoghuc donoghuc left a comment

Choose a reason for hiding this comment

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

LGTM!

@kaisecheng kaisecheng merged commit 8678581 into elastic:main Feb 28, 2025
9 checks passed
mergify bot pushed a commit that referenced this pull request Feb 28, 2025
@kaisecheng @mergify
Fix empty node stats pipelines (#17185)
e48f1b2 
Fixed an issue where the `/_node/stats` API displayed empty pipeline metrics
when X-Pack monitoring was enabled

(cherry picked from commit 8678581)
@mergify mergify bot mentioned this pull request Feb 28, 2025
Merged
5 tasks
mergify bot pushed a commit that referenced this pull request Feb 28, 2025
@kaisecheng @mergify
Fix empty node stats pipelines (#17185)
d6b48f2 
Fixed an issue where the `/_node/stats` API displayed empty pipeline metrics
when X-Pack monitoring was enabled

(cherry picked from commit 8678581)
@mergify mergify bot mentioned this pull request Feb 28, 2025
Open
5 tasks
@kaisecheng
Copy link
Contributor Author

@logstashmachine backport 8.18

@kaisecheng
Copy link
Contributor Author

@logstashmachine backport 9.0

github-actions bot pushed a commit that referenced this pull request Feb 28, 2025
@kaisecheng @logstashmachine
Fix empty node stats pipelines (#17185)
2cb2bc4 
Fixed an issue where the `/_node/stats` API displayed empty pipeline metrics
when X-Pack monitoring was enabled

(cherry picked from commit 8678581)
@github-actions github-actions bot mentioned this pull request Feb 28, 2025
Merged
5 tasks
github-actions bot pushed a commit that referenced this pull request Feb 28, 2025
@kaisecheng @logstashmachine
Fix empty node stats pipelines (#17185)
25cc531 
Fixed an issue where the `/_node/stats` API displayed empty pipeline metrics
when X-Pack monitoring was enabled

(cherry picked from commit 8678581)
@github-actions github-actions bot mentioned this pull request Feb 28, 2025
Merged
5 tasks
kaisecheng added a commit that referenced this pull request Feb 28, 2025
@github-actions @kaisecheng
Fix empty node stats pipelines (#17185) (#17204)
c6149e6 
Fixed an issue where the `/_node/stats` API displayed empty pipeline metrics
when X-Pack monitoring was enabled

(cherry picked from commit 8678581)

Co-authored-by: kaisecheng <69120390+kaisecheng@users.noreply.github.com>
kaisecheng added a commit that referenced this pull request Feb 28, 2025
@github-actions @kaisecheng
Fix empty node stats pipelines (#17185) (#17202)
d10aeef 
Fixed an issue where the `/_node/stats` API displayed empty pipeline metrics
when X-Pack monitoring was enabled

(cherry picked from commit 8678581)

Co-authored-by: kaisecheng <69120390+kaisecheng@users.noreply.github.com>
kaisecheng added a commit that referenced this pull request Feb 28, 2025
@mergify @kaisecheng
Fix empty node stats pipelines (#17185) (#17197)
bb6ffd0 
Fixed an issue where the `/_node/stats` API displayed empty pipeline metrics
when X-Pack monitoring was enabled

(cherry picked from commit 8678581)

Co-authored-by: kaisecheng <69120390+kaisecheng@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@donoghuc donoghuc donoghuc approved these changes

Assignees
No one assigned
Labels
backport-8.x Automated backport to the 8.x branch with mergify backport-8.17 Automated backport with mergify backport-8.18 Automated backport with mergify backport-9.0 Automated backport to the 9.0 branch with mergify v8.18.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Node Stats API doesn't return pipelines when Internal Monitoring is Enabled
3 participants
@kaisecheng @elasticmachine @donoghuc