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

Add remote and local address metadata to loggers #1195

Merged
merged 1 commit into from
Jun 3, 2021

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Jun 2, 2021

Motivation:

In some cases having the local and remote address for a connection in
log messages can make debugging easier.

Modifications:

  • Add the local and remote address to logs in the idle handler, error
    handler and client transport
  • Adopt GRPCLogger a little more widely to avoid having to explicitly
    set the source

Result:

More logging metadata

@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Jun 2, 2021
@glbrntt glbrntt requested a review from Lukasa June 2, 2021 09:09
Sources/GRPC/GRPCLogger.swift Show resolved Hide resolved
@glbrntt
Copy link
Collaborator Author

glbrntt commented Jun 2, 2021

Looks like an allocation regression 👀

Copy link
Collaborator

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

Allocation regression could well be the strings you need to construct to represent the socket addresses. Those will surely be longer than 15 bytes and so will need to heap-allocate.

@glbrntt glbrntt force-pushed the gb-more-log-metadata branch from 630a6f9 to 8470a5b Compare June 2, 2021 12:50
Motivation:

In some cases having the local and remote address for a connection in
log messages can make debugging easier.

Modifications:

- Add the local and remote address to logs in the idle handler, error
  handler and client transport
- Adopt `GRPCLogger` a little more widely to avoid having to explicitly
  set the `source`

Result:

More logging metadata
@glbrntt glbrntt force-pushed the gb-more-log-metadata branch from 8470a5b to fcb8430 Compare June 3, 2021 08:49
@glbrntt glbrntt merged commit 9013e4e into grpc:main Jun 3, 2021
@glbrntt glbrntt deleted the gb-more-log-metadata branch June 3, 2021 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants