Skip to content
This repository has been archived by the owner on Dec 27, 2022. It is now read-only.

[DNM] Add logOperationTime util with implementation in protocol module #398

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions modules/protocol/src/vector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
ProtocolError,
jsonifyError,
} from "@connext/vector-types";
import { getCreate2MultisigAddress, getRandomBytes32 } from "@connext/vector-utils";
import { getCreate2MultisigAddress, getRandomBytes32, logOperationTime } from "@connext/vector-utils";
import { Evt } from "evt";
import pino from "pino";

Expand Down Expand Up @@ -132,6 +132,7 @@ export class Vector implements IVectorProtocol {
): Promise<Result<FullChannelState, OutboundChannelUpdateError>> {
const method = "executeUpdate";
const methodId = getRandomBytes32();
const label = logOperationTime(this.logger, method);
this.logger.debug({
method,
methodId,
Expand Down Expand Up @@ -178,6 +179,7 @@ export class Vector implements IVectorProtocol {
);
}

logOperationTime(this.logger, method, label);
return outboundRes;
}

Expand All @@ -199,7 +201,7 @@ export class Vector implements IVectorProtocol {
}
const method = "onReceiveProtocolMessage";
const methodId = getRandomBytes32();
this.logger.debug({ method, methodId }, "Method start");
const label = logOperationTime(this.logger, method);
Copy link
Contributor

Choose a reason for hiding this comment

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

We will likely want to keep the methodId to log throughout the other parts of the method so the logs are easily searchable. Maybe we should use the label as the methodId instead.

General purpose of the methodId is to get a unique strings so that you can easily identify all logs that came from a given method when searching in log dna or whatever other log parser

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. This functionality is already internal to the logOperationTime method. I actually had it like that at first, but worried that not all methods would follow this standard of generating a methodId. Will add this back tho, thanks


if (msg.isError) {
this.logger.error(
Expand Down Expand Up @@ -267,7 +269,7 @@ export class Vector implements IVectorProtocol {
updatedTransfers: updatedActiveTransfers,
updatedTransfer,
});
this.logger.debug({ method, methodId }, "Method complete");
logOperationTime(this.logger, method, label);
},
);

Expand Down Expand Up @@ -355,7 +357,7 @@ export class Vector implements IVectorProtocol {
public async setup(params: ProtocolParams.Setup): Promise<Result<FullChannelState, OutboundChannelUpdateError>> {
const method = "setup";
const methodId = getRandomBytes32();
this.logger.debug({ method, methodId }, "Method start");
const label = logOperationTime(this.logger, method);
// Validate all parameters
const error = this.validateParamSchema(params, ProtocolParams.SetupSchema);
if (error) {
Expand Down Expand Up @@ -400,14 +402,15 @@ export class Vector implements IVectorProtocol {
},
"Method complete",
);
logOperationTime(this.logger, method, label);
return returnVal;
}

// Adds a deposit that has *already occurred* onchain into the multisig
public async deposit(params: ProtocolParams.Deposit): Promise<Result<FullChannelState, OutboundChannelUpdateError>> {
const method = "deposit";
const methodId = getRandomBytes32();
this.logger.debug({ method, methodId }, "Method start");
const label = logOperationTime(this.logger, method);
// Validate all input
const error = this.validateParamSchema(params, ProtocolParams.DepositSchema);
if (error) {
Expand Down
37 changes: 37 additions & 0 deletions modules/utils/src/logOperationTime.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import pino from "pino";
import { performance } from 'perf_hooks';

/// This method is used to log the start and end time of an executed method. We are using the
/// perf_hooks (performance) lib provided by node to calculate this - much more accurate than
/// Date.now(). On first call, a unique label string is generated and returned. On second call,
/// same label string should be passed in to measure and log the diff in time (i.e. amount of
/// time method took to execute in ms).
export const logOperationTime = (log: pino.BaseLogger, method: string, methodId: string, startMark?: string): string => {
const mark = `${method}-${methodId}`
if (startMark) {
const stopMark = `${mark}-stop`;
const timestamp = performance.mark(stopMark);
performance.measure(mark, startMark, stopMark);
const diff = performance.getEntries({name : mark, entryType: "measure"});

// Depending on how long method took, up severity of logging call. This is a default
// arrangement that was copied over from indra.
const message = `${method} : finished; took ${diff} ms`;
if (diff < 2) {
log.debug(message);
} else if (diff < 200) {
log.info(message);
} else {
log.warn(message);
}
} else {
// Generate the label we'll be using (for later ref) using unique hexstring.
const startMark = `${mark}-start`;
performance.mark(startMark);
// Debug log notify method started.
const message = `${method}: started`;
log.debug(message);
}
// We return the startMark so it can be passed back in for later method call.
return startMark;
};
Loading