-
Notifications
You must be signed in to change notification settings - Fork 212
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
Chore/collectd install prompt #275
Chore/collectd install prompt #275
Conversation
tool/processors/collectd/collectd.go
Outdated
@@ -21,7 +21,7 @@ func (p *processor) Process(ctx *runtime.Context, config *data.Config) { | |||
if ctx.OsParameter == util.OsTypeWindows { | |||
return | |||
} | |||
yes := util.Yes("Do you want to monitor metrics from CollectD?") | |||
yes := util.Yes("Do you want to monitor metrics from CollectD? Note that depending on the system, some require the CollectD software to be installed on your server (e.g. Amazon Linux 2) or the /usr/share/collectd/types.db file to be created.") |
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.
Since the CloudWatch Agent could fail to start, I'd recommend making the the "Note" into a flashy "WARNING" which is more likely to be seen by users:
yes := util.Yes("Do you want to monitor metrics from CollectD? WARNING: CollectD must be installed or the Agent will fail to start")
Another idea would be to modify the configuration validator to output a more helpful error message:
ERROR: collectd is configured, but is not installed. Please install collectd or change the config.
I'd like input from others on the preferred solution.
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 would be important not to break existing user workflows such as causing the config generator to fail if collectd is not installed properly. Someone could config the agent ahead of installing collectd.
What's the current agent behavior if collectd is configured but not installed?
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.
The Agent fails to start:
awsdocs/amazon-cloudwatch-user-guide#54
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 agree with @adam-mateen on this that it should be made more obvious that there will be an issue if collectd is not installed prior to starting the CW agent.
- This doesn't change any behavior - it just makes it clearer to the user that the agent will fail to start if collectd isn't installed
- The one thing I'd be a little hesitant on is the length of the input prompt string now, but that's mostly a nitpick. For someone working on a smaller terminal, the formatting might get a little messed up with such a long string. That being said, if there's another prompt in the repo that is also very long, then I have no qualms with 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.
Hi all, thanks for the review, I think the recommendation @adam-mateen made for option 1 fits the length (there's a longer prompt from func wantEC2TagDimensions
) and is sufficient for the user to be aware of aware of the CollectD installation. Shall I proceed with the refactor for option 1?
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 the wording is sufficient. I'd say go for it
Co-authored-by: Adam Mateen <[email protected]>
Co-authored-by: Github Action <[email protected]>
Description of the issue
#274
Following the wizard, there is a prompt to
monitor metrics from CollectD
. If this is enabled, there will be errors downstream when configuring the agent with the created JSON template.The error downstream is:
Error running agent: Error parsing /opt/aws/amazon-cloudwatch-agent/etc/amazon-cloudwatch-agent.toml, open /usr/share/collectd/types.db: no such file or directory
This is error is also mentioned in an issue in the amazon-cloudwatch-user-guide.
The cause of this is because some systems don't have CollectD installed as explained in AWS documentation for retrieving custom metrics with collectd
Description of changes
Add a note in the wizard when users are prompted to enable CollectD from the wizard. This note informs users that they need to have CollectD installed (I tested this fix on Amazon Linux 2) or have the
/usr/share/collectd/types.db file
file created (I tested this fix on RHEL8). Credit to these fixes can be found in the issue in the amazon-cloudwatch-user-guide.License
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Tests
Amazon Linux 2
Run Command
AmazonCloudWatch-ManageAgent
RHEL8
/usr/share/collectd/types.db file
file. Run the following:Run Command
AmazonCloudWatch-ManageAgent