Details

Reviewers

varun
• jon
michal
ashoat

Commits

rCOMM578d87f96f4f: [services][backup] PullBackup - fix tracing span

Summary

Logging in async_stream wasn't displaying log spans correctly, this diff fixes that.
I needed to introduce a new dependency tracing-futures (official package from tracing vendor) because the core tracing::Instrumented<T> doesn't implement Stream<T>. With this dependency, we can use <impl Stream>::instrument() and <impl Stream>::in_current_span() which are required to correctly trace streaming logs.

Depends on D6246

Question: Should we display user_id in logs? Or is it sensitive data?

Test Plan

Ran integration tests and ensured logs are displayed correctly

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Jan 11 2023, 3:54 PM

bartek held this revision as a draft.

Herald added subscribers: atul, tomek. · View Herald TranscriptJan 11 2023, 3:54 PM

bartek edited the summary of this revision. (Show Details)Jan 11 2023, 3:58 PM

bartek edited the test plan for this revision. (Show Details)

bartek published this revision for review.Jan 11 2023, 4:02 PM

bartek edited the summary of this revision. (Show Details)

bartek attached a referenced file: F326317: Screenshot 2023-01-11 at 23.46.17.png. (Show Details)

Harbormaster completed remote builds in B15275: Diff 20831.Jan 11 2023, 4:09 PM

bartek edited the summary of this revision. (Show Details)Jan 11 2023, 4:21 PM

Fix typo

Harbormaster completed remote builds in B15278: Diff 20834.Jan 11 2023, 4:37 PM

Dependencies look good, but somebody should look at the Rust code, so I'll resign instead of accepting to leave in review state

Question: Should we display user_id in logs? Or is it sensitive data?

Prefer not to do this if possible – it's not as sensitive as other data, but it still creates a log of when users access the backup service, which I'd prefer if we didn't have

bartek mentioned this in D6198: [services][backup] CreateBackup 2/3 - handle non-data inputs.Jan 12 2023, 8:53 AM

Rebase

Harbormaster failed remote builds in B15347: Diff 20927!Jan 13 2023, 4:01 AM

your jsi_codegen.yml is different from master, not sure why gate is failing.

This revision is now accepted and ready to land.Jan 13 2023, 12:00 PM

Harbormaster completed remote builds in B15347: Diff 20927.Jan 13 2023, 12:05 PM

Rebase

Harbormaster completed remote builds in B15477: Diff 21099.Jan 19 2023, 10:03 AM

Closed by commit rCOMM578d87f96f4f: [services][backup] PullBackup - fix tracing span (authored by bartek). · Explain WhyJan 19 2023, 10:08 AM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMM578d87f96f4f: [services][backup] PullBackup - fix tracing span.