Page MenuHomePhabricator

[draft] [services] Blob - Add Logs
AbandonedPublic

Authored by michal on Jul 27 2022, 6:32 AM.
Tags
None
Referenced Files
F2110423: D4652.diff
Tue, Jun 25, 7:49 PM
Unknown Object (File)
Wed, Jun 19, 10:23 AM
Unknown Object (File)
Wed, Jun 19, 10:23 AM
Unknown Object (File)
Wed, Jun 19, 10:18 AM
Unknown Object (File)
May 18 2024, 12:38 PM
Unknown Object (File)
May 10 2024, 12:01 PM
Unknown Object (File)
Apr 14 2024, 5:16 AM
Unknown Object (File)
Apr 14 2024, 1:36 AM

Details

Summary

Adding Logs to the blob service. Putting this up as a draft since I'm not 100% sure about this. Curious about your opinions.

Test Plan
cd services
yarn run-blob-service-in-sandbox

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

karol retitled this revision from [services] Blob - Add Logs to [draft] [services] Blob - Add Logs.Jul 27 2022, 6:35 AM
karol edited the summary of this revision. (Show Details)
karol edited the test plan for this revision. (Show Details)
karol added reviewers: ashoat, tomek, max.

I think more logs are a good idea!! The only thing is that we should make sure not to print any "personally identifiable information" in there... eg. let's avoid printing IP address, or user ID, or device ID, or anything like that.

Resigning to let the others take a pass :)

Accepting, but @karol please make sure that we don't log any sensitive data.

services/blob/src/AwsS3Bucket.cpp
56

What is the objectName? Can it be something sensitive?

71–72

I think it's safer to have it in a single log because there could be a couple of instances running at the same time and we should be able to tell which log belongs to which. E.g. if we have two instances and in the log we have:

current name 1
current name 2
new name A
new name B

do we know if we renamed 1 to A or to B?

services/blob/src/DatabaseEntities/ReverseIndexItem.cpp
36–45

I think it would be great if we could avoid the repetition of method name. One idea I have is to create a variable at the beginning of the method and use it to log - this logging would add the prefix to every log it writes, something like the suggestion:

What do you think?

services/blob/src/S3Tools.cpp
9

Is this import used here?

This revision is now accepted and ready to land.Jul 28 2022, 4:11 AM

I think this diff should be canceled because the Blob service going to Rust now. I can commandeer and close it if @tomek doesn't mind.

michal abandoned this revision.
michal added a reviewer: karol.

Old diff, that isn't applicable to the current blob service.