Tools: Add CLI to create tombstones in obj store #3006
Conversation
Signed-off-by: Harshitha1234 <[email protected]>
cmd/thanos/tools_delete.go
Outdated
| kingpin "gopkg.in/alecthomas/kingpin.v2" | ||
| ) | ||
|
|
||
| type Tombstone struct { |
There was a problem hiding this comment.
Probably we need Thanos tombstone package (:
Signed-off-by: Harshitha1234 <[email protected]>
Signed-off-by: Harshitha1234 <[email protected]>
cmd/thanos/tools_delete.go
Outdated
|
|
||
| ts := tombstone.NewTombstone(*matcher, minTime.PrometheusTimestamp(), maxTime.PrometheusTimestamp()) | ||
|
|
||
| err = tombstone.UploadTombstone(ts, bkt, logger) |
There was a problem hiding this comment.
what about... return tombstone.Upload.... (:
cmd/thanos/tools_delete.go
Outdated
| return err | ||
| } | ||
|
|
||
| ts := tombstone.NewTombstone(*matcher, minTime.PrometheusTimestamp(), maxTime.PrometheusTimestamp()) |
There was a problem hiding this comment.
I think single tombstones can have more than one matcher? 🤔
There was a problem hiding this comment.
Yes and we check the validity of the user input for the matchers using
_, err = parser.ParseMetricSelector(*matchers) and store them in the form of a string.
cmd/thanos/tools_delete.go
Outdated
| ) | ||
|
|
||
| func registerDelete(m map[string]setupFunc, app *kingpin.CmdClause, pre string) { | ||
| cmd := app.Command("delete", "Delete series command") |
There was a problem hiding this comment.
worth to mention
- from where?
- is this deleted immediately?
- where are the detailed docs?
pkg/tombstone/tombstone.go
Outdated
|
|
||
| const ( | ||
| // TombstoneDir is the name of directory to upload tombstones. | ||
| TombstoneDir = "tombstones" |
pkg/tombstone/tombstone.go
Outdated
|
|
||
| // Tombstone represents a tombstone. | ||
| type Tombstone struct { | ||
| Matcher string `json:"matcher"` |
There was a problem hiding this comment.
We will be slicing the matchers during store API masking and while tombstone creation, we store it as a string. 🙂
pkg/tombstone/tombstone.go
Outdated
| ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) | ||
| defer cancel() | ||
|
|
||
| err = objstore.UploadFile(ctx, logger, bkt, tsPath, path.Join(TombstoneDir, GenName(tombstone))) |
| Matcher: matcher, | ||
| MinTime: minTime, | ||
| MaxTime: maxTime, | ||
| CreationTime: timestamp.FromTime(time.Now()), |
There was a problem hiding this comment.
Similar to Alertmanager Silencers wonder if we can allow specifying human readable reason why like reason field
and author field where people can optionally specify author of deletion.. 🤔 Maybe too much (:
cc @metalmatze
There was a problem hiding this comment.
Added the author and reason flags 👍
pkg/tombstone/tombstone.go
Outdated
|
|
||
| // GenName generates file name based on Matcher, MinTime and MaxTime of a tombstone. | ||
| func GenName(ts Tombstone) string { | ||
| hash := xxhash.Sum64([]byte(ts.Matcher + string(ts.MinTime) + string(ts.MaxTime))) |
There was a problem hiding this comment.
interesting (: What's the purpose of this name? Was this logic part of the design? 🤔
The only problem is that it will cause weird errors when someone will try to delete same things. Maybe that's ok - we should at least note it in design or comment here
There was a problem hiding this comment.
If a same request is repeated in the future, this logic helps overwrite the previous tombstone file and avoids duplicate tombstones. Yeah sure, will add this to the design proposal. 🙂
| return err | ||
| } | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) |
pkg/tombstone/tombstone_test.go
Outdated
| bkt := objstore.WithNoopInstr(objstore.NewInMemBucket()) | ||
|
|
||
| { | ||
| SampleTombstone := NewTombstone("up{a=\"b\"}", 00, 9999999) |
cmd/thanos/tools_delete.go
Outdated
|
|
||
| objStoreConfig := regCommonObjStoreFlags(cmd, "", true) | ||
|
|
||
| minTime := model.TimeOrDuration(cmd.Flag("min-time", "Start of time range limit to delete. Option can be a constant time in RFC3339 format or time duration relative to current time, such as -1d or 2h45m. Valid duration units are ms, s, m, h, d, w, y."). |
There was a problem hiding this comment.
Does this mean the deletion can be from -10d to -1d? That does not make much sense, right? (: I think we should be explicit in help that -1d will calculate the actual timestamp now
There was a problem hiding this comment.
Updated the flag help 👍
Signed-off-by: Harshitha1234 <[email protected]>
|
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Signed-off-by: Harshitha1234 [email protected]
Changes
thanos tools deletecommand tool to create tombstones in object storageTODO
Verification