-
Notifications
You must be signed in to change notification settings - Fork 104
Conversation
replay
commented
Aug 3, 2017
- Fixes the initialization of the connection to the Cassandra index from the writer, previously this always tried to connect to localhost.
- Adds auth support and insecure ssl support to the reader.
- Better error handling in the reader.
- A useful help message for the writer.
- A status URL on the writer that can be used for health checks.
@@ -173,11 +188,18 @@ func processFromChan(files chan string, wg *sync.WaitGroup) { | |||
req.Header.Set("Content-Type", "application/json") | |||
req.Header.Set("Content-Encoding", "gzip") | |||
|
|||
_, err = client.Do(req) | |||
if len(*httpAuth) > 0 { | |||
req.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(*httpAuth))) |
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.
Rather than manually adding the header, you can call
req.URL.User = url.UserPassword(user, pass)
But as you have the user/pass in a single string, what you are doing is probably easier.
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.
ok thx, i'll leave it then
req.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(*httpAuth))) | ||
} | ||
|
||
resp, err := client.Do(req) |
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.
You need to drain the resp.Body in order for the connection to be re-used.
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.
👍
@@ -131,6 +171,7 @@ func main() { | |||
server.Index.Init() | |||
|
|||
http.HandleFunc(*uriPath, server.chunksHandler) | |||
http.HandleFunc("/status", server.statusHandler) |
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.
if this is just for healthchecking, the convention is to use "/healthz"
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.
👍
@woodsaj that's all fixed |
Just some feedback, i tested this on a large deploy it appears working. Here is a validation that the metrics are stored, though i still do not see them in the graphs.
|
thx for checking @tehlers320 . to make them show up you'll need to restart metrictank, because otherwise it will not re-read the index. |
That's the part i don't understand, so my ID "1.fcc5e1772ffe25a18dc412e5a06afd43" is already there because i'm mirroring traffic from the old system. I gave it a restart anyways, no historical metrics. consolidateBy(average,sum,min,max) also does not bring up historical metrics. Sorry to keep bothering you about this. |
Do the retention configurations in MT and your imported whisper archives match? currently the importer does not modify the archives/retentions, so if these settings don't match some data might not show up depending on the differences. |
ahh ugh, thats it . Thanks @replay Well i really don't like my old retention schema darn! |
I think sooner or later we'll probably need to add the ability to update retentions to the importer, because I bet you're not the only one with that problem. |
@tehlers320 did you see the |
@replay yes i tried reading only archive 5 but archive 5 This is what i inherited 8h:2y doesn't seem to work well with the chunk system i think. |