[amtool] - Add new command to update silence #1123

lebinh · 2017-11-27T07:09:30Z

This adds a new command, update (and also its alias, extend), to update
existing silence in Alertmanager. User can use this command to update the
expiration or comment on existing silences. The API already support this
so I only expose the same functionality to amtool.

Don't allow update CreatedBy field as it is "Created" not "Updated", so
we should keep the original author.

Sample usage:

$ ./amtool silence add -e 2h -c test foo=bar
2c9ff0c4-9133-4c82-b4a1-01806c3a6673

$ ./amtool silence query foo=bar
ID                                    Matchers  Ends At                  Created By  Comment
2c9ff0c4-9133-4c82-b4a1-01806c3a6673  foo=bar   2017-11-27 08:49:04 UTC  @binh       test

$ ./amtool silence extend -e 4h -c 'updated test' 2c9ff0c4-9133-4c82-b4a1-01806c3a6673
ID                                    Matchers  Ends At                  Created By  Comment
2c9ff0c4-9133-4c82-b4a1-01806c3a6673  foo=bar   2017-11-27 10:49:28 UTC  @binh       updated test

$ ./amtool silence query foo=bar
ID                                    Matchers  Ends At                  Created By  Comment
2c9ff0c4-9133-4c82-b4a1-01806c3a6673  foo=bar   2017-11-27 10:49:28 UTC  @binh       updated test

josedonizetti · 2017-11-30T15:34:14Z

@lebinh I know most of amtool code isn't tested. But I really would like to change that. Especially on new code. Do you mind writing tests here?

lebinh · 2017-12-05T06:54:26Z

Okay, I'll try to add some test soon

stuartnelson3 · 2017-12-07T14:03:05Z

I'll take a look at this tomorrow. Can you resolve the conflicts?

This adds a new command, update (and also its alias, extend), to update existing silence in Alertmanager. User can use this command to update the expiration or comment on existing silences. The API already support this so I only expose the same functionality to amtool. Don't allow update CreatedBy field as it is "Created" not "Updated", so we should keep the original author.

lebinh · 2017-12-08T02:08:14Z

Resolved, although I haven't got around to add any test yet.

stuartnelson3

Few small requests.

I'm not sure if users would want to update date.start on a silence, to be honest. If that's not something you're interested in we can leave it out and just update the flag name for date.end, and if people want date.start it can be added later.

stuartnelson3 · 2017-12-08T09:12:36Z

cli/silence_update.go

+	Aliases: []string{"extend"},
+	Args:    cobra.MinimumNArgs(1),
+	Short:   "Update silences",
+	Long:    `Extend or update existing silence in Alertmanager.`,


Could you update this with a longer description, with a usage example? It would also be good to note that expires-on will override expires.

I'dd add that in once we decided on the parameter names :)

stuartnelson3 · 2017-12-08T09:16:04Z

cli/silence_update.go

+}
+
+func init() {
+	updateCmd.Flags().StringP("expires", "e", "", "Duration of silence (100h)")


I'm thinking we should rename these. Maybe expires should become duration, with d as the short flag?

I was just following the parameter names in silence add command here. duration does make more sense than expires though, but if we are going to change we need to update add command as well. That will be a breaking change though.

stuartnelson3 · 2017-12-08T09:19:21Z

cli/silence_update.go

+
+func init() {
+	updateCmd.Flags().StringP("expires", "e", "", "Duration of silence (100h)")
+	updateCmd.Flags().String("expire-on", "", "Expire at a certain time (Overwrites expires) RFC3339 format 2006-01-02T15:04:05Z07:00")


In alertmanager, you can also update the starting time when editing a silence.

How about adding that as well, and naming the flags date.start and date.end?

hm, thinking more about the name .. not sure i like date. @grobie , naming guru, any recommendation?

Should we just follow the field names in Alertmanager API, startsAt and endsAt?

hrm, those are more json-idiomatic names. for a CLI I think something different would be more appropriate.

stuartnelson3

Thanks for pointing out the consistency you were maintaining with silence add.

I'll go ahead and merge this. Would you like to prepare another PR that updates both update and add's API?

I'm thinking we change expires to duration, and expire-on to date.end, and add date.start to both commands.

…ave item suffix (prometheus#1123) In some cases the file might be called "temp" instead of the usual format "temp<index>_<item>" as described in the kernel docs: https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface In this case, treat this as an _input file containing the current temperature reading. Fixes prometheus#1122 Signed-off-by: Paul Gier <pgier@redhat.com>

* collector/diskstats: don't fail if there are extra stats, just ignore… (prometheus#1125) * collector/diskstats: don't fail if there are extra stats, just ignore them Signed-off-by: Paul Gier <pgier@redhat.com> Signed-off-by: Ben Kochie <superq@gmail.com> * collector/hwmon_linux: handle temperature sensor file which doesn't have item suffix (prometheus#1123) In some cases the file might be called "temp" instead of the usual format "temp<index>_<item>" as described in the kernel docs: https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface In this case, treat this as an _input file containing the current temperature reading. Fixes prometheus#1122 Signed-off-by: Paul Gier <pgier@redhat.com> Signed-off-by: Ben Kochie <superq@gmail.com> * Handle 'Unknown' as measurement value. (prometheus#1113) We use the output-compatible perccli and storcli.py does not handle 'Unknown' as a result: ``` sg="Error parsing \"/var/lib/node_exporter/perccli.prom\": text format parsing error in line 222: expected float as value, got \"Unknown\"" source="textfile.go:212" ``` I know, the perccli should not return 'Unknown' but this error breaks all other useful measurements because the prom file is not parsable. My if condition fixes this. Signed-off-by: Andreas Wirooks <4233401+nudgegoonies@users.noreply.github.com> Signed-off-by: Ben Kochie <superq@gmail.com> * circleci: switch to 2.1 config Signed-off-by: Ben Kochie <superq@gmail.com> * Convert to Go modules (prometheus#1178) * Convert to Go modules * Update promu config. * Convert to Go modules. * Update vendoring. * Update Makefile.common. * Update circleci config. * Use Prometheus release tar for promtool. * Fixup unpack * Use temp dir for unpacking tools. * Use BSD compatible tar command. * OpenBSD mkdir doesn't support `-v`. Signed-off-by: Ben Kochie <superq@gmail.com> * Add fallback for missing /proc/1/mounts (prometheus#1172) * Add fallback for missing /proc/1/mounts On some systems, `/proc/1/mounts` is hidden from non-root users due to the `hidepid` procfs feature. Attempt to fallback to `/proc/mounts` if `/proc/1/mounts` is not found. Signed-off-by: Ben Kochie <superq@gmail.com> * Add tests. Signed-off-by: Ben Kochie <superq@gmail.com> * Add CHANGELOG entry. Signed-off-by: Ben Kochie <superq@gmail.com> * Release v0.17.0 (prometheus#1168) * Update CHANGELOG * Update VERSION Signed-off-by: Ben Kochie <superq@gmail.com>

lebinh force-pushed the binh/silence-extend branch from bf70b81 to 5991fff Compare December 8, 2017 02:05

stuartnelson3 suggested changes Dec 8, 2017

View reviewed changes

stuartnelson3 approved these changes Dec 11, 2017

View reviewed changes

stuartnelson3 merged commit dbff31d into prometheus:master Dec 11, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[amtool] - Add new command to update silence #1123

[amtool] - Add new command to update silence #1123

lebinh commented Nov 27, 2017

josedonizetti commented Nov 30, 2017

lebinh commented Dec 5, 2017

stuartnelson3 commented Dec 7, 2017

lebinh commented Dec 8, 2017

stuartnelson3 left a comment

stuartnelson3 Dec 8, 2017

lebinh Dec 11, 2017

stuartnelson3 Dec 8, 2017

lebinh Dec 11, 2017

stuartnelson3 Dec 11, 2017

stuartnelson3 Dec 8, 2017

stuartnelson3 Dec 9, 2017

lebinh Dec 11, 2017

stuartnelson3 Dec 11, 2017

stuartnelson3 left a comment

[amtool] - Add new command to update silence #1123

[amtool] - Add new command to update silence #1123

Conversation

lebinh commented Nov 27, 2017

josedonizetti commented Nov 30, 2017

lebinh commented Dec 5, 2017

stuartnelson3 commented Dec 7, 2017

lebinh commented Dec 8, 2017

stuartnelson3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stuartnelson3 left a comment

Choose a reason for hiding this comment