Skip to content

Commit 02769b9

Browse files
fabriziodemarianicklaslandreas-karlsson
authored
feat: Add status to telemetry data (#257)
Co-authored-by: Nicklas Lundin <nicklasl@spotify.com> Co-authored-by: Andreas Karlsson <andreas.karlsson.se@gmail.com>
1 parent 3c764d9 commit 02769b9

5 files changed

Lines changed: 324 additions & 52 deletions

File tree

‎packages/sdk/proto/confidence/telemetry/v1/telemetry.proto‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,29 @@ message LibraryTraces {
2020

2121
message Trace {
2222
TraceId id = 1;
23-
// only used for timed events.
23+
24+
// DEPRECATED
2425
optional uint64 millisecond_duration = 2;
26+
27+
oneof trace {
28+
RequestTrace request_trace = 3;
29+
CountTrace count_trace = 4;
30+
}
31+
32+
message CountTrace {}
33+
34+
message RequestTrace {
35+
uint64 millisecond_duration = 1;
36+
Status status = 2;
37+
38+
enum Status {
39+
STATUS_UNSPECIFIED = 0;
40+
STATUS_SUCCESS = 1;
41+
STATUS_ERROR = 2;
42+
STATUS_TIMEOUT = 3;
43+
STATUS_CACHED = 4;
44+
}
45+
}
2546
}
2647

2748
enum Library {

‎packages/sdk/src/FlagResolverClient.ts‎

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { FlagEvaluation } from '.';
22
import { AccessiblePromise } from './AccessiblePromise';
33
import { Applier, FlagResolution } from './FlagResolution';
4-
import { Telemetry, Meter } from './Telemetry';
4+
import { Telemetry, TraceConsumer } from './Telemetry';
55
import { CacheProvider } from './flag-cache';
66
import { Context } from './context';
77
import { FetchBuilder, InternalFetch, SimpleFetch, TimeUnit } from './fetch-util';
@@ -14,6 +14,7 @@ import {
1414
import { Sdk } from './generated/confidence/flags/resolver/v1/types';
1515
import {
1616
LibraryTraces_Library,
17+
LibraryTraces_Trace_RequestTrace_Status as TraceStatus,
1718
LibraryTraces_TraceId,
1819
Monitoring,
1920
} from './generated/confidence/telemetry/v1/telemetry';
@@ -106,12 +107,12 @@ export class FetchingFlagResolverClient implements FlagResolverClient {
106107
private readonly applyDebounce: number;
107108
private readonly resolveTimeout: number;
108109
private readonly baseUrl: string;
109-
private readonly markLatency: Meter;
110+
private readonly traceConsumer: TraceConsumer;
110111
private readonly waitUntil: WaitUntil | undefined;
111112
private readonly cacheReadThrough: (
112113
context: Context,
113114
supplier: () => Promise<ResolveFlagsResponse>,
114-
) => Promise<ResolveFlagsResponse>;
115+
) => Promise<{ response: ResolveFlagsResponse; isFromCache: boolean }>;
115116

116117
constructor({
117118
fetchImplementation,
@@ -128,7 +129,7 @@ export class FetchingFlagResolverClient implements FlagResolverClient {
128129
waitUntil,
129130
cacheProvider,
130131
}: FlagResolverClientOptions) {
131-
this.markLatency = telemetry.registerMeter({
132+
this.traceConsumer = telemetry.registerLibraryTraces({
132133
library: LibraryTraces_Library.LIBRARY_CONFIDENCE,
133134
version: sdk.version,
134135
id: LibraryTraces_TraceId.TRACE_ID_RESOLVE_LATENCY,
@@ -155,13 +156,32 @@ export class FetchingFlagResolverClient implements FlagResolverClient {
155156
if (cacheProvider) {
156157
this.cacheReadThrough = (context, supplier) => {
157158
const cache = cacheProvider(this.clientSecret);
158-
return cache.get(context, supplier);
159+
let isFromCache = true; // Default to true, will be set to false if supplier is called
160+
161+
// Create a wrapper supplier that sets the flag when called
162+
const wrappedSupplier = async () => {
163+
isFromCache = false;
164+
return supplier();
165+
};
166+
167+
return cache.get(context, wrappedSupplier).then(response => {
168+
return { response, isFromCache };
169+
});
159170
};
160171
} else {
161-
this.cacheReadThrough = (_context, supplier) => supplier();
172+
this.cacheReadThrough = (_context, supplier) => supplier().then(response => ({ response, isFromCache: false }));
162173
}
163174
}
164175

176+
private markLatency(latency: number, status: TraceStatus): void {
177+
this.traceConsumer({
178+
requestTrace: {
179+
millisecondDuration: Math.round(latency),
180+
status,
181+
},
182+
});
183+
}
184+
165185
resolve(context: Context): PendingResolution {
166186
const request: ResolveFlagsRequest = {
167187
clientSecret: this.clientSecret,
@@ -177,18 +197,19 @@ export class FetchingFlagResolverClient implements FlagResolverClient {
177197
this.resolveTimeout,
178198
new ResolveError('TIMEOUT', 'Resolve timeout'),
179199
);
180-
const start = Date.now();
200+
const start = performance.now();
181201

182202
return this.cacheReadThrough(context, () => this.resolveFlagsJson(request, signalWithTimeout))
183-
.then(response => FlagResolution.ready(context, response, this.createApplier(response.resolveToken)))
184-
.catch(error => {
185-
if (error instanceof ResolveError) {
186-
return FlagResolution.failed(context, error.code, error.message);
187-
}
188-
return FlagResolution.failed(context, 'GENERAL', error.message);
203+
.then(({ response, isFromCache }) => {
204+
const latency = performance.now() - start;
205+
this.markLatency(latency, isFromCache ? TraceStatus.STATUS_CACHED : TraceStatus.STATUS_SUCCESS);
206+
return FlagResolution.ready(context, response, this.createApplier(response.resolveToken));
189207
})
190-
.finally(() => {
191-
this.markLatency(Date.now() - start);
208+
.catch(error => {
209+
const latency = performance.now() - start;
210+
const errorCode: FlagEvaluation.ErrorCode = error instanceof ResolveError ? error.code : 'GENERAL';
211+
this.markLatency(latency, errorCode === 'TIMEOUT' ? TraceStatus.STATUS_TIMEOUT : TraceStatus.STATUS_ERROR);
212+
return FlagResolution.failed(context, errorCode, error.message);
192213
});
193214
});
194215
}

‎packages/sdk/src/Telemetry.test.ts‎

Lines changed: 69 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,22 @@
1-
import { LibraryTraces_Library, LibraryTraces_TraceId } from './generated/confidence/telemetry/v1/telemetry';
1+
import {
2+
LibraryTraces_Library,
3+
LibraryTraces_TraceId,
4+
LibraryTraces_Trace_RequestTrace_Status,
5+
} from './generated/confidence/telemetry/v1/telemetry';
26
import { Telemetry } from './Telemetry';
37

48
describe('Telemetry', () => {
5-
it('registerCounter and increment counter', () => {
9+
it('registerLibraryTraces and increment counter', () => {
610
const telemetry = new Telemetry({ disabled: false, environment: 'backend' });
7-
const counter = telemetry.registerCounter({
11+
const traceConsumer = telemetry.registerLibraryTraces({
812
library: LibraryTraces_Library.LIBRARY_CONFIDENCE,
913
version: '9.9.9',
1014
id: LibraryTraces_TraceId.TRACE_ID_STALE_FLAG,
1115
});
1216
// increment the registered counter 3 times
13-
counter();
14-
counter();
15-
counter();
17+
traceConsumer({});
18+
traceConsumer({});
19+
traceConsumer({});
1620
const snapshot = telemetry.getSnapshot();
1721
expect(snapshot).toBeTruthy();
1822
expect(snapshot?.libraryTraces.length).toEqual(1);
@@ -25,55 +29,100 @@ describe('Telemetry', () => {
2529
]);
2630
});
2731

28-
it('registerMeter and add measurement', () => {
32+
it('registerLibraryTraces and add requestTrace', () => {
2933
const telemetry = new Telemetry({ disabled: false, environment: 'client' });
30-
const meter = telemetry.registerMeter({
34+
const traceConsumer = telemetry.registerLibraryTraces({
3135
library: LibraryTraces_Library.LIBRARY_CONFIDENCE,
3236
version: '9.9.9',
3337
id: LibraryTraces_TraceId.TRACE_ID_RESOLVE_LATENCY,
3438
});
3539
// measure 3 times with different values
36-
meter(10);
37-
meter(20);
38-
meter(30);
40+
traceConsumer({
41+
requestTrace: { millisecondDuration: 10, status: LibraryTraces_Trace_RequestTrace_Status.STATUS_SUCCESS },
42+
});
43+
traceConsumer({
44+
requestTrace: { millisecondDuration: 20, status: LibraryTraces_Trace_RequestTrace_Status.STATUS_SUCCESS },
45+
});
46+
traceConsumer({
47+
requestTrace: { millisecondDuration: 30, status: LibraryTraces_Trace_RequestTrace_Status.STATUS_ERROR },
48+
});
3949
const snapshot = telemetry.getSnapshot();
4050
expect(snapshot).toBeTruthy();
4151
expect(snapshot?.libraryTraces.length).toEqual(1);
4252
expect(snapshot?.libraryTraces[0].library).toEqual(LibraryTraces_Library.LIBRARY_CONFIDENCE);
4353
expect(snapshot?.libraryTraces[0].libraryVersion).toEqual('9.9.9');
4454
expect(snapshot?.libraryTraces[0].traces).toEqual([
45-
{ id: LibraryTraces_TraceId.TRACE_ID_RESOLVE_LATENCY, millisecondDuration: 10 },
46-
{ id: LibraryTraces_TraceId.TRACE_ID_RESOLVE_LATENCY, millisecondDuration: 20 },
47-
{ id: LibraryTraces_TraceId.TRACE_ID_RESOLVE_LATENCY, millisecondDuration: 30 },
55+
{
56+
id: LibraryTraces_TraceId.TRACE_ID_RESOLVE_LATENCY,
57+
requestTrace: { millisecondDuration: 10, status: LibraryTraces_Trace_RequestTrace_Status.STATUS_SUCCESS },
58+
},
59+
{
60+
id: LibraryTraces_TraceId.TRACE_ID_RESOLVE_LATENCY,
61+
requestTrace: { millisecondDuration: 20, status: LibraryTraces_Trace_RequestTrace_Status.STATUS_SUCCESS },
62+
},
63+
{
64+
id: LibraryTraces_TraceId.TRACE_ID_RESOLVE_LATENCY,
65+
requestTrace: { millisecondDuration: 30, status: LibraryTraces_Trace_RequestTrace_Status.STATUS_ERROR },
66+
},
4867
]);
4968
});
5069

5170
it('snapshot is empty when telemetry is disabled', () => {
5271
const telemetry = new Telemetry({ disabled: true, environment: 'client' });
53-
const counter = telemetry.registerCounter({
72+
const traceConsumer = telemetry.registerLibraryTraces({
5473
library: LibraryTraces_Library.LIBRARY_CONFIDENCE,
5574
version: '9.9.9',
5675
id: LibraryTraces_TraceId.TRACE_ID_STALE_FLAG,
5776
});
58-
counter();
77+
traceConsumer({});
5978
const snapshot = telemetry.getSnapshot();
6079
expect(snapshot.libraryTraces.length).toBe(0);
6180
});
6281

63-
it('monitoring gets cleared after snapshot is obtained', () => {
82+
it('traceConsumer gets cleared after snapshot is obtained', () => {
6483
const telemetry = new Telemetry({ disabled: false, environment: 'client' });
65-
const counter = telemetry.registerCounter({
84+
const traceConsumer = telemetry.registerLibraryTraces({
6685
library: LibraryTraces_Library.LIBRARY_CONFIDENCE,
6786
version: '9.9.9',
6887
id: LibraryTraces_TraceId.TRACE_ID_STALE_FLAG,
6988
});
70-
counter();
89+
traceConsumer({});
7190
const snapshotFirst = telemetry.getSnapshot();
7291
expect(snapshotFirst).toBeTruthy();
7392
expect(snapshotFirst?.libraryTraces.length).toEqual(1);
7493
expect(snapshotFirst?.libraryTraces[0].traces).toEqual([{ id: LibraryTraces_TraceId.TRACE_ID_STALE_FLAG }]);
7594
const snapshotSecond = telemetry.getSnapshot();
76-
// the counter is still registered but the traces are cleared
7795
expect(snapshotSecond?.libraryTraces.length).toBe(0);
7896
});
97+
98+
it('registerLibraryTraces and log cached requestTrace', () => {
99+
const telemetry = new Telemetry({ disabled: false, environment: 'client' });
100+
const traceConsumer = telemetry.registerLibraryTraces({
101+
library: LibraryTraces_Library.LIBRARY_CONFIDENCE,
102+
version: '9.9.9',
103+
id: LibraryTraces_TraceId.TRACE_ID_RESOLVE_LATENCY,
104+
});
105+
106+
traceConsumer({
107+
requestTrace: {
108+
millisecondDuration: 5,
109+
status: LibraryTraces_Trace_RequestTrace_Status.STATUS_CACHED,
110+
},
111+
});
112+
113+
const snapshot = telemetry.getSnapshot();
114+
expect(snapshot).toBeTruthy();
115+
expect(snapshot?.libraryTraces.length).toEqual(1);
116+
expect(snapshot?.libraryTraces[0].library).toEqual(LibraryTraces_Library.LIBRARY_CONFIDENCE);
117+
expect(snapshot?.libraryTraces[0].libraryVersion).toEqual('9.9.9');
118+
expect(snapshot?.libraryTraces[0].traces).toEqual([
119+
{
120+
id: LibraryTraces_TraceId.TRACE_ID_RESOLVE_LATENCY,
121+
requestTrace: {
122+
millisecondDuration: 5,
123+
status: LibraryTraces_Trace_RequestTrace_Status.STATUS_CACHED,
124+
},
125+
},
126+
]);
127+
});
79128
});

‎packages/sdk/src/Telemetry.ts‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ export type Tag = {
1616
id: LibraryTraces_TraceId;
1717
};
1818

19-
export type Counter = () => void;
20-
export type Meter = (value: number) => void;
2119
export type TraceConsumer = (trace: Omit<LibraryTraces_Trace, 'id'>) => void;
2220
export class Telemetry {
2321
private readonly disabled: boolean;
@@ -31,7 +29,7 @@ export class Telemetry {
3129
this.platform = opts.environment === 'client' ? Platform.PLATFORM_JS_WEB : Platform.PLATFORM_JS_SERVER;
3230
}
3331

34-
private registerLibraryTraces({ library, version, id }: Tag): TraceConsumer {
32+
public registerLibraryTraces({ library, version, id }: Tag): TraceConsumer {
3533
if (this.disabled) {
3634
return () => {};
3735
}
@@ -50,16 +48,6 @@ export class Telemetry {
5048
};
5149
}
5250

53-
registerCounter(tag: Tag): Counter {
54-
const traceConsumer = this.registerLibraryTraces(tag);
55-
return () => traceConsumer({});
56-
}
57-
58-
registerMeter(tag: Tag): Meter {
59-
const traceConsumer = this.registerLibraryTraces(tag);
60-
return (millisecondDuration: number) => traceConsumer({ millisecondDuration });
61-
}
62-
6351
getSnapshot(): Monitoring {
6452
const libraryTraces = this.libraryTraces
6553
.filter(({ traces }) => traces.length > 0)

0 commit comments

Comments
 (0)