Skip to content

Commit b5ddafb

Browse files
n1ru4ln1ru4l
authored
feat: skip presigned url step + analytics (#5542)
1 parent b28312c commit b5ddafb

File tree

9 files changed

+333
-228
lines changed

9 files changed

+333
-228
lines changed

‎packages/services/cdn-worker/src/analytics.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,15 @@ type Event =
5757
| {
5858
type: 'r2';
5959
action:
60-
| 'HEAD artifact'
60+
| 'GET artifact'
6161
| 'GET cdn-legacy-keys'
6262
| 'GET cdn-access-token'
6363
| 'GET persistedOperation'
6464
| 'HEAD appDeploymentIsEnabled';
65-
statusCode: number;
65+
// Either 3 digit status code or error code e.g. timeout, http error etc.
66+
statusCodeOrErrCode: number | string;
67+
/** duration in milliseconds */
68+
duration: number;
6669
}
6770
| {
6871
type: 'response';
@@ -97,7 +100,8 @@ export function createAnalytics(
97100
});
98101
case 'r2':
99102
return engines.r2.writeDataPoint({
100-
blobs: [event.action, event.statusCode.toString(), targetId],
103+
blobs: [event.action, event.statusCodeOrErrCode.toString(), targetId],
104+
doubles: [event.duration],
101105
indexes: [targetId.substring(0, 32)],
102106
});
103107
case 'response':

‎packages/services/cdn-worker/src/artifact-handler.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export const createArtifactRequestHandler = (deps: ArtifactRequestHandler) => {
129129

130130
const eTag = request.headers.get('if-none-match');
131131

132-
const result = await deps.artifactStorageReader.generateArtifactReadUrl(
132+
const result = await deps.artifactStorageReader.readArtifact(
133133
params.targetId,
134134
params.contractName,
135135
params.artifactType,

‎packages/services/cdn-worker/src/artifact-storage-reader.ts‎

Lines changed: 132 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,16 @@ export class ArtifactStorageReader {
6565
endpoint: string;
6666
bucketName: string;
6767
},
68+
// private s3Mirror: {
69+
// client: AwsClient;
70+
// endpoint: string;
71+
// bucketName: string;
72+
// },
6873
private analytics: Analytics | null,
6974
) {}
7075

71-
/** Generate a pre-signed url for reading an artifact from a bucket for a limited time period. */
72-
async generateArtifactReadUrl(
76+
/** Read an artifact from S3 */
77+
async readArtifact(
7378
targetId: string,
7479
contractName: string | null,
7580
artifactType: ArtifactsType,
@@ -81,55 +86,57 @@ export class ArtifactStorageReader {
8186

8287
const key = buildArtifactStorageKey(targetId, artifactType, contractName);
8388

84-
const headResponse = await this.s3.client.fetch(
89+
const headers: HeadersInit = {};
90+
91+
if (etagValue) {
92+
headers['if-none-match'] = etagValue;
93+
}
94+
95+
const response = await this.s3.client.fetch(
8596
[this.s3.endpoint, this.s3.bucketName, key].join('/'),
8697
{
87-
method: 'HEAD',
98+
method: 'GET',
99+
headers,
88100
aws: {
89101
signQuery: true,
90102
},
91103
timeout: READ_TIMEOUT_MS,
104+
onAttempt: args => {
105+
this.analytics?.track(
106+
{
107+
type: 'r2',
108+
statusCodeOrErrCode:
109+
args.result.type === 'error'
110+
? String(args.result.error.name ?? 'unknown')
111+
: args.result.response.status,
112+
action: 'GET artifact',
113+
duration: args.duration,
114+
},
115+
targetId,
116+
);
117+
},
92118
},
93119
);
94-
this.analytics?.track(
95-
{
96-
type: 'r2',
97-
statusCode: headResponse.status,
98-
action: 'HEAD artifact',
99-
},
100-
targetId,
101-
);
102-
103-
if (headResponse.status === 200) {
104-
if (etagValue && headResponse.headers.get('etag') === etagValue) {
105-
return { type: 'notModified' } as const;
106-
}
107-
108-
const getResponse = await this.s3.client.fetch(
109-
[this.s3.endpoint, this.s3.bucketName, key].join('/'),
110-
{
111-
method: 'GET',
112-
aws: {
113-
signQuery: true,
114-
},
115-
timeout: READ_TIMEOUT_MS,
116-
},
117-
);
118120

119-
if (getResponse.ok) {
120-
return {
121-
type: 'response',
122-
response: getResponse,
123-
} as const;
124-
}
121+
if (response.status === 404) {
122+
return { type: 'notFound' } as const;
123+
}
125124

126-
throw new Error(`GET request failed with status ${getResponse.status}`);
125+
if (response.status === 304) {
126+
return {
127+
type: 'notModified',
128+
} as const;
127129
}
128-
if (headResponse.status === 404) {
129-
return { type: 'notFound' } as const;
130+
131+
if (response.status === 200) {
132+
return {
133+
type: 'response',
134+
response,
135+
} as const;
130136
}
131-
const body = await headResponse.text();
132-
throw new Error(`HEAD request failed with status ${headResponse.status}: ${body}`);
137+
138+
const body = await response.text();
139+
throw new Error(`GET request failed with status ${response.status}: ${body}`);
133140
}
134141

135142
async isAppDeploymentEnabled(targetId: string, appName: string, appVersion: string) {
@@ -143,16 +150,22 @@ export class ArtifactStorageReader {
143150
signQuery: true,
144151
},
145152
timeout: READ_TIMEOUT_MS,
153+
onAttempt: args => {
154+
this.analytics?.track(
155+
{
156+
type: 'r2',
157+
statusCodeOrErrCode:
158+
args.result.type === 'error'
159+
? String(args.result.error.name ?? 'unknown')
160+
: args.result.response.status,
161+
action: 'HEAD appDeploymentIsEnabled',
162+
duration: args.duration,
163+
},
164+
targetId,
165+
);
166+
},
146167
},
147168
);
148-
this.analytics?.track(
149-
{
150-
type: 'r2',
151-
statusCode: response.status,
152-
action: 'HEAD appDeploymentIsEnabled',
153-
},
154-
targetId,
155-
);
156169

157170
return response.status === 200;
158171
}
@@ -180,18 +193,23 @@ export class ArtifactStorageReader {
180193
},
181194
headers,
182195
timeout: READ_TIMEOUT_MS,
196+
onAttempt: args => {
197+
this.analytics?.track(
198+
{
199+
type: 'r2',
200+
statusCodeOrErrCode:
201+
args.result.type === 'error'
202+
? String(args.result.error.name ?? 'unknown')
203+
: args.result.response.status,
204+
action: 'GET persistedOperation',
205+
duration: args.duration,
206+
},
207+
targetId,
208+
);
209+
},
183210
},
184211
);
185212

186-
this.analytics?.track(
187-
{
188-
type: 'r2',
189-
statusCode: response.status,
190-
action: 'GET persistedOperation',
191-
},
192-
targetId,
193-
);
194-
195213
if (etagValue && response.status === 304) {
196214
return { type: 'notModified' } as const;
197215
}
@@ -211,4 +229,62 @@ export class ArtifactStorageReader {
211229
const body = await response.text();
212230
throw new Error(`HEAD request failed with status ${response.status}: ${body}`);
213231
}
232+
233+
async readLegacyAccessKey(targetId: string) {
234+
const response = await this.s3.client.fetch(
235+
[this.s3.endpoint, this.s3.bucketName, 'cdn-legacy-keys', targetId].join('/'),
236+
{
237+
method: 'GET',
238+
timeout: READ_TIMEOUT_MS,
239+
onAttempt: args => {
240+
this.analytics?.track(
241+
{
242+
type: 'r2',
243+
statusCodeOrErrCode:
244+
args.result.type === 'error'
245+
? String(args.result.error.name ?? 'unknown')
246+
: args.result.response.status,
247+
action: 'GET cdn-legacy-keys',
248+
duration: args.duration,
249+
},
250+
targetId,
251+
);
252+
},
253+
},
254+
);
255+
256+
return response;
257+
}
258+
259+
async readAccessKey(targetId: string, keyId: string) {
260+
const s3KeyParts = ['cdn-keys', targetId, keyId];
261+
262+
const response = await this.s3.client.fetch(
263+
[this.s3.endpoint, this.s3.bucketName, ...s3KeyParts].join('/'),
264+
{
265+
method: 'GET',
266+
aws: {
267+
// This boolean makes Google Cloud Storage & AWS happy.
268+
signQuery: true,
269+
},
270+
timeout: READ_TIMEOUT_MS,
271+
onAttempt: args => {
272+
this.analytics?.track(
273+
{
274+
type: 'r2',
275+
statusCodeOrErrCode:
276+
args.result.type === 'error'
277+
? String(args.result.error.name ?? 'unknown')
278+
: args.result.response.status,
279+
action: 'GET cdn-access-token',
280+
duration: args.duration,
281+
},
282+
targetId,
283+
);
284+
},
285+
},
286+
);
287+
288+
return response;
289+
}
214290
}

‎packages/services/cdn-worker/src/aws.ts‎

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,25 @@ type AwsRequestInit = RequestInit & {
4949
* Timeout in milliseconds for each fetch call.
5050
*/
5151
timeout?: number;
52+
/** Hook being invoked for each attempt for gathering analytics or similar. */
53+
onAttempt?: (args: {
54+
/** attempt number */
55+
attempt: number;
56+
/** attempt duration in ms */
57+
duration: number;
58+
/** result */
59+
result:
60+
| {
61+
// HTTP or other unexpected error
62+
type: 'error';
63+
error: Error;
64+
}
65+
| {
66+
// HTTP response sent by upstream server
67+
type: 'success';
68+
response: Response;
69+
};
70+
}) => void;
5271
};
5372

5473
export type AWSClientConfig = {
@@ -82,7 +101,7 @@ export class AwsClient {
82101
this.service = args.service;
83102
this.region = args.region;
84103
this.cache = args.cache || new Map();
85-
this.retries = args.retries != null ? args.retries : 10; // Up to 25.6 secs
104+
this.retries = args.retries != null ? args.retries : 3;
86105
this.initRetryMs = args.initRetryMs || 50;
87106
this._fetch = args.fetch || fetch.bind(globalThis);
88107
}
@@ -129,18 +148,31 @@ export class AwsClient {
129148

130149
async fetch(input: RequestInfo, init: AwsRequestInit): Promise<Response> {
131150
for (let i = 0; i <= this.retries; i++) {
132-
const fetched = this._fetch(...(await this.sign(input, init)));
133-
if (i === this.retries) {
134-
return fetched; // No need to await if we're returning anyway
135-
}
151+
const attemptStart = performance.now();
136152
try {
137-
const res = await fetched;
138-
if (res.status < 500 && res.status !== 429 && res.status !== 499) {
139-
return res;
153+
const response = await this._fetch(...(await this.sign(input, init)));
154+
const duration = performance.now() - attemptStart;
155+
init.onAttempt?.({ attempt: i, duration, result: { type: 'success', response } });
156+
157+
if (
158+
(response.status < 500 && response.status !== 429 && response.status !== 499) ||
159+
i === this.retries
160+
) {
161+
return response;
140162
}
141163
} catch (error) {
164+
const duration = performance.now() - attemptStart;
142165
// Retry also when there's an exception
143166
console.error(error);
167+
init.onAttempt?.({
168+
attempt: i,
169+
duration,
170+
result: { type: 'error', error: error as Error },
171+
});
172+
173+
if (i === this.retries) {
174+
throw error;
175+
}
144176
}
145177
await new Promise(resolve =>
146178
setTimeout(resolve, Math.random() * this.initRetryMs * Math.pow(2, i)),

‎packages/services/cdn-worker/src/dev.ts‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@ const PORT = process.env.PORT ? parseInt(process.env.PORT, 10) : 4010;
2626
const artifactStorageReader = new ArtifactStorageReader(s3, null);
2727

2828
const handleRequest = createRequestHandler({
29-
isKeyValid: createIsKeyValid({ s3, getCache: null, waitUntil: null, analytics: null }),
29+
isKeyValid: createIsKeyValid({
30+
artifactStorageReader,
31+
getCache: null,
32+
waitUntil: null,
33+
analytics: null,
34+
}),
3035
async getArtifactAction(targetId, contractName, artifactType, eTag) {
31-
return artifactStorageReader.generateArtifactReadUrl(
32-
targetId,
33-
contractName,
34-
artifactType,
35-
eTag,
36-
);
36+
return artifactStorageReader.readArtifact(targetId, contractName, artifactType, eTag);
3737
},
3838
async fetchText(url) {
3939
const r = await fetch(url);
@@ -47,7 +47,12 @@ const handleRequest = createRequestHandler({
4747
});
4848

4949
const handleArtifactRequest = createArtifactRequestHandler({
50-
isKeyValid: createIsKeyValid({ s3, getCache: null, waitUntil: null, analytics: null }),
50+
isKeyValid: createIsKeyValid({
51+
artifactStorageReader,
52+
getCache: null,
53+
waitUntil: null,
54+
analytics: null,
55+
}),
5156
isAppDeploymentActive: createIsAppDeploymentActive({
5257
artifactStorageReader,
5358
getCache: null,

0 commit comments

Comments
 (0)