Skip to content
This repository was archived by the owner on Jul 10, 2025. It is now read-only.

feat(logs): Use debug.js library for logging [DXJ-327] #285

Merged
merged 21 commits into from
Mar 10, 2023

Conversation

coder11
Copy link
Contributor

@coder11 coder11 commented Mar 9, 2023

closes: DXJ-327, DXJ-118, DXJ-71

@linear
Copy link

linear bot commented Mar 9, 2023

@coder11 coder11 changed the title Fix/logging should work dxj 327 feat(logs): Use debug.js library for logging [DXJ-327] Mar 10, 2023
@coder11 coder11 marked this pull request as ready for review March 10, 2023 17:41
@linear
Copy link

linear bot commented Mar 10, 2023

DXJ-118 FluenceJS debug logging improvements

--log-level debug brings this kind of stuff from FluenceJS.
It's not a debug info, it's a mess. Nothing debuggable here, nothing readable.
It should be like this

Interpreted particle abcde-abcde-abcde: OK

Interpreted particle abcde-abcde-abcde: Error: Canon state is 1,2,3 but expected 891415 too bad

Sending particle abcde-abcde-abcde to relay "12D3KooWMigkP4jkVyufq5JnDJL6nXvyjeaDNpRfEZqQhsG3sYCU"

Executing particle abcde-abcde-abcde call (run-console print) args: [ "will upload", "/Users/folex/Development/fluencelabs/subrosy/src/scheduled/scheduled.simple.air", "12D3KooWHCJbJKGDfCgHSoCuK9q4STyRnVveqLoXAPBbXHTZx9Cv"]

Interpreted particle abcde-abcde-abcde: OK (empty next_peer_pks)

These should be absolutely forbidden for the eternity:

  • multiline json bigger than 5 lines at once

  • i'd argue ALL multiline json should be forbidden, but maybe it's too remote of a goal

  • \n\"\n\"\n\"\n\"\n\"\n\"  <- THIS STUFF

I'd also recommend enabling timestamps in logs and having some log formatting like eg aqua has:

2022.10.12 00:33:58 [DEBUG] Call time: 8214ms

DXJ-71 Use 'debug' for logging in FluenceJS

Swap logLevel for debug for the following reasons:

  • less complicated way to configure logging across different envs (nodejs, browsers etc)
  • uniform logging config with libp2p
  • possibility to configure logging transparently throughout multiple npm deps
  • more precise control over logging scopes (i.e different components)

Do not forget to update logging in marine-js

Don't forget to mention that "verbose" should be turned on for logs to work in browser

@coder11 coder11 requested a review from shamsartem March 10, 2023 17:47
export const registerService: RegisterService = ({ peer, def, serviceId, service }) => {
log.debug('registering aqua service %o', { def, serviceId, service });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a log that service was registered successfully? not sure

@@ -134,37 +136,41 @@ export class RelayConnection extends FluenceConnection {

async connect(onIncomingParticle: ParticleHandler) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to add log that connection succeeded?

item.particle.logTo('debug', 'sending particle:');
this.connection?.sendParticle(item.nextPeerIds, item.particle.toString()).then(
() => {
log.trace('id %s. sending particle into network', item.particle.id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
log.trace('id %s. sending particle into network', item.particle.id);
log.trace('id %s. sending particle to relay', item.particle.id);

@@ -226,7 +227,7 @@ export class EphemeralNetwork {
}

private async _send(from: PeerIdB58, to: PeerIdB58[], particle: string) {
log.info(`Sending particle from ${from}, to ${JSON.stringify(to)}`);
log.trace(`Sending particle from %s, to %j`, from, to);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add particle id here?

@coder11 coder11 requested a review from folex March 10, 2023 18:26
const hello = await helloTest();
console.log('hello test finished, result: ', hello);

// TODO: some wired error shit about SharedArrayBuffer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a task for that? just so we don't forget

@coder11 coder11 merged commit e95c34a into master Mar 10, 2023
@coder11 coder11 deleted the fix/logging-should-work-dxj-327 branch March 10, 2023 20:03
@fluencebot fluencebot mentioned this pull request Mar 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants