Fix listeners leaks – EXP-206 (#18321)

* [server] fix leaking prebuild update listeners

- check for client be defined in gitpod-server-impl
- add prebuild subscribers only if the thing is not disposed yet
   - this might have happened frequently on very short living workspacePageClose events

* [server] fix metric registration

don't override default registry, which breaks other metrics.

* fixup for getMetricsAsArray

* [server] bump prom-client

Author: AlexTugarev

components/gitpod-db/package.json

@@ -33,7 +33,7 @@
     "typeorm": "0.2.38",
     "ioredis": "^5.3.2",
     "ioredis-mock": "^8.7.0",
-    "prom-client": "^13.2.0"
+    "prom-client": "^14.2.0"
   },
   "devDependencies": {
     "@testdeck/mocha": "^0.3.3",

components/gitpod-protocol/package.json

@@ -60,7 +60,7 @@
         "nice-grpc-common": "^2.0.0",
         "opentracing": "^0.14.5",
         "parse-duration": "^1.0.3",
-        "prom-client": "^13.2.0",
+        "prom-client": "^14.2.0",
         "random-number-csprng": "^1.0.2",
         "react": "17.0.2",
         "react-dom": "17.0.2",

components/server/package.json

@@ -83,7 +83,7 @@
     "passport-gitlab2": "5.0.0",
     "passport-http": "^0.3.0",
     "probot": "12.1.1",
-    "prom-client": "^13.2.0",
+    "prom-client": "^14.2.0",
     "rate-limiter-flexible": "^2.3.6",
     "redlock": "^5.0.0-beta.2",
     "reflect-metadata": "^0.1.10",

components/server/src/monitoring-endpoints.ts

@@ -13,12 +13,16 @@ import { registerServerMetrics } from "./prometheus-metrics";
 @injectable()
 export class MonitoringEndpointsApp {
     public create(): express.Application {
-        let registry = prometheusClient.register;
+        const registry = prometheusClient.register;
 
         prometheusClient.collectDefaultMetrics({ register: registry });
         registerDBMetrics(registry);
         registerServerMetrics(registry);
-        registry = prometheusClient.Registry.merge([registry, redisMetricsRegistry()]);
+
+        // Append redis metrics to default registry
+        redisMetricsRegistry()
+            .getMetricsAsArray()
+            .forEach((metric) => registry.registerMetric(metric as any));
 
         const monApp = express();
         monApp.get("/metrics", async (req, res) => {

components/server/src/workspace/gitpod-server-impl.ts

@@ -308,11 +308,13 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         log.debug({ userId: this.userID }, "initializeClient");
 
         this.listenForWorkspaceInstanceUpdates();
-
         this.listenForPrebuildUpdates().catch((err) => log.error("error registering for prebuild updates", err));
     }
 
     private async listenForPrebuildUpdates() {
+        if (!this.client) {
+            return;
+        }
         // 'registering for prebuild updates for all projects this user has access to
         const projects = await this.getAccessibleProjects();
 
@@ -328,8 +330,10 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
                 ctx,
             );
 
-        for (const projectId of projects) {
-            this.disposables.pushAll([this.subscriber.listenForPrebuildUpdates(projectId, handler)]);
+        if (!this.disposables.disposed) {
+            for (const projectId of projects) {
+                this.disposables.push(this.subscriber.listenForPrebuildUpdates(projectId, handler));
+            }
         }
 
         // TODO(at) we need to keep the list of accessible project up to date
@@ -451,7 +455,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
                 // Once we have those, we should remove this.
                 //
                 const ws = await this.workspaceDb.trace(ctx).findById(workspaceID);
-                if (!!ws && !!wsi && ws.ownerId !== this.userID) {
+                const relatedPrebuildFound = !!ws && !!wsi && ws.ownerId !== this.userID;
+                if (relatedPrebuildFound && !this.disposables.disposed) {
                     const resetListenerFromRedis = this.subscriber.listenForWorkspaceInstanceUpdates(
                         ws.ownerId,
                         (ctx, instance) => {
@@ -2968,19 +2973,23 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
         }
 
         const project = await this.projectsService.createProject(params, user);
+
         // update client registration for the logged in user
-        const prebuildUpdateHandler = (ctx: TraceContext, update: PrebuildWithStatus) =>
-            TraceContext.withSpan(
-                "forwardPrebuildUpdateToClient",
-                (ctx) => {
-                    traceClientMetadata(ctx, this.clientMetadata);
-                    TraceContext.setJsonRPCMetadata(ctx, "onPrebuildUpdate");
+        if (this.client && !this.disposables.disposed) {
+            const prebuildUpdateHandler = (ctx: TraceContext, update: PrebuildWithStatus) =>
+                TraceContext.withSpan(
+                    "forwardPrebuildUpdateToClient",
+                    (ctx) => {
+                        traceClientMetadata(ctx, this.clientMetadata);
+                        TraceContext.setJsonRPCMetadata(ctx, "onPrebuildUpdate");
+
+                        this.client?.onPrebuildUpdate(update);
+                    },
+                    ctx,
+                );
 
-                    this.client?.onPrebuildUpdate(update);
-                },
-                ctx,
-            );
-        this.disposables.pushAll([this.subscriber.listenForPrebuildUpdates(project.id, prebuildUpdateHandler)]);
+            this.disposables.pushAll([this.subscriber.listenForPrebuildUpdates(project.id, prebuildUpdateHandler)]);
+        }
 
         return project;
     }

components/ws-manager-bridge/package.json

@@ -31,7 +31,7 @@
     "express": "^4.17.3",
     "inversify": "^6.0.1",
     "ioredis": "^5.3.2",
-    "prom-client": "^13.2.0",
+    "prom-client": "^14.2.0",
     "reflect-metadata": "^0.1.13"
   },
   "devDependencies": {

yarn.lock

@@ -13609,7 +13609,14 @@ progress@^2.0.0:
   resolved "https://registry.npmjs.org/progress/-/progress-2.0.3.tgz"
   integrity sha512-7PiHtLll5LdnKIMw100I+8xJXR5gW2QwWYkT6iJva0bXitZKa/XMrSbdmg3r2Xnaidz9Qumd0VPaMrZlF9V9sA==
 
-prom-client@^13.2.0, "prom-client@~11.3.0 || ^12.0.0 || ^13.0.0":
+prom-client@^14.2.0:
+  version "14.2.0"
+  resolved "https://registry.yarnpkg.com/prom-client/-/prom-client-14.2.0.tgz#ca94504e64156f6506574c25fb1c34df7812cf11"
+  integrity sha512-sF308EhTenb/pDRPakm+WgiN+VdM/T1RaHj1x+MvAuT8UiQP8JmOEbxVqtkbfR4LrvOg5n7ic01kRBDGXjYikA==
+  dependencies:
+    tdigest "^0.1.1"
+
+"prom-client@~11.3.0 || ^12.0.0 || ^13.0.0":
   version "13.2.0"
   resolved "https://registry.npmjs.org/prom-client/-/prom-client-13.2.0.tgz"
   integrity sha512-wGr5mlNNdRNzEhRYXgboUU2LxHWIojxscJKmtG3R8f4/KiWqyYgXTLHs0+Ted7tG3zFT7pgHJbtomzZ1L0ARaQ==