-
Notifications
You must be signed in to change notification settings - Fork 20
chore: add additional logging around resource and memory useage #465
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4617974
e5ae295
1e56a3f
f201d1a
d7c650b
f3bcbca
5ebc004
854ca01
8974e8b
9b440dc
1eed376
d6fb26c
2c09de6
3c3f452
44106ee
9659dd8
86de75d
5fbc7a7
b839ad5
c8e45da
90172e6
0d04fba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import * as vscode from "vscode" | |
import { WebSocket } from "ws" | ||
import { coderSessionTokenHeader } from "./api" | ||
import { errToStr } from "./api-helper" | ||
import { getMemoryLogger } from "./memoryLogger" | ||
import { type Storage } from "./storage" | ||
|
||
// These are the template IDs of our notifications. | ||
|
@@ -17,9 +18,17 @@ export class Inbox implements vscode.Disposable { | |
readonly #storage: Storage | ||
#disposed = false | ||
#socket: WebSocket | ||
#messageCount = 0 | ||
#workspaceId: string | ||
#memoryInterval: NodeJS.Timeout | ||
|
||
constructor(workspace: Workspace, httpAgent: ProxyAgent, restClient: Api, storage: Storage) { | ||
const logger = getMemoryLogger() | ||
this.#storage = storage | ||
this.#workspaceId = workspace.id | ||
|
||
logger.trackResourceCreated("InboxWebSocket", workspace.id) | ||
logger.info(`Creating inbox for workspace: ${workspace.owner_name}/${workspace.name} (${workspace.id})`) | ||
|
||
const baseUrlRaw = restClient.getAxiosInstance().defaults.baseURL | ||
if (!baseUrlRaw) { | ||
|
@@ -38,6 +47,8 @@ export class Inbox implements vscode.Disposable { | |
const socketProto = baseUrl.protocol === "https:" ? "wss:" : "ws:" | ||
const socketUrl = `${socketProto}//${baseUrl.host}/api/v2/notifications/inbox/watch?format=plaintext&templates=${watchTemplatesParam}&targets=${watchTargetsParam}` | ||
|
||
logger.debug(`Connecting to inbox WebSocket at: ${socketUrl}`) | ||
|
||
const token = restClient.getAxiosInstance().defaults.headers.common[coderSessionTokenHeader] as string | undefined | ||
this.#socket = new WebSocket(new URL(socketUrl), { | ||
agent: httpAgent, | ||
|
@@ -50,35 +61,74 @@ export class Inbox implements vscode.Disposable { | |
}) | ||
|
||
this.#socket.on("open", () => { | ||
logger.info(`Inbox WebSocket connection opened for workspace: ${workspace.id}`) | ||
this.#storage.writeToCoderOutputChannel("Listening to Coder Inbox") | ||
}) | ||
|
||
this.#socket.on("error", (error) => { | ||
logger.error(`Inbox WebSocket error for workspace: ${workspace.id}`, error) | ||
this.notifyError(error) | ||
this.dispose() | ||
}) | ||
|
||
this.#socket.on("close", (code, reason) => { | ||
logger.info(`Inbox WebSocket closed for workspace: ${workspace.id}, code: ${code}, reason: ${reason || "none"}`) | ||
if (!this.#disposed) { | ||
this.dispose() | ||
} | ||
}) | ||
|
||
this.#socket.on("message", (data) => { | ||
this.#messageCount++ | ||
|
||
// Log periodic message stats | ||
if (this.#messageCount % 10 === 0) { | ||
logger.info(`Inbox received ${this.#messageCount} messages for workspace: ${workspace.id}`) | ||
logger.logMemoryUsage("INBOX_WEBSOCKET") | ||
} | ||
Comment on lines
+84
to
+88
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want a little more granularity here? The current setup means that we have to wait for the first 10 messages before we log anything. I don't know how quickly the messages are coming in, but I'm worried we'll never log anything. Wondering if we could have something more logarithmic, where maybe we could have:
|
||
|
||
try { | ||
const inboxMessage = JSON.parse(data.toString()) as GetInboxNotificationResponse | ||
|
||
logger.debug(`Inbox notification received: ${inboxMessage.notification.title}`) | ||
vscode.window.showInformationMessage(inboxMessage.notification.title) | ||
} catch (error) { | ||
logger.error(`Error processing inbox message for workspace: ${workspace.id}`, error) | ||
this.notifyError(error) | ||
} | ||
}) | ||
|
||
// Log memory stats periodically | ||
this.#memoryInterval = setInterval( | ||
() => { | ||
if (!this.#disposed) { | ||
logger.logMemoryUsage("INBOX_PERIODIC") | ||
} else { | ||
clearInterval(this.#memoryInterval) | ||
} | ||
}, | ||
5 * 60 * 1000, | ||
) // Every 5 minutes | ||
} | ||
|
||
dispose() { | ||
brettkolodny marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const logger = getMemoryLogger() | ||
|
||
if (!this.#disposed) { | ||
logger.info(`Disposing inbox for workspace: ${this.#workspaceId} after ${this.#messageCount} messages`) | ||
this.#storage.writeToCoderOutputChannel("No longer listening to Coder Inbox") | ||
this.#socket.close() | ||
this.#disposed = true | ||
logger.trackResourceDisposed("InboxWebSocket", this.#workspaceId) | ||
} | ||
|
||
clearInterval(this.#memoryInterval) | ||
} | ||
|
||
private notifyError(error: unknown) { | ||
const logger = getMemoryLogger() | ||
const message = errToStr(error, "Got empty error while monitoring Coder Inbox") | ||
|
||
logger.error(`Inbox error for workspace: ${this.#workspaceId}`, error) | ||
this.#storage.writeToCoderOutputChannel(message) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be better to refactor this class to take a single init object. That way, we can dependency-inject the logger, and also make it easier to add new parameters down the line. Moving the logger directly to the constructor parameters would give us a five-argument function, which feels a bit unwieldy