WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Commit 7116bfd

Browse files
committed
feat(payments-next): Capture error presence in CaptureTimingWithStatsD
Because: * Currently failing methods and successful methods are in the same bucket, and separating out the timing of the two distribution populations is not trivial This commit: * Adds in `error: 'true' | 'false'` to the StatsD call depending on whether the wrapped method throws an error Closes #PAY-3201
1 parent 4f18ae8 commit 7116bfd

File tree

2 files changed

+79
-5
lines changed

2 files changed

+79
-5
lines changed

libs/shared/metrics/statsd/src/lib/statsd.decorator.spec.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,16 @@ class TestClass {
3131
syncMethodWithCustomHandler() {
3232
return 'Custom Handler';
3333
}
34+
35+
@CaptureTimingWithStatsD()
36+
syncMethodWithError() {
37+
throw new Error('Sync error');
38+
}
39+
40+
@CaptureTimingWithStatsD()
41+
async asyncMethodWithError() {
42+
throw new Error('Async error');
43+
}
3444
}
3545

3646
describe('CaptureTimingWithStatsD', () => {
@@ -54,6 +64,7 @@ describe('CaptureTimingWithStatsD', () => {
5464
expect.any(Number),
5565
{
5666
sourceClass: 'TestClass',
67+
error: 'false',
5768
}
5869
);
5970
});
@@ -66,6 +77,7 @@ describe('CaptureTimingWithStatsD', () => {
6677
expect.any(Number),
6778
{
6879
sourceClass: 'TestClass',
80+
error: 'false',
6981
}
7082
);
7183
});
@@ -75,4 +87,54 @@ describe('CaptureTimingWithStatsD', () => {
7587
expect(mockCallback).toHaveBeenCalledTimes(1);
7688
expect(mockCallback).toHaveBeenCalledWith(expect.any(Number));
7789
});
90+
91+
it('should track error=true for synchronous methods that throw', () => {
92+
expect(() => instance.syncMethodWithError()).toThrow('Sync error');
93+
expect(mockStatsD.timing).toHaveBeenCalledTimes(2);
94+
expect(mockStatsD.timing).toHaveBeenCalledWith(
95+
'TestClass_syncMethodWithError',
96+
expect.any(Number),
97+
{
98+
sourceClass: 'TestClass',
99+
error: 'true',
100+
}
101+
);
102+
});
103+
104+
it('should track error=true for asynchronous methods that throw', async () => {
105+
await expect(instance.asyncMethodWithError()).rejects.toThrow('Async error');
106+
expect(mockStatsD.timing).toHaveBeenCalledTimes(2);
107+
expect(mockStatsD.timing).toHaveBeenCalledWith(
108+
'TestClass_asyncMethodWithError',
109+
expect.any(Number),
110+
{
111+
sourceClass: 'TestClass',
112+
error: 'true',
113+
}
114+
);
115+
});
116+
117+
it('should track error=false for successful synchronous methods', () => {
118+
instance.syncMethod();
119+
expect(mockStatsD.timing).toHaveBeenCalledWith(
120+
'TestClass',
121+
expect.any(Number),
122+
{
123+
methodName: 'syncMethod',
124+
error: 'false',
125+
}
126+
);
127+
});
128+
129+
it('should track error=false for successful asynchronous methods', async () => {
130+
await instance.asyncMethod();
131+
expect(mockStatsD.timing).toHaveBeenCalledWith(
132+
'TestClass',
133+
expect.any(Number),
134+
{
135+
methodName: 'asyncMethod',
136+
error: 'false',
137+
}
138+
);
139+
});
78140
});

libs/shared/metrics/statsd/src/lib/statsd.decorator.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,39 +22,51 @@ export function CaptureTimingWithStatsD<
2222
const originalDef = descriptor.value;
2323

2424
descriptor.value = function (this: T, ...args: any[]) {
25-
const defaultHandler = function (this: T, elapsed: number) {
25+
const defaultHandler = function (this: T, elapsed: number, error = false) {
2626
this.statsd.timing(`${this.constructor.name}_${key}`, elapsed, {
2727
sourceClass: this.constructor.name,
28+
error: error.toString(),
2829
...options?.tags,
2930
});
3031
this.statsd.timing(this.constructor.name, elapsed, {
3132
methodName: key,
33+
error: error.toString(),
3234
...options?.tags,
3335
});
3436
};
3537
const handler = options?.handle || defaultHandler;
3638

3739
const start = performance.now();
38-
const originalReturnValue = originalDef.apply(this, args);
40+
let originalReturnValue;
41+
let syncError = false;
42+
43+
try {
44+
originalReturnValue = originalDef.apply(this, args);
45+
} catch (err) {
46+
syncError = true;
47+
const end = performance.now();
48+
handler.apply(this, [end - start, true]);
49+
throw err;
50+
}
3951

4052
if (originalReturnValue instanceof Promise) {
4153
return originalReturnValue
4254
.then((value) => {
4355
const end = performance.now();
44-
handler.apply(this, [end - start]);
56+
handler.apply(this, [end - start, false]);
4557

4658
return value;
4759
})
4860
.catch((err) => {
4961
const end = performance.now();
50-
handler.apply(this, [end - start]);
62+
handler.apply(this, [end - start, true]);
5163

5264
throw err;
5365
});
5466
}
5567

5668
const end = performance.now();
57-
handler.apply(this, [end - start]);
69+
handler.apply(this, [end - start, syncError]);
5870

5971
return originalReturnValue;
6072
};

0 commit comments

Comments
 (0)