[services] Tunnelbroker - Use glog instead of `std::cout`
ClosedPublic
Actions

Authored by • max on May 9 2022, 7:15 AM.

Details

Reviewers

• karol
tomek
varun

Commits

rCOMMd0a9304f95eb: [services] Tunnelbroker - Use glog instead of `std::cout`

Summary

Following our convention to use glog, convert the std:cout messages to the LOG format with different log levels:

INFO: For debug purposes,
ERROR: For errors.

Related linear task: ENG-801

Test Plan

Start the service in INFO and ERROR-only modes.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• max created this revision.May 9 2022, 7:15 AM

Herald added subscribers: • abosh, • karol, atul and 3 others. · View Herald TranscriptMay 9 2022, 7:15 AM

• max edited the summary of this revision. (Show Details)May 9 2022, 7:16 AM

• max added reviewers: • karol, tomek, varun.

Harbormaster returned this revision to the author for changes because remote builds failed.May 9 2022, 8:18 AM

Harbormaster failed remote builds in B8929: Diff 12438!

Harbormaster completed remote builds in B8929: Diff 12438.May 9 2022, 8:36 AM

• max requested review of this revision.May 9 2022, 8:36 AM

Following our convention to use glog

Could you remind me where this convention exists? I've searched the codebase and haven't found any usage of it. Also, do we need to install it somehow?

This revision now requires changes to proceed.May 10 2022, 12:25 AM

• max planned changes to this revision.May 11 2022, 12:10 AM

In D3969#111452, @palys-swm wrote:

Following our convention to use glog

Could you remind me where this convention exists?

The related task is ENG-801.
Before this task, we had a discussion in the Comm Dev Team chat and voted for glog because we already have it as a dependency from Folly.

Also, do we need to install it somehow?

No, because we already installed it as a dependency for Folly.

I've searched the codebase and haven't found any usage of it?

This is the first glog usage in our code (our Folly dependency uses it internally).

This is the first glog usage in our code (our Folly dependency uses it internally).

Are we sure that we don't need to explicitly add glog as our dependency? @karol-bisztyga what's your opinion?

This is a good point, I think since we explicitly use it ourselves and it's no longer just a folly dependency for us, then I think we should extract its installation into a separate script. I'm going to accept this as it looks ok to me but we should follow up about this.

This revision is now accepted and ready to land.May 13 2022, 6:01 AM

Given @karol-bisztyga's comment, it is critical that we create a Linear task for the follow-up before landing this diff

In D3969#113597, @ashoat wrote:

Given @karol-bisztyga's comment, it is critical that we create a Linear task for the follow-up before landing this diff

Sure! ENG-1144 was created for this one.

• max mentioned this in D4054: [services] Extract glog installation from folly installation.May 16 2022, 6:10 AM

Rebase on master.

Harbormaster completed remote builds in B9162: Diff 12739.May 16 2022, 7:04 AM

Closed by commit rCOMMd0a9304f95eb: [services] Tunnelbroker - Use glog instead of `std::cout` (authored by • max). · Explain WhyMay 16 2022, 7:05 AM

This revision was automatically updated to reflect the committed changes.

• max added a commit: rCOMMd0a9304f95eb: [services] Tunnelbroker - Use glog instead of `std::cout`.

• max mentioned this in rCOMMf1b0af3fbd23: [services] Extract glog installation from folly installation.May 20 2022, 4:15 AM