Skip to content

Commit 6043cc0

Browse files
geroplroboquat
geropl
authored and
roboquat
committed
[server] Finish spans
1 parent 4fa1859 commit 6043cc0

File tree

2 files changed

+110
-86
lines changed

2 files changed

+110
-86
lines changed

‎components/server/src/workspace/headless-log-controller.ts

Lines changed: 69 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { accessHeadlessLogs } from "../auth/rate-limiter";
3333
import { BearerAuth } from "../auth/bearer-authenticator";
3434
import { ProjectsService } from "../projects/projects-service";
3535
import { HostContextProvider } from "../auth/host-context-provider";
36+
import { TraceContext } from "@gitpod/gitpod-protocol/lib/util/tracing";
3637

3738
export const HEADLESS_LOGS_PATH_PREFIX = "/headless-logs";
3839
export const HEADLESS_LOG_DOWNLOAD_PATH_PREFIX = "/headless-log-download";
@@ -54,79 +55,86 @@ export class HeadlessLogController {
5455
authenticateAndAuthorize,
5556
asyncHandler(async (req: express.Request, res: express.Response) => {
5657
const span = opentracing.globalTracer().startSpan(HEADLESS_LOGS_PATH_PREFIX);
57-
const params = { instanceId: req.params.instanceId, terminalId: req.params.terminalId };
58-
const user = req.user as User; // verified by authenticateAndAuthorize
59-
60-
const instanceId = params.instanceId;
61-
const ws = await this.authorizeHeadlessLogAccess(span, user, instanceId, res);
62-
if (!ws) {
63-
return;
64-
}
65-
const { workspace, instance } = ws;
58+
try {
59+
const params = { instanceId: req.params.instanceId, terminalId: req.params.terminalId };
60+
const user = req.user as User; // verified by authenticateAndAuthorize
6661

67-
const logCtx = { userId: user.id, instanceId, workspaceId: workspace!.id };
68-
log.debug(logCtx, HEADLESS_LOGS_PATH_PREFIX);
62+
const instanceId = params.instanceId;
63+
const ws = await this.authorizeHeadlessLogAccess(span, user, instanceId, res);
64+
if (!ws) {
65+
return;
66+
}
67+
const { workspace, instance } = ws;
6968

70-
const aborted = new Deferred<boolean>();
71-
try {
72-
const head = {
73-
"Content-Type": "text/html; charset=utf-8", // is text/plain, but with that node.js won't stream...
74-
"Transfer-Encoding": "chunked",
75-
"Cache-Control": "no-cache, no-store, must-revalidate", // make sure stream are not re-used on reconnect
76-
};
77-
res.writeHead(200, head);
69+
const logCtx = { userId: user.id, instanceId, workspaceId: workspace!.id };
70+
log.debug(logCtx, HEADLESS_LOGS_PATH_PREFIX);
7871

79-
const abort = (err: any) => {
80-
aborted.resolve(true);
81-
log.debug(logCtx, "headless-log: aborted");
82-
};
83-
req.on("abort", abort); // abort handling if the reqeuest was aborted
72+
const aborted = new Deferred<boolean>();
73+
try {
74+
const head = {
75+
"Content-Type": "text/html; charset=utf-8", // is text/plain, but with that node.js won't stream...
76+
"Transfer-Encoding": "chunked",
77+
"Cache-Control": "no-cache, no-store, must-revalidate", // make sure stream are not re-used on reconnect
78+
};
79+
res.writeHead(200, head);
8480

85-
const queue = new Queue(); // Make sure we forward in the correct order
86-
const writeToResponse = async (chunk: string) =>
87-
queue.enqueue(
88-
() =>
89-
new Promise<void>(async (resolve, reject) => {
90-
if (aborted.isResolved && (await aborted.promise)) {
91-
return;
92-
}
81+
const abort = (err: any) => {
82+
aborted.resolve(true);
83+
log.debug(logCtx, "headless-log: aborted");
84+
};
85+
req.on("abort", abort); // abort handling if the reqeuest was aborted
9386

94-
const done = res.write(chunk, "utf-8", (err?: Error | null) => {
95-
if (err) {
96-
reject(err); // propagate write error to upstream
87+
const queue = new Queue(); // Make sure we forward in the correct order
88+
const writeToResponse = async (chunk: string) =>
89+
queue.enqueue(
90+
() =>
91+
new Promise<void>(async (resolve, reject) => {
92+
if (aborted.isResolved && (await aborted.promise)) {
9793
return;
9894
}
99-
});
100-
// handle as per doc: https://nodejs.org/api/stream.html#stream_writable_write_chunk_encoding_callback
101-
if (!done) {
102-
res.once("drain", resolve);
103-
} else {
104-
setImmediate(resolve);
105-
}
106-
}),
95+
96+
const done = res.write(chunk, "utf-8", (err?: Error | null) => {
97+
if (err) {
98+
reject(err); // propagate write error to upstream
99+
return;
100+
}
101+
});
102+
// handle as per doc: https://nodejs.org/api/stream.html#stream_writable_write_chunk_encoding_callback
103+
if (!done) {
104+
res.once("drain", resolve);
105+
} else {
106+
setImmediate(resolve);
107+
}
108+
}),
109+
);
110+
const logEndpoint = HeadlessLogEndpoint.fromWithOwnerToken(instance);
111+
await this.headlessLogService.streamWorkspaceLogWhileRunning(
112+
logCtx,
113+
logEndpoint,
114+
instanceId,
115+
params.terminalId,
116+
writeToResponse,
117+
aborted,
107118
);
108-
const logEndpoint = HeadlessLogEndpoint.fromWithOwnerToken(instance);
109-
await this.headlessLogService.streamWorkspaceLogWhileRunning(
110-
logCtx,
111-
logEndpoint,
112-
instanceId,
113-
params.terminalId,
114-
writeToResponse,
115-
aborted,
116-
);
117119

118-
// In an ideal world, we'd use res.addTrailers()/response.trailer here. But despite being introduced with HTTP/1.1 in 1999, trailers are not supported by popular proxies (nginx, for example).
119-
// So we resort to this hand-written solution
120-
res.write(`\n${HEADLESS_LOG_STREAM_STATUS_CODE}: 200`);
120+
// In an ideal world, we'd use res.addTrailers()/response.trailer here. But despite being introduced with HTTP/1.1 in 1999, trailers are not supported by popular proxies (nginx, for example).
121+
// So we resort to this hand-written solution
122+
res.write(`\n${HEADLESS_LOG_STREAM_STATUS_CODE}: 200`);
121123

122-
res.end();
123-
} catch (err) {
124-
log.debug(logCtx, "error streaming headless logs", err);
124+
res.end();
125+
} catch (err) {
126+
log.debug(logCtx, "error streaming headless logs", err);
125127

126-
res.write(`\n${HEADLESS_LOG_STREAM_STATUS_CODE}: 500`);
127-
res.end();
128+
res.write(`\n${HEADLESS_LOG_STREAM_STATUS_CODE}: 500`);
129+
res.end();
130+
} finally {
131+
aborted.resolve(true); // ensure that the promise gets resolved eventually!
132+
}
133+
} catch (e) {
134+
TraceContext.setError({ span }, e);
135+
throw e;
128136
} finally {
129-
aborted.resolve(true); // ensure that the promise gets resolved eventually!
137+
span.finish();
130138
}
131139
}),
132140
]);

‎components/server/src/workspace/workspace-starter.ts

Lines changed: 41 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -451,12 +451,19 @@ export class WorkspaceStarter {
451451

452452
protected async notifyOnPrebuildQueued(ctx: TraceContext, workspaceId: string) {
453453
const span = TraceContext.startSpan("notifyOnPrebuildQueued", ctx);
454-
const prebuild = await this.workspaceDb.trace({ span }).findPrebuildByWorkspaceID(workspaceId);
455-
if (prebuild) {
456-
const info = (await this.workspaceDb.trace({ span }).findPrebuildInfos([prebuild.id]))[0];
457-
if (info) {
458-
await this.messageBus.notifyOnPrebuildUpdate({ info, status: "queued" });
454+
try {
455+
const prebuild = await this.workspaceDb.trace({ span }).findPrebuildByWorkspaceID(workspaceId);
456+
if (prebuild) {
457+
const info = (await this.workspaceDb.trace({ span }).findPrebuildInfos([prebuild.id]))[0];
458+
if (info) {
459+
await this.messageBus.notifyOnPrebuildUpdate({ info, status: "queued" });
460+
}
459461
}
462+
} catch (e) {
463+
TraceContext.setError({ span }, e);
464+
throw e;
465+
} finally {
466+
span.finish();
460467
}
461468
}
462469

@@ -507,6 +514,8 @@ export class WorkspaceStarter {
507514
"cannot properly fail workspace instance during start",
508515
err,
509516
);
517+
} finally {
518+
span.finish();
510519
}
511520
}
512521

@@ -1389,27 +1398,34 @@ export class WorkspaceStarter {
13891398
user: User,
13901399
): Promise<{ initializer: GitInitializer | CompositeInitializer; disposable: Disposable }> {
13911400
const span = TraceContext.startSpan("createInitializerForCommit", ctx);
1392-
const mainGit = this.createGitInitializer({ span }, workspace, context, user);
1393-
if (!context.additionalRepositoryCheckoutInfo || context.additionalRepositoryCheckoutInfo.length === 0) {
1394-
return mainGit;
1395-
}
1396-
const subRepoInitializers = [mainGit];
1397-
for (const subRepo of context.additionalRepositoryCheckoutInfo) {
1398-
subRepoInitializers.push(this.createGitInitializer({ span }, workspace, subRepo, user));
1399-
}
1400-
const inits = await Promise.all(subRepoInitializers);
1401-
const compositeInit = new CompositeInitializer();
1402-
const compositeDisposable = new DisposableCollection();
1403-
for (const r of inits) {
1404-
const wsinit = new WorkspaceInitializer();
1405-
wsinit.setGit(r.initializer);
1406-
compositeInit.addInitializer(wsinit);
1407-
compositeDisposable.push(r.disposable);
1401+
try {
1402+
const mainGit = this.createGitInitializer({ span }, workspace, context, user);
1403+
if (!context.additionalRepositoryCheckoutInfo || context.additionalRepositoryCheckoutInfo.length === 0) {
1404+
return mainGit;
1405+
}
1406+
const subRepoInitializers = [mainGit];
1407+
for (const subRepo of context.additionalRepositoryCheckoutInfo) {
1408+
subRepoInitializers.push(this.createGitInitializer({ span }, workspace, subRepo, user));
1409+
}
1410+
const inits = await Promise.all(subRepoInitializers);
1411+
const compositeInit = new CompositeInitializer();
1412+
const compositeDisposable = new DisposableCollection();
1413+
for (const r of inits) {
1414+
const wsinit = new WorkspaceInitializer();
1415+
wsinit.setGit(r.initializer);
1416+
compositeInit.addInitializer(wsinit);
1417+
compositeDisposable.push(r.disposable);
1418+
}
1419+
return {
1420+
initializer: compositeInit,
1421+
disposable: compositeDisposable,
1422+
};
1423+
} catch (e) {
1424+
TraceContext.setError({ span }, e);
1425+
throw e;
1426+
} finally {
1427+
span.finish();
14081428
}
1409-
return {
1410-
initializer: compositeInit,
1411-
disposable: compositeDisposable,
1412-
};
14131429
}
14141430

14151431
protected async createGitInitializer(

0 commit comments

Comments
 (0)