Skip to content
Merged
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
18 changes: 9 additions & 9 deletions handwritten/spanner/src/metrics/metrics-tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ import {Spanner} from '..';
* by MetricsTracer to monitor and report metrics for each individual gRPC call attempt.
*/
class MetricAttemptTracer {
private _startTime: Date;
private _startTime: number;
public status: string;

constructor() {
this._startTime = new Date(Date.now());
this._startTime = performance.now();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The performance global is only available in Node.js v16+. To maintain compatibility with older supported Node.js versions (such as Node 14), you should import performance from the perf_hooks module at the top of the file.

this.status = Status[Status.UNKNOWN];
}

Expand Down Expand Up @@ -67,12 +67,12 @@ class MetricAttemptTracer {
*/
class MetricOperationTracer {
private _attemptCount: number;
private _startTime: Date;
private _startTime: number;
private _currentAttempt: MetricAttemptTracer | null;

constructor() {
this._attemptCount = 0;
this._startTime = new Date(Date.now());
this._startTime = performance.now();
this._currentAttempt = null;
}

Expand Down Expand Up @@ -173,13 +173,13 @@ export class MetricsTracer {
}

/**
* Returns the difference in milliseconds between two Date objects.
* Returns the difference in milliseconds between two times.
* @param start The start time.
* @param end The end time.
* @returns The time difference in milliseconds.
*/
private _getMillisecondTimeDifference(start: Date, end: Date): number {
return end.valueOf() - start.valueOf();
private _getMillisecondTimeDifference(start: number, end: number): number {
return end - start;
}

/**
Expand Down Expand Up @@ -235,7 +235,7 @@ export class MetricsTracer {
if (!this.enabled) return;
this.currentOperation!.currentAttempt!.status = Status[statusCode];
const attemptAttributes = this._createAttemptOtelAttributes();
const endTime = new Date(Date.now());
const endTime = performance.now();
const attemptLatencyMilliseconds = this._getMillisecondTimeDifference(
this.currentOperation!.currentAttempt!.startTime,
endTime,
Expand Down Expand Up @@ -265,7 +265,7 @@ export class MetricsTracer {
*/
public recordOperationCompletion() {
if (!this.enabled || !this.currentOperation) return;
const endTime = new Date(Date.now());
const endTime = performance.now();
const operationAttributes = this._createOperationOtelAttributes();
const operationLatencyMilliseconds = this._getMillisecondTimeDifference(
this.currentOperation!.startTime,
Expand Down
36 changes: 36 additions & 0 deletions handwritten/spanner/test/metrics/metrics-tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,21 @@ describe('MetricsTracer', () => {
);
});

it('should record fractional latency with sub-millisecond precision', () => {
const nowStub = sandbox.stub(performance, 'now');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The test relies on the global performance object, which is only available in Node.js v16+. For compatibility with older Node.js versions, consider importing performance from perf_hooks and stubbing that instance instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Spanner supports node versions above 18

nowStub.onCall(0).returns(100.0); // op start
nowStub.onCall(1).returns(200.5); // attempt start
nowStub.onCall(2).returns(202.75); // attempt end

tracer.recordOperationStart();
tracer.recordAttemptStart();
tracer.recordAttemptCompletion(Status.OK);

assert.strictEqual(fakeAttemptLatency.record.calledOnce, true);
const [[latency]] = fakeAttemptLatency.record.args;
assert.strictEqual(latency, 2.25); // 202.75 - 200.5
});

it('should do nothing if disabled', () => {
tracer.enabled = false;
tracer.recordAttemptStart();
Expand Down Expand Up @@ -142,6 +157,27 @@ describe('MetricsTracer', () => {
assert.strictEqual(opAttrs[Constants.METRIC_LABEL_KEY_STATUS], 'OK');
});

it('should record fractional operation latency with sub-millisecond precision', () => {
const nowStub = sandbox.stub(performance, 'now');
nowStub.onCall(0).returns(100.5); // op start
nowStub.onCall(1).returns(105.0); // attempt start
nowStub.onCall(2).returns(108.0); // attempt end
nowStub.onCall(3).returns(110.25); // op end

sandbox.stub(MetricsTracerFactory, 'getInstance').returns({
clearCurrentTracer: sinon.spy(),
} as any);

tracer.recordOperationStart();
tracer.recordAttemptStart();
tracer.recordAttemptCompletion(Status.OK);
tracer.recordOperationCompletion();

assert.strictEqual(fakeOperationLatency.record.calledOnce, true);
const [[latency]] = fakeOperationLatency.record.args;
assert.strictEqual(latency, 9.75); // 110.25 - 100.5
});

it('should do nothing if disabled', () => {
tracer.enabled = false;
tracer.recordOperationCompletion();
Expand Down
Loading