Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Collect version metadata for Fluentd #5057
Collect version metadata for Fluentd #5057
Changes from 3 commits
7d8b2f2
70f0013
2d14166
d4dea05
cbbda0a
6eadc85
3083b6a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
It seems the the api doesn't provide currently the version, but can be added easily upstream here, example:
https://github.com/fluent/fluentd/blob/master/lib/fluent/plugin/in_monitor_agent.rb#L65-L68
Even if we push this change upstream, it will be available only for new versions, so we will still need to retrieve versions via command line for old versions.
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.
Yup. I'll open an issue (or most likely a PR directly) on the fluentd repo to discuss adding the version to their monitoring REST API.
Agreed that this is not a blocker w.r.t. this PR, though? (i.e. we can add the API-based version lookup later.)
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.
@AlexandreYang I opened fluent/fluentd#2705 and fluent/fluentd#2706 on the Fluentd repo.
Will discuss there whether supporting accessing a remote Fluentd instance makes any sense at all from a monitoring perspective.
I'm now less sure that it can actually be a possible setup: most examples from the Fluentd monitoring docs show calls to
localhost
or similar.So, if people always only install the integration on the host where Fluentd is installed, then the benefits we'd get from using the API are less clear IMO.