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 a static-source Logger #1165

Merged
merged 1 commit into from
Apr 22, 2021
Merged

Add a static-source Logger #1165

merged 1 commit into from
Apr 22, 2021

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Apr 22, 2021

Motivation:

swift-log includes a source field which -- in most cases -- should be
the name of the module. Currently swift-log parses the #file to use
the directory containing the file in the hope that it is the module
name.

In our tests we have a tear down step which validates that all caputred
logs have 'GRPC' as their source. However, this requires manually
providing the source in a bunch of places, that's a bit of a pain and
only validates the logs we emit.

Modifications:

  • Add an internal GRPCLogger which wraps a Logger providing only
    trace and debug (for now) and always sets the source to GRPC.

Result:

It's harder to get the source wrong.

Motivation:

swift-log includes a `source` field which -- in most cases -- should be
the name of the module. Currently swift-log parses the `#file` to use
the directory containing the file in the hope that it is the module
name.

In our tests we have a tear down step which validates that all caputred
logs have 'GRPC' as their source. However, this requires manually
providing the source in a bunch of places, that's a bit of a pain and
only validates the logs we emit.

Modifications:

- Add an internal `GRPCLogger` which wraps a `Logger` providing only
  `trace` and `debug` (for now) and always sets the source to `GRPC`.

Result:

It's harder to get the source wrong.
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Apr 22, 2021
@glbrntt glbrntt requested a review from Lukasa April 22, 2021 09:06
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.

Nice one, seems like a very good idea.

@glbrntt glbrntt merged commit 66f74fc into grpc:main Apr 22, 2021
@glbrntt glbrntt deleted the gb-grpc-logger branch April 22, 2021 09:42
@weissi
Copy link
Contributor

weissi commented Apr 22, 2021

@glbrntt I fixed that in SwiftLog: apple/swift-log#187 . It now uses #fileID for Swift >= 5.3 which guarantees that the first(!!) component is the module name. It's always ModuleName/FileName.swift. It may add other components in the future but the Swift language guide documents that the module name is the first.

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