-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Make logstash/node_stats metricset work for Stack Monitoring without xpack.enabled flag #21546
Make logstash/node_stats metricset work for Stack Monitoring without xpack.enabled flag #21546
Conversation
Pinging @elastic/stack-monitoring (Stack monitoring) |
Pinging @elastic/integrations-services (Team:Services) |
I'm seeing this error when running this:
|
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.
LGTM! UI loads up as expected!
…sh/node_stats_xpack_flag
…sh/node_stats_xpack_flag # Conflicts: # metricbeat/module/logstash/node_stats/data_xpack.go
4ebe049
to
d7e5860
Compare
|
||
func (m *MetricSet) init() error { | ||
if m.XPack { | ||
return m.CheckPipelineGraphAPIsAvailable() |
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.
I think we will need to call m.CheckPipelineGraphAPIsAvailable()
every time now, right, since we plan to always call the Logstash Pipeline Graph APIs in the Fetch()
method, no?
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.
I removed it because I didn't see any call to this method anywhere, haha. Anyways I guess it should be included so I added it back into the Fetch
call directly.
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.
This is looking mostly good. Just left one comment/question.
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.
I left a comment/question a few minutes ago. Besides that, everything in this PR LGTM. So I'm approving it for merge now. Feel free to address my comment before merging or after, in the feature branch, as you prefer.
This reverts commit da828ea.
…xpack.enabled flag (elastic#21546)
Ready to test