cmd: Add log parser #98

jodh-intel · 2018-02-28T12:46:30Z

Add a log parsing tool (kata-log-parser) that reads multiple logfmt
[*] logfiles and writes their output in time order showing time
differences between log entries.

The tool can also check logs for validity and convert the logs to
various alternative formats.

For full details:

$ kata-log-parser --help

Fixes #93.

[*] - https://brandur.org/logfmt

Signed-off-by: James O. D. Hunt james.o.hunt@intel.com

jodh-intel · 2018-02-28T12:47:44Z

Blocked on the following trivial PRs which just need a few acks:

grahamwhaley · 2018-02-28T13:43:01Z

I'm guessing the CI fail is due to the pending deps:

ERROR: kata-log-parser: failed to parse time for value 2018-02-28T13:06:08Z (entry: &{Count:0 TimeDelta:0

So, I'll mark this as DNM to stop the CI trying until we have those deps merged then.

jodh-intel · 2018-02-28T13:45:24Z

Yes - we really need those 3 other PRs merged...

grahamwhaley

hands up - I didn't scrutinise the code - but, for the general overview and intention,
lgtm
few minor queries...

grahamwhaley · 2018-02-28T13:45:27Z

.ci/run.sh

+		eval "$cmd"
+	done
+
+	dir="."


just a note: the dir feels strangely redundant (just *.log would match in . anyhow, no?). And, I'd probably have put a $(pwd) maybe, but that is probably just personal preference.

grahamwhaley · 2018-02-28T13:49:01Z

cmd/log-parser/Gopkg.toml

+# required = ["github.com/user/thing/cmd/thing"]
+# ignored = ["github.com/user/project/pkgX", "bitbucket.org/user/project/pkgA/pkgY"]
+#
+# [[constraint]]


we could nuke the example comments if you fancy.

grahamwhaley · 2018-02-28T13:50:45Z

cmd/log-parser/Makefile

+default: install
+
+check:
+	go test .


I guess we are relying on the global CI scripts to do any static analysis

Yep - when any PR is raised on this repo, the .ci/static-checks.sh will be run on all packages. In fact, that script is currently checking a leeeetle too much - see #100 :)

grahamwhaley · 2018-02-28T13:51:40Z

cmd/log-parser/README.md

+them, adding a time delta showing how much time has elapsed between each log
+entry.
+
+It is checks the logs for validity and can re-format the logs and output them in a different format.


s/It is/It/

grahamwhaley · 2018-02-28T13:52:40Z

cmd/log-parser/README.md

+
+## Component logfiles
+
+The primary logfiles the tool reads are:


I presume the tool itself is component neutral? It will work with any set of appropriately formatted log files I think?

I've added a new section to the doc explaining the tools expectations.

grahamwhaley · 2018-02-28T13:55:18Z

cmd/log-parser/agent.go

@@ -0,0 +1,94 @@
+// Copyright (c) 2017-2018 Intel Corporation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");


SPDX headers here?

grahamwhaley · 2018-02-28T13:58:11Z

cmd/log-parser/hexbytes.go

+)
+
+// HexByteReader is an I/O Reader type.
+type HexByteReader struct {


just curious, this is not something we could use package "encoding/hex" for is it?
(which I've used, but had some fun with before, as it only handles a single case (lower iirc), and I needed it to handle upper :-) )

I don't think that package handles escaped hex values?

jodh-intel · 2018-02-28T14:23:17Z

Hi @klynnrif - please could you take a look at the new cmd/log-parser/README.md on this PR?

jodh-intel · 2018-02-28T17:43:51Z

Removing label now dependent PRs have landed...

klynnrif

Edited cmd/log-parser/README.md for grammar, format, and text flow. I might have changed the meaning with some edits - apologies in advance :). Thanks!

klynnrif · 2018-02-28T22:22:28Z

cmd/log-parser/README.md

+* [Usage](#usage)
+
+`kata-log-parser` is a tool that combines logfiles generated by the various
+system components, sorts them by timestamp and re-displays the log entries,


Lines 8-9 suggested rewrite: system components, sorts them by timestamp, and re-displays the log entries. A time delta is added to shows how much time has elapsed between each log entry.

klynnrif · 2018-02-28T22:25:51Z

cmd/log-parser/README.md

+system components, sorts them by timestamp and re-displays the log entries,
+adding a time delta showing how much time has elapsed between each log entry.
+
+The tool also checks the validity of all log records and can re-format the


Lines 11-12 suggested rewrite: The tool also checks the validity of all log records, can re-format the logs, and output them in a different format.

klynnrif · 2018-02-28T22:28:14Z

cmd/log-parser/README.md

+The tool also checks the validity of all log records and can re-format the
+logs and output them in a different format.
+
+For full details,


For more information on the kata-log-parser tool, use the help command:

klynnrif · 2018-02-28T22:33:26Z

cmd/log-parser/README.md

+## Logfile requirements
+
+The tool reads logfiles in the [logfmt](https://brandur.org/logfmt) structured
+logging format, for example as created by the golang


Lines 23-24 suggested rewrite: logging format. For example, a logfile created by the golang logrus package.

klynnrif · 2018-02-28T22:35:11Z

cmd/log-parser/README.md

+logging format, for example as created by the golang
+[logrus](https://godoc.org/github.com/sirupsen/logrus) package.
+
+The tool requires the following fields to be defined for each log record:


The tool requires that the following fields are defined for each log record:

klynnrif · 2018-02-28T22:37:16Z

cmd/log-parser/README.md

+  format and including a nanosecond value.
+
+- Source field (`source`): a single word that specifies the name of a unique
+  part of the system (for example, `proxy`, `runtime`, `shim`).


part of the system (e.g. proxy, runtime, shim).

klynnrif · 2018-02-28T22:37:47Z

cmd/log-parser/README.md

+  part of the system (for example, `proxy`, `runtime`, `shim`).
+
+- Name field (`name`): a single word that specifies the name of the
+  application that generated the log record (for example, `kata-runtime`).


application that generates the log record (e.g. kata-runtime).

klynnrif · 2018-02-28T22:54:11Z

cmd/log-parser/README.md

+
+  This log also includes embedded log entries from the
+  [agent](https://github.com/katacontainers/agent). `kata-log-parser`
+  automatically unpacks and displays these, discarding the proxies encapsulating


automatically unpacks and displays the entries. During this process, the encapsulating proxy log messages are discarded.

klynnrif · 2018-02-28T22:55:03Z

cmd/log-parser/README.md

+
+## Usage
+
+To merge together all logs:


To merge all logs:

klynnrif · 2018-02-28T22:55:59Z

cmd/log-parser/README.md

+   ```
+1. Create a container.
+1. Collect the logs.
+    1. Save the proxy log (which also includes agent log details):


Save the proxy log, which includes agent log details:

jodh-intel · 2018-03-01T10:03:55Z

Hi @klynnrif - thanks for reviewing. Doc updated.

jodh-intel · 2018-03-01T12:16:53Z

Hi @chavafg, @devimc - could you take a look please?

jodh-intel · 2018-03-01T14:03:41Z

The CI is correctly detecting that the runtime logs are invalid:

ERROR: kata-log-parser: missing pid: {Count:0 TimeDelta:0 Filename:/home/jenkins/jenkins_slave/workspace/kata-containers-tests-ubuntu-16-04-PR/go/src/github.com/kata-containers/tests/kata-runtime-cc.log Line:1 Time:2018-03-01 13:38:29.691863391 +0000 UTC Pid:0 Level:debug Msg:loaded configuration Source:runtime Name: Data:map[format:TOML]}

This is because the version of cc-runtime in kata is too old. Therefore, blocked on:

cc-runtime: Update to 3.0.20 runtime#43.

jodh-intel · 2018-03-01T16:42:37Z

Hi @klynnrif - thanks for re-reviewing. Could you add a "lgtm" comment as, alas, pullapprove doesn't honour the "thumbs-ups" 😄

klynnrif · 2018-03-01T16:51:56Z

lgtm

klynnrif · 2018-03-01T16:52:52Z

@jodh-intel that is unfortunate :) "thumbs-ups" should always be honored. haha! Thanks!

jodh-intel · 2018-03-01T18:50:58Z

kata-containers/runtime#43 is now merged so re-started CI...

Hi again @klynnrif - actually, I think you've got to use the approve button as the top right is still showing a red cross against your review.

klynnrif

lgtm

klynnrif · 2018-03-01T19:07:34Z

@jodh-intel Okay, I think I'm set now :) let me know if not. Thanks!

Add a log parsing tool (`kata-log-parser`) that reads multiple logfmt [*] logfiles and writes their output in time order showing time differences between log entries. The tool can also check logs for validity and convert the logs to various alternative formats. For full details: ``` $ kata-log-parser --help ``` Fixes kata-containers#93. [*] - https://brandur.org/logfmt Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

Call the log parser at the end of the test run. It will check all component log files and display a detailed error message if any issues are found. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

jodh-intel · 2018-03-02T14:00:31Z

CI is now failing due to an agent issue: kata-containers/agent#159.

jodh-intel · 2018-03-02T15:30:23Z

Blocked on kata-containers/agent#159.

sboeuf · 2018-03-02T15:37:05Z

LGTM

sboeuf · 2018-03-02T15:38:04Z

@jodh-intel I don't think you should bother with the CI failing right now. Kata is moving a lot and everything is not stable yet. This should not gate your PR from being merged.

jodh-intel · 2018-03-02T15:39:37Z

More specifically, blocked on:

Change block device driver in runtime config to block #107
vendor: Update virtcontainers vendoring clearcontainers/runtime#1044
a PR to re-vendor cc-runtime into https://github.com/kata-containers/runtime.

jodh-intel · 2018-03-02T15:43:24Z

@sboeuf - wfm :)

WDYT @grahamwhaley? We have 3 acks on this PR already... ;)

grahamwhaley · 2018-03-02T15:51:31Z

Welllll - this works two ways - if we make the CI stable now, then we may abate major breakage as we 'move fast', which is a good thing. On the other hand, if we know we are fixing CI things that are going to be thrown in the bin (which I'm not convinced of...), then fine.

I'd prefer we got CI running to stop us shooting our own feet off in the next couple of weeks.

jodh-intel · 2018-03-02T15:54:32Z

I agree that we want a stable CI.

It's worth stating that this PR does not add any new tests - it adds a new (unused) tool. We also can see from the CI logs that the static checks pass for this PR.

sboeuf · 2018-03-02T16:22:28Z

As explained by @jodh-intel, this PR cannot make the build worst as it is not introducing new tests. I think we should go ahead and merge it.

chavafg · 2018-03-03T00:21:55Z

lgtm

chavafg · 2018-03-03T00:22:38Z

Agree to merge this one..

jodh-intel added the review label Feb 28, 2018

grahamwhaley added the do-not-merge label Feb 28, 2018

grahamwhaley approved these changes Feb 28, 2018

View reviewed changes

jodh-intel force-pushed the add-log-parser branch from a723fc2 to 142adba Compare February 28, 2018 14:22

jodh-intel force-pushed the add-log-parser branch 5 times, most recently from 209edce to 45db9f0 Compare February 28, 2018 17:43

jodh-intel removed the do-not-merge label Feb 28, 2018

jodh-intel force-pushed the add-log-parser branch 2 times, most recently from 32589ab to 635a7ab Compare February 28, 2018 18:06

klynnrif suggested changes Feb 28, 2018

View reviewed changes

jodh-intel force-pushed the add-log-parser branch 5 times, most recently from bab5322 to 3d3d916 Compare March 1, 2018 09:35

jodh-intel mentioned this pull request Mar 1, 2018

Remove losetup EBUSY errors #101

Closed

jodh-intel force-pushed the add-log-parser branch 4 times, most recently from 94a06ed to 5e07fb4 Compare March 1, 2018 12:15

jodh-intel mentioned this pull request Mar 1, 2018

cmd: Add log parser clearcontainers/tests#590

Closed

klynnrif approved these changes Mar 1, 2018

View reviewed changes

jodh-intel force-pushed the add-log-parser branch 3 times, most recently from f84a8a1 to 26438aa Compare March 2, 2018 09:19

jodh-intel mentioned this pull request Mar 2, 2018

CI: Set CI variable for Jenkins #106

Merged

jodh-intel added 2 commits March 2, 2018 10:47

CI: Add call to log parser

a03a589

Call the log parser at the end of the test run. It will check all component log files and display a detailed error message if any issues are found. Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>

jodh-intel force-pushed the add-log-parser branch from 26438aa to a03a589 Compare March 2, 2018 10:48

jodh-intel mentioned this pull request Mar 2, 2018

[RFC] Generate enriched errors containers/virtcontainers#657

Closed

chavafg merged commit 5981afd into kata-containers:master Mar 3, 2018

chavafg removed the review label Mar 3, 2018

jodh-intel mentioned this pull request Mar 7, 2018

requirements: add initial document kata-containers/documentation#17

Merged


		## Component logfiles

		The primary logfiles the tool reads are:

cmd: Add log parser #98

cmd: Add log parser #98

Conversation

jodh-intel commented Feb 28, 2018

jodh-intel commented Feb 28, 2018

grahamwhaley commented Feb 28, 2018

jodh-intel commented Feb 28, 2018

grahamwhaley 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jodh-intel commented Feb 28, 2018

jodh-intel commented Feb 28, 2018

klynnrif 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

Choose a reason for hiding this comment

jodh-intel commented Mar 1, 2018

jodh-intel commented Mar 1, 2018

jodh-intel commented Mar 1, 2018

jodh-intel commented Mar 1, 2018

klynnrif commented Mar 1, 2018 • edited by amshinde Loading

klynnrif commented Mar 1, 2018

jodh-intel commented Mar 1, 2018

klynnrif left a comment

Choose a reason for hiding this comment

klynnrif commented Mar 1, 2018

jodh-intel commented Mar 2, 2018

jodh-intel commented Mar 2, 2018

sboeuf commented Mar 2, 2018

sboeuf commented Mar 2, 2018 • edited Loading

jodh-intel commented Mar 2, 2018

jodh-intel commented Mar 2, 2018

grahamwhaley commented Mar 2, 2018

jodh-intel commented Mar 2, 2018

sboeuf commented Mar 2, 2018

chavafg commented Mar 3, 2018 • edited by amshinde Loading

chavafg commented Mar 3, 2018

klynnrif commented Mar 1, 2018 •

edited by amshinde

Loading

sboeuf commented Mar 2, 2018 •

edited

Loading

chavafg commented Mar 3, 2018 •

edited by amshinde

Loading