Skip to content

Commit 2823606

Browse files
mads-hartmannroboquat
mads-hartmann
authored and
roboquat
committed
Ensure we close spans
1 parent 72c6ebe commit 2823606

File tree

16 files changed

+250
-156
lines changed

16 files changed

+250
-156
lines changed

‎components/common-go/tracing/tracing.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,8 @@ func ApplyOWI(span opentracing.Span, owi logrus.Fields) {
110110
}
111111
}
112112

113-
// GetTraceID extracts the ueber-trace-id from the context
113+
// GetTraceID extracts the uber-trace-id from the context which encodes
114+
// {trace-id}:{span-id}:{parent-span-id}:{flags}
114115
func GetTraceID(span opentracing.Span) string {
115116
var buf bytes.Buffer
116117
err := opentracing.GlobalTracer().Inject(span.Context(), opentracing.Binary, &buf)

‎components/content-service/pkg/initializer/initializer.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,9 @@ func (e *EmptyInitializer) Run(ctx context.Context, mappings []archive.IDMapping
6060
type CompositeInitializer []Initializer
6161

6262
// Run calls run on all child initializers
63-
func (e CompositeInitializer) Run(ctx context.Context, mappings []archive.IDMapping) (csapi.WorkspaceInitSource, error) {
64-
_, ctx = opentracing.StartSpanFromContext(ctx, "CompositeInitializer.Run")
63+
func (e CompositeInitializer) Run(ctx context.Context, mappings []archive.IDMapping) (_ csapi.WorkspaceInitSource, err error) {
64+
span, ctx := opentracing.StartSpanFromContext(ctx, "CompositeInitializer.Run")
65+
defer tracing.FinishSpan(span, &err)
6566
for _, init := range e {
6667
_, err := init.Run(ctx, mappings)
6768
if err != nil {

‎components/content-service/pkg/initializer/prebuild.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ func runGitInit(ctx context.Context, gInit *GitInitializer) (err error) {
112112
tracelog.String("IsWorkingCopy", fmt.Sprintf("%v", git.IsWorkingCopy(gInit.Location))),
113113
tracelog.String("location", fmt.Sprintf("%v", gInit.Location)),
114114
)
115+
defer tracing.FinishSpan(span, &err)
115116
if git.IsWorkingCopy(gInit.Location) {
116117
out, err := gInit.GitWithOutput(ctx, "stash", "push", "-u")
117118
if err != nil {

‎components/image-builder-api/typescript/src/sugar.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,15 @@ export class PromisifiedImageBuilderClient {
237237

238238
const stream = this.client.logs(request, withTracing({ span }));
239239
return new Promise<void>((resolve, reject) => {
240-
stream.on('end', () => resolve())
241-
stream.on('error', err => reject(err));
240+
stream.on('end', () => {
241+
span.finish();
242+
resolve()
243+
})
244+
stream.on('error', err => {
245+
TraceContext.setError({ span }, err);
246+
span.finish();
247+
reject(err)
248+
});
242249
stream.on('data', (resp: LogsResponse) => {
243250
if (cb(new TextDecoder("utf-8").decode(resp.getContent_asU8())) === 'stop') {
244251
stream.cancel()

‎components/server/ee/src/monitoring-endpoint-ee.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ async function checkWorkspaceHealth(
6464
timestamp: Date.now(),
6565
};
6666
if (extra && numUnhealthy > 0) {
67+
span.finish();
6768
return {
6869
...returnValue,
6970
extra: {

‎components/server/ee/src/prebuilds/bitbucket-app.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,19 @@ export class BitbucketApp {
4444
// we should send a UNAUTHORIZED signal.
4545
res.statusCode = 401;
4646
res.send();
47+
span.finish();
4748
return;
4849
}
49-
const data = toData(req.body);
50-
if (data) {
51-
await this.handlePushHook({ span }, data, user);
50+
try {
51+
const data = toData(req.body);
52+
if (data) {
53+
await this.handlePushHook({ span }, data, user);
54+
}
55+
} catch (err) {
56+
TraceContext.setError({ span }, err);
57+
throw err;
58+
} finally {
59+
span.finish();
5260
}
5361
} else {
5462
console.warn(`Ignoring unsupported bitbucket event: ${req.header("X-Event-Key")}`);

‎components/server/ee/src/prebuilds/bitbucket-server-app.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,17 @@ export class BitbucketServerApp {
4545
// we should send a UNAUTHORIZED signal.
4646
res.statusCode = 401;
4747
res.send();
48+
span.finish();
4849
return;
4950
}
50-
await this.handlePushHook({ span }, user, payload);
51+
try {
52+
await this.handlePushHook({ span }, user, payload);
53+
} catch (err) {
54+
TraceContext.setError({ span }, err);
55+
throw err;
56+
} finally {
57+
span.finish();
58+
}
5159
} else {
5260
console.warn(`Ignoring unsupported BBS event.`, { headers: req.headers });
5361
}

‎components/server/ee/src/prebuilds/github-enterprise-app.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,24 @@ export class GitHubEnterpriseApp {
4545
try {
4646
user = await this.findUser({ span }, payload, req);
4747
} catch (error) {
48+
TraceContext.setError({ span }, error);
4849
log.error("Cannot find user.", error, {});
4950
}
5051
if (!user) {
5152
res.statusCode = 401;
5253
res.send();
54+
span.finish();
5355
return;
5456
}
55-
await this.handlePushHook({ span }, payload, user);
57+
58+
try {
59+
await this.handlePushHook({ span }, payload, user);
60+
} catch (err) {
61+
TraceContext.setError({ span }, err);
62+
throw err;
63+
} finally {
64+
span.finish();
65+
}
5666
} else {
5767
log.info("Unknown GitHub Enterprise event received", { event });
5868
}

‎components/server/ee/src/prebuilds/gitlab-app.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,23 @@ export class GitLabApp {
4343
try {
4444
user = await this.findUser({ span }, context, req);
4545
} catch (error) {
46+
TraceContext.setError({ span }, error);
4647
log.error("Cannot find user.", error, {});
4748
}
4849
if (!user) {
4950
res.statusCode = 503;
5051
res.send();
52+
span.finish();
5153
return;
5254
}
53-
await this.handlePushHook({ span }, context, user);
55+
try {
56+
await this.handlePushHook({ span }, context, user);
57+
} catch (err) {
58+
TraceContext.setError({ span }, err);
59+
throw err;
60+
} finally {
61+
span.finish();
62+
}
5463
} else {
5564
log.debug("Unknown GitLab event received", { event });
5665
}

‎components/server/ee/src/prebuilds/prebuild-manager.ts

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -297,27 +297,34 @@ export class PrebuildManager {
297297
) {
298298
const span = TraceContext.startSpan("storePrebuildInfo", ctx);
299299
const { userId, teamId, name: projectName, id: projectId } = project;
300-
await this.workspaceDB.trace({ span }).storePrebuildInfo({
301-
id: pws.id,
302-
buildWorkspaceId: pws.buildWorkspaceId,
303-
basedOnPrebuildId: ws.basedOnPrebuildId,
304-
teamId,
305-
userId,
306-
projectName,
307-
projectId,
308-
startedAt: pws.creationTime,
309-
startedBy: "", // TODO
310-
startedByAvatar: "", // TODO
311-
cloneUrl: pws.cloneURL,
312-
branch: pws.branch || "unknown",
313-
changeAuthor: commit.author,
314-
changeAuthorAvatar: commit.authorAvatarUrl,
315-
changeDate: commit.authorDate || "",
316-
changeHash: commit.sha,
317-
changeTitle: commit.commitMessage,
318-
// changePR
319-
changeUrl: ws.contextURL,
320-
});
300+
try {
301+
await this.workspaceDB.trace({ span }).storePrebuildInfo({
302+
id: pws.id,
303+
buildWorkspaceId: pws.buildWorkspaceId,
304+
basedOnPrebuildId: ws.basedOnPrebuildId,
305+
teamId,
306+
userId,
307+
projectName,
308+
projectId,
309+
startedAt: pws.creationTime,
310+
startedBy: "", // TODO
311+
startedByAvatar: "", // TODO
312+
cloneUrl: pws.cloneURL,
313+
branch: pws.branch || "unknown",
314+
changeAuthor: commit.author,
315+
changeAuthorAvatar: commit.authorAvatarUrl,
316+
changeDate: commit.authorDate || "",
317+
changeHash: commit.sha,
318+
changeTitle: commit.commitMessage,
319+
// changePR
320+
changeUrl: ws.contextURL,
321+
});
322+
} catch (err) {
323+
TraceContext.setError(ctx, err);
324+
throw err;
325+
} finally {
326+
span.finish();
327+
}
321328
}
322329

323330
private async shouldRateLimitPrebuild(span: opentracing.Span, cloneURL: string): Promise<boolean> {
@@ -367,6 +374,7 @@ export class PrebuildManager {
367374
const { inactivityPeriodForRepos } = this.config;
368375
if (!inactivityPeriodForRepos) {
369376
// skipping is disabled if `inactivityPeriodForRepos` is not set
377+
span.finish();
370378
return false;
371379
}
372380
try {
@@ -377,7 +385,10 @@ export class PrebuildManager {
377385
);
378386
} catch (error) {
379387
log.error("cannot compute activity for repository", { cloneURL }, error);
388+
TraceContext.setError(ctx, error);
380389
return false;
390+
} finally {
391+
span.finish();
381392
}
382393
}
383394
}

0 commit comments

Comments
 (0)