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

Fix sciond logging for processing path replies. #3021

Merged
merged 3 commits into from
Aug 21, 2019

Conversation

kormat
Copy link
Contributor

@kormat kormat commented Aug 21, 2019

  • The logger does not expect an error after the string, so removing
    nil.
  • The logger does not do a good job with []error, so created
    common.FmtErrors to handle this.

Before:

2019-08-21 09:01:33.830842+0000 [WARN] Failed to verify reply debug_id=747f80c4 trace_id=64dc60702aa9bf7 LOG15_ERROR="Key(<nil>) is not a string: <nil>" LOG15_ERROR="Key([]error) is not a string: [Failed to verify seg seg="55c11074ec66 2019-08-21 09:01:31+0000 1-ff00:0:110 1>41 1-ff00:0:111"
    DEBUGTHISTHING 2 Failed to verify seg seg="55c11074ec66 2019-08-21 09:01:31+0000 1-ff00:0:110 1>41 1-ff00:0:111"
    DEBUGTHISTHING 2]" LOG15_ERROR="Normalized odd number of arguments by adding nil"

After:

2019-08-20 15:59:05.693430+0000 [WARN] Failed to verify reply debug_id=e544675c trace_id=6963201a9432fbae errors=
>  Failed to verify seg seg="55c11074ec66 2019-08-20 15:59:01+0000 1-ff00:0:110 1>41 1-ff00:0:111"
>      DEBUGTHISTHING 2
>  Failed to verify seg seg="55c11074ec66 2019-08-20 15:59:01+0000 1-ff00:0:110 1>41 1-ff00:0:111"
>      DEBUGTHISTHING 2

This change is Reviewable

@kormat kormat requested a review from lukedirtwalker August 21, 2019 09:07
- The logger does not expect an `error` after the string, so removing
  `nil`.
- The logger does not do a good job with `[]error`, so created
  `common.FmtErrors` to handle this.

Before:
```
2019-08-21 09:01:33.830842+0000 [WARN] Failed to verify reply debug_id=747f80c4 trace_id=64dc60702aa9bf7 LOG15_ERROR="Key(<nil>) is not a string: <nil>" LOG15_ERROR="Key([]error) is not a string: [Failed to verify seg seg="55c11074ec66 2019-08-21 09:01:31+0000 1-ff00:0:110 1>41 1-ff00:0:111"
    DEBUGTHISTHING 2 Failed to verify seg seg="55c11074ec66 2019-08-21 09:01:31+0000 1-ff00:0:110 1>41 1-ff00:0:111"
    DEBUGTHISTHING 2]" LOG15_ERROR="Normalized odd number of arguments by adding nil"
```

After:
```
2019-08-20 15:59:05.693430+0000 [WARN] Failed to verify reply debug_id=e544675c trace_id=6963201a9432fbae errors=
>  Failed to verify seg seg="55c11074ec66 2019-08-20 15:59:01+0000 1-ff00:0:110 1>41 1-ff00:0:111"
>      DEBUGTHISTHING 2
>  Failed to verify seg seg="55c11074ec66 2019-08-20 15:59:01+0000 1-ff00:0:110 1>41 1-ff00:0:111"
>      DEBUGTHISTHING 2
```
Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @kormat)


go/lib/common/errors.go, line 189 at r1 (raw file):

// FmtErrors formats a slice of errors for logging.
func FmtErrors(es []error) string {
	s := make([]string, 0)

make([]string, 0, len(es))

Copy link
Contributor Author

@kormat kormat left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)


go/lib/common/errors.go, line 189 at r1 (raw file):

Previously, lukedirtwalker (Lukas Vogel) wrote…

make([]string, 0, len(es))

Done. ish.

Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, 1 of 1 files at r3.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Collaborator

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@kormat kormat merged commit c5c1874 into scionproto:master Aug 21, 2019
@kormat kormat deleted the fix_sd_log branch August 21, 2019 09:30
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