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

Logging improvements #941

Merged
merged 4 commits into from
Sep 28, 2023
Merged

Logging improvements #941

merged 4 commits into from
Sep 28, 2023

Conversation

iand
Copy link

@iand iand commented Sep 27, 2023

Makes the logging a little more consistent and adds debug logging for key routing events to help with testing during integration.

@iand iand changed the title Loggiing improvements Logging improvements Sep 27, 2023
@iand iand added the v2 All issues related to the v2 rewrite label Sep 27, 2023
@iand iand marked this pull request as ready for review September 27, 2023 15:09
Comment on lines +16 to +21
AttrKeyError = "error"
AttrKeyPeerID = "peer_id"
AttrKeyKey = "key"
AttrKeyCacheHit = "hit"
AttrKeyInEvent = "in_event"
AttrKeyOutEvent = "out_event"
Copy link
Contributor

Choose a reason for hiding this comment

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

If they are only used in the tele package we could unexport them

Copy link
Author

Choose a reason for hiding this comment

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

They're designed to be used wherever we need them, such as logging an attribute

Copy link
Contributor

Choose a reason for hiding this comment

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

all good 👍

d.log.Warn(msg, tele.LogAttrError(err))
return
}
d.log.With(args...).Warn(msg, tele.LogAttrError(err))
Copy link
Contributor

Choose a reason for hiding this comment

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

not necessary, but now that we have helper methods for logging error messages we could use the more performant LogAttrs method here. Error logs shouldn't live in the hot path anyway so feel free to ignore.

@iand iand merged commit 6a4249c into v2-develop Sep 28, 2023
11 checks passed
@iand iand deleted the v2-logging branch September 28, 2023 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 All issues related to the v2 rewrite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants