add trace logging to NativePolicyService

related to #163418
microsoft · Nov 16, 2022 · dbd70fc · dbd70fc
1 parent 8df188a
commit dbd70fc
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 31 deletions.
diff --git a/src/vs/code/electron-main/main.ts b/src/vs/code/electron-main/main.ts
@@ -188,7 +188,7 @@ class CodeMain {
 		services.set(IUserDataProfilesMainService, userDataProfilesMainService);
 
 		// Policy
-		const policyService = isWindows && productService.win32RegValueName ? disposables.add(new NativePolicyService(productService.win32RegValueName))
+		const policyService = isWindows && productService.win32RegValueName ? disposables.add(new NativePolicyService(logService, productService.win32RegValueName))
 			: environmentMainService.policyFile ? disposables.add(new FilePolicyService(environmentMainService.policyFile, fileService, logService))
 				: new NullPolicyService();
 		services.set(IPolicyService, policyService);

diff --git a/src/vs/code/node/cliProcessMain.ts b/src/vs/code/node/cliProcessMain.ts
@@ -153,7 +153,7 @@ class CliMain extends Disposable {
 		services.set(IUserDataProfilesService, userDataProfilesService);
 
 		// Policy
-		const policyService = isWindows && productService.win32RegValueName ? this._register(new NativePolicyService(productService.win32RegValueName))
+		const policyService = isWindows && productService.win32RegValueName ? this._register(new NativePolicyService(logService, productService.win32RegValueName))
 			: environmentService.policyFile ? this._register(new FilePolicyService(environmentService.policyFile, fileService, logService))
 				: new NullPolicyService();
 		services.set(IPolicyService, policyService);

diff --git a/src/vs/platform/configuration/common/configurations.ts b/src/vs/platform/configuration/common/configurations.ts
@@ -107,13 +107,13 @@ export class PolicyConfiguration extends Disposable implements IPolicyConfigurat
 	}
 
 	async initialize(): Promise<ConfigurationModel> {
-		this.update(await this.registerPolicyDefinitions(this.defaultConfiguration.configurationModel.keys), false);
+		this.update(await this.updatePolicyDefinitions(this.defaultConfiguration.configurationModel.keys), false);
 		this._register(this.policyService.onDidChange(policyNames => this.onDidChangePolicies(policyNames)));
-		this._register(this.defaultConfiguration.onDidChangeConfiguration(async ({ properties }) => this.update(await this.registerPolicyDefinitions(properties), true)));
+		this._register(this.defaultConfiguration.onDidChangeConfiguration(async ({ properties }) => this.update(await this.updatePolicyDefinitions(properties), true)));
 		return this._configurationModel;
 	}
 
-	private async registerPolicyDefinitions(properties: string[]): Promise<string[]> {
+	private async updatePolicyDefinitions(properties: string[]): Promise<string[]> {
 		const policyDefinitions: IStringDictionary<PolicyDefinition> = {};
 		const keys: string[] = [];
 		const configurationProperties = Registry.as<IConfigurationRegistry>(Extensions.Configuration).getConfigurationProperties();
@@ -136,7 +136,7 @@ export class PolicyConfiguration extends Disposable implements IPolicyConfigurat
 		}
 
 		if (!isEmptyObject(policyDefinitions)) {
-			await this.policyService.registerPolicyDefinitions(policyDefinitions);
+			await this.policyService.updatePolicyDefinitions(policyDefinitions);
 		}
 
 		return keys;

diff --git a/src/vs/platform/policy/common/filePolicyService.ts b/src/vs/platform/policy/common/filePolicyService.ts
@@ -4,14 +4,13 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { ThrottledDelayer } from 'vs/base/common/async';
-import { IStringDictionary } from 'vs/base/common/collections';
 import { Event } from 'vs/base/common/event';
 import { Iterable } from 'vs/base/common/iterator';
 import { isObject } from 'vs/base/common/types';
 import { URI } from 'vs/base/common/uri';
 import { FileOperationError, FileOperationResult, IFileService } from 'vs/platform/files/common/files';
 import { ILogService } from 'vs/platform/log/common/log';
-import { AbstractPolicyService, IPolicyService, PolicyDefinition, PolicyName, PolicyValue } from 'vs/platform/policy/common/policy';
+import { AbstractPolicyService, IPolicyService, PolicyName, PolicyValue } from 'vs/platform/policy/common/policy';
 
 function keysDiff<T>(a: Map<string, T>, b: Map<string, T>): string[] {
 	const result: string[] = [];
@@ -41,7 +40,7 @@ export class FilePolicyService extends AbstractPolicyService implements IPolicyS
 		this._register(onDidChangePolicyFile(() => this.throttledDelayer.trigger(() => this.refresh())));
 	}
 
-	protected async initializePolicies(policyDefinitions: IStringDictionary<PolicyDefinition>): Promise<void> {
+	protected async _updatePolicyDefinitions(): Promise<void> {
 		await this.refresh();
 	}
 

diff --git a/src/vs/platform/policy/common/policy.ts b/src/vs/platform/policy/common/policy.ts
@@ -19,7 +19,7 @@ export interface IPolicyService {
 	readonly _serviceBrand: undefined;
 
 	readonly onDidChange: Event<readonly PolicyName[]>;
-	registerPolicyDefinitions(policyDefinitions: IStringDictionary<PolicyDefinition>): Promise<IStringDictionary<PolicyValue>>;
+	updatePolicyDefinitions(policyDefinitions: IStringDictionary<PolicyDefinition>): Promise<IStringDictionary<PolicyValue>>;
 	getPolicyValue(name: PolicyName): PolicyValue | undefined;
 	serialize(): IStringDictionary<{ definition: PolicyDefinition; value: PolicyValue }> | undefined;
 }
@@ -33,12 +33,12 @@ export abstract class AbstractPolicyService extends Disposable implements IPolic
 	protected readonly _onDidChange = this._register(new Emitter<readonly PolicyName[]>());
 	readonly onDidChange = this._onDidChange.event;
 
-	async registerPolicyDefinitions(policyDefinitions: IStringDictionary<PolicyDefinition>): Promise<IStringDictionary<PolicyValue>> {
+	async updatePolicyDefinitions(policyDefinitions: IStringDictionary<PolicyDefinition>): Promise<IStringDictionary<PolicyValue>> {
 		const size = Object.keys(this.policyDefinitions).length;
 		this.policyDefinitions = { ...policyDefinitions, ...this.policyDefinitions };
 
 		if (size !== Object.keys(this.policyDefinitions).length) {
-			await this.initializePolicies(policyDefinitions);
+			await this._updatePolicyDefinitions(policyDefinitions);
 		}
 
 		return Iterable.reduce(this.policies.entries(), (r, [name, value]) => ({ ...r, [name]: value }), {});
@@ -52,13 +52,13 @@ export abstract class AbstractPolicyService extends Disposable implements IPolic
 		return Iterable.reduce<[PolicyName, PolicyDefinition], IStringDictionary<{ definition: PolicyDefinition; value: PolicyValue }>>(Object.entries(this.policyDefinitions), (r, [name, definition]) => ({ ...r, [name]: { definition, value: this.policies.get(name)! } }), {});
 	}
 
-	protected abstract initializePolicies(policyDefinitions: IStringDictionary<PolicyDefinition>): Promise<void>;
+	protected abstract _updatePolicyDefinitions(policyDefinitions: IStringDictionary<PolicyDefinition>): Promise<void>;
 }
 
 export class NullPolicyService implements IPolicyService {
 	readonly _serviceBrand: undefined;
 	readonly onDidChange = Event.None;
-	async registerPolicyDefinitions() { return {}; }
+	async updatePolicyDefinitions() { return {}; }
 	getPolicyValue() { return undefined; }
 	serialize() { return undefined; }
 }
diff --git a/src/vs/platform/policy/common/policyIpc.ts b/src/vs/platform/policy/common/policyIpc.ts
@@ -29,7 +29,7 @@ export class PolicyChannel implements IServerChannel {
 
 	call(_: unknown, command: string, arg?: any): Promise<any> {
 		switch (command) {
-			case 'registerPolicyDefinitions': return this.service.registerPolicyDefinitions(arg as IStringDictionary<PolicyDefinition>);
+			case 'updatePolicyDefinitions': return this.service.updatePolicyDefinitions(arg as IStringDictionary<PolicyDefinition>);
 		}
 
 		throw new Error(`Call not found: ${command}`);
@@ -66,8 +66,8 @@ export class PolicyChannelClient extends AbstractPolicyService implements IPolic
 		});
 	}
 
-	protected async initializePolicies(policyDefinitions: IStringDictionary<PolicyDefinition>): Promise<void> {
-		const result = await this.channel.call<{ [name: PolicyName]: PolicyValue }>('registerPolicyDefinitions', policyDefinitions);
+	protected async _updatePolicyDefinitions(policyDefinitions: IStringDictionary<PolicyDefinition>): Promise<void> {
+		const result = await this.channel.call<{ [name: PolicyName]: PolicyValue }>('updatePolicyDefinitions', policyDefinitions);
 		for (const name in result) {
 			this.policies.set(name, result[name]);
 		}

diff --git a/src/vs/platform/policy/node/nativePolicyService.ts b/src/vs/platform/policy/node/nativePolicyService.ts
@@ -6,39 +6,51 @@
 import { AbstractPolicyService, IPolicyService, PolicyDefinition } from 'vs/platform/policy/common/policy';
 import { IStringDictionary } from 'vs/base/common/collections';
 import { Throttler } from 'vs/base/common/async';
-import { createWatcher, Watcher } from 'vscode-policy-watcher';
+import { createWatcher, PolicyUpdate, Watcher } from 'vscode-policy-watcher';
 import { MutableDisposable } from 'vs/base/common/lifecycle';
+import { ILogService } from 'vs/platform/log/common/log';
 
 export class NativePolicyService extends AbstractPolicyService implements IPolicyService {
 
 	private throttler = new Throttler();
 	private watcher = this._register(new MutableDisposable<Watcher>());
 
-	constructor(private readonly productName: string) {
+	constructor(
+		@ILogService private readonly logService: ILogService,
+		private readonly productName: string
+	) {
 		super();
 	}
 
-	protected async initializePolicies(policyDefinitions: IStringDictionary<PolicyDefinition>): Promise<void> {
+	protected async _updatePolicyDefinitions(policyDefinitions: IStringDictionary<PolicyDefinition>): Promise<void> {
+		this.logService.trace(`NativePolicyService#_updatePolicyDefinitions - Found ${policyDefinitions.length} policy definitions`);
+
 		await this.throttler.queue(() => new Promise<void>((c, e) => {
 			try {
 				this.watcher.value = createWatcher(this.productName, policyDefinitions, update => {
-					for (const key in update) {
-						const value = update[key] as any;
-
-						if (value === undefined) {
-							this.policies.delete(key);
-						} else {
-							this.policies.set(key, value);
-						}
-					}
-
-					this._onDidChange.fire(Object.keys(update));
+					this._onDidPolicyChange(update);
 					c();
 				});
 			} catch (err) {
+				this.logService.error(`NativePolicyService#_updatePolicyDefinitions - Error creating watcher:`, err);
 				e(err);
 			}
 		}));
 	}
 
+	private _onDidPolicyChange(update: PolicyUpdate<IStringDictionary<PolicyDefinition>>): void {
+		this.logService.trace(`NativePolicyService#_onDidPolicyChange - Updated policy values: ${Object.keys(update).join(', ')}`);
+
+		for (const key in update) {
+			const value = update[key] as any;
+
+			if (value === undefined) {
+				this.policies.delete(key);
+			} else {
+				this.policies.set(key, value);
+			}
+		}
+
+		this._onDidChange.fire(Object.keys(update));
+	}
 }