Conversation
added 12 commits
December 31, 2016 15:58
The primary goal of an alertmanager tool is to provide a cli interface for the prometheus alertmanager. My vision for this tool has two parts: - Silence management (query, add, delete) - Alert management (query, maybe more in future?) Resolves: prometheus#567
mattbostock
suggested changes
Jan 23, 2017
mattbostock
left a comment
mattbostock
left a comment
There was a problem hiding this comment.
I've added some initial comments.
The build failed so I haven't yet been able to try it out - see my comment on cli/silence_test.go.
cmd/amtool/main.go
Outdated
| @@ -0,0 +1,9 @@ | |||
| // Copyright © 2016 Kellen Fox <[email protected]> | |||
There was a problem hiding this comment.
The copyright belongs to Cloudflare, assuming you worked on this on your work machine during work hours. I don't think there's a good reason to put the copyright notice in this file but I am not a lawyer.
Owner
Author
There was a problem hiding this comment.
I'll just delete. The cobra autogen tool puts a copyright notice in.
cli/root.go
Outdated
| var RootCmd = &cobra.Command{ | ||
| Use: "amtool", | ||
| Short: "Alertmanager CLI", | ||
| Long: `View and modify the current alertmanager state.`, |
cli/root.go
Outdated
| Long: `View and modify the current alertmanager state.`, | ||
| // Uncomment the following line if your bare application | ||
| // has an action associated with it: | ||
| // Run: func(cmd *cobra.Command, args []string) { }, |
There was a problem hiding this comment.
I think these commands can be removed?
cli/root.go
Outdated
| func Execute() { | ||
| if err := RootCmd.Execute(); err != nil { | ||
| fmt.Println(err) | ||
| os.Exit(-1) |
There was a problem hiding this comment.
Exit codes are uint8, unless you want to return a 255 exit code?
Suggest os.Exit(1).
Owner
Author
There was a problem hiding this comment.
Hmm. I just copy/paste'ed this from the cobra docs. Changed to 1
cli/root.go
Outdated
| } | ||
| } | ||
|
|
||
| func getAlertmanagerUrl() { |
There was a problem hiding this comment.
Owner
Author
There was a problem hiding this comment.
This function isn't used. I've deleted it
cli/root.go
Outdated
| func getAlertmanagerUrl() { | ||
| am := viper.GetString("alertmanager") | ||
| if am == "" { | ||
| fmt.Println("Alertmanager url not specified in the config file or on the command line") |
cli/silence.go
Outdated
| return output | ||
| } | ||
|
|
||
| // Thanks @vendemiat for help with this one |
There was a problem hiding this comment.
I'd put that in the commit message, that way you still get to say thanks but it doesn't clutter the code.
cli/silence_test.go
Outdated
| &types.Matcher{Name: "bar", Value: "baz", IsRegex: false}, | ||
| } | ||
|
|
||
| testGroups := MatcherGroup{ |
There was a problem hiding this comment.
Build fails with:
cli/silence_test.go:54: undefined: MatcherGroup
Owner
Author
|
Hopefully It builds for you now. Currently all the tests seem broken due to the mismatched references to github.com/Kellel/alertmanager vs github.com/prometheus/alertmanager Not really sure how to deal with this |
mattbostock
reviewed
Jan 23, 2017
cli/root.go
Outdated
|
|
||
| RootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (default is $HOME/.amtool.yaml)") | ||
| RootCmd.PersistentFlags().String("alertmanager", "", "Alertmanager to talk to") | ||
| viper.BindPFlag("alertmanager", RootCmd.PersistentFlags().Lookup("alertmanager")) |
There was a problem hiding this comment.
alertmanager.url for consistency with Prometheus' CLI flags.
mattbostock
reviewed
Jan 23, 2017
cli/root.go
Outdated
| func init() { | ||
| cobra.OnInitialize(initConfig) | ||
|
|
||
| RootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (default is $HOME/.amtool.yaml)") |
There was a problem hiding this comment.
Prometheus and Alertmanager default to a .yml extension.
|
It would be good to show a friendlier error if someone forgets to specify the Alertmanager URL: |
|
Ditto for a friendlier error when the URL is incorrect: |
added 2 commits
January 23, 2017 08:48
Error messages are maybe better?
|
Rough notes from a pairing session between @Kellel and I, documenting my first time using amtool:
|
mattbostock
reviewed
Jan 24, 2017
cli/silence.go
Outdated
| var silenceCmd = &cobra.Command{ | ||
| Use: "silence", | ||
| Short: "Manage silences", | ||
| Long: `Add, expire or view silences For more information and additional flags see query help`, |
There was a problem hiding this comment.
Can probably remove 'For more information and additional flags see query help', not sure it adds anything.
Owner
Author
|
I've reformatted the list from my session with @mattbostock I've prioritized the items to help myself focus (x=complete, ● = todo) BUGS(Need immediate attention) x panic REQUIRED Features(Required for merge)
PROBABLY(Things that are lower priority, but most likely need to be added before merging to core
NICE to haves(things that I would like to implement, but I find lower priority)
DEBATABLE(things I'm not sure if I agree with. I will hold on to these and see if the core devs want to see them implemented as well)
|
added 3 commits
February 24, 2017 15:00
* also allow for man page generation * update vendors to include cobra/doc * split silence commands into multiple files * move some ommon gits into utils.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This is the first version of an alertmanager tool.