-
Notifications
You must be signed in to change notification settings - Fork 9.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
etcd-dump-logs: add decoder support #9790
Conversation
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.
Some minor formatting issue. Can we also rebase from current master branch? We've bumped up the base test image to use Go 1.10.2.
Will try this new patch as well.
tools/etcd-dump-logs/README.md
Outdated
Lists all the interested entries from WAL log. | ||
|
||
``` | ||
|
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.
remove this unneeded blank line at 39?
tools/etcd-dump-logs/README.md
Outdated
Entry types () count is : 4 | ||
``` | ||
|
||
[decoder_correctoutputformat.sh]: https://github.com/wenjiaswe/etcd/blob/decoder-support/tools/etcd-dump-logs/testdecoder/decoder_correctoutputformat.sh |
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.
[decoder_correctoutputformat.sh]: ./testdecoder/decoder_correctoutputformat.sh
?
@@ -45,6 +45,9 @@ func TestEtcdDumpLogEntryType(t *testing.T) { | |||
t.Skipf("%q does not exist", dumpLogsBinary) | |||
} | |||
|
|||
decoder_correctoutputformat := path.Join(binDir + "/testdecoder/decoder_correctoutputformat.sh") |
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.
filepath.Join(binDir, "/testdecoder/decoder_correctoutputformat.sh")
?
@@ -45,6 +45,9 @@ func TestEtcdDumpLogEntryType(t *testing.T) { | |||
t.Skipf("%q does not exist", dumpLogsBinary) | |||
} | |||
|
|||
decoder_correctoutputformat := path.Join(binDir + "/testdecoder/decoder_correctoutputformat.sh") | |||
decoder_wrongoutputformat := path.Join(binDir + "/testdecoder/decoder_wrongoutputformat.sh") |
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.
filepath.Join(binDir, "/testdecoder/decoder_wrongoutputformat.sh")
?
tools/etcd-dump-logs/main.go
Outdated
@@ -31,6 +35,8 @@ import ( | |||
"github.com/coreos/etcd/wal/walpb" | |||
|
|||
"go.uber.org/zap" | |||
"bytes" |
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.
Can you move "bytes"
and "os"
at the same level as "encoding/hex"
? Just minor gofmt
issue.
tools/etcd-dump-logs/README.md
Outdated
etcd-dump-logs inspects etcd db files. | ||
|
||
``` | ||
|
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.
Remove unnecessary blank line at 6?
c0a49a3
to
3ef3cff
Compare
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.
@gyuho Thank you very much for your review, I have addressed all your comments and rebased from the latest master branch, I have reran the unit tests and they all passed:
$ PASSES=unit PKG=./tools/etcd-dump-logs TESTCASE="\bTestEtcdDumpLogEntryType\b" ./test
Running with TEST_CPUS: 1,2,4
Starting 'unit' pass at Wed May 30 15:44:58 PDT 2018
Running unit tests...
3ef3cff
to
4dfad83
Compare
tools/etcd-dump-logs/main.go
Outdated
if err != nil { | ||
log.Panic(err) | ||
} | ||
os.Stderr.WriteString("decoder stderr: " + stderr.String()) |
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 stderr.String() != "" {
os.Stderr.WriteString("decoder stderr: " + stderr.String())
}
?
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.
Thanks! Fixed!
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.
I manually tried it and works great. Thanks a lot!
Just minor nits on decoder stderr:
output. And lgtm.
4dfad83
to
5871905
Compare
tools/etcd-dump-logs/README.md
Outdated
9 6 norm ID:10 lease_grant:<TTL:1 ID:1 > decoder output format is not right, print output anyway jhjadbjdjhjaajja | ||
12 7 norm ID:13 auth_enable:<> decoder output format is not right, print output anyway jhjdcbcejj | ||
27 8 norm ??? decoder output format is not right, print output anyway cf | ||
decoder stderr: |
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.
Remove this?
tools/etcd-dump-logs/README.md
Outdated
9 6 norm ID:10 lease_grant:<TTL:1 ID:1 > ERROR jhjadbjdjhjaajja | ||
12 7 norm ID:13 auth_enable:<> ERROR jhjdcbcejj | ||
27 8 norm ??? ERROR cf | ||
decoder stderr: |
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.
Remove this?
Manually addressed README.md typos. Merging. Thanks. |
Thanks, @gyuho ! |
etcd-dump-logs: add decoder support
Added "stream-decoder" flag so user could set the decoder and decoder args they want to use to decode each WAL log entries.
Expected behavior of decoder:
The decoder should be able to take hex encoded lines of binary as input (from etcd-dump-logs), process it, output the status and decoded data, preferably in the format of <DECODER_STATUS>|<DECODED_DATA>. (If the output is not in the preferable format, result will be put into decoded data column regardlessly)
Expected behavior of etcd-dump-logs:
When the stream-decoder is specified, etcd-dump-logs will feed the hex encoded lines of WAL LOG binary into the decoder and output the decoder_status and decoded_data for each entry.
Two simple decoders were added for testing and demonstrating purpose:
Both decoders (decoder_correctoutputformat.sh and decoder_wrongoutputformat.sh ) are simply converting 1234567890 to abcdefghij. decoder_correctoutputformat.sh outputs decoder_status and decoded_data in the preferable format as above (<DECODER_STATUS>|<DECODED_DATA>), while decoder_wrongoutputformat.sh does not. The different result from etcd-dump-logs can be found in decoder_correctoutputformat.output and decoder_wrongoutputformat.output.
Combination usage of etcd-dump-logs and Auger:
Other than the two "fake" decoder above, I would like to mention a real usage of this added feature using Auger as decoder. Auger (please refer to Auger link for further details if you are interested) is a very useful tool in kubernetes community to inspect and analyze kubernetes objects stored in etcd. Now with this change, we could use Auger as the decoder to decode kubernetes objects in WAL log and have them output nicely. Here is the example of using etcd-dump-logs and Auger.
README.md is added.
As etcd-dump-logs is getting more powerful and complicated, I think it's easier to have a README.md to explain the usage.
@gyuho @xiang90 @jpbetz