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

[INDY-1187] Implement log processor #573

Merged
merged 16 commits into from
Mar 27, 2018

Conversation

skhoroshavin
Copy link
Contributor

Signed-off-by: Sergey Khoroshavin [email protected]

@skhoroshavin skhoroshavin changed the title [INDY-1187] [WIP] Implement advanced log processor [skip ci] [INDY-1187] Implement advanced log processor [skip ci] Mar 20, 2018
Signed-off-by: Sergey Khoroshavin <[email protected]>
@skhoroshavin skhoroshavin changed the title [INDY-1187] Implement advanced log processor [skip ci] [INDY-1187] Implement log processor [skip ci] Mar 20, 2018
filename: warnings_<node>.log
pattern: <timestamp> | <message>
merge_nodes: no
commits:
Copy link
Contributor

Choose a reason for hiding this comment

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

What does commits mean here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just an identifier for log sink. It's used as a target in rule log line in chains.

only_timestamped: no # Whether to discard non-timestamped lines


chains:
Copy link
Contributor

Choose a reason for hiding this comment

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

We need documentation and description how to use it and what each parameter means here.

monitoring: warnings
- log line: warnings

tag_commits:
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be a good idea to have a list of prepared chain items for common operations such as ViewChange, CatchUp, Connection, etc.
It would be great to have these rules in a separate file so that we don't need to edit neither the script nor config if the format of some log entries changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is next in my TODO list

merge_nodes: no
commits:
filename: commits.log
pattern: <timestamp> | <node> | <instId> <seqNo>
Copy link
Contributor

Choose a reason for hiding this comment

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

How to use it? Is it some pre-defined variables?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some of them (timestamp, node, level, source, func, message) are indeed pre-defined, others (in this case instId and seqNo) are values of user defined attributes by tag rule

Sergey Khoroshavin added 2 commits March 20, 2018 18:58
@ashcherbakov
Copy link
Contributor

test this please

- `node_group`: regex group number that matches node identifier
- `only_timestamped`: whether to discard non-timestamped messages

### logs
Copy link
Contributor

@ashcherbakov ashcherbakov Mar 26, 2018

Choose a reason for hiding this comment

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

Maybe better to name it output_logs?
Or even better move logs, timelogs and counters into output section, so that we have only 3 top-level configs: input, output, matchers (chains)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

output section makes sense, thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

filename: output.log
pattern: <timestamp> | <node> | <source> | <func> | <message>
merge_nodes: yes
state_trie:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why state_trie?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ashcherbakov this is a leftover from debugging session when I found incorrect state trie condition while analyzing logs from INDY-1180, will remove

Signed-off-by: Sergey Khoroshavin <[email protected]>
@skhoroshavin skhoroshavin changed the title [INDY-1187] Implement log processor [skip ci] [INDY-1187] Implement log processor Mar 26, 2018
@dsurnin
Copy link
Contributor

dsurnin commented Mar 26, 2018

Only suggestion - add more example/predefined configs
but it could be done later

@skhoroshavin
Copy link
Contributor Author

@dsurnin main problem with examples is my lack of experience with plenum, hope to catch up this soon

@ashcherbakov
Copy link
Contributor

test this please

2 similar comments
@skhoroshavin
Copy link
Contributor Author

test this please

@skhoroshavin
Copy link
Contributor Author

test this please

@ashcherbakov ashcherbakov merged commit 3cc7954 into hyperledger:master Mar 27, 2018
@skhoroshavin skhoroshavin deleted the log-processor branch March 27, 2018 08:52
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.

3 participants