[services][blob] Use tracing for logs
ClosedPublic
Actions

Authored by bartek on Nov 28 2022, 2:14 AM.

Details

Reviewers

varun
ashoat
tomek
marcin
• jon
• max

Commits

rCOMM22fde1efd8e0: [services][blob] Use tracing for logs

Summary

Related ENG-2308

Similar to identity service, I introduced the tracing library here. I enhanced its configuration to take level filter from environment variables. It defaults to INFO everywhere, what corresponds to the identity service behavior.

Default environment variable name is RUST_LOG but it can be changed if needed.

A few examples:

cargo run - (default) showing INFO logs and higher (warn/error)
RUST_LOG=debug - showing DEBUG+, includes dependencies (e.g. AWS logs)
RUST_LOG=blob=trace - showing TRACE+ logs for the blob service, INFO+ for dependencies

Depends on D5728

Adding @ashoat as this introduces a new dependency (already used in identity service though).

Test Plan

RUST_LOG=blob=trace cargo run and see the logs

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Nov 28 2022, 2:14 AM

bartek held this revision as a draft.

Herald added a subscriber: atul. · View Herald TranscriptNov 28 2022, 2:14 AM

bartek added a child revision: D5730: [services][blob] Add meaningful logs in gRPC handler.Nov 28 2022, 2:24 AM

Harbormaster completed remote builds in B13770: Diff 18855.Nov 28 2022, 2:27 AM

bartek published this revision for review.Nov 28 2022, 2:54 AM

looks to be pretty standard way to use tracing

cc @max and @tomek on usage of tracing over log. Seems that tracing crate is preferred for asynchronous tracing.

tomek accepted this revision.Nov 28 2022, 9:59 AM

tomek added inline comments.

services/blob/src/main.rs
49–55 ↗	(On Diff #18855)	It's a matter of style, but even though this comment is useful, it might get easily out of sync with the code. A lot better approach would be to extract a function with the exact same name as this comment. By doing that we're ensuring that the name and code remain in sync.

ashoat accepted this revision.Nov 28 2022, 12:06 PM

varun accepted this revision.Nov 28 2022, 11:11 PM

This revision is now accepted and ready to land.Nov 28 2022, 11:11 PM

• max accepted this revision.Nov 29 2022, 2:32 AM

• max added inline comments.

services/blob/src/main.rs
49–55 ↗	(On Diff #18855)	It's a matter of style, but even though this comment is useful, it might get easily out of sync with the code. A lot better approach would be to extract a function with the exact same name as this comment. By doing that we're ensuring that the name and code remain in sync. I agree with @tomek, can we extract this?