Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ class WebSocketConnectionFactory extends Disposable implements IRemoteAgentHostC
address,
entry.connectionToken,
ahpLoggingEnabled
? { logsHome: this._environmentService.logsHome, connectionId: address, transport: 'websocket' }
? { logsHome: this._environmentService.logsHome, logId: address, connectionId: address, transport: 'websocket' }
: undefined,
);
const connection = this._instantiationService.createInstance(AgentHostProtocolClient, address, transportFactory, { clientInfo: this._clientInfo() });
Expand Down
22 changes: 19 additions & 3 deletions src/vs/platform/agentHost/common/ahpJsonlLogger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { VSBuffer } from '../../../base/common/buffer.js';
import { StringSHA1 } from '../../../base/common/hash.js';
import { Disposable } from '../../../base/common/lifecycle.js';
import { MarshalledId } from '../../../base/common/marshallingIds.js';
import { joinPath } from '../../../base/common/resources.js';
Expand All @@ -25,13 +26,17 @@ interface IAhpLogMeta {

export interface IAhpJsonlLoggerOptions {
readonly logsHome: URI;
/** Stable identity shared by every transport connection to the same logical host. */
readonly logId: string;
readonly connectionId: string;
readonly transport: string;
readonly maxFileSizeBytes?: number;
readonly maxFiles?: number;
}

const AHP_LOG_DIR = 'ahp';
const AHP_LOG_FILE_PREFIX = 'ahp';
const AHP_LOG_FILE_EXTENSION = '.jsonl';
const DEFAULT_MAX_FILE_SIZE_BYTES = 75 * 1024 * 1024;
const DEFAULT_MAX_FILES = 5;
// Cap the size of any single coalesced writeFile to avoid producing huge
Expand Down Expand Up @@ -76,7 +81,7 @@ export class AhpJsonlLogger extends Disposable {
this._directory = joinPath(this._options.logsHome, AHP_LOG_DIR);
// Truncate connectionId to avoid filesystem filename length limits (e.g. 255 on ext4/APFS)
const safeConnectionId = sanitizeFilePart(this._options.connectionId).slice(0, 64);
this._baseName = `ahp-${toFileTimestamp(new Date())}-${safeConnectionId}.jsonl`;
this._baseName = `${getAhpLogFilePrefix(this._options.logId)}${toFileTimestamp(new Date())}-${safeConnectionId}${AHP_LOG_FILE_EXTENSION}`;
this._maxFileSizeBytes = this._options.maxFileSizeBytes ?? DEFAULT_MAX_FILE_SIZE_BYTES;
this._maxFiles = this._options.maxFiles ?? DEFAULT_MAX_FILES;
this._currentFile = joinPath(this._directory, this._baseName);
Expand Down Expand Up @@ -195,8 +200,8 @@ export class AhpJsonlLogger extends Disposable {
if (segment === 0) {
return joinPath(this._directory, this._baseName);
}
const currentBaseName = this._baseName.slice(0, -'.jsonl'.length);
return joinPath(this._directory, `${currentBaseName}.${segment}.jsonl`);
const currentBaseName = this._baseName.slice(0, -AHP_LOG_FILE_EXTENSION.length);
return joinPath(this._directory, `${currentBaseName}.${segment}${AHP_LOG_FILE_EXTENSION}`);
}

private async _getFileSize(resource: URI): Promise<number> {
Expand All @@ -212,6 +217,11 @@ export function getAhpLogByteLength(text: string): number {
return VSBuffer.fromString(text).byteLength;
}

/** Tests whether a JSONL filename belongs to the given logical Agent Host connection. */
export function isAhpLogFileFor(logId: string, name: string): boolean {
return name.startsWith(getAhpLogFilePrefix(logId)) && name.endsWith(AHP_LOG_FILE_EXTENSION);
}

export function stringifyAhpLogEntry(value: unknown): string {
return JSON.stringify(value, _ahpReplacer);
}
Expand Down Expand Up @@ -256,6 +266,12 @@ function toFileTimestamp(date: Date): string {
return date.toISOString().replace(/[:.]/g, '-');
}

function getAhpLogFilePrefix(logId: string): string {
const hash = new StringSHA1();
hash.update(logId);
return `${AHP_LOG_FILE_PREFIX}-${hash.digest()}-`;
}

function sanitizeFilePart(value: string): string {
return value.replace(/[\\/:\*\?"<>\|\s]+/g, '-').replace(/^-+|-+$/g, '') || 'connection';
}
4 changes: 2 additions & 2 deletions src/vs/platform/agentHost/common/relayTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export class ReconnectingRelayTransport extends ReconnectingTransport {
constructor(
establish: () => Promise<IRelayConnectionHandle>,
channel: IRelayChannel,
createAhpLogger: () => AhpJsonlLogger | undefined,
createAhpLogger: (connectionId: string) => AhpJsonlLogger | undefined,
logService: ILogService,
logPrefix: string,
clientConnectionKind: AgentHostClientConnectionKind,
Expand All @@ -153,7 +153,7 @@ export class ReconnectingRelayTransport extends ReconnectingTransport {
// is what owns and disposes it, so a logger built before `establish`
// resolves would be leaked on every failed attempt.
return {
transport: new RelayTransport(connectionHandle.connectionId, channel, createAhpLogger(), logService, logPrefix, clientConnectionKind),
transport: new RelayTransport(connectionHandle.connectionId, channel, createAhpLogger(connectionHandle.connectionId), logService, logPrefix, clientConnectionKind),
close: connectionHandle.close,
};
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ export class LocalAgentHostServiceClient extends Disposable implements IAgentHos
this._ahpLogger = this._configurationService.getValue<boolean>(AgentHostAhpJsonlLoggingSettingId)
? this._register(this._instantiationService.createInstance(AhpJsonlLogger, {
logsHome: environmentService.logsHome,
logId: this.clientId,
connectionId: this.clientId,
transport: 'local',
}))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,9 @@ export class SSHRelayClientFactory implements ISSHRelayClientFactory {
}
};
return this._instantiationService.createInstance(AgentHostProtocolClient, address, () => {
// Logged under the seed channel id: the re-established id is not known
// until `establish()` resolves, after the logger has to exist.
const createLogger = () => ahpLoggingEnabled ? this._instantiationService.createInstance(
const createLogger = (activeConnectionId: string) => ahpLoggingEnabled ? this._instantiationService.createInstance(
AhpJsonlLogger,
{ logsHome: this._environmentService.logsHome, connectionId, transport: 'ssh' },
{ logsHome: this._environmentService.logsHome, logId: address, connectionId: activeConnectionId, transport: 'ssh' },
) : undefined;
return new ReconnectingRelayTransport(
establish,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ export class WSLRelayClientFactory implements IWSLRelayClientFactory {
};
const transportFactory = () => {
const ahpLoggingEnabled = !!this._configurationService.getValue<boolean>(AgentHostAhpJsonlLoggingSettingId);
const createLogger = () => ahpLoggingEnabled ? this._instantiationService.createInstance(
const createLogger = (activeConnectionId: string) => ahpLoggingEnabled ? this._instantiationService.createInstance(
AhpJsonlLogger,
{ logsHome: this._environmentService.logsHome, connectionId, transport: 'wsl' },
{ logsHome: this._environmentService.logsHome, logId: address, connectionId: activeConnectionId, transport: 'wsl' },
) : undefined;
return this._instantiationService.createInstance(
ReconnectingRelayTransport,
Expand Down
4 changes: 3 additions & 1 deletion src/vs/platform/agentHost/node/webSocketTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,13 @@ export class WebSocketProtocolServer extends Disposable implements IProtocolServ
if (!this._ahpLogOptions) {
return undefined;
}
const connectionId = `agent-host-${++this._connectionCount}-${generateUuid()}`;
return this._ahpLogOptions.instantiationService.createInstance(
AhpJsonlLogger,
{
logsHome: this._ahpLogOptions.logsHome,
connectionId: `agent-host-${++this._connectionCount}-${generateUuid()}`,
logId: connectionId,
connectionId,
transport: 'websocket',
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ suite('AgentHostIpcChannelTransport', () => {
const fileService = ds.add(new FileService(new NullLogService()));
ds.add(fileService.registerProvider('file', ds.add(new InMemoryFileSystemProvider())));
const logger = ds.add(new AhpJsonlLogger(
{ logsHome: URI.file('/logs'), connectionId: 'local-client', transport: 'local' },
{ logsHome: URI.file('/logs'), logId: 'local-client', connectionId: 'local-client', transport: 'local' },
fileService,
new NullLogService(),
));
Expand Down
15 changes: 9 additions & 6 deletions src/vs/platform/agentHost/test/common/ahpJsonlLogger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { FileService } from '../../../files/common/fileService.js';
import { IFileWriteOptions } from '../../../files/common/files.js';
import { InMemoryFileSystemProvider } from '../../../files/common/inMemoryFilesystemProvider.js';
import { NullLogService } from '../../../log/common/log.js';
import { AhpJsonlLogger, getAhpLogByteLength, stringifyAhpLogEntry } from '../../common/ahpJsonlLogger.js';
import { AhpJsonlLogger, getAhpLogByteLength, isAhpLogFileFor, stringifyAhpLogEntry } from '../../common/ahpJsonlLogger.js';

suite('AhpJsonlLogger', () => {

Expand All @@ -22,7 +22,7 @@ suite('AhpJsonlLogger', () => {
store.add(fileService.registerProvider('file', store.add(new InMemoryFileSystemProvider())));

const logger = store.add(new AhpJsonlLogger(
{ logsHome: URI.file('/logs'), connectionId: 'conn:1', transport: 'websocket' },
{ logsHome: URI.file('/logs'), logId: 'logical-host', connectionId: 'conn:1', transport: 'websocket' },
fileService,
new NullLogService(),
));
Expand Down Expand Up @@ -106,6 +106,7 @@ suite('AhpJsonlLogger', () => {
},
},
]);
assert.strictEqual(isAhpLogFileFor('logical-host', basename(logger.resource)), true);

for (const entry of parsed) {
assert.strictEqual(entry.jsonrpc, '2.0');
Expand All @@ -118,7 +119,7 @@ suite('AhpJsonlLogger', () => {
store.add(fileService.registerProvider('file', store.add(new InMemoryFileSystemProvider())));

const logger = store.add(new AhpJsonlLogger(
{ logsHome: URI.file('/logs'), connectionId: 'rotating', transport: 'websocket', maxFileSizeBytes: 1, maxFiles: 2 },
{ logsHome: URI.file('/logs'), logId: 'logical-host', connectionId: 'rotating', transport: 'websocket', maxFileSizeBytes: 1, maxFiles: 2 },
fileService,
new NullLogService(),
));
Expand All @@ -141,10 +142,12 @@ suite('AhpJsonlLogger', () => {
assert.deepStrictEqual({
firstFileExists: await fileService.exists(firstResource),
ids: parsed.map(entry => entry.id),
segmentsMatchLogicalHost: [rotated1, rotated2].every(resource => isAhpLogFileFor('logical-host', basename(resource))),
rootsAreJsonRpc: parsed.every(entry => entry.jsonrpc === '2.0' && (entry.method !== undefined || (entry.id !== undefined && (Object.hasOwn(entry, 'result') || Object.hasOwn(entry, 'error'))))),
}, {
firstFileExists: false,
ids: [2, 3],
segmentsMatchLogicalHost: true,
rootsAreJsonRpc: true,
});
});
Expand All @@ -155,7 +158,7 @@ suite('AhpJsonlLogger', () => {
store.add(fileService.registerProvider('file', provider));

const logger = store.add(new AhpJsonlLogger(
{ logsHome: URI.file('/logs'), connectionId: 'batched', transport: 'websocket' },
{ logsHome: URI.file('/logs'), logId: 'batched', connectionId: 'batched', transport: 'websocket' },
fileService,
new NullLogService(),
));
Expand Down Expand Up @@ -188,7 +191,7 @@ suite('AhpJsonlLogger', () => {
store.add(fileService.registerProvider('file', store.add(new InMemoryFileSystemProvider())));

const logger = store.add(new AhpJsonlLogger(
{ logsHome: URI.file('/logs'), connectionId: 'flush-order', transport: 'websocket' },
{ logsHome: URI.file('/logs'), logId: 'flush-order', connectionId: 'flush-order', transport: 'websocket' },
fileService,
new NullLogService(),
));
Expand All @@ -213,7 +216,7 @@ suite('AhpJsonlLogger', () => {
store.add(fileService.registerProvider('file', store.add(new InMemoryFileSystemProvider())));

const logger = store.add(new AhpJsonlLogger(
{ logsHome: URI.file('/logs'), connectionId: 'conn:1', transport: 'websocket' },
{ logsHome: URI.file('/logs'), logId: 'logical-host', connectionId: 'conn:1', transport: 'websocket' },
fileService,
new NullLogService(),
));
Expand Down
23 changes: 23 additions & 0 deletions src/vs/platform/agentHost/test/common/relayTransport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,29 @@ suite('ReconnectingRelayTransport', () => {
assert.deepStrictEqual(received, [{ id: 'connected' }]);
});

test('creates each reconnect logger with the established connectionId', async () => {
const loggerConnectionIds: string[] = [];
const createTransport = (connectionId: string) => disposables.add(new ReconnectingRelayTransport(
async () => ({ connectionId }),
mockChannel,
activeConnectionId => {
loggerConnectionIds.push(activeConnectionId);
return undefined;
},
new NullLogService(),
'[ReconnectingRelayTransport]',
AgentHostClientConnectionKind.DevTunnel
));

const initial = createTransport('relay-1');
await initial.connect();
initial.dispose();
const reconnected = createTransport('relay-2');
await reconnected.connect();

assert.deepStrictEqual(loggerConnectionIds, ['relay-1', 'relay-2']);
});

test('warns and drops messages sent before adopting a channel', () => {
const logService = new RecordingLogService();
const transport = disposables.add(new ReconnectingRelayTransport(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ class CloudSandboxConnectionFactory extends Disposable implements IRemoteAgentHo
ahpLogger: ahpLoggingEnabled
? this._instantiationService.createInstance(AhpJsonlLogger, {
logsHome: this._environmentService.logsHome,
logId: address,
connectionId: staged.clientId,
transport: 'webpubsub',
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,11 @@ export class DevContainerAgentHostConnector implements IDevContainerAgentHostCon
}
};
const transportFactory = () => {
// Post-reconnect logs use the original channel id because the new id is assigned asynchronously by `establish`.
const createLogger = () => this._configurationService.getValue<boolean>(AgentHostAhpJsonlLoggingSettingId)
const createLogger = (activeConnectionId: string) => this._configurationService.getValue<boolean>(AgentHostAhpJsonlLoggingSettingId)
? this._instantiationService.createInstance(AhpJsonlLogger, {
logsHome: this._environmentService.logsHome,
connectionId,
logId: address,
connectionId: activeConnectionId,
transport: 'devcontainer',
})
: undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ export class TunnelAgentHostService extends Disposable implements ITunnelAgentHo
// Bind the narrowed connection before the closure: TypeScript does not
// carry the discriminant narrowing into the `find` callback below.
const connection = entry.connection;
const address = getEntryAddress(entry);
const cachedTunnel = this._storage.getCachedTunnels().find(cached => cached.tunnelId === connection.tunnelId);
const tunnel: ITunnelInfo = {
tunnelId: connection.tunnelId,
Expand Down Expand Up @@ -307,9 +308,9 @@ export class TunnelAgentHostService extends Disposable implements ITunnelAgentHo
() => new ReconnectingRelayTransport(
establish,
this._mainService,
() => ahpLoggingEnabled ? this._instantiationService.createInstance(
activeConnectionId => ahpLoggingEnabled ? this._instantiationService.createInstance(
AhpJsonlLogger,
{ logsHome: this._environmentService.logsHome, connectionId: result.connectionId, transport: 'tunnel' },
{ logsHome: this._environmentService.logsHome, logId: address, connectionId: activeConnectionId, transport: 'tunnel' },
) : undefined,
this._logService,
LOG_PREFIX,
Expand Down
Loading