Skip to content
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

cmd, dashboard, internal, log, node: logging feature #17097

Merged
merged 5 commits into from
Jul 11, 2018

Conversation

kurkomisi
Copy link
Contributor

This PR contains an implementation of log saver and visualizer.

log/handler.go Outdated
@@ -70,6 +74,103 @@ func FileHandler(path string, fmtr Format) (Handler, error) {
return closingHandler{f, StreamHandler(f, fmtr)}, nil
}

type writeCounter struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc pls.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls rename to countingWriter

log/handler.go Outdated
count uint
}

func (w *writeCounter) Write(p []byte) (n int, err error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc pls.

log/handler.go Outdated
// at the given path. When a file's size reaches the limit, the handler creates
// a new file named after the timestamp of the first log record it will contain.
func RotatingFileHandler(path string, limit uint, formatter Format) Handler {
if err := os.MkdirAll(path, 0755); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's restrict the permissions to 0700, since we don't really want everyone on the machine reading our logs.

log/handler.go Outdated
// a new file named after the timestamp of the first log record it will contain.
func RotatingFileHandler(path string, limit uint, formatter Format) Handler {
if err := os.MkdirAll(path, 0755); err != nil {
panic(err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's return an error instead, panics can turn nasty if deep enough in the code. Same for all other panics too in this method btw.

log/handler.go Outdated
counter := new(writeCounter)
h := StreamHandler(counter, formatter)

re := regexp.MustCompile(".log$")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"\\.log$"

dashboard/log.go Outdated
}
b := make([]byte, len(buf)+len(chunk))
copy(b, buf)
copy(b[len(buf):], chunk)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

append

dashboard/log.go Outdated
var l *LogsMessage
// Update the history.
db.lock.Lock()
if len(db.history.Logs.Chunk) == 2 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bytes.Equal

dashboard/log.go Outdated
Past: false,
Last: true,
},
Chunk: json.RawMessage("[]"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

separate it out as emptyChunk.

default:
lvl = '';
}
if (lvl === '' || typeof t !== 'string' || t.length < 19 || typeof msg !== 'string' || !Array.isArray(ctx)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use new Date(t) and handle any errors, cleaner than string manipulation.


for (let i = 0; i < ctx.length; i += 2) {
const key = ctx[i];
const value = ctx[i + 1];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick, use val, same length as key.

padding = value.length;
fieldPadding.set(key, padding);
}
content += ` <span style="color:${color}">${key}</span>=${value}${'&nbsp;'.repeat(padding - value.length)}`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't pad the last one

@karalabe
Copy link
Member

karalabe commented Jul 4, 2018

You need to vendor in the missing packages. Also, afaik we already have a file system notification package vendored in. Perhaps use that instead?

@karalabe
Copy link
Member

karalabe commented Jul 4, 2018

You also need to gofmt and govendor, linter is failing.

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something seems off, I'm getting errors in the logs when I load the page.

WARN [07-09|12:01:34.919] Failed to load the asset                 path=/service-worker-7c7a96ce6fa87adc81b6af59edceb61b59781902a923313485bf6f4d32a1fad3.js err="Asset service-worker-7c7a96ce6fa87adc81b6af59edceb61b59781902a923313485bf6f4d32a1fad3.js not found"

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Service Worker issue was at my end. PR LGTM!

@karalabe karalabe merged commit a9835c1 into ethereum:master Jul 11, 2018
firmianavan pushed a commit to firmianavan/go-ethereum that referenced this pull request Aug 28, 2018
* cmd, dashboard, internal, log, node: logging feature

* cmd, dashboard, internal, log: requested changes

* dashboard, vendor: gofmt, govendor, use vendored file watcher

* dashboard, log: gofmt -s -w, goimports

* dashboard, log: gosimple
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 4, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 13, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 14, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 14, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 14, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 15, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 15, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants