vlagent: implement Kubernetes log collector#815
Conversation
9a03420 to
faedb5b
Compare
valyala
left a comment
There was a problem hiding this comment.
Looks great overall!
@vadimalekseev , please document the added functionality in docs/victorialogs/vlagent.md, and mention it at docs/victorialogs/CHANGELOG.md.
faedb5b to
ceeb226
Compare
|
maybe makes sense to move k8s client configuration from this PR and https://github.com/VictoriaMetrics/VictoriaMetrics/blob/1a68d4ac8a032dfa96cea8b984e17a3cb23f825d/lib/promscrape/discovery/kubernetes/kubeconfig.go to a separate package |
9c17ba6 to
b44ea2d
Compare
|
Added documentation.
upd: |
e704457 to
4d6b440
Compare
4fe8d5f to
bedc0a7
Compare
f9a9997 to
1c3b7b4
Compare
8757ae4 to
fbc8332
Compare
fbc8332 to
01cb965
Compare
|
@vadimalekseev , thank you for implementing discovery and collecting of logs in Kubernetes by |
| func (er podWatchStream) readEvents(h func(event watchEvent)) error { | ||
| lr := newLineReader(er.r) | ||
| for { | ||
| line, err := lr.readLine() |
There was a problem hiding this comment.
It should be easier to use json.Decoder here in the way similar to the code used in Kubernetes service dicsovery for VictoriaMetrics - https://github.com/VictoriaMetrics/VictoriaMetrics/blob/0cb90f91fcb109e895cb34d57a93e9a3e8d5bc01/lib/promscrape/discovery/kubernetes/api_watcher.go#L871-L876 . Then you won't need the code for manual reading of lines before parsing the next JSON-encoded event in the stream.
There was a problem hiding this comment.
Also, the readEvents() function always returns non-nil error. I think it shouldn't return io.EOF error when Kubernetes API server closes the response stream. It should return nil in this case, since this is the expected state.
As for the io.UnexpectedEOF handling, could you verify whether it should be treated as io.EOF or it is better to return the io.UnexpectedEOF as is. I think that it must be returned to the caller of the readEvents() function as is, since it usually means that the connection is closed in the middle of reading of JSON-encoded event (e.g. this is an error, which must be reported to the user, so he could decide what to do next).
There was a problem hiding this comment.
could you verify whether it should be treated as io.EOF or it is better to return the io.UnexpectedEOF as is. I think that it must be returned to the caller of the readEvents() function as is, since it usually means that the connection is closed in the middle of reading of JSON-encoded event (e.g. this is an error, which must be reported to the user, so he could decide what to do next).
In our test environment, all connection interruption errors appeared as io.UnexpectedEOF. This is why a added this commit 5895712 . I'm not sure if we encountered io.EOF errors, as we have been handling those since the initial vlagent release. Therefore, I believe we should handle both cases.
I can deploy a test release with verbose error logging to the test environment to verify which errors we are receiving.
upd:
I couldn't find those logs in our test environment. Most likely, it happened in my local Kubernetes cluster. It's possible my laptop went into sleep mode while the Kubernetes API was streaming JSON.
valyala
left a comment
There was a problem hiding this comment.
See the recommendation about using json.Decoder for reading JSON-encoded event stream without the need to manually read lines from the stream.
Describe Your Changes
Fixes: #538
Checklist
The following checks are mandatory: