Convert console logs to use logger #3141

joaquincasares · 2022-05-24T01:05:52Z

Description

As we move to standardize our logging, use the Bunyan logger to capture enriched logs instead of using console.log().

Tests

Have CircleCI run tests over the PR and monitor Loggly logs for changed lines.

How will this change be monitored? Are there sufficient logs?

Have CircleCI run tests over the PR and monitor Loggly logs for changed lines.

creator-node/src/apiHelpers.js

dmanjunath

this looks good to me but i want @SidSethi to confirm

SidSethi

thanks for handling this!! 🙌

creator-node/src/apiHelpers.js

creator-node/src/utils.js

creator-node/src/apiHelpers.js

creator-node/src/utils.js

creator-node/src/apiHelpers.js

SidSethi

my b - meant to Request Changes per comments (also unit tests failing)

joaquincasares · 2022-05-25T21:40:29Z

@SidSethi this is now ready for another review.

Thanks again!

SidSethi

looks great! thanks for the quick iterations here 🙂

also just checking since the PR desc is a bit unclear - you tested these with a local setup to make sure logs look good right?

joaquincasares · 2022-05-26T02:38:54Z

also just checking since the PR desc is a bit unclear - you tested these with a local setup to make sure logs look good right?

Whoops, I fixed the description and will be running test-mad-dog-e2e-full to ensure our tests come back clean since it's going to be time consuming to test each individual path. However, since we're heavily looking at logging right now we should be able to see issues occurring within Loggly pretty quickly, especially when this the scope of changes in this PR are pretty straightforward.

dmanjunath · 2022-05-26T16:25:17Z

@joaquincasares i don't think mad-dog-e2e-full works, doesn't even pass on master. i think what @SidSethi is asking is to verify that there's no regressions to the logs and if you run the creator node tests or the service locally it outputs logs from the changes made here

joaquincasares · 2022-05-26T17:34:52Z

@dmanjunath ah man, I didn't realize mad-dog-full was broken on master.

Indeed, Sid and I were talking yesterday. I said "since the code is clean enough to look for bugs and since all the CircleCI tests ran successfully (minus mad-dog-full), how would you feel if we merge this after running mad-dog-full?". Sid's response was "yeah fair enough".

Since mad dog is broken, I can run this branch locally for a bit and see how many of the logs I capture, but for this PR it seems a bit harder to fully test since it’d have to go down different code paths, some of which are error paths.

Thoughts?

dmanjunath · 2022-05-26T17:49:10Z

@joaquincasares ideally we would reproduce at least one of the paths locally, don't need to test every single one. the intent is to test that the console.log successfully got switched to logger (which it should be from the code). you can test this by adding a function call somewhere in the content node code locally that exercises one of these paths. my suggestion would be to add redis.getLock on content node init and see if that prints out the log

joaquincasares

I confirmed these two endpoints appeared within my remote-dev env.

I did see this stacktrace, but it seems unrelated to this PR:

Error: ENOENT: no such file or directory, stat '/usr/src/app/local-storage/%40audius%2Flibs%3Adiscovery-node-timestamp'
    at Object.statSync (fs.js:1131:3)
    at _rm (/usr/src/audius-libs/node_modules/node-localstorage/LocalStorage.js:29:12)
    at LocalStorage.removeItem (/usr/src/audius-libs/node_modules/node-localstorage/LocalStorage.js:248:9)
    at DiscoveryProviderSelection.clearCached (/usr/src/audius-libs/dist/index.js:58323:22)
    at DiscoveryProvider._callee46$ (/usr/src/audius-libs/dist/index.js:60974:38)
    at tryCatch (/usr/src/audius-libs/node_modules/@audius/anchor-audius-data/dist/index.js:99738:40)
    at Generator.invoke [as _invoke] (/usr/src/audius-libs/node_modules/@audius/anchor-audius-data/dist/index.js:99969:22)
    at Generator.next (/usr/src/audius-libs/node_modules/@audius/anchor-audius-data/dist/index.js:99794:21)
    at asyncGeneratorStep (/usr/src/audius-libs/dist/index.js:236:24)
    at _next (/usr/src/audius-libs/dist/index.js:258:9)
    at /usr/src/audius-libs/dist/index.js:265:7
    at new Promise (<anonymous>)
    at DiscoveryProvider.<anonymous> (/usr/src/audius-libs/dist/index.js:254:12)
    at DiscoveryProvider.getHealthyDiscoveryProviderEndpoint (/usr/src/audius-libs/dist/index.js:61001:53)
    at DiscoveryProvider._callee45$ (/usr/src/audius-libs/dist/index.js:60788:29)
    at tryCatch (/usr/src/audius-libs/node_modules/@audius/anchor-audius-data/dist/index.js:99738:40)
    at Generator.invoke [as _invoke] (/usr/src/audius-libs/node_modules/@audius/anchor-audius-data/dist/index.js:99969:22)
    at Generator.next (/usr/src/audius-libs/node_modules/@audius/anchor-audius-data/dist/index.js:99794:21)
    at asyncGeneratorStep (/usr/src/audius-libs/dist/index.js:236:24)
    at _next (/usr/src/audius-libs/dist/index.js:258:9)
    at /usr/src/audius-libs/dist/index.js:265:7
    at new Promise (<anonymous>)
    at DiscoveryProvider.<anonymous> (/usr/src/audius-libs/dist/index.js:254:12)
    at DiscoveryProvider._makeRequest (/usr/src/audius-libs/dist/index.js:60942:30)
    at DiscoveryProvider._callee45$ (/usr/src/audius-libs/dist/index.js:60892:29)
    at tryCatch (/usr/src/audius-libs/node_modules/@audius/anchor-audius-data/dist/index.js:99738:40)
    at Generator.invoke [as _invoke] (/usr/src/audius-libs/node_modules/@audius/anchor-audius-data/dist/index.js:99969:22)
    at Generator.next (/usr/src/audius-libs/node_modules/@audius/anchor-audius-data/dist/index.js:99794:21) {
  errno: -2,
  syscall: 'stat',
  code: 'ENOENT',
  path: '/usr/src/app/local-storage/%40audius%2Flibs%3Adiscovery-node-timestamp'
}

joaquincasares · 2022-05-27T02:53:41Z

creator-node/src/blacklistManager.js

@@ -719,12 +719,12 @@ class BlacklistManager {
      }
    })
    stream.on('end', function () {
-      console.log(
+      logger.info(


confirmed on remote-dev.

joaquincasares · 2022-05-27T02:56:25Z

creator-node/src/redis.js

    // set allows you to set an optional expire param
    return redisClient.set(key, true, 'EX', expiration)
  }

  static async getLock(key) {
-    console.log(`GETTING LOCK ${key}`)
+    logger.info(`GETTING LOCK ${key}`)


confirmed on remote-dev

dmanjunath · 2022-05-27T14:10:54Z

yeah that stacktrace is fine, it comes from libs while caching the discovery provider endpoint. @SidSethi do you want to take one more look before merge?

SidSethi · 2022-05-27T14:15:44Z

yeah that stacktrace is fine, it comes from libs while caching the discovery provider endpoint. @SidSethi do you want to take one more look before merge?

whoops, @joaquincasares sorry to send you down a rabbit hole 😬 . i didnt read your earlier msg closely enough, i meant that you can merge once base mad-dog passes

good to merge!!

…on processing (#3141)

[a82b6d2] [C-2391 PLAT-833] Handle get-discovery-notification error, notification processing (#3141) Dylan Jeffers [b6a8470] [PAY-1073] Mobile messages settings screen (#3131) Reed [c567b35] Separate chat text input to own component (#3142) Reed [1a38cb1] [PAY-1090] - Subscribe to special access gates to unlock new tracks (#3108) Saliou Diallo [9e5a45d] Set dev optimizely key (#3126) Sebastian Klingler [c5e86fa] [PLAT-699] Add tastemaker notification to web (#3140) Dylan Jeffers [4bc608c] [C-2295] Only show now-playing-artwork-tile when song enqueued (#3137) Dylan Jeffers [aa34b32] [PAY-1039] Mobile chat header navs to profile (#3139) Reed [55f5819] Optimizely attributes - use null if empty instead of undefined (#3134) nicoback2 [92ab79e] [C-2387] Improve permalink and user handle caching (#3117) Dylan Jeffers [086af81] [PAY-1107][PAY-1112] Center mobile chat text input, enlarge icon (#3136) Reed [fcec3f5] [PAY-1123] Fix mobile chat scroll being stuck (#3135) Reed [a276e86] Revert "Allow Optimizely targeting w CodePush on mobile C-2394" (#3133) nicoback2 [3335b4a] [C-2357] Update the icon color of the podcast skip buttons to accept the theme color (#3132) Kyle Shanks [1af5730] Fix dapp store versionCode (#3121) Dylan Jeffers [7787115] Fix app store version fetched from Apple in Fastlane being out of date first 24h after new release C-2397 (#3128) nicoback2 [06c181d] Allow Optimizely targeting w CodePush on mobile C-2394 (#3127) nicoback2 [ff07c32] [PAY-1038] Fix mobile profile page button borders (#3130) Reed [9286959] [PAY-1027] Mobile chat websockets (#3129) Reed [81ef3b3] [PAY-1031] Mobile chat list views use full height (#3123) Reed [41e3f5d] [C-2231] Remove audius-data, dynamically import web3modal (#1771) Raymond Jacobson [34d80be] [PAY-1111] Remove "Done" row on ios keyboard for mobile chats (#3124) Reed [9eb83a8] [PAY-1102] - Add supporters info drawer (#3112) Saliou Diallo [6fe7abe] Add dependencies, bundle to mobile README (#3111) Reed [dc8d4f2] Show header shadow in chat screens (#3106) Reed [f871a38] [PAY-906] Mobile chats KeyboardAvoidingView (#3122) Reed [fd00bf2] Fix chat reactions with createEntityAdapter (#3118) Reed [9876137] Switch to 1s (#3120) Raymond Jacobson [fe9efea] Tip using both erc_wallet and wallet (#3049) Michael Piazza [fdfc0b6] Update dapp store readme (#3119) Raymond Jacobson [ca58e69] [C-2359, C-2380] Update duration text for podcast tiles (#3116) Kyle Shanks [6441421] [PLAT-695] Add trending notifications to valid_types (#3109) Dylan Jeffers [4233ee6] [PAY-1083] - Only allow prompt modal when single track is being uploaded (#3115) Saliou Diallo [36909d1] Always include user signature for stream (#3114) Raymond Jacobson [b90eaf6] [C-2386] Fix notification button not toggling panel (#3110) Dylan Jeffers [a3c2e91] Check if messagesStatus undefined in ChatScreen (#3113) Reed [64d89ca] [C-2382] Update dapp-store cli, short_description length (#3104) Dylan Jeffers [680931d] [C-2361, C-2360, C-2355, C-2346] Update skip 15s button behavior on mobile for podcasts (#3105) Kyle Shanks [3eb742f] [C-2353, C-2358] Update playback position tracking for web and mobile (#3097) Kyle Shanks [e06c4d9] Bump sdk to 2.0.3-beta.1 (#3098) Isaac Solo [8191694] Fix "Pick some for me" button on android (#3102) Sebastian Klingler

pull-request-size bot added size/S size/M and removed size/S labels May 24, 2022

joaquincasares requested a review from dmanjunath May 24, 2022 01:34

joaquincasares commented May 24, 2022

View reviewed changes

creator-node/src/apiHelpers.js Outdated Show resolved Hide resolved

dmanjunath requested a review from SidSethi May 24, 2022 12:48

dmanjunath approved these changes May 24, 2022

View reviewed changes

SidSethi approved these changes May 25, 2022

View reviewed changes

creator-node/src/apiHelpers.js Outdated Show resolved Hide resolved

creator-node/src/utils.js Outdated Show resolved Hide resolved

joaquincasares added 5 commits May 25, 2022 17:50

convert console logs to use logger

9add341

relative module

35b117a

simplify import

72b0612

logger -> childLogger

2969910

./logging

bb87b38

joaquincasares force-pushed the jc-inf-93.cn.console.log branch from fc11357 to bb87b38 Compare May 25, 2022 18:01

SidSethi reviewed May 25, 2022

View reviewed changes

creator-node/src/apiHelpers.js Outdated Show resolved Hide resolved

creator-node/src/utils.js Outdated Show resolved Hide resolved

creator-node/src/apiHelpers.js Outdated Show resolved Hide resolved

SidSethi suggested changes May 25, 2022

View reviewed changes

joaquincasares added 2 commits May 25, 2022 20:51

remove childLogger post-rebase

462bfc8

undo rename

139e54c

pull-request-size bot added size/S and removed size/M labels May 25, 2022

joaquincasares requested a review from SidSethi May 25, 2022 20:56

SidSethi approved these changes May 26, 2022

View reviewed changes

joaquincasares commented May 27, 2022

View reviewed changes

joaquincasares merged commit 3fc3ddb into master May 31, 2022

joaquincasares deleted the jc-inf-93.cn.console.log branch May 31, 2022 19:29

sliptype pushed a commit that referenced this pull request Sep 10, 2023

[C-2391 PLAT-833] Handle get-discovery-notification error, notificati…

421b74d

…on processing (#3141)

AudiusProject deleted a comment from linear bot Sep 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert console logs to use logger #3141

Convert console logs to use logger #3141

joaquincasares commented May 24, 2022 •

edited

Loading

dmanjunath left a comment

SidSethi left a comment

SidSethi left a comment

joaquincasares commented May 25, 2022

SidSethi left a comment

joaquincasares commented May 26, 2022

dmanjunath commented May 26, 2022

joaquincasares commented May 26, 2022

dmanjunath commented May 26, 2022 •

edited

Loading

joaquincasares left a comment

joaquincasares May 27, 2022

joaquincasares May 27, 2022

dmanjunath commented May 27, 2022

SidSethi commented May 27, 2022

Convert console logs to use logger #3141

Convert console logs to use logger #3141

Conversation

joaquincasares commented May 24, 2022 • edited Loading

Description

Tests

How will this change be monitored? Are there sufficient logs?

dmanjunath left a comment

Choose a reason for hiding this comment

SidSethi left a comment

Choose a reason for hiding this comment

SidSethi left a comment

Choose a reason for hiding this comment

joaquincasares commented May 25, 2022

SidSethi left a comment

Choose a reason for hiding this comment

joaquincasares commented May 26, 2022

dmanjunath commented May 26, 2022

joaquincasares commented May 26, 2022

dmanjunath commented May 26, 2022 • edited Loading

joaquincasares left a comment

Choose a reason for hiding this comment

joaquincasares May 27, 2022

Choose a reason for hiding this comment

joaquincasares May 27, 2022

Choose a reason for hiding this comment

dmanjunath commented May 27, 2022

SidSethi commented May 27, 2022

joaquincasares commented May 24, 2022 •

edited

Loading

dmanjunath commented May 26, 2022 •

edited

Loading