From 7db665fde28d218211ae909684bf158852af6f7a Mon Sep 17 00:00:00 2001 From: linglingye001 <143174321+linglingye001@users.noreply.github.com> Date: Mon, 18 Nov 2024 16:07:14 +0800 Subject: [PATCH 01/15] Failover support (#98) * resolve conflicts * resolve conflicts * resolve conflicts * add tests * resolve conflicts and update * fix lint * resolve conflicts * resolve comments * update package-lock * update * update * update failover error * update * update * update failoverable error with 'ENOTFOUND' * fix lint * update * added ENOENT error * update * update error message in test * update test * update test * update * resolve conflicts --- package-lock.json | 3 + src/AzureAppConfigurationImpl.ts | 216 ++++++++++++--------- src/AzureAppConfigurationOptions.ts | 10 +- src/ConfigurationClientManager.ts | 288 ++++++++++++++++++++++++++++ src/ConfigurationClientWrapper.ts | 49 +++++ src/common/utils.ts | 8 + src/load.ts | 82 +------- src/refresh/RefreshTimer.ts | 59 ------ src/requestTracing/constants.ts | 1 + src/requestTracing/utils.ts | 24 ++- test/failover.test.ts | 112 +++++++++++ test/requestTracing.test.ts | 16 +- test/utils/testHelper.ts | 117 +++++++---- 13 files changed, 700 insertions(+), 285 deletions(-) create mode 100644 src/ConfigurationClientManager.ts create mode 100644 src/ConfigurationClientWrapper.ts create mode 100644 test/failover.test.ts diff --git a/package-lock.json b/package-lock.json index 498f59e..7cc0a77 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2524,6 +2524,7 @@ "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.8.tgz", "integrity": "sha512-PXwfBhYu0hBCPw8Dn0E+WDYb7af3dSLVWKi3HGv84IdF4TyFoC0ysxFd0Goxw7nSv4T/PzEJQxsYsEiFCKo2BA==", "dev": true, + "license": "MIT", "dependencies": { "braces": "^3.0.3", "picomatch": "^2.3.1" @@ -2869,6 +2870,7 @@ "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-1.9.0.tgz", "integrity": "sha512-xIp7/apCFJuUHdDLWe8O1HIkb0kQrOMb/0u6FXQjemHn/ii5LrIzU6bdECnsiTF/GjZkMEKg1xdiZwNqDYlZ6g==", "dev": true, + "license": "MIT", "dependencies": { "isarray": "0.0.1" } @@ -3085,6 +3087,7 @@ "resolved": "https://registry.npmjs.org/rollup/-/rollup-3.29.5.tgz", "integrity": "sha512-GVsDdsbJzzy4S/v3dqWPJ7EfvZJfCHiDqe80IyrF59LYuP+e6U1LJoUqeuqRbwAWoMNoXivMNeNAOf5E22VA1w==", "dev": true, + "license": "MIT", "bin": { "rollup": "dist/bin/rollup" }, diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 977872a..08f3bb3 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -35,6 +35,7 @@ import { AzureKeyVaultKeyValueAdapter } from "./keyvault/AzureKeyVaultKeyValueAd import { RefreshTimer } from "./refresh/RefreshTimer.js"; import { getConfigurationSettingWithTrace, listConfigurationSettingsWithTrace, requestTracingEnabled } from "./requestTracing/utils.js"; import { KeyFilter, LabelFilter, SettingSelector } from "./types.js"; +import { ConfigurationClientManager } from "./ConfigurationClientManager.js"; type PagedSettingSelector = SettingSelector & { /** @@ -56,10 +57,10 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { */ #sortedTrimKeyPrefixes: string[] | undefined; readonly #requestTracingEnabled: boolean; - #client: AppConfigurationClient; - #clientEndpoint: string | undefined; + #clientManager: ConfigurationClientManager; #options: AzureAppConfigurationOptions | undefined; #isInitialLoadCompleted: boolean = false; + #isFailoverRequest: boolean = false; // Refresh #refreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS; @@ -78,13 +79,11 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { #featureFlagSelectors: PagedSettingSelector[] = []; constructor( - client: AppConfigurationClient, - clientEndpoint: string | undefined, - options: AzureAppConfigurationOptions | undefined + clientManager: ConfigurationClientManager, + options: AzureAppConfigurationOptions | undefined, ) { - this.#client = client; - this.#clientEndpoint = clientEndpoint; this.#options = options; + this.#clientManager = clientManager; // Enable request tracing if not opt-out this.#requestTracingEnabled = requestTracingEnabled(); @@ -197,35 +196,66 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return { requestTracingEnabled: this.#requestTracingEnabled, initialLoadCompleted: this.#isInitialLoadCompleted, - appConfigOptions: this.#options + appConfigOptions: this.#options, + isFailoverRequest: this.#isFailoverRequest }; } - async #loadSelectedKeyValues(): Promise { - const loadedSettings: ConfigurationSetting[] = []; + async #executeWithFailoverPolicy(funcToExecute: (client: AppConfigurationClient) => Promise): Promise { + const clientWrappers = await this.#clientManager.getClients(); - // validate selectors - const selectors = getValidKeyValueSelectors(this.#options?.selectors); + let successful: boolean; + for (const clientWrapper of clientWrappers) { + successful = false; + try { + const result = await funcToExecute(clientWrapper.client); + this.#isFailoverRequest = false; + successful = true; + clientWrapper.updateBackoffStatus(successful); + return result; + } catch (error) { + if (isFailoverableError(error)) { + clientWrapper.updateBackoffStatus(successful); + this.#isFailoverRequest = true; + continue; + } - for (const selector of selectors) { - const listOptions: ListConfigurationSettingsOptions = { - keyFilter: selector.keyFilter, - labelFilter: selector.labelFilter - }; + throw error; + } + } - const settings = listConfigurationSettingsWithTrace( - this.#requestTraceOptions, - this.#client, - listOptions - ); + this.#clientManager.refreshClients(); + throw new Error("All clients failed to get configuration settings."); + } - for await (const setting of settings) { - if (!isFeatureFlag(setting)) { // exclude feature flags - loadedSettings.push(setting); + async #loadSelectedKeyValues(): Promise { + // validate selectors + const selectors = getValidKeyValueSelectors(this.#options?.selectors); + + const funcToExecute = async (client) => { + const loadedSettings: ConfigurationSetting[] = []; + for (const selector of selectors) { + const listOptions: ListConfigurationSettingsOptions = { + keyFilter: selector.keyFilter, + labelFilter: selector.labelFilter + }; + + const settings = listConfigurationSettingsWithTrace( + this.#requestTraceOptions, + client, + listOptions + ); + + for await (const setting of settings) { + if (!isFeatureFlag(setting)) { // exclude feature flags + loadedSettings.push(setting); + } } } - } - return loadedSettings; + return loadedSettings; + }; + + return await this.#executeWithFailoverPolicy(funcToExecute) as ConfigurationSetting[]; } /** @@ -279,29 +309,42 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } async #loadFeatureFlags() { - const featureFlagSettings: ConfigurationSetting[] = []; - for (const selector of this.#featureFlagSelectors) { - const listOptions: ListConfigurationSettingsOptions = { - keyFilter: `${featureFlagPrefix}${selector.keyFilter}`, - labelFilter: selector.labelFilter - }; + // Temporary map to store feature flags, key is the key of the setting, value is the raw value of the setting + const funcToExecute = async (client) => { + const featureFlagSettings: ConfigurationSetting[] = []; + // deep copy selectors to avoid modification if current client fails + const selectors = JSON.parse( + JSON.stringify(this.#featureFlagSelectors) + ); - const pageEtags: string[] = []; - const pageIterator = listConfigurationSettingsWithTrace( - this.#requestTraceOptions, - this.#client, - listOptions - ).byPage(); - for await (const page of pageIterator) { - pageEtags.push(page.etag ?? ""); - for (const setting of page.items) { - if (isFeatureFlag(setting)) { - featureFlagSettings.push(setting); + for (const selector of selectors) { + const listOptions: ListConfigurationSettingsOptions = { + keyFilter: `${featureFlagPrefix}${selector.keyFilter}`, + labelFilter: selector.labelFilter + }; + + const pageEtags: string[] = []; + const pageIterator = listConfigurationSettingsWithTrace( + this.#requestTraceOptions, + client, + listOptions + ).byPage(); + for await (const page of pageIterator) { + pageEtags.push(page.etag ?? ""); + for (const setting of page.items) { + if (isFeatureFlag(setting)) { + featureFlagSettings.push(setting); + } } } + selector.pageEtags = pageEtags; } - selector.pageEtags = pageEtags; - } + + this.#featureFlagSelectors = selectors; + return featureFlagSettings; + }; + + const featureFlagSettings = await this.#executeWithFailoverPolicy(funcToExecute) as ConfigurationSetting[]; // parse feature flags const featureFlags = await Promise.all( @@ -389,7 +432,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { // check if any refresh task failed for (const result of results) { if (result.status === "rejected") { - throw result.reason; + console.warn("Refresh failed:", result.reason); } } @@ -430,13 +473,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } if (needRefresh) { - try { - await this.#loadSelectedAndWatchedKeyValues(); - } catch (error) { - // if refresh failed, backoff - this.#refreshTimer.backoff(); - throw error; - } + await this.#loadSelectedAndWatchedKeyValues(); } this.#refreshTimer.reset(); @@ -454,39 +491,32 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } // check if any feature flag is changed - let needRefresh = false; - for (const selector of this.#featureFlagSelectors) { - const listOptions: ListConfigurationSettingsOptions = { - keyFilter: `${featureFlagPrefix}${selector.keyFilter}`, - labelFilter: selector.labelFilter, - pageEtags: selector.pageEtags - }; - const pageIterator = listConfigurationSettingsWithTrace( - this.#requestTraceOptions, - this.#client, - listOptions - ).byPage(); - - for await (const page of pageIterator) { - if (page._response.status === 200) { // created or changed - needRefresh = true; - break; + const funcToExecute = async (client) => { + for (const selector of this.#featureFlagSelectors) { + const listOptions: ListConfigurationSettingsOptions = { + keyFilter: `${featureFlagPrefix}${selector.keyFilter}`, + labelFilter: selector.labelFilter, + pageEtags: selector.pageEtags + }; + + const pageIterator = listConfigurationSettingsWithTrace( + this.#requestTraceOptions, + client, + listOptions + ).byPage(); + + for await (const page of pageIterator) { + if (page._response.status === 200) { // created or changed + return true; + } } } + return false; + }; - if (needRefresh) { - break; // short-circuit if result from any of the selectors is changed - } - } - + const needRefresh: boolean = await this.#executeWithFailoverPolicy(funcToExecute); if (needRefresh) { - try { - await this.#loadFeatureFlags(); - } catch (error) { - // if refresh failed, backoff - this.#featureFlagRefreshTimer.backoff(); - throw error; - } + await this.#loadFeatureFlags(); } this.#featureFlagRefreshTimer.reset(); @@ -540,14 +570,18 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { * Get a configuration setting by key and label. If the setting is not found, return undefine instead of throwing an error. */ async #getConfigurationSetting(configurationSettingId: ConfigurationSettingId, customOptions?: GetConfigurationSettingOptions): Promise { - let response: GetConfigurationSettingResponse | undefined; - try { - response = await getConfigurationSettingWithTrace( + const funcToExecute = async (client) => { + return getConfigurationSettingWithTrace( this.#requestTraceOptions, - this.#client, + client, configurationSettingId, customOptions ); + }; + + let response: GetConfigurationSettingResponse | undefined; + try { + response = await this.#executeWithFailoverPolicy(funcToExecute); } catch (error) { if (isRestError(error) && error.statusCode === 404) { response = undefined; @@ -634,7 +668,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } #createFeatureFlagReference(setting: ConfigurationSetting): string { - let featureFlagReference = `${this.#clientEndpoint}kv/${setting.key}`; + let featureFlagReference = `${this.#clientManager.endpoint.origin}/kv/${setting.key}`; if (setting.label && setting.label.trim().length !== 0) { featureFlagReference += `?label=${setting.label}`; } @@ -794,3 +828,9 @@ function getValidFeatureFlagSelectors(selectors?: SettingSelector[]): SettingSel return getValidSelectors(selectors); } } + +function isFailoverableError(error: any): boolean { + // ENOTFOUND: DNS lookup failed, ENOENT: no such file or directory + return isRestError(error) && (error.code === "ENOTFOUND" || error.code === "ENOENT" || + (error.statusCode !== undefined && (error.statusCode === 401 || error.statusCode === 403 || error.statusCode === 408 || error.statusCode === 429 || error.statusCode >= 500))); +} diff --git a/src/AzureAppConfigurationOptions.ts b/src/AzureAppConfigurationOptions.ts index f88ad67..4aa3f99 100644 --- a/src/AzureAppConfigurationOptions.ts +++ b/src/AzureAppConfigurationOptions.ts @@ -12,7 +12,7 @@ export const MaxRetryDelayInMs = 60000; export interface AzureAppConfigurationOptions { /** - * Specify what key-values to include in the configuration provider. + * Specifies what key-values to include in the configuration provider. * * @remarks * If no selectors are specified then all key-values with no label will be included. @@ -47,4 +47,12 @@ export interface AzureAppConfigurationOptions { * Specifies options used to configure feature flags. */ featureFlagOptions?: FeatureFlagOptions; + + /** + * Specifies whether to enable replica discovery or not. + * + * @remarks + * If not specified, the default value is true. + */ + replicaDiscoveryEnabled?: boolean; } diff --git a/src/ConfigurationClientManager.ts b/src/ConfigurationClientManager.ts new file mode 100644 index 0000000..59e03aa --- /dev/null +++ b/src/ConfigurationClientManager.ts @@ -0,0 +1,288 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { AppConfigurationClient, AppConfigurationClientOptions } from "@azure/app-configuration"; +import { ConfigurationClientWrapper } from "./ConfigurationClientWrapper.js"; +import { TokenCredential } from "@azure/identity"; +import { AzureAppConfigurationOptions, MaxRetries, MaxRetryDelayInMs } from "./AzureAppConfigurationOptions.js"; +import { isBrowser, isWebWorker } from "./requestTracing/utils.js"; +import * as RequestTracing from "./requestTracing/constants.js"; +import { shuffleList } from "./common/utils.js"; + +const TCP_ORIGIN_KEY_NAME = "_origin._tcp"; +const ALT_KEY_NAME = "_alt"; +const TCP_KEY_NAME = "_tcp"; +const ENDPOINT_KEY_NAME = "Endpoint"; +const ID_KEY_NAME = "Id"; +const SECRET_KEY_NAME = "Secret"; +const TRUSTED_DOMAIN_LABELS = [".azconfig.", ".appconfig."]; +const FALLBACK_CLIENT_REFRESH_EXPIRE_INTERVAL = 60 * 60 * 1000; // 1 hour in milliseconds +const MINIMAL_CLIENT_REFRESH_INTERVAL = 30 * 1000; // 30 seconds in milliseconds +const SRV_QUERY_TIMEOUT = 30 * 1000; // 30 seconds in milliseconds + +export class ConfigurationClientManager { + #isFailoverable: boolean; + #dns: any; + endpoint: URL; + #secret : string; + #id : string; + #credential: TokenCredential; + #clientOptions: AppConfigurationClientOptions | undefined; + #appConfigOptions: AzureAppConfigurationOptions | undefined; + #validDomain: string; + #staticClients: ConfigurationClientWrapper[]; + #dynamicClients: ConfigurationClientWrapper[]; + #lastFallbackClientRefreshTime: number = 0; + #lastFallbackClientRefreshAttempt: number = 0; + + constructor ( + connectionStringOrEndpoint?: string | URL, + credentialOrOptions?: TokenCredential | AzureAppConfigurationOptions, + appConfigOptions?: AzureAppConfigurationOptions + ) { + let staticClient: AppConfigurationClient; + const credentialPassed = instanceOfTokenCredential(credentialOrOptions); + + if (typeof connectionStringOrEndpoint === "string" && !credentialPassed) { + const connectionString = connectionStringOrEndpoint; + this.#appConfigOptions = credentialOrOptions as AzureAppConfigurationOptions; + this.#clientOptions = getClientOptions(this.#appConfigOptions); + const ConnectionStringRegex = /Endpoint=(.*);Id=(.*);Secret=(.*)/; + const regexMatch = connectionString.match(ConnectionStringRegex); + if (regexMatch) { + const endpointFromConnectionStr = regexMatch[1]; + this.endpoint = getValidUrl(endpointFromConnectionStr); + this.#id = regexMatch[2]; + this.#secret = regexMatch[3]; + } else { + throw new Error(`Invalid connection string. Valid connection strings should match the regex '${ConnectionStringRegex.source}'.`); + } + staticClient = new AppConfigurationClient(connectionString, this.#clientOptions); + } else if ((connectionStringOrEndpoint instanceof URL || typeof connectionStringOrEndpoint === "string") && credentialPassed) { + let endpoint = connectionStringOrEndpoint; + // ensure string is a valid URL. + if (typeof endpoint === "string") { + endpoint = getValidUrl(endpoint); + } + + const credential = credentialOrOptions as TokenCredential; + this.#appConfigOptions = appConfigOptions as AzureAppConfigurationOptions; + this.#clientOptions = getClientOptions(this.#appConfigOptions); + this.endpoint = endpoint; + this.#credential = credential; + staticClient = new AppConfigurationClient(this.endpoint.origin, this.#credential, this.#clientOptions); + } else { + throw new Error("A connection string or an endpoint with credential must be specified to create a client."); + } + + this.#staticClients = [new ConfigurationClientWrapper(this.endpoint.origin, staticClient)]; + this.#validDomain = getValidDomain(this.endpoint.hostname.toLowerCase()); + } + + async init() { + if (this.#appConfigOptions?.replicaDiscoveryEnabled === false || isBrowser() || isWebWorker()) { + this.#isFailoverable = false; + return; + } + + try { + this.#dns = await import("dns/promises"); + }catch (error) { + this.#isFailoverable = false; + console.warn("Failed to load the dns module:", error.message); + return; + } + + this.#isFailoverable = true; + } + + async getClients() : Promise { + if (!this.#isFailoverable) { + return this.#staticClients; + } + + const currentTime = Date.now(); + // Filter static clients whose backoff time has ended + let availableClients = this.#staticClients.filter(client => client.backoffEndTime <= currentTime); + if (currentTime >= this.#lastFallbackClientRefreshAttempt + MINIMAL_CLIENT_REFRESH_INTERVAL && + (!this.#dynamicClients || + // All dynamic clients are in backoff means no client is available + this.#dynamicClients.every(client => currentTime < client.backoffEndTime) || + currentTime >= this.#lastFallbackClientRefreshTime + FALLBACK_CLIENT_REFRESH_EXPIRE_INTERVAL)) { + this.#lastFallbackClientRefreshAttempt = currentTime; + await this.#discoverFallbackClients(this.endpoint.hostname); + return availableClients.concat(this.#dynamicClients); + } + + // If there are dynamic clients, filter and concatenate them + if (this.#dynamicClients && this.#dynamicClients.length > 0) { + availableClients = availableClients.concat( + this.#dynamicClients + .filter(client => client.backoffEndTime <= currentTime)); + } + + return availableClients; + } + + async refreshClients() { + const currentTime = Date.now(); + if (this.#isFailoverable && + currentTime >= new Date(this.#lastFallbackClientRefreshAttempt + MINIMAL_CLIENT_REFRESH_INTERVAL).getTime()) { + this.#lastFallbackClientRefreshAttempt = currentTime; + await this.#discoverFallbackClients(this.endpoint.hostname); + } + } + + async #discoverFallbackClients(host: string) { + let result; + try { + result = await Promise.race([ + new Promise((_, reject) => setTimeout(() => reject(new Error("SRV record query timed out.")), SRV_QUERY_TIMEOUT)), + this.#querySrvTargetHost(host) + ]); + } catch (error) { + throw new Error(`Failed to build fallback clients, ${error.message}`); + } + + const srvTargetHosts = shuffleList(result) as string[]; + const newDynamicClients: ConfigurationClientWrapper[] = []; + for (const host of srvTargetHosts) { + if (isValidEndpoint(host, this.#validDomain)) { + const targetEndpoint = `https://${host}`; + if (host.toLowerCase() === this.endpoint.hostname.toLowerCase()) { + continue; + } + const client = this.#credential ? + new AppConfigurationClient(targetEndpoint, this.#credential, this.#clientOptions) : + new AppConfigurationClient(buildConnectionString(targetEndpoint, this.#secret, this.#id), this.#clientOptions); + newDynamicClients.push(new ConfigurationClientWrapper(targetEndpoint, client)); + } + } + + this.#dynamicClients = newDynamicClients; + this.#lastFallbackClientRefreshTime = Date.now(); + } + + /** + * Query SRV records and return target hosts. + */ + async #querySrvTargetHost(host: string): Promise { + const results: string[] = []; + + try { + // Look up SRV records for the origin host + const originRecords = await this.#dns.resolveSrv(`${TCP_ORIGIN_KEY_NAME}.${host}`); + if (originRecords.length === 0) { + return results; + } + + // Add the first origin record to results + const originHost = originRecords[0].name; + results.push(originHost); + + // Look up SRV records for alternate hosts + let index = 0; + // eslint-disable-next-line no-constant-condition + while (true) { + const currentAlt = `${ALT_KEY_NAME}${index}`; + const altRecords = await this.#dns.resolveSrv(`${currentAlt}.${TCP_KEY_NAME}.${originHost}`); + if (altRecords.length === 0) { + break; // No more alternate records, exit loop + } + + altRecords.forEach(record => { + const altHost = record.name; + if (altHost) { + results.push(altHost); + } + }); + index++; + } + } catch (err) { + if (err.code === "ENOTFOUND") { + return results; // No more SRV records found, return results + } else { + throw new Error(`Failed to lookup SRV records: ${err.message}`); + } + } + + return results; + } +} + +/** + * Builds a connection string from the given endpoint, secret, and id. + * Returns an empty string if either secret or id is empty. + */ +function buildConnectionString(endpoint, secret, id: string): string { + if (!secret || !id) { + return ""; + } + + return `${ENDPOINT_KEY_NAME}=${endpoint};${ID_KEY_NAME}=${id};${SECRET_KEY_NAME}=${secret}`; +} + +/** + * Extracts a valid domain from the given endpoint URL based on trusted domain labels. + */ +export function getValidDomain(host: string): string { + for (const label of TRUSTED_DOMAIN_LABELS) { + const index = host.lastIndexOf(label); + if (index !== -1) { + return host.substring(index); + } + } + + return ""; +} + +/** + * Checks if the given host ends with the valid domain. + */ +export function isValidEndpoint(host: string, validDomain: string): boolean { + if (!validDomain) { + return false; + } + + return host.toLowerCase().endsWith(validDomain.toLowerCase()); +} + +function getClientOptions(options?: AzureAppConfigurationOptions): AppConfigurationClientOptions | undefined { + // user-agent + let userAgentPrefix = RequestTracing.USER_AGENT_PREFIX; // Default UA for JavaScript Provider + const userAgentOptions = options?.clientOptions?.userAgentOptions; + if (userAgentOptions?.userAgentPrefix) { + userAgentPrefix = `${userAgentOptions.userAgentPrefix} ${userAgentPrefix}`; // Prepend if UA prefix specified by user + } + + // retry options + const defaultRetryOptions = { + maxRetries: MaxRetries, + maxRetryDelayInMs: MaxRetryDelayInMs, + }; + const retryOptions = Object.assign({}, defaultRetryOptions, options?.clientOptions?.retryOptions); + + return Object.assign({}, options?.clientOptions, { + retryOptions, + userAgentOptions: { + userAgentPrefix + } + }); +} + +function getValidUrl(endpoint: string): URL { + try { + return new URL(endpoint); + } catch (error) { + if (error.code === "ERR_INVALID_URL") { + throw new Error("Invalid endpoint URL.", { cause: error }); + } else { + throw error; + } + } +} + +export function instanceOfTokenCredential(obj: unknown) { + return obj && typeof obj === "object" && "getToken" in obj && typeof obj.getToken === "function"; +} + diff --git a/src/ConfigurationClientWrapper.ts b/src/ConfigurationClientWrapper.ts new file mode 100644 index 0000000..7dd6f41 --- /dev/null +++ b/src/ConfigurationClientWrapper.ts @@ -0,0 +1,49 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { AppConfigurationClient } from "@azure/app-configuration"; + +const MaxBackoffDuration = 10 * 60 * 1000; // 10 minutes in milliseconds +const MinBackoffDuration = 30 * 1000; // 30 seconds in milliseconds +const MAX_SAFE_EXPONENTIAL = 30; // Used to avoid overflow. bitwise operations in JavaScript are limited to 32 bits. It overflows at 2^31 - 1. +const JITTER_RATIO = 0.25; + +export class ConfigurationClientWrapper { + endpoint: string; + client: AppConfigurationClient; + backoffEndTime: number = 0; // Timestamp + #failedAttempts: number = 0; + + constructor(endpoint: string, client: AppConfigurationClient) { + this.endpoint = endpoint; + this.client = client; + } + + updateBackoffStatus(successfull: boolean) { + if (successfull) { + this.#failedAttempts = 0; + this.backoffEndTime = Date.now(); + } else { + this.#failedAttempts += 1; + this.backoffEndTime = Date.now() + calculateBackoffDuration(this.#failedAttempts); + } + } +} + +export function calculateBackoffDuration(failedAttempts: number) { + if (failedAttempts <= 1) { + return MinBackoffDuration; + } + + // exponential: minBackoff * 2 ^ (failedAttempts - 1) + const exponential = Math.min(failedAttempts - 1, MAX_SAFE_EXPONENTIAL); + let calculatedBackoffDuration = MinBackoffDuration * (1 << exponential); + if (calculatedBackoffDuration > MaxBackoffDuration) { + calculatedBackoffDuration = MaxBackoffDuration; + } + + // jitter: random value between [-1, 1) * jitterRatio * calculatedBackoffMs + const jitter = JITTER_RATIO * (Math.random() * 2 - 1); + + return calculatedBackoffDuration * (1 + jitter); +} diff --git a/src/common/utils.ts b/src/common/utils.ts index ad827bb..8682484 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -22,3 +22,11 @@ export function jsonSorter(key, value) { } return value; } + +export function shuffleList(array: T[]): T[] { + for (let i = array.length - 1; i > 0; i--) { + const j = Math.floor(Math.random() * (i + 1)); + [array[i], array[j]] = [array[j], array[i]]; + } + return array; +} diff --git a/src/load.ts b/src/load.ts index ce3d39c..4d24174 100644 --- a/src/load.ts +++ b/src/load.ts @@ -1,12 +1,11 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { AppConfigurationClient, AppConfigurationClientOptions } from "@azure/app-configuration"; import { TokenCredential } from "@azure/identity"; import { AzureAppConfiguration } from "./AzureAppConfiguration.js"; import { AzureAppConfigurationImpl } from "./AzureAppConfigurationImpl.js"; -import { AzureAppConfigurationOptions, MaxRetries, MaxRetryDelayInMs } from "./AzureAppConfigurationOptions.js"; -import * as RequestTracing from "./requestTracing/constants.js"; +import { AzureAppConfigurationOptions } from "./AzureAppConfigurationOptions.js"; +import { ConfigurationClientManager, instanceOfTokenCredential } from "./ConfigurationClientManager.js"; const MIN_DELAY_FOR_UNHANDLED_ERROR: number = 5000; // 5 seconds @@ -31,43 +30,18 @@ export async function load( appConfigOptions?: AzureAppConfigurationOptions ): Promise { const startTimestamp = Date.now(); - let client: AppConfigurationClient; - let clientEndpoint: string | undefined; let options: AzureAppConfigurationOptions | undefined; + const clientManager = new ConfigurationClientManager(connectionStringOrEndpoint, credentialOrOptions, appConfigOptions); + await clientManager.init(); - // input validation - if (typeof connectionStringOrEndpoint === "string" && !instanceOfTokenCredential(credentialOrOptions)) { - const connectionString = connectionStringOrEndpoint; + if (!instanceOfTokenCredential(credentialOrOptions)) { options = credentialOrOptions as AzureAppConfigurationOptions; - const clientOptions = getClientOptions(options); - client = new AppConfigurationClient(connectionString, clientOptions); - clientEndpoint = getEndpoint(connectionStringOrEndpoint); - } else if ((connectionStringOrEndpoint instanceof URL || typeof connectionStringOrEndpoint === "string") && instanceOfTokenCredential(credentialOrOptions)) { - // ensure string is a valid URL. - if (typeof connectionStringOrEndpoint === "string") { - try { - const endpointUrl = new URL(connectionStringOrEndpoint); - clientEndpoint = endpointUrl.toString(); - } catch (error) { - if (error.code === "ERR_INVALID_URL") { - throw new Error("Invalid endpoint URL.", { cause: error }); - } else { - throw error; - } - } - } else { - clientEndpoint = connectionStringOrEndpoint.toString(); - } - const credential = credentialOrOptions as TokenCredential; - options = appConfigOptions; - const clientOptions = getClientOptions(options); - client = new AppConfigurationClient(clientEndpoint, credential, clientOptions); } else { - throw new Error("A connection string or an endpoint with credential must be specified to create a client."); + options = appConfigOptions; } try { - const appConfiguration = new AzureAppConfigurationImpl(client, clientEndpoint, options); + const appConfiguration = new AzureAppConfigurationImpl(clientManager, options); await appConfiguration.load(); return appConfiguration; } catch (error) { @@ -81,45 +55,3 @@ export async function load( throw error; } } - -function instanceOfTokenCredential(obj: unknown) { - return obj && typeof obj === "object" && "getToken" in obj && typeof obj.getToken === "function"; -} - -function getClientOptions(options?: AzureAppConfigurationOptions): AppConfigurationClientOptions | undefined { - // user-agent - let userAgentPrefix = RequestTracing.USER_AGENT_PREFIX; // Default UA for JavaScript Provider - const userAgentOptions = options?.clientOptions?.userAgentOptions; - if (userAgentOptions?.userAgentPrefix) { - userAgentPrefix = `${userAgentOptions.userAgentPrefix} ${userAgentPrefix}`; // Prepend if UA prefix specified by user - } - - // retry options - const defaultRetryOptions = { - maxRetries: MaxRetries, - maxRetryDelayInMs: MaxRetryDelayInMs, - }; - const retryOptions = Object.assign({}, defaultRetryOptions, options?.clientOptions?.retryOptions); - - return Object.assign({}, options?.clientOptions, { - retryOptions, - userAgentOptions: { - userAgentPrefix - } - }); -} - -function getEndpoint(connectionString: string): string | undefined { - const parts = connectionString.split(";"); - const endpointPart = parts.find(part => part.startsWith("Endpoint=")); - - if (endpointPart) { - let endpoint = endpointPart.split("=")[1]; - if (!endpoint.endsWith("/")) { - endpoint += "/"; - } - return endpoint; - } - - return undefined; -} diff --git a/src/refresh/RefreshTimer.ts b/src/refresh/RefreshTimer.ts index ce48594..45fdf0b 100644 --- a/src/refresh/RefreshTimer.ts +++ b/src/refresh/RefreshTimer.ts @@ -1,30 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -/** - * The backoff time is between the minimum and maximum backoff time, based on the number of attempts. - * An exponential backoff strategy is used, with a jitter factor to prevent clients from retrying at the same time. - * - * The backoff time is calculated as follows: - * - `basic backoff time` = `MinimumBackoffInMs` * 2 ^ `attempts`, and it is no larger than the `MaximumBackoffInMs`. - * - based on jitter ratio, the jittered time is between [-1, 1) * `JitterRatio` * basic backoff time. - * - the final backoff time is the basic backoff time plus the jittered time. - * - * Note: the backoff time usually is no larger than the refresh interval, which is specified by the user. - * - If the interval is less than the minimum backoff, the interval is used. - * - If the interval is between the minimum and maximum backoff, the interval is used as the maximum backoff. - * - Because of the jitter, the maximum backoff time is actually `MaximumBackoffInMs` * (1 + `JitterRatio`). - */ - -const MIN_BACKOFF_IN_MS = 30 * 1000; // 30s -const MAX_BACKOFF_IN_MS = 10 * 60 * 1000; // 10min -const MAX_SAFE_EXPONENTIAL = 30; // Used to avoid overflow. bitwise operations in JavaScript are limited to 32 bits. It overflows at 2^31 - 1. -const JITTER_RATIO = 0.25; - export class RefreshTimer { - #minBackoff: number = MIN_BACKOFF_IN_MS; - #maxBackoff: number = MAX_BACKOFF_IN_MS; - #failedAttempts: number = 0; #backoffEnd: number; // Timestamp #interval: number; @@ -43,43 +20,7 @@ export class RefreshTimer { return Date.now() >= this.#backoffEnd; } - backoff(): void { - this.#failedAttempts += 1; - this.#backoffEnd = Date.now() + this.#calculateBackoffTime(); - } - reset(): void { - this.#failedAttempts = 0; this.#backoffEnd = Date.now() + this.#interval; } - - #calculateBackoffTime(): number { - let minBackoffMs: number; - let maxBackoffMs: number; - if (this.#interval <= this.#minBackoff) { - return this.#interval; - } - - // _minBackoff <= _interval - if (this.#interval <= this.#maxBackoff) { - minBackoffMs = this.#minBackoff; - maxBackoffMs = this.#interval; - } else { - minBackoffMs = this.#minBackoff; - maxBackoffMs = this.#maxBackoff; - } - - // exponential: minBackoffMs * 2^(failedAttempts-1) - const exponential = Math.min(this.#failedAttempts - 1, MAX_SAFE_EXPONENTIAL); - let calculatedBackoffMs = minBackoffMs * (1 << exponential); - if (calculatedBackoffMs > maxBackoffMs) { - calculatedBackoffMs = maxBackoffMs; - } - - // jitter: random value between [-1, 1) * jitterRatio * calculatedBackoffMs - const jitter = JITTER_RATIO * (Math.random() * 2 - 1); - - return calculatedBackoffMs * (1 + jitter); - } - } diff --git a/src/requestTracing/constants.ts b/src/requestTracing/constants.ts index d46cdfd..60dbb81 100644 --- a/src/requestTracing/constants.ts +++ b/src/requestTracing/constants.ts @@ -45,4 +45,5 @@ export enum RequestType { } // Tag names +export const FAILOVER_REQUEST_TAG = "Failover"; export const KEY_VAULT_CONFIGURED_TAG = "UsesKeyVault"; diff --git a/src/requestTracing/utils.ts b/src/requestTracing/utils.ts index de33573..8a2fdbc 100644 --- a/src/requestTracing/utils.ts +++ b/src/requestTracing/utils.ts @@ -19,7 +19,8 @@ import { REQUEST_TYPE_KEY, RequestType, SERVICE_FABRIC_ENV_VAR, - CORRELATION_CONTEXT_HEADER_NAME + CORRELATION_CONTEXT_HEADER_NAME, + FAILOVER_REQUEST_TAG } from "./constants"; // Utils @@ -28,17 +29,18 @@ export function listConfigurationSettingsWithTrace( requestTracingEnabled: boolean; initialLoadCompleted: boolean; appConfigOptions: AzureAppConfigurationOptions | undefined; + isFailoverRequest: boolean; }, client: AppConfigurationClient, listOptions: ListConfigurationSettingsOptions ) { - const { requestTracingEnabled, initialLoadCompleted, appConfigOptions } = requestTracingOptions; + const { requestTracingEnabled, initialLoadCompleted, appConfigOptions, isFailoverRequest } = requestTracingOptions; const actualListOptions = { ...listOptions }; if (requestTracingEnabled) { actualListOptions.requestOptions = { customHeaders: { - [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(appConfigOptions, initialLoadCompleted) + [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(appConfigOptions, initialLoadCompleted, isFailoverRequest) } }; } @@ -51,18 +53,19 @@ export function getConfigurationSettingWithTrace( requestTracingEnabled: boolean; initialLoadCompleted: boolean; appConfigOptions: AzureAppConfigurationOptions | undefined; + isFailoverRequest: boolean; }, client: AppConfigurationClient, configurationSettingId: ConfigurationSettingId, getOptions?: GetConfigurationSettingOptions, ) { - const { requestTracingEnabled, initialLoadCompleted, appConfigOptions } = requestTracingOptions; + const { requestTracingEnabled, initialLoadCompleted, appConfigOptions, isFailoverRequest } = requestTracingOptions; const actualGetOptions = { ...getOptions }; if (requestTracingEnabled) { actualGetOptions.requestOptions = { customHeaders: { - [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(appConfigOptions, initialLoadCompleted) + [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(appConfigOptions, initialLoadCompleted, isFailoverRequest) } }; } @@ -70,7 +73,7 @@ export function getConfigurationSettingWithTrace( return client.getConfigurationSetting(configurationSettingId, actualGetOptions); } -export function createCorrelationContextHeader(options: AzureAppConfigurationOptions | undefined, isInitialLoadCompleted: boolean): string { +export function createCorrelationContextHeader(options: AzureAppConfigurationOptions | undefined, isInitialLoadCompleted: boolean, isFailoverRequest: boolean): string { /* RequestType: 'Startup' during application starting up, 'Watch' after startup completed. Host: identify with defined envs @@ -100,6 +103,10 @@ export function createCorrelationContextHeader(options: AzureAppConfigurationOpt contextParts.push(tag); } + if (isFailoverRequest) { + contextParts.push(FAILOVER_REQUEST_TAG); + } + return contextParts.join(","); } @@ -146,7 +153,7 @@ function isDevEnvironment(): boolean { return false; } -function isBrowser() { +export function isBrowser() { // https://developer.mozilla.org/en-US/docs/Web/API/Window const isWindowDefinedAsExpected = typeof window === "object" && typeof Window === "function" && window instanceof Window; // https://developer.mozilla.org/en-US/docs/Web/API/Document @@ -155,7 +162,7 @@ function isBrowser() { return isWindowDefinedAsExpected && isDocumentDefinedAsExpected; } -function isWebWorker() { +export function isWebWorker() { // https://developer.mozilla.org/en-US/docs/Web/API/WorkerGlobalScope const workerGlobalScopeDefined = typeof WorkerGlobalScope !== "undefined"; // https://developer.mozilla.org/en-US/docs/Web/API/WorkerNavigator @@ -165,3 +172,4 @@ function isWebWorker() { return workerGlobalScopeDefined && importScriptsAsGlobalFunction && isNavigatorDefinedAsExpected; } + diff --git a/test/failover.test.ts b/test/failover.test.ts new file mode 100644 index 0000000..c97a127 --- /dev/null +++ b/test/failover.test.ts @@ -0,0 +1,112 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import * as chai from "chai"; +import * as chaiAsPromised from "chai-as-promised"; +chai.use(chaiAsPromised); +const expect = chai.expect; +import { load } from "./exportedApi"; +import { createMockedConnectionString, createMockedFeatureFlag, createMockedKeyValue, mockConfigurationManagerGetClients, restoreMocks } from "./utils/testHelper"; +import { getValidDomain, isValidEndpoint } from "../src/ConfigurationClientManager"; + +const mockedKVs = [{ + key: "app.settings.fontColor", + value: "red", +}, { + key: "app.settings.fontSize", + value: "40", +}].map(createMockedKeyValue); + +const mockedFeatureFlags = [{ + key: "app.settings.fontColor", + value: "red", +}].map(createMockedKeyValue).concat([ + createMockedFeatureFlag("Beta", { enabled: true }), + createMockedFeatureFlag("Alpha_1", { enabled: true }), + createMockedFeatureFlag("Alpha_2", { enabled: false }), +]); + +describe("failover", function () { + this.timeout(15000); + + afterEach(() => { + restoreMocks(); + }); + + it("should failover to replica and load key values from config store", async () => { + const isFailoverable = true; + mockConfigurationManagerGetClients(isFailoverable, mockedKVs); + + const connectionString = createMockedConnectionString(); + // replicaDiscoveryEnabled is default to true + const settings = await load(connectionString); + expect(settings).not.undefined; + expect(settings.get("app.settings.fontColor")).eq("red"); + expect(settings.get("app.settings.fontSize")).eq("40"); + }); + + it("should failover to replica and load feature flags from config store", async () => { + const isFailoverable = true; + mockConfigurationManagerGetClients(isFailoverable, mockedFeatureFlags); + + const connectionString = createMockedConnectionString(); + // replicaDiscoveryEnabled is default to true + const settings = await load(connectionString, { + featureFlagOptions: { + enabled: true, + selectors: [{ + keyFilter: "*" + }] + } + }); + expect(settings).not.undefined; + expect(settings.get("feature_management")).not.undefined; + expect(settings.get("feature_management").feature_flags).not.undefined; + }); + + it("should throw error when all clients failed", async () => { + const isFailoverable = false; + mockConfigurationManagerGetClients(isFailoverable); + + const connectionString = createMockedConnectionString(); + return expect(load(connectionString)).eventually.rejectedWith("All clients failed to get configuration settings."); + }); + + it("should validate endpoint", () => { + const fakeHost = "fake.azconfig.io"; + const validDomain = getValidDomain(fakeHost); + + expect(isValidEndpoint("azure.azconfig.io", validDomain)).to.be.true; + expect(isValidEndpoint("azure.privatelink.azconfig.io", validDomain)).to.be.true; + expect(isValidEndpoint("azure-replica.azconfig.io", validDomain)).to.be.true; + expect(isValidEndpoint("azure.badazconfig.io", validDomain)).to.be.false; + expect(isValidEndpoint("azure.azconfigbad.io", validDomain)).to.be.false; + expect(isValidEndpoint("azure.appconfig.azure.com", validDomain)).to.be.false; + expect(isValidEndpoint("azure.azconfig.bad.io", validDomain)).to.be.false; + + const fakeHost2 = "foobar.appconfig.azure.com"; + const validDomain2 = getValidDomain(fakeHost2); + + expect(isValidEndpoint("azure.appconfig.azure.com", validDomain2)).to.be.true; + expect(isValidEndpoint("azure.z1.appconfig.azure.com", validDomain2)).to.be.true; + expect(isValidEndpoint("azure-replia.z1.appconfig.azure.com", validDomain2)).to.be.true; // Note: Typo "azure-replia" + expect(isValidEndpoint("azure.privatelink.appconfig.azure.com", validDomain2)).to.be.true; + expect(isValidEndpoint("azconfig.appconfig.azure.com", validDomain2)).to.be.true; + expect(isValidEndpoint("azure.azconfig.io", validDomain2)).to.be.false; + expect(isValidEndpoint("azure.badappconfig.azure.com", validDomain2)).to.be.false; + expect(isValidEndpoint("azure.appconfigbad.azure.com", validDomain2)).to.be.false; + + const fakeHost3 = "foobar.azconfig-test.io"; + const validDomain3 = getValidDomain(fakeHost3); + + expect(isValidEndpoint("azure.azconfig-test.io", validDomain3)).to.be.false; + expect(isValidEndpoint("azure.azconfig.io", validDomain3)).to.be.false; + + const fakeHost4 = "foobar.z1.appconfig-test.azure.com"; + const validDomain4 = getValidDomain(fakeHost4); + + expect(isValidEndpoint("foobar.z2.appconfig-test.azure.com", validDomain4)).to.be.false; + expect(isValidEndpoint("foobar.appconfig-test.azure.com", validDomain4)).to.be.false; + expect(isValidEndpoint("foobar.appconfig.azure.com", validDomain4)).to.be.false; + }); +}); diff --git a/test/requestTracing.test.ts b/test/requestTracing.test.ts index d4e7edc..62d0c5b 100644 --- a/test/requestTracing.test.ts +++ b/test/requestTracing.test.ts @@ -54,9 +54,7 @@ describe("request tracing", function () { it("should have request type in correlation-context header", async () => { try { - await load(createMockedConnectionString(fakeEndpoint), { - clientOptions - }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; expect(headerPolicy.headers.get("Correlation-Context")).eq("RequestType=Startup"); @@ -80,9 +78,7 @@ describe("request tracing", function () { it("should detect env in correlation-context header", async () => { process.env.NODE_ENV = "development"; try { - await load(createMockedConnectionString(fakeEndpoint), { - clientOptions - }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -94,9 +90,7 @@ describe("request tracing", function () { it("should detect host type in correlation-context header", async () => { process.env.WEBSITE_SITE_NAME = "website-name"; try { - await load(createMockedConnectionString(fakeEndpoint), { - clientOptions - }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); @@ -109,9 +103,7 @@ describe("request tracing", function () { for (const indicator of ["TRUE", "true"]) { process.env.AZURE_APP_CONFIGURATION_TRACING_DISABLED = indicator; try { - await load(createMockedConnectionString(fakeEndpoint), { - clientOptions - }); + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); } catch (e) { /* empty */ } expect(headerPolicy.headers).not.undefined; const correlationContext = headerPolicy.headers.get("Correlation-Context"); diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index 6e787dd..261b9b5 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -10,6 +10,8 @@ import { RestError } from "@azure/core-rest-pipeline"; import { promisify } from "util"; const sleepInMs = promisify(setTimeout); import * as crypto from "crypto"; +import { ConfigurationClientManager } from "../../src/ConfigurationClientManager"; +import { ConfigurationClientWrapper } from "../../src/ConfigurationClientWrapper"; const TEST_CLIENT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_TENANT_ID = "00000000-0000-0000-0000-000000000000"; @@ -38,6 +40,50 @@ function _filterKVs(unfilteredKvs: ConfigurationSetting[], listOptions: any) { }); } +function getMockedIterator(pages: ConfigurationSetting[][], kvs: ConfigurationSetting[], listOptions: any) { + const mockIterator: AsyncIterableIterator & { byPage(): AsyncIterableIterator } = { + [Symbol.asyncIterator](): AsyncIterableIterator { + kvs = _filterKVs(pages.flat(), listOptions); + return this; + }, + next() { + const value = kvs.shift(); + return Promise.resolve({ done: !value, value }); + }, + byPage(): AsyncIterableIterator { + let remainingPages; + const pageEtags = listOptions?.pageEtags ? [...listOptions.pageEtags] : undefined; // a copy of the original list + return { + [Symbol.asyncIterator](): AsyncIterableIterator { + remainingPages = [...pages]; + return this; + }, + next() { + const pageItems = remainingPages.shift(); + const pageEtag = pageEtags?.shift(); + if (pageItems === undefined) { + return Promise.resolve({ done: true, value: undefined }); + } else { + const items = _filterKVs(pageItems ?? [], listOptions); + const etag = _sha256(JSON.stringify(items)); + const statusCode = pageEtag === etag ? 304 : 200; + return Promise.resolve({ + done: false, + value: { + items, + etag, + _response: { status: statusCode } + } + }); + } + } + }; + } + }; + + return mockIterator as any; +} + /** * Mocks the listConfigurationSettings method of AppConfigurationClient to return the provided pages of ConfigurationSetting. * E.g. @@ -49,48 +95,34 @@ function _filterKVs(unfilteredKvs: ConfigurationSetting[], listOptions: any) { function mockAppConfigurationClientListConfigurationSettings(...pages: ConfigurationSetting[][]) { sinon.stub(AppConfigurationClient.prototype, "listConfigurationSettings").callsFake((listOptions) => { - let kvs = _filterKVs(pages.flat(), listOptions); - const mockIterator: AsyncIterableIterator & { byPage(): AsyncIterableIterator } = { - [Symbol.asyncIterator](): AsyncIterableIterator { - kvs = _filterKVs(pages.flat(), listOptions); - return this; - }, - next() { - const value = kvs.shift(); - return Promise.resolve({ done: !value, value }); - }, - byPage(): AsyncIterableIterator { - let remainingPages; - const pageEtags = listOptions?.pageEtags ? [...listOptions.pageEtags] : undefined; // a copy of the original list - return { - [Symbol.asyncIterator](): AsyncIterableIterator { - remainingPages = [...pages]; - return this; - }, - next() { - const pageItems = remainingPages.shift(); - const pageEtag = pageEtags?.shift(); - if (pageItems === undefined) { - return Promise.resolve({ done: true, value: undefined }); - } else { - const items = _filterKVs(pageItems ?? [], listOptions); - const etag = _sha256(JSON.stringify(items)); - const statusCode = pageEtag === etag ? 304 : 200; - return Promise.resolve({ - done: false, - value: { - items, - etag, - _response: { status: statusCode } - } - }); - } - } - }; - } - }; + const kvs = _filterKVs(pages.flat(), listOptions); + return getMockedIterator(pages, kvs, listOptions); + }); +} - return mockIterator as any; +function mockConfigurationManagerGetClients(isFailoverable: boolean, ...pages: ConfigurationSetting[][]) { + // Stub the getClients method on the class prototype + sinon.stub(ConfigurationClientManager.prototype, "getClients").callsFake(async () => { + const clients: ConfigurationClientWrapper[] = []; + const fakeEndpoint = createMockedEndpoint("fake"); + const fakeStaticClientWrapper = new ConfigurationClientWrapper(fakeEndpoint, new AppConfigurationClient(createMockedConnectionString(fakeEndpoint))); + sinon.stub(fakeStaticClientWrapper.client, "listConfigurationSettings").callsFake(() => { + throw new RestError("Internal Server Error", { statusCode: 500 }); + }); + clients.push(fakeStaticClientWrapper); + + if (!isFailoverable) { + return clients; + } + + const fakeReplicaEndpoint = createMockedEndpoint("fake-replica"); + const fakeDynamicClientWrapper = new ConfigurationClientWrapper(fakeReplicaEndpoint, new AppConfigurationClient(createMockedConnectionString(fakeReplicaEndpoint))); + clients.push(fakeDynamicClientWrapper); + sinon.stub(fakeDynamicClientWrapper.client, "listConfigurationSettings").callsFake((listOptions) => { + const kvs = _filterKVs(pages.flat(), listOptions); + return getMockedIterator(pages, kvs, listOptions); + }); + return clients; }); } @@ -198,6 +230,7 @@ export { sinon, mockAppConfigurationClientListConfigurationSettings, mockAppConfigurationClientGetConfigurationSetting, + mockConfigurationManagerGetClients, mockSecretClientGetSecret, restoreMocks, @@ -210,4 +243,4 @@ export { createMockedFeatureFlag, sleepInMs -}; +}; From 477f18de35107c6ec7ac6d6b9f4df0a32925082d Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> Date: Tue, 19 Nov 2024 01:36:23 +0800 Subject: [PATCH 02/15] add dns module to rollup whitelist (#134) --- rollup.config.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rollup.config.mjs b/rollup.config.mjs index 1cd15df..8ad5164 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -4,7 +4,7 @@ import dts from "rollup-plugin-dts"; export default [ { - external: ["@azure/app-configuration", "@azure/keyvault-secrets", "@azure/core-rest-pipeline", "crypto"], + external: ["@azure/app-configuration", "@azure/keyvault-secrets", "@azure/core-rest-pipeline", "crypto", "dns/promises"], input: "src/index.ts", output: [ { From 2315e4c341b6a6629fc10f6286f38a2cf19c1fbb Mon Sep 17 00:00:00 2001 From: linglingye001 <143174321+linglingye001@users.noreply.github.com> Date: Wed, 27 Nov 2024 14:58:45 +0800 Subject: [PATCH 03/15] Load balance support (#135) * load balance support * improve test --- src/AzureAppConfigurationImpl.ts | 22 ++++++- src/AzureAppConfigurationOptions.ts | 8 +++ src/requestTracing/constants.ts | 3 + src/requestTracing/utils.ts | 7 ++- test/failover.test.ts | 6 +- test/loadBalance.test.ts | 96 +++++++++++++++++++++++++++++ test/utils/testHelper.ts | 15 ++++- 7 files changed, 150 insertions(+), 7 deletions(-) create mode 100644 test/loadBalance.test.ts diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 08f3bb3..bde471e 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -75,9 +75,12 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { #featureFlagRefreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS; #featureFlagRefreshTimer: RefreshTimer; - // selectors + // Selectors #featureFlagSelectors: PagedSettingSelector[] = []; + // Load balancing + #lastSuccessfulEndpoint: string = ""; + constructor( clientManager: ConfigurationClientManager, options: AzureAppConfigurationOptions | undefined, @@ -202,7 +205,21 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } async #executeWithFailoverPolicy(funcToExecute: (client: AppConfigurationClient) => Promise): Promise { - const clientWrappers = await this.#clientManager.getClients(); + let clientWrappers = await this.#clientManager.getClients(); + if (this.#options?.loadBalancingEnabled && this.#lastSuccessfulEndpoint !== "" && clientWrappers.length > 1) { + let nextClientIndex = 0; + // Iterate through clients to find the index of the client with the last successful endpoint + for (const clientWrapper of clientWrappers) { + nextClientIndex++; + if (clientWrapper.endpoint === this.#lastSuccessfulEndpoint) { + break; + } + } + // If we found the last successful client, rotate the list so that the next client is at the beginning + if (nextClientIndex < clientWrappers.length) { + clientWrappers = [...clientWrappers.slice(nextClientIndex), ...clientWrappers.slice(0, nextClientIndex)]; + } + } let successful: boolean; for (const clientWrapper of clientWrappers) { @@ -210,6 +227,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { try { const result = await funcToExecute(clientWrapper.client); this.#isFailoverRequest = false; + this.#lastSuccessfulEndpoint = clientWrapper.endpoint; successful = true; clientWrapper.updateBackoffStatus(successful); return result; diff --git a/src/AzureAppConfigurationOptions.ts b/src/AzureAppConfigurationOptions.ts index 4aa3f99..56b47b5 100644 --- a/src/AzureAppConfigurationOptions.ts +++ b/src/AzureAppConfigurationOptions.ts @@ -55,4 +55,12 @@ export interface AzureAppConfigurationOptions { * If not specified, the default value is true. */ replicaDiscoveryEnabled?: boolean; + + /** + * Specifies whether to enable load balance or not. + * + * @remarks + * If not specified, the default value is false. + */ + loadBalancingEnabled?: boolean; } diff --git a/src/requestTracing/constants.ts b/src/requestTracing/constants.ts index 60dbb81..8b39b63 100644 --- a/src/requestTracing/constants.ts +++ b/src/requestTracing/constants.ts @@ -44,6 +44,9 @@ export enum RequestType { WATCH = "Watch" } +export const FEATURES_KEY = "Features"; + // Tag names export const FAILOVER_REQUEST_TAG = "Failover"; export const KEY_VAULT_CONFIGURED_TAG = "UsesKeyVault"; +export const LOAD_BALANCE_CONFIGURED_TAG = "LB"; diff --git a/src/requestTracing/utils.ts b/src/requestTracing/utils.ts index 8a2fdbc..d48bd5e 100644 --- a/src/requestTracing/utils.ts +++ b/src/requestTracing/utils.ts @@ -20,7 +20,9 @@ import { RequestType, SERVICE_FABRIC_ENV_VAR, CORRELATION_CONTEXT_HEADER_NAME, - FAILOVER_REQUEST_TAG + FAILOVER_REQUEST_TAG, + FEATURES_KEY, + LOAD_BALANCE_CONFIGURED_TAG } from "./constants"; // Utils @@ -84,6 +86,9 @@ export function createCorrelationContextHeader(options: AzureAppConfigurationOpt keyValues.set(REQUEST_TYPE_KEY, isInitialLoadCompleted ? RequestType.WATCH : RequestType.STARTUP); keyValues.set(HOST_TYPE_KEY, getHostType()); keyValues.set(ENV_KEY, isDevEnvironment() ? DEV_ENV_VAL : undefined); + if (options?.loadBalancingEnabled) { + keyValues.set(FEATURES_KEY, LOAD_BALANCE_CONFIGURED_TAG); + } const tags: string[] = []; if (options?.keyVaultOptions) { diff --git a/test/failover.test.ts b/test/failover.test.ts index c97a127..2671a1f 100644 --- a/test/failover.test.ts +++ b/test/failover.test.ts @@ -35,7 +35,7 @@ describe("failover", function () { it("should failover to replica and load key values from config store", async () => { const isFailoverable = true; - mockConfigurationManagerGetClients(isFailoverable, mockedKVs); + mockConfigurationManagerGetClients([], isFailoverable, mockedKVs); const connectionString = createMockedConnectionString(); // replicaDiscoveryEnabled is default to true @@ -47,7 +47,7 @@ describe("failover", function () { it("should failover to replica and load feature flags from config store", async () => { const isFailoverable = true; - mockConfigurationManagerGetClients(isFailoverable, mockedFeatureFlags); + mockConfigurationManagerGetClients([], isFailoverable, mockedFeatureFlags); const connectionString = createMockedConnectionString(); // replicaDiscoveryEnabled is default to true @@ -66,7 +66,7 @@ describe("failover", function () { it("should throw error when all clients failed", async () => { const isFailoverable = false; - mockConfigurationManagerGetClients(isFailoverable); + mockConfigurationManagerGetClients([], isFailoverable); const connectionString = createMockedConnectionString(); return expect(load(connectionString)).eventually.rejectedWith("All clients failed to get configuration settings."); diff --git a/test/loadBalance.test.ts b/test/loadBalance.test.ts new file mode 100644 index 0000000..f0e04b0 --- /dev/null +++ b/test/loadBalance.test.ts @@ -0,0 +1,96 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import * as chai from "chai"; +import * as chaiAsPromised from "chai-as-promised"; +chai.use(chaiAsPromised); +const expect = chai.expect; +import { load } from "./exportedApi.js"; +import { restoreMocks, createMockedConnectionString, sleepInMs, createMockedEndpoint, mockConfigurationManagerGetClients, mockAppConfigurationClientLoadBalanceMode } from "./utils/testHelper.js"; +import { AppConfigurationClient } from "@azure/app-configuration"; +import { ConfigurationClientWrapper } from "../src/ConfigurationClientWrapper.js"; + +const fakeEndpoint_1 = createMockedEndpoint("fake_1"); +const fakeEndpoint_2 = createMockedEndpoint("fake_2"); +const fakeClientWrapper_1 = new ConfigurationClientWrapper(fakeEndpoint_1, new AppConfigurationClient(createMockedConnectionString(fakeEndpoint_1))); +const fakeClientWrapper_2 = new ConfigurationClientWrapper(fakeEndpoint_2, new AppConfigurationClient(createMockedConnectionString(fakeEndpoint_2))); +const clientRequestCounter_1 = {count: 0}; +const clientRequestCounter_2 = {count: 0}; + +describe("load balance", function () { + this.timeout(10000); + + beforeEach(() => { + }); + + afterEach(() => { + restoreMocks(); + }); + + it("should load balance the request when loadBalancingEnabled", async () => { + mockConfigurationManagerGetClients([fakeClientWrapper_1, fakeClientWrapper_2], false); + mockAppConfigurationClientLoadBalanceMode(fakeClientWrapper_1, clientRequestCounter_1); + mockAppConfigurationClientLoadBalanceMode(fakeClientWrapper_2, clientRequestCounter_2); + + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + loadBalancingEnabled: true, + featureFlagOptions: { + enabled: true, + selectors: [{ + keyFilter: "*" + }], + refresh: { + enabled: true, + refreshIntervalInMs: 2000 // 2 seconds for quick test. + } + } + }); + // one request for key values, one request for feature flags + expect(clientRequestCounter_1.count).eq(1); + expect(clientRequestCounter_2.count).eq(1); + + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + // refresh request for feature flags + expect(clientRequestCounter_1.count).eq(2); + expect(clientRequestCounter_2.count).eq(1); + + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + expect(clientRequestCounter_1.count).eq(2); + expect(clientRequestCounter_2.count).eq(2); + }); + + it("should not load balance the request when loadBalance disabled", async () => { + clientRequestCounter_1.count = 0; + clientRequestCounter_2.count = 0; + mockConfigurationManagerGetClients([fakeClientWrapper_1, fakeClientWrapper_2], false); + mockAppConfigurationClientLoadBalanceMode(fakeClientWrapper_1, clientRequestCounter_1); + mockAppConfigurationClientLoadBalanceMode(fakeClientWrapper_2, clientRequestCounter_2); + + const connectionString = createMockedConnectionString(); + // loadBalancingEnabled is default to false + const settings = await load(connectionString, { + featureFlagOptions: { + enabled: true, + selectors: [{ + keyFilter: "*" + }], + refresh: { + enabled: true, + refreshIntervalInMs: 2000 // 2 seconds for quick test. + } + } + }); + // one request for key values, one request for feature flags + expect(clientRequestCounter_1.count).eq(2); + expect(clientRequestCounter_2.count).eq(0); + + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + // refresh request for feature flags + expect(clientRequestCounter_1.count).eq(3); + expect(clientRequestCounter_2.count).eq(0); + }); +}); diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index 261b9b5..9284f87 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -100,9 +100,21 @@ function mockAppConfigurationClientListConfigurationSettings(...pages: Configura }); } -function mockConfigurationManagerGetClients(isFailoverable: boolean, ...pages: ConfigurationSetting[][]) { +function mockAppConfigurationClientLoadBalanceMode(clientWrapper: ConfigurationClientWrapper, countObject: { count: number }) { + const emptyPages: ConfigurationSetting[][] = []; + sinon.stub(clientWrapper.client, "listConfigurationSettings").callsFake((listOptions) => { + countObject.count += 1; + const kvs = _filterKVs(emptyPages.flat(), listOptions); + return getMockedIterator(emptyPages, kvs, listOptions); + }); +} + +function mockConfigurationManagerGetClients(fakeClientWrappers: ConfigurationClientWrapper[], isFailoverable: boolean, ...pages: ConfigurationSetting[][]) { // Stub the getClients method on the class prototype sinon.stub(ConfigurationClientManager.prototype, "getClients").callsFake(async () => { + if (fakeClientWrappers?.length > 0) { + return fakeClientWrappers; + } const clients: ConfigurationClientWrapper[] = []; const fakeEndpoint = createMockedEndpoint("fake"); const fakeStaticClientWrapper = new ConfigurationClientWrapper(fakeEndpoint, new AppConfigurationClient(createMockedConnectionString(fakeEndpoint))); @@ -230,6 +242,7 @@ export { sinon, mockAppConfigurationClientListConfigurationSettings, mockAppConfigurationClientGetConfigurationSetting, + mockAppConfigurationClientLoadBalanceMode, mockConfigurationManagerGetClients, mockSecretClientGetSecret, restoreMocks, From 3f2ffeb1abd7cad8a19a2a7006f5febf7b19f60a Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> Date: Mon, 2 Dec 2024 16:52:41 +0800 Subject: [PATCH 04/15] Avoid multiple refresh (#136) * add lock for refresh operation * bind context * fix lint * update * simplify * add more testcases * add more comments --- package.json | 2 +- src/AzureAppConfigurationImpl.ts | 14 +++++ test/featureFlag.test.ts | 2 +- test/json.test.ts | 8 +-- test/keyvault.test.ts | 2 +- test/load.test.ts | 2 +- test/refresh.test.ts | 98 ++++++++++++++++++++++++++++---- test/requestTracing.test.ts | 4 +- test/utils/testHelper.ts | 15 +++-- 9 files changed, 123 insertions(+), 24 deletions(-) diff --git a/package.json b/package.json index 0b493ea..b194a72 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "dev": "rollup --config --watch", "lint": "eslint src/ test/", "fix-lint": "eslint src/ test/ --fix", - "test": "mocha out/test/*.test.{js,cjs,mjs} --parallel" + "test": "mocha out/test/refresh.test.{js,cjs,mjs} --parallel" }, "repository": { "type": "git", diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 1bbf077..1541c40 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -40,6 +40,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { #isInitialLoadCompleted: boolean = false; // Refresh + #refreshInProgress: boolean = false; + #refreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS; #onRefreshListeners: Array<() => any> = []; /** @@ -350,6 +352,18 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { throw new Error("Refresh is not enabled for key-values or feature flags."); } + if (this.#refreshInProgress) { + return; + } + this.#refreshInProgress = true; + try { + await this.#refreshTasks(); + } finally { + this.#refreshInProgress = false; + } + } + + async #refreshTasks(): Promise { const refreshTasks: Promise[] = []; if (this.#refreshEnabled) { refreshTasks.push(this.#refreshKeyValues()); diff --git a/test/featureFlag.test.ts b/test/featureFlag.test.ts index 1bf8eae..897efe9 100644 --- a/test/featureFlag.test.ts +++ b/test/featureFlag.test.ts @@ -60,7 +60,7 @@ describe("feature flags", function () { this.timeout(10000); before(() => { - mockAppConfigurationClientListConfigurationSettings(mockedKVs); + mockAppConfigurationClientListConfigurationSettings([mockedKVs]); }); after(() => { diff --git a/test/json.test.ts b/test/json.test.ts index c139d53..47a3f67 100644 --- a/test/json.test.ts +++ b/test/json.test.ts @@ -20,7 +20,7 @@ describe("json", function () { }); it("should load and parse if content type is application/json", async () => { - mockAppConfigurationClientListConfigurationSettings([jsonKeyValue]); + mockAppConfigurationClientListConfigurationSettings([[jsonKeyValue]]); const connectionString = createMockedConnectionString(); const settings = await load(connectionString); @@ -34,7 +34,7 @@ describe("json", function () { }); it("should not parse key-vault reference", async () => { - mockAppConfigurationClientListConfigurationSettings([jsonKeyValue, keyVaultKeyValue]); + mockAppConfigurationClientListConfigurationSettings([[jsonKeyValue, keyVaultKeyValue]]); const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { @@ -50,7 +50,7 @@ describe("json", function () { }); it("should parse different kinds of legal values", async () => { - mockAppConfigurationClientListConfigurationSettings([ + mockAppConfigurationClientListConfigurationSettings([[ /** * A JSON value MUST be an object, array, number, or string, false, null, true * See https://www.ietf.org/rfc/rfc4627.txt @@ -69,7 +69,7 @@ describe("json", function () { createMockedJsonKeyValue("json.settings.emptyString", ""), // should fail JSON.parse and use string value as fallback createMockedJsonKeyValue("json.settings.illegalString", "[unclosed"), // should fail JSON.parse - ]); + ]]); const connectionString = createMockedConnectionString(); const settings = await load(connectionString); expect(settings).not.undefined; diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index cf01235..2877243 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -19,7 +19,7 @@ const mockedData = [ function mockAppConfigurationClient() { // eslint-disable-next-line @typescript-eslint/no-unused-vars const kvs = mockedData.map(([key, vaultUri, _value]) => createMockedKeyVaultReference(key, vaultUri)); - mockAppConfigurationClientListConfigurationSettings(kvs); + mockAppConfigurationClientListConfigurationSettings([kvs]); } function mockNewlyCreatedKeyVaultSecretClients() { diff --git a/test/load.test.ts b/test/load.test.ts index ce22b1a..6d2c94b 100644 --- a/test/load.test.ts +++ b/test/load.test.ts @@ -80,7 +80,7 @@ describe("load", function () { this.timeout(10000); before(() => { - mockAppConfigurationClientListConfigurationSettings(mockedKVs); + mockAppConfigurationClientListConfigurationSettings([mockedKVs]); }); after(() => { diff --git a/test/refresh.test.ts b/test/refresh.test.ts index fcffaee..5fbeb97 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -23,6 +23,15 @@ function addSetting(key: string, value: any) { mockedKVs.push(createMockedKeyValue({ key, value })); } +let listKvRequestCount = 0; +const listKvCallback = () => { + listKvRequestCount++; +}; +let getKvRequestCount = 0; +const getKvCallback = () => { + getKvRequestCount++; +}; + describe("dynamic refresh", function () { this.timeout(10000); @@ -32,12 +41,14 @@ describe("dynamic refresh", function () { { value: "40", key: "app.settings.fontSize" }, { value: "30", key: "app.settings.fontSize", label: "prod" } ].map(createMockedKeyValue); - mockAppConfigurationClientListConfigurationSettings(mockedKVs); - mockAppConfigurationClientGetConfigurationSetting(mockedKVs); + mockAppConfigurationClientListConfigurationSettings([mockedKVs], listKvCallback); + mockAppConfigurationClientGetConfigurationSetting(mockedKVs, getKvCallback); }); afterEach(() => { restoreMocks(); + listKvRequestCount = 0; + getKvRequestCount = 0; }); it("should throw error when refresh is not enabled but refresh is called", async () => { @@ -139,6 +150,8 @@ describe("dynamic refresh", function () { ] } }); + expect(listKvRequestCount).eq(1); + expect(getKvRequestCount).eq(0); expect(settings).not.undefined; expect(settings.get("app.settings.fontColor")).eq("red"); expect(settings.get("app.settings.fontSize")).eq("40"); @@ -149,10 +162,14 @@ describe("dynamic refresh", function () { // within refreshInterval, should not really refresh await settings.refresh(); expect(settings.get("app.settings.fontColor")).eq("red"); + expect(listKvRequestCount).eq(1); // no more request should be sent during the refresh interval + expect(getKvRequestCount).eq(0); // no more request should be sent during the refresh interval // after refreshInterval, should really refresh await sleepInMs(2 * 1000 + 1); await settings.refresh(); + expect(listKvRequestCount).eq(2); + expect(getKvRequestCount).eq(1); expect(settings.get("app.settings.fontColor")).eq("blue"); }); @@ -167,6 +184,8 @@ describe("dynamic refresh", function () { ] } }); + expect(listKvRequestCount).eq(1); + expect(getKvRequestCount).eq(0); expect(settings).not.undefined; expect(settings.get("app.settings.fontColor")).eq("red"); expect(settings.get("app.settings.fontSize")).eq("40"); @@ -174,11 +193,13 @@ describe("dynamic refresh", function () { // delete setting 'app.settings.fontColor' const newMockedKVs = mockedKVs.filter(elem => elem.key !== "app.settings.fontColor"); restoreMocks(); - mockAppConfigurationClientListConfigurationSettings(newMockedKVs); - mockAppConfigurationClientGetConfigurationSetting(newMockedKVs); + mockAppConfigurationClientListConfigurationSettings([newMockedKVs], listKvCallback); + mockAppConfigurationClientGetConfigurationSetting(newMockedKVs, getKvCallback); await sleepInMs(2 * 1000 + 1); await settings.refresh(); + expect(listKvRequestCount).eq(2); + expect(getKvRequestCount).eq(2); // one conditional request to detect change and one request as part of loading all kvs (because app.settings.fontColor doesn't exist in the response of listKv request) expect(settings.get("app.settings.fontColor")).eq(undefined); }); @@ -193,6 +214,8 @@ describe("dynamic refresh", function () { ] } }); + expect(listKvRequestCount).eq(1); + expect(getKvRequestCount).eq(0); expect(settings).not.undefined; expect(settings.get("app.settings.fontColor")).eq("red"); expect(settings.get("app.settings.fontSize")).eq("40"); @@ -200,6 +223,8 @@ describe("dynamic refresh", function () { updateSetting("app.settings.fontSize", "50"); // unwatched setting await sleepInMs(2 * 1000 + 1); await settings.refresh(); + expect(listKvRequestCount).eq(1); + expect(getKvRequestCount).eq(1); expect(settings.get("app.settings.fontSize")).eq("40"); }); @@ -215,6 +240,8 @@ describe("dynamic refresh", function () { ] } }); + expect(listKvRequestCount).eq(1); + expect(getKvRequestCount).eq(0); expect(settings).not.undefined; expect(settings.get("app.settings.fontColor")).eq("red"); expect(settings.get("app.settings.fontSize")).eq("40"); @@ -224,6 +251,8 @@ describe("dynamic refresh", function () { updateSetting("app.settings.fontSize", "50"); await sleepInMs(2 * 1000 + 1); await settings.refresh(); + expect(listKvRequestCount).eq(2); + expect(getKvRequestCount).eq(2); // two getKv request for two watched settings expect(settings.get("app.settings.fontSize")).eq("50"); expect(settings.get("app.settings.bgColor")).eq("white"); }); @@ -309,6 +338,8 @@ describe("dynamic refresh", function () { ] } }); + expect(listKvRequestCount).eq(1); + expect(getKvRequestCount).eq(1); // app.settings.bgColor doesn't exist in the response of listKv request, so an additional getKv request is made to get it. expect(settings).not.undefined; expect(settings.get("app.settings.fontColor")).eq("red"); expect(settings.get("app.settings.fontSize")).eq("40"); @@ -317,10 +348,45 @@ describe("dynamic refresh", function () { updateSetting("app.settings.fontColor", "blue"); await sleepInMs(2 * 1000 + 1); await settings.refresh(); + expect(listKvRequestCount).eq(1); + expect(getKvRequestCount).eq(2); // should not refresh expect(settings.get("app.settings.fontColor")).eq("red"); }); + it("should not refresh any more when there is refresh in progress", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + refreshOptions: { + enabled: true, + refreshIntervalInMs: 2000, + watchedSettings: [ + { key: "app.settings.fontColor" } + ] + } + }); + expect(listKvRequestCount).eq(1); + expect(getKvRequestCount).eq(0); + expect(settings).not.undefined; + expect(settings.get("app.settings.fontColor")).eq("red"); + + // change setting + updateSetting("app.settings.fontColor", "blue"); + + // after refreshInterval, should really refresh + await sleepInMs(2 * 1000 + 1); + for (let i = 0; i < 5; i++) { // in practice, refresh should not be used in this way + settings.refresh(); // refresh "concurrently" + } + expect(listKvRequestCount).to.be.at.most(2); + expect(getKvRequestCount).to.be.at.most(1); + + await sleepInMs(1000); // wait for all 5 refresh attempts to finish + + expect(listKvRequestCount).eq(2); + expect(getKvRequestCount).eq(1); + expect(settings.get("app.settings.fontColor")).eq("blue"); + }); }); describe("dynamic refresh feature flags", function () { @@ -331,14 +397,16 @@ describe("dynamic refresh feature flags", function () { afterEach(() => { restoreMocks(); + listKvRequestCount = 0; + getKvRequestCount = 0; }); it("should refresh feature flags when enabled", async () => { mockedKVs = [ createMockedFeatureFlag("Beta", { enabled: true }) ]; - mockAppConfigurationClientListConfigurationSettings(mockedKVs); - mockAppConfigurationClientGetConfigurationSetting(mockedKVs); + mockAppConfigurationClientListConfigurationSettings([mockedKVs], listKvCallback); + mockAppConfigurationClientGetConfigurationSetting(mockedKVs, getKvCallback); const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { @@ -353,6 +421,8 @@ describe("dynamic refresh feature flags", function () { } } }); + expect(listKvRequestCount).eq(2); // one listKv request for kvs and one listKv request for feature flags + expect(getKvRequestCount).eq(0); expect(settings).not.undefined; expect(settings.get("feature_management")).not.undefined; expect(settings.get("feature_management").feature_flags).not.undefined; @@ -371,6 +441,8 @@ describe("dynamic refresh feature flags", function () { await sleepInMs(2 * 1000 + 1); await settings.refresh(); + expect(listKvRequestCount).eq(4); // 2 + 2 more requests: one conditional request to detect change and one request to reload all feature flags + expect(getKvRequestCount).eq(0); expect(settings.get("feature_management").feature_flags[0].id).eq("Beta"); expect(settings.get("feature_management").feature_flags[0].enabled).eq(false); @@ -387,8 +459,8 @@ describe("dynamic refresh feature flags", function () { createMockedFeatureFlag("Beta_1", { enabled: true }), createMockedFeatureFlag("Beta_2", { enabled: true }), ]; - mockAppConfigurationClientListConfigurationSettings(page1, page2); - mockAppConfigurationClientGetConfigurationSetting([...page1, ...page2]); + mockAppConfigurationClientListConfigurationSettings([page1, page2], listKvCallback); + mockAppConfigurationClientGetConfigurationSetting([...page1, ...page2], getKvCallback); const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { @@ -403,6 +475,8 @@ describe("dynamic refresh feature flags", function () { } } }); + expect(listKvRequestCount).eq(2); + expect(getKvRequestCount).eq(0); let refreshSuccessfulCount = 0; settings.onRefresh(() => { @@ -411,16 +485,20 @@ describe("dynamic refresh feature flags", function () { await sleepInMs(2 * 1000 + 1); await settings.refresh(); + expect(listKvRequestCount).eq(3); // one conditional request to detect change + expect(getKvRequestCount).eq(0); expect(refreshSuccessfulCount).eq(0); // no change in feature flags, because page etags are the same. // change feature flag Beta_1 to false page2[0] = createMockedFeatureFlag("Beta_1", { enabled: false }); restoreMocks(); - mockAppConfigurationClientListConfigurationSettings(page1, page2); - mockAppConfigurationClientGetConfigurationSetting([...page1, ...page2]); + mockAppConfigurationClientListConfigurationSettings([page1, page2], listKvCallback); + mockAppConfigurationClientGetConfigurationSetting([...page1, ...page2], getKvCallback); await sleepInMs(2 * 1000 + 1); await settings.refresh(); + expect(listKvRequestCount).eq(5); // 3 + 2 more requests: one conditional request to detect change and one request to reload all feature flags + expect(getKvRequestCount).eq(0); expect(refreshSuccessfulCount).eq(1); // change in feature flags, because page etags are different. }); }); diff --git a/test/requestTracing.test.ts b/test/requestTracing.test.ts index d4e7edc..a64d2c4 100644 --- a/test/requestTracing.test.ts +++ b/test/requestTracing.test.ts @@ -123,10 +123,10 @@ describe("request tracing", function () { }); it("should have request type in correlation-context header when refresh is enabled", async () => { - mockAppConfigurationClientListConfigurationSettings([{ + mockAppConfigurationClientListConfigurationSettings([[{ key: "app.settings.fontColor", value: "red" - }].map(createMockedKeyValue)); + }].map(createMockedKeyValue)]); const settings = await load(createMockedConnectionString(fakeEndpoint), { clientOptions, diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index 6e787dd..bf05882 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -42,13 +42,16 @@ function _filterKVs(unfilteredKvs: ConfigurationSetting[], listOptions: any) { * Mocks the listConfigurationSettings method of AppConfigurationClient to return the provided pages of ConfigurationSetting. * E.g. * - mockAppConfigurationClientListConfigurationSettings([item1, item2, item3]) // single page - * - mockAppConfigurationClientListConfigurationSettings([item1, item2], [item3], [item4]) // multiple pages * * @param pages List of pages, each page is a list of ConfigurationSetting */ -function mockAppConfigurationClientListConfigurationSettings(...pages: ConfigurationSetting[][]) { +function mockAppConfigurationClientListConfigurationSettings(pages: ConfigurationSetting[][], customCallback?: (listOptions) => any) { sinon.stub(AppConfigurationClient.prototype, "listConfigurationSettings").callsFake((listOptions) => { + if (customCallback) { + customCallback(listOptions); + } + let kvs = _filterKVs(pages.flat(), listOptions); const mockIterator: AsyncIterableIterator & { byPage(): AsyncIterableIterator } = { [Symbol.asyncIterator](): AsyncIterableIterator { @@ -94,8 +97,12 @@ function mockAppConfigurationClientListConfigurationSettings(...pages: Configura }); } -function mockAppConfigurationClientGetConfigurationSetting(kvList) { +function mockAppConfigurationClientGetConfigurationSetting(kvList, customCallback?: (options) => any) { sinon.stub(AppConfigurationClient.prototype, "getConfigurationSetting").callsFake((settingId, options) => { + if (customCallback) { + customCallback(options); + } + const found = kvList.find(elem => elem.key === settingId.key && elem.label === settingId.label); if (found) { if (options?.onlyIfChanged && settingId.etag === found.etag) { @@ -210,4 +217,4 @@ export { createMockedFeatureFlag, sleepInMs -}; +}; From ad0290b98ead61165d376486053054de1a7103bb Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> Date: Mon, 2 Dec 2024 17:10:04 +0800 Subject: [PATCH 05/15] small fix (#138) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index b194a72..0b493ea 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "dev": "rollup --config --watch", "lint": "eslint src/ test/", "fix-lint": "eslint src/ test/ --fix", - "test": "mocha out/test/refresh.test.{js,cjs,mjs} --parallel" + "test": "mocha out/test/*.test.{js,cjs,mjs} --parallel" }, "repository": { "type": "git", From 9300106b37781ded6be9ca81fe52570734a6bd30 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang Date: Thu, 12 Dec 2024 15:55:54 +0800 Subject: [PATCH 06/15] add replica count tracing --- package-lock.json | 2 +- src/AzureAppConfigurationImpl.ts | 9 ++-- src/ConfigurationClientManager.ts | 8 +++- src/requestTracing/constants.ts | 10 +++-- src/requestTracing/utils.ts | 70 +++++++++++++++++-------------- 5 files changed, 57 insertions(+), 42 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7cc0a77..e3a0c07 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6,7 +6,7 @@ "packages": { "": { "name": "@azure/app-configuration-provider", - "version": "1.1.2", + "version": "2.0.0-preview.1", "license": "MIT", "dependencies": { "@azure/app-configuration": "^1.6.1", diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index d709a80..5a66cfb 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -33,7 +33,7 @@ import { } from "./featureManagement/constants.js"; import { AzureKeyVaultKeyValueAdapter } from "./keyvault/AzureKeyVaultKeyValueAdapter.js"; import { RefreshTimer } from "./refresh/RefreshTimer.js"; -import { getConfigurationSettingWithTrace, listConfigurationSettingsWithTrace, requestTracingEnabled } from "./requestTracing/utils.js"; +import { RequestTracingOptions, getConfigurationSettingWithTrace, listConfigurationSettingsWithTrace, requestTracingEnabled } from "./requestTracing/utils.js"; import { KeyFilter, LabelFilter, SettingSelector } from "./types.js"; import { ConfigurationClientManager } from "./ConfigurationClientManager.js"; @@ -197,11 +197,12 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return this.#featureFlagEnabled && !!this.#options?.featureFlagOptions?.refresh?.enabled; } - get #requestTraceOptions() { + get #requestTraceOptions(): RequestTracingOptions { return { - requestTracingEnabled: this.#requestTracingEnabled, - initialLoadCompleted: this.#isInitialLoadCompleted, + enabled: this.#requestTracingEnabled, appConfigOptions: this.#options, + initialLoadCompleted: this.#isInitialLoadCompleted, + replicaCount: this.#clientManager.getReplicaCount(), isFailoverRequest: this.#isFailoverRequest }; } diff --git a/src/ConfigurationClientManager.ts b/src/ConfigurationClientManager.ts index 59e03aa..2421a78 100644 --- a/src/ConfigurationClientManager.ts +++ b/src/ConfigurationClientManager.ts @@ -30,7 +30,7 @@ export class ConfigurationClientManager { #clientOptions: AppConfigurationClientOptions | undefined; #appConfigOptions: AzureAppConfigurationOptions | undefined; #validDomain: string; - #staticClients: ConfigurationClientWrapper[]; + #staticClients: ConfigurationClientWrapper[]; // there should always be only one static client #dynamicClients: ConfigurationClientWrapper[]; #lastFallbackClientRefreshTime: number = 0; #lastFallbackClientRefreshAttempt: number = 0; @@ -96,7 +96,11 @@ export class ConfigurationClientManager { this.#isFailoverable = true; } - async getClients() : Promise { + getReplicaCount(): number { + return this.#dynamicClients.length; + } + + async getClients(): Promise { if (!this.#isFailoverable) { return this.#staticClients; } diff --git a/src/requestTracing/constants.ts b/src/requestTracing/constants.ts index 8b39b63..30a12f4 100644 --- a/src/requestTracing/constants.ts +++ b/src/requestTracing/constants.ts @@ -37,16 +37,20 @@ export const CONTAINER_APP_ENV_VAR = "CONTAINER_APP_NAME"; export const KUBERNETES_ENV_VAR = "KUBERNETES_PORT"; export const SERVICE_FABRIC_ENV_VAR = "Fabric_NodeName"; // See: https://docs.microsoft.com/en-us/azure/service-fabric/service-fabric-environment-variables-reference -// Request Type +// Request type export const REQUEST_TYPE_KEY = "RequestType"; export enum RequestType { STARTUP = "Startup", WATCH = "Watch" } -export const FEATURES_KEY = "Features"; +// Replica count +export const REPLICA_COUNT_KEY = "ReplicaCount"; // Tag names -export const FAILOVER_REQUEST_TAG = "Failover"; export const KEY_VAULT_CONFIGURED_TAG = "UsesKeyVault"; +export const FAILOVER_REQUEST_TAG = "Failover"; + +// Compact feature tags +export const FEATURES_KEY = "Features"; export const LOAD_BALANCE_CONFIGURED_TAG = "LB"; diff --git a/src/requestTracing/utils.ts b/src/requestTracing/utils.ts index d48bd5e..23ec460 100644 --- a/src/requestTracing/utils.ts +++ b/src/requestTracing/utils.ts @@ -20,29 +20,31 @@ import { RequestType, SERVICE_FABRIC_ENV_VAR, CORRELATION_CONTEXT_HEADER_NAME, + REPLICA_COUNT_KEY, FAILOVER_REQUEST_TAG, FEATURES_KEY, LOAD_BALANCE_CONFIGURED_TAG } from "./constants"; +export interface RequestTracingOptions { + enabled: boolean; + appConfigOptions: AzureAppConfigurationOptions | undefined; + initialLoadCompleted: boolean; + replicaCount: number; + isFailoverRequest: boolean; +} + // Utils export function listConfigurationSettingsWithTrace( - requestTracingOptions: { - requestTracingEnabled: boolean; - initialLoadCompleted: boolean; - appConfigOptions: AzureAppConfigurationOptions | undefined; - isFailoverRequest: boolean; - }, + requestTracingOptions: RequestTracingOptions, client: AppConfigurationClient, listOptions: ListConfigurationSettingsOptions ) { - const { requestTracingEnabled, initialLoadCompleted, appConfigOptions, isFailoverRequest } = requestTracingOptions; - const actualListOptions = { ...listOptions }; - if (requestTracingEnabled) { + if (requestTracingOptions.enabled) { actualListOptions.requestOptions = { customHeaders: { - [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(appConfigOptions, initialLoadCompleted, isFailoverRequest) + [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(requestTracingOptions) } }; } @@ -51,23 +53,17 @@ export function listConfigurationSettingsWithTrace( } export function getConfigurationSettingWithTrace( - requestTracingOptions: { - requestTracingEnabled: boolean; - initialLoadCompleted: boolean; - appConfigOptions: AzureAppConfigurationOptions | undefined; - isFailoverRequest: boolean; - }, + requestTracingOptions: RequestTracingOptions, client: AppConfigurationClient, configurationSettingId: ConfigurationSettingId, getOptions?: GetConfigurationSettingOptions, ) { - const { requestTracingEnabled, initialLoadCompleted, appConfigOptions, isFailoverRequest } = requestTracingOptions; const actualGetOptions = { ...getOptions }; - if (requestTracingEnabled) { + if (requestTracingOptions.enabled) { actualGetOptions.requestOptions = { customHeaders: { - [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(appConfigOptions, initialLoadCompleted, isFailoverRequest) + [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(requestTracingOptions) } }; } @@ -75,29 +71,43 @@ export function getConfigurationSettingWithTrace( return client.getConfigurationSetting(configurationSettingId, actualGetOptions); } -export function createCorrelationContextHeader(options: AzureAppConfigurationOptions | undefined, isInitialLoadCompleted: boolean, isFailoverRequest: boolean): string { +export function createCorrelationContextHeader(requestTracingOptions: RequestTracingOptions): string { /* RequestType: 'Startup' during application starting up, 'Watch' after startup completed. Host: identify with defined envs - Env: identify by env `NODE_ENV` which is a popular but not standard.usually the value can be "development", "production". + Env: identify by env `NODE_ENV` which is a popular but not standard. Usually, the value can be "development", "production". + ReplicaCount: identify how many replicas are found + Features: LB UsersKeyVault + Failover */ const keyValues = new Map(); - keyValues.set(REQUEST_TYPE_KEY, isInitialLoadCompleted ? RequestType.WATCH : RequestType.STARTUP); + const tags: string[] = []; + + keyValues.set(REQUEST_TYPE_KEY, requestTracingOptions.initialLoadCompleted ? RequestType.WATCH : RequestType.STARTUP); keyValues.set(HOST_TYPE_KEY, getHostType()); keyValues.set(ENV_KEY, isDevEnvironment() ? DEV_ENV_VAL : undefined); - if (options?.loadBalancingEnabled) { - keyValues.set(FEATURES_KEY, LOAD_BALANCE_CONFIGURED_TAG); - } - const tags: string[] = []; - if (options?.keyVaultOptions) { - const { credential, secretClients, secretResolver } = options.keyVaultOptions; + const appConfigOptions = requestTracingOptions.appConfigOptions; + if (appConfigOptions?.keyVaultOptions) { + const { credential, secretClients, secretResolver } = appConfigOptions.keyVaultOptions; if (credential !== undefined || secretClients?.length || secretResolver !== undefined) { tags.push(KEY_VAULT_CONFIGURED_TAG); } } + if (requestTracingOptions.isFailoverRequest) { + tags.push(FAILOVER_REQUEST_TAG); + } + if (requestTracingOptions.replicaCount > 0) { + keyValues.set(REPLICA_COUNT_KEY, requestTracingOptions.replicaCount.toString()); + } + + // Compact tags: Features=LB+... + if (appConfigOptions?.loadBalancingEnabled) { + keyValues.set(FEATURES_KEY, LOAD_BALANCE_CONFIGURED_TAG); + } + const contextParts: string[] = []; for (const [k, v] of keyValues) { if (v !== undefined) { @@ -108,10 +118,6 @@ export function createCorrelationContextHeader(options: AzureAppConfigurationOpt contextParts.push(tag); } - if (isFailoverRequest) { - contextParts.push(FAILOVER_REQUEST_TAG); - } - return contextParts.join(","); } From 9a38443ff44ffc2c7eba4f829a3f965d0644a25a Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 13 Dec 2024 13:36:25 +0800 Subject: [PATCH 07/15] fix bug --- src/ConfigurationClientManager.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ConfigurationClientManager.ts b/src/ConfigurationClientManager.ts index 2421a78..b11e935 100644 --- a/src/ConfigurationClientManager.ts +++ b/src/ConfigurationClientManager.ts @@ -32,6 +32,7 @@ export class ConfigurationClientManager { #validDomain: string; #staticClients: ConfigurationClientWrapper[]; // there should always be only one static client #dynamicClients: ConfigurationClientWrapper[]; + #replicaCount: number = 0; #lastFallbackClientRefreshTime: number = 0; #lastFallbackClientRefreshAttempt: number = 0; @@ -97,7 +98,7 @@ export class ConfigurationClientManager { } getReplicaCount(): number { - return this.#dynamicClients.length; + return this.#replicaCount; } async getClients(): Promise { @@ -165,6 +166,7 @@ export class ConfigurationClientManager { this.#dynamicClients = newDynamicClients; this.#lastFallbackClientRefreshTime = Date.now(); + this.#replicaCount = this.#dynamicClients.length; } /** From ddf3d6017420ec3dcb0b5ebebc3947dd9e412cc7 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 13 Dec 2024 13:45:03 +0800 Subject: [PATCH 08/15] audit vulnerablitiy --- package-lock.json | 173 +++++++++++++++++++++++----------------------- 1 file changed, 87 insertions(+), 86 deletions(-) diff --git a/package-lock.json b/package-lock.json index e3a0c07..44b53a0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1138,10 +1138,11 @@ } }, "node_modules/ansi-colors": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/ansi-colors/-/ansi-colors-4.1.1.tgz", - "integrity": "sha512-JoX0apGbHaUJBNl6yF+p6JAFYZ666/hhCGKN5t9QFjbJQKUU/g8MNbFDbvfrgKXvI1QpZplPOnwIo99lX/AAmA==", + "version": "4.1.3", + "resolved": "https://registry.npmjs.org/ansi-colors/-/ansi-colors-4.1.3.tgz", + "integrity": "sha512-/6w/C21Pm1A7aZitlI5Ni/2J6FFQN8i1Cvz3kHABAAbw93v/NlvKdVOqz7CCWz/3iv/JplRSEEZ83XION15ovw==", "dev": true, + "license": "MIT", "engines": { "node": ">=6" } @@ -1425,10 +1426,11 @@ "dev": true }, "node_modules/cross-spawn": { - "version": "7.0.3", - "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz", - "integrity": "sha512-iRDPJKUPVEND7dHPO8rkbOnPpyDygcDFtWjpeWNCgy8WP2rXcxXL8TskReQl6OrB2G7+UJrags1q15Fudc7G6w==", + "version": "7.0.6", + "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.6.tgz", + "integrity": "sha512-uV2QOWP2nWzsy2aMp8aRibhi9dlzF5Hgh5SHaB9OiTGEyDTiJJyx0uy51QXdyWbtAHNua4XJzUKca3OzKUd3vA==", "dev": true, + "license": "MIT", "dependencies": { "path-key": "^3.1.0", "shebang-command": "^2.0.0", @@ -1439,11 +1441,12 @@ } }, "node_modules/debug": { - "version": "4.3.4", - "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.4.tgz", - "integrity": "sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ==", + "version": "4.4.0", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.4.0.tgz", + "integrity": "sha512-6WTZ/IxCY/T6BALoZHaE4ctp9xm+Z5kY/pzYaCHRFeyVhojxlrm+46y68HA6hr0TcwEssoxNiDEUJQjfPZ/RYA==", + "license": "MIT", "dependencies": { - "ms": "2.1.2" + "ms": "^2.1.3" }, "engines": { "node": ">=6.0" @@ -1501,10 +1504,11 @@ } }, "node_modules/diff": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/diff/-/diff-5.0.0.tgz", - "integrity": "sha512-/VTCrvm5Z0JGty/BWHljh+BAiw3IK+2j87NGMu8Nwc/f48WoDAC395uomO9ZD117ZOBaHmkX1oyLvkVM/aIT3w==", + "version": "5.2.0", + "resolved": "https://registry.npmjs.org/diff/-/diff-5.2.0.tgz", + "integrity": "sha512-uIFDxqpRZGZ6ThOk84hEfqWoHx2devRFvpTZcTHur85vImfaxUbTW9Ryh4CpCuDnToOP1CEtXKIgytHBPVff5A==", "dev": true, + "license": "BSD-3-Clause", "engines": { "node": ">=0.3.1" } @@ -2574,32 +2578,32 @@ } }, "node_modules/mocha": { - "version": "10.2.0", - "resolved": "https://registry.npmjs.org/mocha/-/mocha-10.2.0.tgz", - "integrity": "sha512-IDY7fl/BecMwFHzoqF2sg/SHHANeBoMMXFlS9r0OXKDssYE1M5O43wUY/9BVPeIvfH2zmEbBfseqN9gBQZzXkg==", - "dev": true, - "dependencies": { - "ansi-colors": "4.1.1", - "browser-stdout": "1.3.1", - "chokidar": "3.5.3", - "debug": "4.3.4", - "diff": "5.0.0", - "escape-string-regexp": "4.0.0", - "find-up": "5.0.0", - "glob": "7.2.0", - "he": "1.2.0", - "js-yaml": "4.1.0", - "log-symbols": "4.1.0", - "minimatch": "5.0.1", - "ms": "2.1.3", - "nanoid": "3.3.3", - "serialize-javascript": "6.0.0", - "strip-json-comments": "3.1.1", - "supports-color": "8.1.1", - "workerpool": "6.2.1", - "yargs": "16.2.0", - "yargs-parser": "20.2.4", - "yargs-unparser": "2.0.0" + "version": "10.8.2", + "resolved": "https://registry.npmjs.org/mocha/-/mocha-10.8.2.tgz", + "integrity": "sha512-VZlYo/WE8t1tstuRmqgeyBgCbJc/lEdopaa+axcKzTBJ+UIdlAB9XnmvTCAH4pwR4ElNInaedhEBmZD8iCSVEg==", + "dev": true, + "license": "MIT", + "dependencies": { + "ansi-colors": "^4.1.3", + "browser-stdout": "^1.3.1", + "chokidar": "^3.5.3", + "debug": "^4.3.5", + "diff": "^5.2.0", + "escape-string-regexp": "^4.0.0", + "find-up": "^5.0.0", + "glob": "^8.1.0", + "he": "^1.2.0", + "js-yaml": "^4.1.0", + "log-symbols": "^4.1.0", + "minimatch": "^5.1.6", + "ms": "^2.1.3", + "serialize-javascript": "^6.0.2", + "strip-json-comments": "^3.1.1", + "supports-color": "^8.1.1", + "workerpool": "^6.5.1", + "yargs": "^16.2.0", + "yargs-parser": "^20.2.9", + "yargs-unparser": "^2.0.0" }, "bin": { "_mocha": "bin/_mocha", @@ -2607,10 +2611,6 @@ }, "engines": { "node": ">= 14.0.0" - }, - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/mochajs" } }, "node_modules/mocha/node_modules/brace-expansion": { @@ -2618,15 +2618,38 @@ "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz", "integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==", "dev": true, + "license": "MIT", "dependencies": { "balanced-match": "^1.0.0" } }, + "node_modules/mocha/node_modules/glob": { + "version": "8.1.0", + "resolved": "https://registry.npmjs.org/glob/-/glob-8.1.0.tgz", + "integrity": "sha512-r8hpEjiQEYlF2QU0df3dS+nxxSIreXQS1qRhMJM0Q5NDdR386C7jb7Hwwod8Fgiuex+k0GFjgft18yvxm5XoCQ==", + "deprecated": "Glob versions prior to v9 are no longer supported", + "dev": true, + "license": "ISC", + "dependencies": { + "fs.realpath": "^1.0.0", + "inflight": "^1.0.4", + "inherits": "2", + "minimatch": "^5.0.1", + "once": "^1.3.0" + }, + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, "node_modules/mocha/node_modules/minimatch": { - "version": "5.0.1", - "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-5.0.1.tgz", - "integrity": "sha512-nLDxIFRyhDblz3qMuq+SoRZED4+miJ/G+tdDrjkkkRnjAsBexeGpgjLEQ0blJy7rHhR2b93rhQY4SvyWu9v03g==", + "version": "5.1.6", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-5.1.6.tgz", + "integrity": "sha512-lKwV/1brpG6mBUFHtb7NUmtABCb2WZZmm2wNiOA5hAb8VdCS4B3dtMWyvcoViccwAW/COERjXLt0zP1zXUN26g==", "dev": true, + "license": "ISC", "dependencies": { "brace-expansion": "^2.0.1" }, @@ -2634,12 +2657,6 @@ "node": ">=10" } }, - "node_modules/mocha/node_modules/ms": { - "version": "2.1.3", - "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", - "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==", - "dev": true - }, "node_modules/mocha/node_modules/supports-color": { "version": "8.1.1", "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-8.1.1.tgz", @@ -2656,21 +2673,10 @@ } }, "node_modules/ms": { - "version": "2.1.2", - "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", - "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==" - }, - "node_modules/nanoid": { - "version": "3.3.3", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.3.tgz", - "integrity": "sha512-p1sjXuopFs0xg+fPASzQ28agW1oHD7xDsd9Xkf3T15H3c/cifrFHVwrh74PdoklAPi+i7MdRsE47vm2r6JoB+w==", - "dev": true, - "bin": { - "nanoid": "bin/nanoid.cjs" - }, - "engines": { - "node": "^10 || ^12 || ^13.7 || ^14 || >=15.0.1" - } + "version": "2.1.3", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", + "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==", + "license": "MIT" }, "node_modules/natural-compare": { "version": "1.4.0", @@ -2957,6 +2963,7 @@ "resolved": "https://registry.npmjs.org/randombytes/-/randombytes-2.1.0.tgz", "integrity": "sha512-vYl3iOX+4CKUWuxGi9Ukhie6fsqXqS9FE2Zaic4tNFD2N2QQaXOMFbuKK4QmDHC0JO6B1Zp41J0LpT0oR68amQ==", "dev": true, + "license": "MIT", "dependencies": { "safe-buffer": "^5.1.0" } @@ -3178,10 +3185,11 @@ } }, "node_modules/serialize-javascript": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/serialize-javascript/-/serialize-javascript-6.0.0.tgz", - "integrity": "sha512-Qr3TosvguFt8ePWqsvRfrKyQXIiW+nGbYpy8XK24NQHE83caxWt+mIymTT19DGFbNWNLfEwsrkSmN64lVWB9ag==", + "version": "6.0.2", + "resolved": "https://registry.npmjs.org/serialize-javascript/-/serialize-javascript-6.0.2.tgz", + "integrity": "sha512-Saa1xPByTTq2gdeFZYLLo+RFE35NHZkAbqZeWNd3BpzppeVisAqpDjcp8dyf6uIvEqJRd46jemmyA4iFIeVk8g==", "dev": true, + "license": "BSD-3-Clause", "dependencies": { "randombytes": "^2.1.0" } @@ -3238,15 +3246,6 @@ "url": "https://opencollective.com/sinon" } }, - "node_modules/sinon/node_modules/diff": { - "version": "5.1.0", - "resolved": "https://registry.npmjs.org/diff/-/diff-5.1.0.tgz", - "integrity": "sha512-D+mk+qE8VC/PAUrlAU34N+VfXev0ghe5ywmpqrawphmVZc1bEfn56uo9qpyGp1p4xpzOHkSW4ztBd6L7Xx4ACw==", - "dev": true, - "engines": { - "node": ">=0.3.1" - } - }, "node_modules/slash": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/slash/-/slash-3.0.0.tgz", @@ -3480,10 +3479,11 @@ } }, "node_modules/workerpool": { - "version": "6.2.1", - "resolved": "https://registry.npmjs.org/workerpool/-/workerpool-6.2.1.tgz", - "integrity": "sha512-ILEIE97kDZvF9Wb9f6h5aXK4swSlKGUcOEGiIYb2OOu/IrDU9iwj0fD//SsA6E5ibwJxpEvhullJY4Sl4GcpAw==", - "dev": true + "version": "6.5.1", + "resolved": "https://registry.npmjs.org/workerpool/-/workerpool-6.5.1.tgz", + "integrity": "sha512-Fs4dNYcsdpYSAfVxhnl1L5zTksjvOJxtC5hzMNl+1t9B8hTJTdKDyZ5ju7ztgPy+ft9tBFXoOlDNiOT9WUXZlA==", + "dev": true, + "license": "Apache-2.0" }, "node_modules/wrap-ansi": { "version": "7.0.0", @@ -3559,10 +3559,11 @@ } }, "node_modules/yargs-parser": { - "version": "20.2.4", - "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-20.2.4.tgz", - "integrity": "sha512-WOkpgNhPTlE73h4VFAFsOnomJVaovO8VqLDzy5saChRBFQFBoMYirowyW+Q9HB4HFF4Z7VZTiG3iSzJJA29yRA==", + "version": "20.2.9", + "resolved": "https://registry.npmjs.org/yargs-parser/-/yargs-parser-20.2.9.tgz", + "integrity": "sha512-y11nGElTIV+CT3Zv9t7VKl+Q3hTQoT9a1Qzezhhl6Rp21gJ/IVTW7Z3y9EWXhuUBC2Shnf+DX0antecpAwSP8w==", "dev": true, + "license": "ISC", "engines": { "node": ">=10" } From eced463a356007ee7ac0553d5a84c1260a8056bc Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 13 Dec 2024 14:04:45 +0800 Subject: [PATCH 09/15] centralize timeout --- test/clientOptions.test.ts | 4 ++-- test/failover.test.ts | 4 ++-- test/featureFlag.test.ts | 4 ++-- test/json.test.ts | 4 +++- test/keyvault.test.ts | 4 ++-- test/load.test.ts | 4 ++-- test/loadBalance.test.ts | 4 ++-- test/refresh.test.ts | 4 ++-- test/requestTracing.test.ts | 4 ++-- test/utils/testHelper.ts | 6 +++++- 10 files changed, 24 insertions(+), 18 deletions(-) diff --git a/test/clientOptions.test.ts b/test/clientOptions.test.ts index 62e1b21..2e9417e 100644 --- a/test/clientOptions.test.ts +++ b/test/clientOptions.test.ts @@ -6,7 +6,7 @@ import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi.js"; -import { createMockedConnectionString } from "./utils/testHelper.js"; +import { MAX_TIME_OUT, createMockedConnectionString } from "./utils/testHelper.js"; import * as nock from "nock"; class HttpRequestCountPolicy { @@ -27,7 +27,7 @@ class HttpRequestCountPolicy { } describe("custom client options", function () { - this.timeout(15000); + this.timeout(MAX_TIME_OUT); const fakeEndpoint = "https://azure.azconfig.io"; beforeEach(() => { diff --git a/test/failover.test.ts b/test/failover.test.ts index 2671a1f..e1f2f04 100644 --- a/test/failover.test.ts +++ b/test/failover.test.ts @@ -6,7 +6,7 @@ import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi"; -import { createMockedConnectionString, createMockedFeatureFlag, createMockedKeyValue, mockConfigurationManagerGetClients, restoreMocks } from "./utils/testHelper"; +import { MAX_TIME_OUT, createMockedConnectionString, createMockedFeatureFlag, createMockedKeyValue, mockConfigurationManagerGetClients, restoreMocks } from "./utils/testHelper"; import { getValidDomain, isValidEndpoint } from "../src/ConfigurationClientManager"; const mockedKVs = [{ @@ -27,7 +27,7 @@ const mockedFeatureFlags = [{ ]); describe("failover", function () { - this.timeout(15000); + this.timeout(MAX_TIME_OUT); afterEach(() => { restoreMocks(); diff --git a/test/featureFlag.test.ts b/test/featureFlag.test.ts index 9294a45..2544e61 100644 --- a/test/featureFlag.test.ts +++ b/test/featureFlag.test.ts @@ -4,7 +4,7 @@ import * as chai from "chai"; import * as chaiAsPromised from "chai-as-promised"; import { load } from "./exportedApi.js"; -import { createMockedConnectionString, createMockedEndpoint, createMockedFeatureFlag, createMockedKeyValue, mockAppConfigurationClientListConfigurationSettings, restoreMocks } from "./utils/testHelper.js"; +import { MAX_TIME_OUT, createMockedConnectionString, createMockedEndpoint, createMockedFeatureFlag, createMockedKeyValue, mockAppConfigurationClientListConfigurationSettings, restoreMocks } from "./utils/testHelper.js"; chai.use(chaiAsPromised); const expect = chai.expect; @@ -199,7 +199,7 @@ const mockedKVs = [{ ]); describe("feature flags", function () { - this.timeout(10000); + this.timeout(MAX_TIME_OUT); before(() => { mockAppConfigurationClientListConfigurationSettings([mockedKVs]); diff --git a/test/json.test.ts b/test/json.test.ts index 47a3f67..cb937bd 100644 --- a/test/json.test.ts +++ b/test/json.test.ts @@ -6,12 +6,14 @@ import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi.js"; -import { mockAppConfigurationClientListConfigurationSettings, restoreMocks, createMockedConnectionString, createMockedKeyVaultReference, createMockedJsonKeyValue } from "./utils/testHelper.js"; +import { MAX_TIME_OUT, mockAppConfigurationClientListConfigurationSettings, restoreMocks, createMockedConnectionString, createMockedKeyVaultReference, createMockedJsonKeyValue } from "./utils/testHelper.js"; const jsonKeyValue = createMockedJsonKeyValue("json.settings.logging", '{"Test":{"Level":"Debug"},"Prod":{"Level":"Warning"}}'); const keyVaultKeyValue = createMockedKeyVaultReference("TestKey", "https://fake-vault-name.vault.azure.net/secrets/fakeSecretName"); describe("json", function () { + this.timeout(MAX_TIME_OUT); + beforeEach(() => { }); diff --git a/test/keyvault.test.ts b/test/keyvault.test.ts index 2877243..e88044e 100644 --- a/test/keyvault.test.ts +++ b/test/keyvault.test.ts @@ -6,7 +6,7 @@ import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi.js"; -import { sinon, createMockedConnectionString, createMockedTokenCredential, mockAppConfigurationClientListConfigurationSettings, mockSecretClientGetSecret, restoreMocks, createMockedKeyVaultReference } from "./utils/testHelper.js"; +import { MAX_TIME_OUT, sinon, createMockedConnectionString, createMockedTokenCredential, mockAppConfigurationClientListConfigurationSettings, mockSecretClientGetSecret, restoreMocks, createMockedKeyVaultReference } from "./utils/testHelper.js"; import { KeyVaultSecret, SecretClient } from "@azure/keyvault-secrets"; const mockedData = [ @@ -27,7 +27,7 @@ function mockNewlyCreatedKeyVaultSecretClients() { mockSecretClientGetSecret(mockedData.map(([_key, secretUri, value]) => [secretUri, value])); } describe("key vault reference", function () { - this.timeout(10000); + this.timeout(MAX_TIME_OUT); beforeEach(() => { mockAppConfigurationClient(); diff --git a/test/load.test.ts b/test/load.test.ts index 6d2c94b..d36a331 100644 --- a/test/load.test.ts +++ b/test/load.test.ts @@ -6,7 +6,7 @@ import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi.js"; -import { mockAppConfigurationClientListConfigurationSettings, restoreMocks, createMockedConnectionString, createMockedEndpoint, createMockedTokenCredential, createMockedKeyValue } from "./utils/testHelper.js"; +import { MAX_TIME_OUT, mockAppConfigurationClientListConfigurationSettings, restoreMocks, createMockedConnectionString, createMockedEndpoint, createMockedTokenCredential, createMockedKeyValue } from "./utils/testHelper.js"; const mockedKVs = [{ key: "app.settings.fontColor", @@ -77,7 +77,7 @@ const mockedKVs = [{ ].map(createMockedKeyValue); describe("load", function () { - this.timeout(10000); + this.timeout(MAX_TIME_OUT); before(() => { mockAppConfigurationClientListConfigurationSettings([mockedKVs]); diff --git a/test/loadBalance.test.ts b/test/loadBalance.test.ts index f0e04b0..59248b3 100644 --- a/test/loadBalance.test.ts +++ b/test/loadBalance.test.ts @@ -6,7 +6,7 @@ import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi.js"; -import { restoreMocks, createMockedConnectionString, sleepInMs, createMockedEndpoint, mockConfigurationManagerGetClients, mockAppConfigurationClientLoadBalanceMode } from "./utils/testHelper.js"; +import { MAX_TIME_OUT, restoreMocks, createMockedConnectionString, sleepInMs, createMockedEndpoint, mockConfigurationManagerGetClients, mockAppConfigurationClientLoadBalanceMode } from "./utils/testHelper.js"; import { AppConfigurationClient } from "@azure/app-configuration"; import { ConfigurationClientWrapper } from "../src/ConfigurationClientWrapper.js"; @@ -18,7 +18,7 @@ const clientRequestCounter_1 = {count: 0}; const clientRequestCounter_2 = {count: 0}; describe("load balance", function () { - this.timeout(10000); + this.timeout(MAX_TIME_OUT); beforeEach(() => { }); diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 5fbeb97..6c537fb 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -6,7 +6,7 @@ import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi.js"; -import { mockAppConfigurationClientListConfigurationSettings, mockAppConfigurationClientGetConfigurationSetting, restoreMocks, createMockedConnectionString, createMockedKeyValue, sleepInMs, createMockedFeatureFlag } from "./utils/testHelper.js"; +import { MAX_TIME_OUT, mockAppConfigurationClientListConfigurationSettings, mockAppConfigurationClientGetConfigurationSetting, restoreMocks, createMockedConnectionString, createMockedKeyValue, sleepInMs, createMockedFeatureFlag } from "./utils/testHelper.js"; import * as uuid from "uuid"; let mockedKVs: any[] = []; @@ -33,7 +33,7 @@ const getKvCallback = () => { }; describe("dynamic refresh", function () { - this.timeout(10000); + this.timeout(MAX_TIME_OUT); beforeEach(() => { mockedKVs = [ diff --git a/test/requestTracing.test.ts b/test/requestTracing.test.ts index d146301..4e5e5fa 100644 --- a/test/requestTracing.test.ts +++ b/test/requestTracing.test.ts @@ -5,7 +5,7 @@ import * as chai from "chai"; import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; -import { createMockedConnectionString, createMockedKeyValue, createMockedTokenCredential, mockAppConfigurationClientListConfigurationSettings, restoreMocks, sleepInMs } from "./utils/testHelper.js"; +import { MAX_TIME_OUT, createMockedConnectionString, createMockedKeyValue, createMockedTokenCredential, mockAppConfigurationClientListConfigurationSettings, restoreMocks, sleepInMs } from "./utils/testHelper.js"; import { load } from "./exportedApi.js"; class HttpRequestHeadersPolicy { @@ -23,7 +23,7 @@ class HttpRequestHeadersPolicy { } describe("request tracing", function () { - this.timeout(15000); + this.timeout(MAX_TIME_OUT); const fakeEndpoint = "https://127.0.0.1"; // sufficient to test the request it sends out const headerPolicy = new HttpRequestHeadersPolicy(); diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index 3b51596..4d3b414 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -13,6 +13,8 @@ import * as crypto from "crypto"; import { ConfigurationClientManager } from "../../src/ConfigurationClientManager"; import { ConfigurationClientWrapper } from "../../src/ConfigurationClientWrapper"; +const MAX_TIME_OUT = 20000; + const TEST_CLIENT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_TENANT_ID = "00000000-0000-0000-0000-000000000000"; const TEST_CLIENT_SECRET = "0000000000000000000000000000000000000000"; @@ -262,5 +264,7 @@ export { createMockedKeyValue, createMockedFeatureFlag, - sleepInMs + sleepInMs, + + MAX_TIME_OUT }; From 4d77f89a02962913a2f0c4b7e739c4910bf1987b Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 13 Dec 2024 14:07:02 +0800 Subject: [PATCH 10/15] fix lint --- test/loadBalance.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/loadBalance.test.ts b/test/loadBalance.test.ts index 59248b3..59bdf0f 100644 --- a/test/loadBalance.test.ts +++ b/test/loadBalance.test.ts @@ -6,7 +6,7 @@ import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; import { load } from "./exportedApi.js"; -import { MAX_TIME_OUT, restoreMocks, createMockedConnectionString, sleepInMs, createMockedEndpoint, mockConfigurationManagerGetClients, mockAppConfigurationClientLoadBalanceMode } from "./utils/testHelper.js"; +import { MAX_TIME_OUT, restoreMocks, createMockedConnectionString, sleepInMs, createMockedEndpoint, mockConfigurationManagerGetClients, mockAppConfigurationClientLoadBalanceMode } from "./utils/testHelper.js"; import { AppConfigurationClient } from "@azure/app-configuration"; import { ConfigurationClientWrapper } from "../src/ConfigurationClientWrapper.js"; From 04f6d23a790c061a88ea8b81d554e5c1e1217c1d Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Fri, 13 Dec 2024 14:49:53 +0800 Subject: [PATCH 11/15] add testcase --- test/requestTracing.test.ts | 16 +++++++++++++++- test/utils/testHelper.ts | 4 ++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/test/requestTracing.test.ts b/test/requestTracing.test.ts index 4e5e5fa..311f857 100644 --- a/test/requestTracing.test.ts +++ b/test/requestTracing.test.ts @@ -5,7 +5,8 @@ import * as chai from "chai"; import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; -import { MAX_TIME_OUT, createMockedConnectionString, createMockedKeyValue, createMockedTokenCredential, mockAppConfigurationClientListConfigurationSettings, restoreMocks, sleepInMs } from "./utils/testHelper.js"; +import { MAX_TIME_OUT, createMockedConnectionString, createMockedKeyValue, createMockedTokenCredential, mockAppConfigurationClientListConfigurationSettings, restoreMocks, sinon, sleepInMs } from "./utils/testHelper.js"; +import { ConfigurationClientManager } from "../src/ConfigurationClientManager.js"; import { load } from "./exportedApi.js"; class HttpRequestHeadersPolicy { @@ -75,6 +76,19 @@ describe("request tracing", function () { expect(correlationContext.includes("UsesKeyVault")).eq(true); }); + it("should have replica count in correlation-context header", async () => { + const replicaCount = 2; + sinon.stub(ConfigurationClientManager.prototype, "getReplicaCount").returns(replicaCount); + try { + await load(createMockedConnectionString(fakeEndpoint), { clientOptions }); + } catch (e) { /* empty */ } + expect(headerPolicy.headers).not.undefined; + const correlationContext = headerPolicy.headers.get("Correlation-Context"); + expect(correlationContext).not.undefined; + expect(correlationContext.includes(`ReplicaCount=${replicaCount}`)).eq(true); + sinon.restore(); + }); + it("should detect env in correlation-context header", async () => { process.env.NODE_ENV = "development"; try { diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index 4d3b414..d3e9a06 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -10,8 +10,8 @@ import { RestError } from "@azure/core-rest-pipeline"; import { promisify } from "util"; const sleepInMs = promisify(setTimeout); import * as crypto from "crypto"; -import { ConfigurationClientManager } from "../../src/ConfigurationClientManager"; -import { ConfigurationClientWrapper } from "../../src/ConfigurationClientWrapper"; +import { ConfigurationClientManager } from "../../src/ConfigurationClientManager.js"; +import { ConfigurationClientWrapper } from "../../src/ConfigurationClientWrapper.js"; const MAX_TIME_OUT = 20000; From 48585042a65f9174f4d435834928086842d77205 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> Date: Wed, 18 Dec 2024 11:28:11 +0800 Subject: [PATCH 12/15] Add feature filter usage tracing (#108) * wip * WIP * add integration test * fix lint * update port * update port * update * update * avoid create new featureFlagTracing object when request tracing is disabled * update * update * add variant and telemetry tracing --- src/AzureAppConfigurationImpl.ts | 50 +++++- src/featureManagement/constants.ts | 13 +- .../FeatureFlagTracingOptions.ts | 95 ++++++++++ src/requestTracing/constants.ts | 15 +- src/requestTracing/utils.ts | 24 ++- test/requestTracing.test.ts | 165 ++++++++++++++++-- test/utils/testHelper.ts | 16 ++ 7 files changed, 351 insertions(+), 27 deletions(-) create mode 100644 src/requestTracing/FeatureFlagTracingOptions.ts diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 1541c40..4523642 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -9,10 +9,11 @@ import { IKeyValueAdapter } from "./IKeyValueAdapter.js"; import { JsonKeyValueAdapter } from "./JsonKeyValueAdapter.js"; import { DEFAULT_REFRESH_INTERVAL_IN_MS, MIN_REFRESH_INTERVAL_IN_MS } from "./RefreshOptions.js"; import { Disposable } from "./common/disposable.js"; -import { FEATURE_FLAGS_KEY_NAME, FEATURE_MANAGEMENT_KEY_NAME } from "./featureManagement/constants.js"; +import { FEATURE_FLAGS_KEY_NAME, FEATURE_MANAGEMENT_KEY_NAME, CONDITIONS_KEY_NAME, CLIENT_FILTERS_KEY_NAME, TELEMETRY_KEY_NAME, VARIANTS_KEY_NAME, ALLOCATION_KEY_NAME, SEED_KEY_NAME, NAME_KEY_NAME, ENABLED_KEY_NAME } from "./featureManagement/constants.js"; import { AzureKeyVaultKeyValueAdapter } from "./keyvault/AzureKeyVaultKeyValueAdapter.js"; import { RefreshTimer } from "./refresh/RefreshTimer.js"; import { getConfigurationSettingWithTrace, listConfigurationSettingsWithTrace, requestTracingEnabled } from "./requestTracing/utils.js"; +import { FeatureFlagTracingOptions } from "./requestTracing/FeatureFlagTracingOptions.js"; import { KeyFilter, LabelFilter, SettingSelector } from "./types.js"; type PagedSettingSelector = SettingSelector & { @@ -38,6 +39,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { #client: AppConfigurationClient; #options: AzureAppConfigurationOptions | undefined; #isInitialLoadCompleted: boolean = false; + #featureFlagTracing: FeatureFlagTracingOptions | undefined; // Refresh #refreshInProgress: boolean = false; @@ -66,6 +68,9 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { // Enable request tracing if not opt-out this.#requestTracingEnabled = requestTracingEnabled(); + if (this.#requestTracingEnabled) { + this.#featureFlagTracing = new FeatureFlagTracingOptions(); + } if (options?.trimKeyPrefixes) { this.#sortedTrimKeyPrefixes = [...options.trimKeyPrefixes].sort((a, b) => b.localeCompare(a)); @@ -175,7 +180,8 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return { requestTracingEnabled: this.#requestTracingEnabled, initialLoadCompleted: this.#isInitialLoadCompleted, - appConfigOptions: this.#options + appConfigOptions: this.#options, + featureFlagTracingOptions: this.#featureFlagTracing }; } @@ -257,8 +263,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } async #loadFeatureFlags() { - // Temporary map to store feature flags, key is the key of the setting, value is the raw value of the setting - const featureFlagsMap = new Map(); + const featureFlagSettings: ConfigurationSetting[] = []; for (const selector of this.#featureFlagSelectors) { const listOptions: ListConfigurationSettingsOptions = { keyFilter: `${featureFlagPrefix}${selector.keyFilter}`, @@ -275,15 +280,21 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { pageEtags.push(page.etag ?? ""); for (const setting of page.items) { if (isFeatureFlag(setting)) { - featureFlagsMap.set(setting.key, setting.value); + featureFlagSettings.push(setting); } } } selector.pageEtags = pageEtags; } + if (this.#requestTracingEnabled && this.#featureFlagTracing !== undefined) { + this.#featureFlagTracing.resetFeatureFlagTracing(); + } + // parse feature flags - const featureFlags = Array.from(featureFlagsMap.values()).map(rawFlag => JSON.parse(rawFlag)); + const featureFlags = await Promise.all( + featureFlagSettings.map(setting => this.#parseFeatureFlag(setting)) + ); // feature_management is a reserved key, and feature_flags is an array of feature flags this.#configMap.set(FEATURE_MANAGEMENT_KEY_NAME, { [FEATURE_FLAGS_KEY_NAME]: featureFlags }); @@ -546,6 +557,33 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } return response; } + + async #parseFeatureFlag(setting: ConfigurationSetting): Promise { + const rawFlag = setting.value; + if (rawFlag === undefined) { + throw new Error("The value of configuration setting cannot be undefined."); + } + const featureFlag = JSON.parse(rawFlag); + if (this.#requestTracingEnabled && this.#featureFlagTracing !== undefined) { + if (featureFlag[CONDITIONS_KEY_NAME] && + featureFlag[CONDITIONS_KEY_NAME][CLIENT_FILTERS_KEY_NAME] && + Array.isArray(featureFlag[CONDITIONS_KEY_NAME][CLIENT_FILTERS_KEY_NAME])) { + for (const filter of featureFlag[CONDITIONS_KEY_NAME][CLIENT_FILTERS_KEY_NAME]) { + this.#featureFlagTracing.updateFeatureFilterTracing(filter[NAME_KEY_NAME]); + } + } + if (featureFlag[VARIANTS_KEY_NAME] && Array.isArray(featureFlag[VARIANTS_KEY_NAME])) { + this.#featureFlagTracing.notifyMaxVariants(featureFlag[VARIANTS_KEY_NAME].length); + } + if (featureFlag[TELEMETRY_KEY_NAME] && featureFlag[TELEMETRY_KEY_NAME][ENABLED_KEY_NAME]) { + this.#featureFlagTracing.usesTelemetry = true; + } + if (featureFlag[ALLOCATION_KEY_NAME] && featureFlag[ALLOCATION_KEY_NAME][SEED_KEY_NAME]) { + this.#featureFlagTracing.usesSeed = true; + } + } + return featureFlag; + } } function getValidSelectors(selectors: SettingSelector[]): SettingSelector[] { diff --git a/src/featureManagement/constants.ts b/src/featureManagement/constants.ts index f0082f4..67afa55 100644 --- a/src/featureManagement/constants.ts +++ b/src/featureManagement/constants.ts @@ -2,4 +2,15 @@ // Licensed under the MIT license. export const FEATURE_MANAGEMENT_KEY_NAME = "feature_management"; -export const FEATURE_FLAGS_KEY_NAME = "feature_flags"; +export const FEATURE_FLAGS_KEY_NAME = "feature_flags"; +export const CONDITIONS_KEY_NAME = "conditions"; +export const CLIENT_FILTERS_KEY_NAME = "client_filters"; +export const TELEMETRY_KEY_NAME = "telemetry"; +export const VARIANTS_KEY_NAME = "variants"; +export const ALLOCATION_KEY_NAME = "allocation"; +export const SEED_KEY_NAME = "seed"; +export const NAME_KEY_NAME = "name"; +export const ENABLED_KEY_NAME = "enabled"; + +export const TIME_WINDOW_FILTER_NAMES = ["TimeWindow", "Microsoft.TimeWindow", "TimeWindowFilter", "Microsoft.TimeWindowFilter"]; +export const TARGETING_FILTER_NAMES = ["Targeting", "Microsoft.Targeting", "TargetingFilter", "Microsoft.TargetingFilter"]; diff --git a/src/requestTracing/FeatureFlagTracingOptions.ts b/src/requestTracing/FeatureFlagTracingOptions.ts new file mode 100644 index 0000000..006d969 --- /dev/null +++ b/src/requestTracing/FeatureFlagTracingOptions.ts @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { TIME_WINDOW_FILTER_NAMES, TARGETING_FILTER_NAMES } from "../featureManagement/constants.js"; +import { CUSTOM_FILTER_KEY, TIME_WINDOW_FILTER_KEY, TARGETING_FILTER_KEY, FF_SEED_USED_TAG, FF_TELEMETRY_USED_TAG, DELIMITER } from "./constants.js"; + +/** + * Tracing for tracking feature flag usage. + */ +export class FeatureFlagTracingOptions { + /** + * Built-in feature filter usage. + */ + usesCustomFilter: boolean = false; + usesTimeWindowFilter: boolean = false; + usesTargetingFilter: boolean = false; + usesTelemetry: boolean = false; + usesSeed: boolean = false; + maxVariants: number = 0; + + resetFeatureFlagTracing(): void { + this.usesCustomFilter = false; + this.usesTimeWindowFilter = false; + this.usesTargetingFilter = false; + this.usesTelemetry = false; + this.usesSeed = false; + this.maxVariants = 0; + } + + updateFeatureFilterTracing(filterName: string): void { + if (TIME_WINDOW_FILTER_NAMES.some(name => name === filterName)) { + this.usesTimeWindowFilter = true; + } else if (TARGETING_FILTER_NAMES.some(name => name === filterName)) { + this.usesTargetingFilter = true; + } else { + this.usesCustomFilter = true; + } + } + + notifyMaxVariants(currentFFTotalVariants: number): void { + if (currentFFTotalVariants > this.maxVariants) { + this.maxVariants = currentFFTotalVariants; + } + } + + usesAnyFeatureFilter(): boolean { + return this.usesCustomFilter || this.usesTimeWindowFilter || this.usesTargetingFilter; + } + + usesAnyTracingFeature() { + return this.usesSeed || this.usesTelemetry; + } + + createFeatureFiltersString(): string { + if (!this.usesAnyFeatureFilter()) { + return ""; + } + + let result: string = ""; + if (this.usesCustomFilter) { + result += CUSTOM_FILTER_KEY; + } + if (this.usesTimeWindowFilter) { + if (result !== "") { + result += DELIMITER; + } + result += TIME_WINDOW_FILTER_KEY; + } + if (this.usesTargetingFilter) { + if (result !== "") { + result += DELIMITER; + } + result += TARGETING_FILTER_KEY; + } + return result; + } + + createFeaturesString(): string { + if (!this.usesAnyTracingFeature()) { + return ""; + } + + let result: string = ""; + if (this.usesSeed) { + result += FF_SEED_USED_TAG; + } + if (this.usesTelemetry) { + if (result !== "") { + result += DELIMITER; + } + result += FF_TELEMETRY_USED_TAG; + } + return result; + } +} diff --git a/src/requestTracing/constants.ts b/src/requestTracing/constants.ts index d46cdfd..f32996b 100644 --- a/src/requestTracing/constants.ts +++ b/src/requestTracing/constants.ts @@ -37,7 +37,7 @@ export const CONTAINER_APP_ENV_VAR = "CONTAINER_APP_NAME"; export const KUBERNETES_ENV_VAR = "KUBERNETES_PORT"; export const SERVICE_FABRIC_ENV_VAR = "Fabric_NodeName"; // See: https://docs.microsoft.com/en-us/azure/service-fabric/service-fabric-environment-variables-reference -// Request Type +// Request type export const REQUEST_TYPE_KEY = "RequestType"; export enum RequestType { STARTUP = "Startup", @@ -46,3 +46,16 @@ export enum RequestType { // Tag names export const KEY_VAULT_CONFIGURED_TAG = "UsesKeyVault"; + +// Feature flag usage tracing +export const FEATURE_FILTER_TYPE_KEY = "Filter"; +export const CUSTOM_FILTER_KEY = "CSTM"; +export const TIME_WINDOW_FILTER_KEY = "TIME"; +export const TARGETING_FILTER_KEY = "TRGT"; + +export const FF_TELEMETRY_USED_TAG = "Telemetry"; +export const FF_MAX_VARIANTS_KEY = "MaxVariants"; +export const FF_SEED_USED_TAG = "Seed"; +export const FF_FEATURES_KEY = "FFFeatures"; + +export const DELIMITER = "+"; diff --git a/src/requestTracing/utils.ts b/src/requestTracing/utils.ts index de33573..e9d0b0d 100644 --- a/src/requestTracing/utils.ts +++ b/src/requestTracing/utils.ts @@ -3,6 +3,7 @@ import { AppConfigurationClient, ConfigurationSettingId, GetConfigurationSettingOptions, ListConfigurationSettingsOptions } from "@azure/app-configuration"; import { AzureAppConfigurationOptions } from "../AzureAppConfigurationOptions.js"; +import { FeatureFlagTracingOptions } from "./FeatureFlagTracingOptions.js"; import { AZURE_FUNCTION_ENV_VAR, AZURE_WEB_APP_ENV_VAR, @@ -10,6 +11,9 @@ import { DEV_ENV_VAL, ENV_AZURE_APP_CONFIGURATION_TRACING_DISABLED, ENV_KEY, + FEATURE_FILTER_TYPE_KEY, + FF_MAX_VARIANTS_KEY, + FF_FEATURES_KEY, HOST_TYPE_KEY, HostType, KEY_VAULT_CONFIGURED_TAG, @@ -28,17 +32,18 @@ export function listConfigurationSettingsWithTrace( requestTracingEnabled: boolean; initialLoadCompleted: boolean; appConfigOptions: AzureAppConfigurationOptions | undefined; + featureFlagTracingOptions: FeatureFlagTracingOptions | undefined; }, client: AppConfigurationClient, listOptions: ListConfigurationSettingsOptions ) { - const { requestTracingEnabled, initialLoadCompleted, appConfigOptions } = requestTracingOptions; + const { requestTracingEnabled, initialLoadCompleted, appConfigOptions, featureFlagTracingOptions } = requestTracingOptions; const actualListOptions = { ...listOptions }; if (requestTracingEnabled) { actualListOptions.requestOptions = { customHeaders: { - [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(appConfigOptions, initialLoadCompleted) + [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(appConfigOptions, featureFlagTracingOptions, initialLoadCompleted) } }; } @@ -51,18 +56,19 @@ export function getConfigurationSettingWithTrace( requestTracingEnabled: boolean; initialLoadCompleted: boolean; appConfigOptions: AzureAppConfigurationOptions | undefined; + featureFlagTracingOptions: FeatureFlagTracingOptions | undefined; }, client: AppConfigurationClient, configurationSettingId: ConfigurationSettingId, getOptions?: GetConfigurationSettingOptions, ) { - const { requestTracingEnabled, initialLoadCompleted, appConfigOptions } = requestTracingOptions; + const { requestTracingEnabled, initialLoadCompleted, appConfigOptions, featureFlagTracingOptions } = requestTracingOptions; const actualGetOptions = { ...getOptions }; if (requestTracingEnabled) { actualGetOptions.requestOptions = { customHeaders: { - [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(appConfigOptions, initialLoadCompleted) + [CORRELATION_CONTEXT_HEADER_NAME]: createCorrelationContextHeader(appConfigOptions, featureFlagTracingOptions, initialLoadCompleted) } }; } @@ -70,7 +76,7 @@ export function getConfigurationSettingWithTrace( return client.getConfigurationSetting(configurationSettingId, actualGetOptions); } -export function createCorrelationContextHeader(options: AzureAppConfigurationOptions | undefined, isInitialLoadCompleted: boolean): string { +export function createCorrelationContextHeader(options: AzureAppConfigurationOptions | undefined, featureFlagTracing: FeatureFlagTracingOptions | undefined, isInitialLoadCompleted: boolean): string { /* RequestType: 'Startup' during application starting up, 'Watch' after startup completed. Host: identify with defined envs @@ -82,6 +88,14 @@ export function createCorrelationContextHeader(options: AzureAppConfigurationOpt keyValues.set(HOST_TYPE_KEY, getHostType()); keyValues.set(ENV_KEY, isDevEnvironment() ? DEV_ENV_VAL : undefined); + if (featureFlagTracing) { + keyValues.set(FEATURE_FILTER_TYPE_KEY, featureFlagTracing.usesAnyFeatureFilter() ? featureFlagTracing.createFeatureFiltersString() : undefined); + keyValues.set(FF_FEATURES_KEY, featureFlagTracing.usesAnyTracingFeature() ? featureFlagTracing.createFeaturesString() : undefined); + if (featureFlagTracing.maxVariants > 0) { + keyValues.set(FF_MAX_VARIANTS_KEY, featureFlagTracing.maxVariants.toString()); + } + } + const tags: string[] = []; if (options?.keyVaultOptions) { const { credential, secretClients, secretResolver } = options.keyVaultOptions; diff --git a/test/requestTracing.test.ts b/test/requestTracing.test.ts index a64d2c4..7bd73ce 100644 --- a/test/requestTracing.test.ts +++ b/test/requestTracing.test.ts @@ -5,22 +5,10 @@ import * as chai from "chai"; import * as chaiAsPromised from "chai-as-promised"; chai.use(chaiAsPromised); const expect = chai.expect; -import { createMockedConnectionString, createMockedKeyValue, createMockedTokenCredential, mockAppConfigurationClientListConfigurationSettings, restoreMocks, sleepInMs } from "./utils/testHelper.js"; +import { createMockedConnectionString, createMockedKeyValue, createMockedFeatureFlag, createMockedTokenCredential, mockAppConfigurationClientListConfigurationSettings, restoreMocks, HttpRequestHeadersPolicy, sleepInMs } from "./utils/testHelper.js"; import { load } from "./exportedApi.js"; -class HttpRequestHeadersPolicy { - headers: any; - name: string; - - constructor() { - this.headers = {}; - this.name = "HttpRequestHeadersPolicy"; - } - sendRequest(req, next) { - this.headers = req.headers; - return next(req).then(resp => resp); - } -} +const CORRELATION_CONTEXT_HEADER_NAME = "Correlation-Context"; describe("request tracing", function () { this.timeout(15000); @@ -150,6 +138,155 @@ describe("request tracing", function () { restoreMocks(); }); + it("should have filter type in correlation-context header if feature flags use feature filters", async () => { + let correlationContext: string = ""; + const listKvCallback = (listOptions) => { + correlationContext = listOptions?.requestOptions?.customHeaders[CORRELATION_CONTEXT_HEADER_NAME] ?? ""; + }; + + mockAppConfigurationClientListConfigurationSettings([[ + createMockedFeatureFlag("Alpha_1", { conditions: { client_filters: [ { name: "Microsoft.TimeWindow" } ] } }), + createMockedFeatureFlag("Alpha_2", { conditions: { client_filters: [ { name: "Microsoft.Targeting" } ] } }), + createMockedFeatureFlag("Alpha_3", { conditions: { client_filters: [ { name: "CustomFilter" } ] } }) + ]], listKvCallback); + + const settings = await load(createMockedConnectionString(fakeEndpoint), { + featureFlagOptions: { + enabled: true, + selectors: [ {keyFilter: "*"} ], + refresh: { + enabled: true, + refreshIntervalInMs: 1000 + } + } + }); + + expect(correlationContext).not.undefined; + expect(correlationContext?.includes("RequestType=Startup")).eq(true); + + await sleepInMs(1000 + 1); + try { + await settings.refresh(); + } catch (e) { /* empty */ } + expect(headerPolicy.headers).not.undefined; + expect(correlationContext).not.undefined; + expect(correlationContext?.includes("RequestType=Watch")).eq(true); + expect(correlationContext?.includes("Filter=CSTM+TIME+TRGT")).eq(true); + + restoreMocks(); + }); + + it("should have max variants in correlation-context header if feature flags use variants", async () => { + let correlationContext: string = ""; + const listKvCallback = (listOptions) => { + correlationContext = listOptions?.requestOptions?.customHeaders[CORRELATION_CONTEXT_HEADER_NAME] ?? ""; + }; + + mockAppConfigurationClientListConfigurationSettings([[ + createMockedFeatureFlag("Alpha_1", { variants: [ {name: "a"}, {name: "b"}] }), + createMockedFeatureFlag("Alpha_2", { variants: [ {name: "a"}, {name: "b"}, {name: "c"}] }), + createMockedFeatureFlag("Alpha_3", { variants: [] }) + ]], listKvCallback); + + const settings = await load(createMockedConnectionString(fakeEndpoint), { + featureFlagOptions: { + enabled: true, + selectors: [ {keyFilter: "*"} ], + refresh: { + enabled: true, + refreshIntervalInMs: 1000 + } + } + }); + + expect(correlationContext).not.undefined; + expect(correlationContext?.includes("RequestType=Startup")).eq(true); + + await sleepInMs(1000 + 1); + try { + await settings.refresh(); + } catch (e) { /* empty */ } + expect(headerPolicy.headers).not.undefined; + expect(correlationContext).not.undefined; + expect(correlationContext?.includes("RequestType=Watch")).eq(true); + expect(correlationContext?.includes("MaxVariants=3")).eq(true); + + restoreMocks(); + }); + + it("should have telemety tag in correlation-context header if feature flags enable telemetry", async () => { + let correlationContext: string = ""; + const listKvCallback = (listOptions) => { + correlationContext = listOptions?.requestOptions?.customHeaders[CORRELATION_CONTEXT_HEADER_NAME] ?? ""; + }; + + mockAppConfigurationClientListConfigurationSettings([[ + createMockedFeatureFlag("Alpha_1", { telemetry: {enabled: true} }) + ]], listKvCallback); + + const settings = await load(createMockedConnectionString(fakeEndpoint), { + featureFlagOptions: { + enabled: true, + selectors: [ {keyFilter: "*"} ], + refresh: { + enabled: true, + refreshIntervalInMs: 1000 + } + } + }); + + expect(correlationContext).not.undefined; + expect(correlationContext?.includes("RequestType=Startup")).eq(true); + + await sleepInMs(1000 + 1); + try { + await settings.refresh(); + } catch (e) { /* empty */ } + expect(headerPolicy.headers).not.undefined; + expect(correlationContext).not.undefined; + expect(correlationContext?.includes("RequestType=Watch")).eq(true); + expect(correlationContext?.includes("FFFeatures=Telemetry")).eq(true); + + restoreMocks(); + }); + + it("should have seed tag in correlation-context header if feature flags use allocation seed", async () => { + let correlationContext: string = ""; + const listKvCallback = (listOptions) => { + correlationContext = listOptions?.requestOptions?.customHeaders[CORRELATION_CONTEXT_HEADER_NAME] ?? ""; + }; + + mockAppConfigurationClientListConfigurationSettings([[ + createMockedFeatureFlag("Alpha_1", { telemetry: {enabled: true} }), + createMockedFeatureFlag("Alpha_2", { allocation: {seed: "123"} }) + ]], listKvCallback); + + const settings = await load(createMockedConnectionString(fakeEndpoint), { + featureFlagOptions: { + enabled: true, + selectors: [ {keyFilter: "*"} ], + refresh: { + enabled: true, + refreshIntervalInMs: 1000 + } + } + }); + + expect(correlationContext).not.undefined; + expect(correlationContext?.includes("RequestType=Startup")).eq(true); + + await sleepInMs(1000 + 1); + try { + await settings.refresh(); + } catch (e) { /* empty */ } + expect(headerPolicy.headers).not.undefined; + expect(correlationContext).not.undefined; + expect(correlationContext?.includes("RequestType=Watch")).eq(true); + expect(correlationContext?.includes("FFFeatures=Seed+Telemetry")).eq(true); + + restoreMocks(); + }); + describe("request tracing in Web Worker environment", () => { let originalNavigator; let originalWorkerNavigator; diff --git a/test/utils/testHelper.ts b/test/utils/testHelper.ts index bf05882..c00c99b 100644 --- a/test/utils/testHelper.ts +++ b/test/utils/testHelper.ts @@ -201,6 +201,20 @@ const createMockedFeatureFlag = (name: string, flagProps?: any, props?: any) => isReadOnly: false }, props)); +class HttpRequestHeadersPolicy { + headers: any; + name: string; + + constructor() { + this.headers = {}; + this.name = "HttpRequestHeadersPolicy"; + } + sendRequest(req, next) { + this.headers = req.headers; + return next(req).then(resp => resp); + } +} + export { sinon, mockAppConfigurationClientListConfigurationSettings, @@ -216,5 +230,7 @@ export { createMockedKeyValue, createMockedFeatureFlag, + HttpRequestHeadersPolicy, + sleepInMs }; From 71aebabca61a3e3fa5df2b7112b1236768932408 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> Date: Thu, 19 Dec 2024 13:51:25 +0800 Subject: [PATCH 13/15] Refresh key value collection based on page etag (#133) * refresh based on page etag * remove watchAll & reorganize the code * add testcase * fix lint & update method name * add comment * update variable name * move public method * add more comments * fix lint * resolve merge conflict --- src/AzureAppConfigurationImpl.ts | 569 ++++++++++++++++--------------- src/RefreshOptions.ts | 3 + test/refresh.test.ts | 88 +++-- 3 files changed, 363 insertions(+), 297 deletions(-) diff --git a/src/AzureAppConfigurationImpl.ts b/src/AzureAppConfigurationImpl.ts index 6f2ee9d..916ece4 100644 --- a/src/AzureAppConfigurationImpl.ts +++ b/src/AzureAppConfigurationImpl.ts @@ -69,20 +69,27 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { // Refresh #refreshInProgress: boolean = false; - #refreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS; #onRefreshListeners: Array<() => any> = []; /** * Aka watched settings. */ #sentinels: ConfigurationSettingId[] = []; - #refreshTimer: RefreshTimer; + #watchAll: boolean = false; + #kvRefreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS; + #kvRefreshTimer: RefreshTimer; // Feature flags - #featureFlagRefreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS; - #featureFlagRefreshTimer: RefreshTimer; + #ffRefreshInterval: number = DEFAULT_REFRESH_INTERVAL_IN_MS; + #ffRefreshTimer: RefreshTimer; - // Selectors - #featureFlagSelectors: PagedSettingSelector[] = []; + /** + * Selectors of key-values obtained from @see AzureAppConfigurationOptions.selectors + */ + #kvSelectors: PagedSettingSelector[] = []; + /** + * Selectors of feature flags obtained from @see AzureAppConfigurationOptions.featureFlagOptions.selectors + */ + #ffSelectors: PagedSettingSelector[] = []; // Load balancing #lastSuccessfulEndpoint: string = ""; @@ -94,7 +101,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#options = options; this.#clientManager = clientManager; - // Enable request tracing if not opt-out + // enable request tracing if not opt-out this.#requestTracingEnabled = requestTracingEnabled(); if (this.#requestTracingEnabled) { this.#featureFlagTracing = new FeatureFlagTracingOptions(); @@ -104,40 +111,40 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#sortedTrimKeyPrefixes = [...options.trimKeyPrefixes].sort((a, b) => b.localeCompare(a)); } + // if no selector is specified, always load key values using the default selector: key="*" and label="\0" + this.#kvSelectors = getValidKeyValueSelectors(options?.selectors); + if (options?.refreshOptions?.enabled) { - const { watchedSettings, refreshIntervalInMs } = options.refreshOptions; - // validate watched settings + const { refreshIntervalInMs, watchedSettings } = options.refreshOptions; if (watchedSettings === undefined || watchedSettings.length === 0) { - throw new Error("Refresh is enabled but no watched settings are specified."); + this.#watchAll = true; // if no watched settings is specified, then watch all + } else { + for (const setting of watchedSettings) { + if (setting.key.includes("*") || setting.key.includes(",")) { + throw new Error("The characters '*' and ',' are not supported in key of watched settings."); + } + if (setting.label?.includes("*") || setting.label?.includes(",")) { + throw new Error("The characters '*' and ',' are not supported in label of watched settings."); + } + this.#sentinels.push(setting); + } } // custom refresh interval if (refreshIntervalInMs !== undefined) { if (refreshIntervalInMs < MIN_REFRESH_INTERVAL_IN_MS) { throw new Error(`The refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); - } else { - this.#refreshInterval = refreshIntervalInMs; - } - } - - for (const setting of watchedSettings) { - if (setting.key.includes("*") || setting.key.includes(",")) { - throw new Error("The characters '*' and ',' are not supported in key of watched settings."); - } - if (setting.label?.includes("*") || setting.label?.includes(",")) { - throw new Error("The characters '*' and ',' are not supported in label of watched settings."); + this.#kvRefreshInterval = refreshIntervalInMs; } - this.#sentinels.push(setting); } - - this.#refreshTimer = new RefreshTimer(this.#refreshInterval); + this.#kvRefreshTimer = new RefreshTimer(this.#kvRefreshInterval); } // feature flag options if (options?.featureFlagOptions?.enabled) { - // validate feature flag selectors - this.#featureFlagSelectors = getValidFeatureFlagSelectors(options.featureFlagOptions.selectors); + // validate feature flag selectors, only load feature flags when enabled + this.#ffSelectors = getValidFeatureFlagSelectors(options.featureFlagOptions.selectors); if (options.featureFlagOptions.refresh?.enabled) { const { refreshIntervalInMs } = options.featureFlagOptions.refresh; @@ -146,11 +153,11 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { if (refreshIntervalInMs < MIN_REFRESH_INTERVAL_IN_MS) { throw new Error(`The feature flag refresh interval cannot be less than ${MIN_REFRESH_INTERVAL_IN_MS} milliseconds.`); } else { - this.#featureFlagRefreshInterval = refreshIntervalInMs; + this.#ffRefreshInterval = refreshIntervalInMs; } } - this.#featureFlagRefreshTimer = new RefreshTimer(this.#featureFlagRefreshInterval); + this.#ffRefreshTimer = new RefreshTimer(this.#ffRefreshInterval); } } @@ -158,40 +165,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { this.#adapters.push(new JsonKeyValueAdapter()); } - // #region ReadonlyMap APIs - get(key: string): T | undefined { - return this.#configMap.get(key); - } - - forEach(callbackfn: (value: any, key: string, map: ReadonlyMap) => void, thisArg?: any): void { - this.#configMap.forEach(callbackfn, thisArg); - } - - has(key: string): boolean { - return this.#configMap.has(key); - } - - get size(): number { - return this.#configMap.size; - } - - entries(): MapIterator<[string, any]> { - return this.#configMap.entries(); - } - - keys(): MapIterator { - return this.#configMap.keys(); - } - - values(): MapIterator { - return this.#configMap.values(); - } - - [Symbol.iterator](): MapIterator<[string, any]> { - return this.#configMap[Symbol.iterator](); - } - // #endregion - get #refreshEnabled(): boolean { return !!this.#options?.refreshOptions?.enabled; } @@ -215,181 +188,42 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { }; } - async #executeWithFailoverPolicy(funcToExecute: (client: AppConfigurationClient) => Promise): Promise { - let clientWrappers = await this.#clientManager.getClients(); - if (this.#options?.loadBalancingEnabled && this.#lastSuccessfulEndpoint !== "" && clientWrappers.length > 1) { - let nextClientIndex = 0; - // Iterate through clients to find the index of the client with the last successful endpoint - for (const clientWrapper of clientWrappers) { - nextClientIndex++; - if (clientWrapper.endpoint === this.#lastSuccessfulEndpoint) { - break; - } - } - // If we found the last successful client, rotate the list so that the next client is at the beginning - if (nextClientIndex < clientWrappers.length) { - clientWrappers = [...clientWrappers.slice(nextClientIndex), ...clientWrappers.slice(0, nextClientIndex)]; - } - } - - let successful: boolean; - for (const clientWrapper of clientWrappers) { - successful = false; - try { - const result = await funcToExecute(clientWrapper.client); - this.#isFailoverRequest = false; - this.#lastSuccessfulEndpoint = clientWrapper.endpoint; - successful = true; - clientWrapper.updateBackoffStatus(successful); - return result; - } catch (error) { - if (isFailoverableError(error)) { - clientWrapper.updateBackoffStatus(successful); - this.#isFailoverRequest = true; - continue; - } - - throw error; - } - } - - this.#clientManager.refreshClients(); - throw new Error("All clients failed to get configuration settings."); + // #region ReadonlyMap APIs + get(key: string): T | undefined { + return this.#configMap.get(key); } - async #loadSelectedKeyValues(): Promise { - // validate selectors - const selectors = getValidKeyValueSelectors(this.#options?.selectors); - - const funcToExecute = async (client) => { - const loadedSettings: ConfigurationSetting[] = []; - for (const selector of selectors) { - const listOptions: ListConfigurationSettingsOptions = { - keyFilter: selector.keyFilter, - labelFilter: selector.labelFilter - }; - - const settings = listConfigurationSettingsWithTrace( - this.#requestTraceOptions, - client, - listOptions - ); - - for await (const setting of settings) { - if (!isFeatureFlag(setting)) { // exclude feature flags - loadedSettings.push(setting); - } - } - } - return loadedSettings; - }; - - return await this.#executeWithFailoverPolicy(funcToExecute) as ConfigurationSetting[]; + forEach(callbackfn: (value: any, key: string, map: ReadonlyMap) => void, thisArg?: any): void { + this.#configMap.forEach(callbackfn, thisArg); } - /** - * Update etag of watched settings from loaded data. If a watched setting is not covered by any selector, a request will be sent to retrieve it. - */ - async #updateWatchedKeyValuesEtag(existingSettings: ConfigurationSetting[]): Promise { - if (!this.#refreshEnabled) { - return; - } - - for (const sentinel of this.#sentinels) { - const matchedSetting = existingSettings.find(s => s.key === sentinel.key && s.label === sentinel.label); - if (matchedSetting) { - sentinel.etag = matchedSetting.etag; - } else { - // Send a request to retrieve key-value since it may be either not loaded or loaded with a different label or different casing - const { key, label } = sentinel; - const response = await this.#getConfigurationSetting({ key, label }); - if (response) { - sentinel.etag = response.etag; - } else { - sentinel.etag = undefined; - } - } - } + has(key: string): boolean { + return this.#configMap.has(key); } - async #loadSelectedAndWatchedKeyValues() { - const keyValues: [key: string, value: unknown][] = []; - const loadedSettings = await this.#loadSelectedKeyValues(); - await this.#updateWatchedKeyValuesEtag(loadedSettings); - - // process key-values, watched settings have higher priority - for (const setting of loadedSettings) { - const [key, value] = await this.#processKeyValues(setting); - keyValues.push([key, value]); - } - - this.#clearLoadedKeyValues(); // clear existing key-values in case of configuration setting deletion - for (const [k, v] of keyValues) { - this.#configMap.set(k, v); - } + get size(): number { + return this.#configMap.size; } - async #clearLoadedKeyValues() { - for (const key of this.#configMap.keys()) { - if (key !== FEATURE_MANAGEMENT_KEY_NAME) { - this.#configMap.delete(key); - } - } + entries(): MapIterator<[string, any]> { + return this.#configMap.entries(); } - async #loadFeatureFlags() { - // Temporary map to store feature flags, key is the key of the setting, value is the raw value of the setting - const funcToExecute = async (client) => { - const featureFlagSettings: ConfigurationSetting[] = []; - // deep copy selectors to avoid modification if current client fails - const selectors = JSON.parse( - JSON.stringify(this.#featureFlagSelectors) - ); - - for (const selector of selectors) { - const listOptions: ListConfigurationSettingsOptions = { - keyFilter: `${featureFlagPrefix}${selector.keyFilter}`, - labelFilter: selector.labelFilter - }; - - const pageEtags: string[] = []; - const pageIterator = listConfigurationSettingsWithTrace( - this.#requestTraceOptions, - client, - listOptions - ).byPage(); - for await (const page of pageIterator) { - pageEtags.push(page.etag ?? ""); - for (const setting of page.items) { - if (isFeatureFlag(setting)) { - featureFlagSettings.push(setting); - } - } - } - selector.pageEtags = pageEtags; - } - - this.#featureFlagSelectors = selectors; - return featureFlagSettings; - }; - - const featureFlagSettings = await this.#executeWithFailoverPolicy(funcToExecute) as ConfigurationSetting[]; - - if (this.#requestTracingEnabled && this.#featureFlagTracing !== undefined) { - this.#featureFlagTracing.resetFeatureFlagTracing(); - } + keys(): MapIterator { + return this.#configMap.keys(); + } - // parse feature flags - const featureFlags = await Promise.all( - featureFlagSettings.map(setting => this.#parseFeatureFlag(setting)) - ); + values(): MapIterator { + return this.#configMap.values(); + } - // feature_management is a reserved key, and feature_flags is an array of feature flags - this.#configMap.set(FEATURE_MANAGEMENT_KEY_NAME, { [FEATURE_FLAGS_KEY_NAME]: featureFlags }); + [Symbol.iterator](): MapIterator<[string, any]> { + return this.#configMap[Symbol.iterator](); } + // #endregion /** - * Load the configuration store for the first time. + * Loads the configuration store for the first time. */ async load() { await this.#loadSelectedAndWatchedKeyValues(); @@ -401,7 +235,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } /** - * Construct hierarchical data object from map. + * Constructs hierarchical data object from map. */ constructConfigurationObject(options?: ConfigurationObjectConstructionOptions): Record { const separator = options?.separator ?? "."; @@ -444,7 +278,7 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } /** - * Refresh the configuration store. + * Refreshes the configuration. */ async refresh(): Promise { if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled) { @@ -462,6 +296,26 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } } + /** + * Registers a callback function to be called when the configuration is refreshed. + */ + onRefresh(listener: () => any, thisArg?: any): Disposable { + if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled) { + throw new Error("Refresh is not enabled for key-values or feature flags."); + } + + const boundedListener = listener.bind(thisArg); + this.#onRefreshListeners.push(boundedListener); + + const remove = () => { + const index = this.#onRefreshListeners.indexOf(boundedListener); + if (index >= 0) { + this.#onRefreshListeners.splice(index, 1); + } + }; + return new Disposable(remove); + } + async #refreshTasks(): Promise { const refreshTasks: Promise[] = []; if (this.#refreshEnabled) { @@ -492,17 +346,141 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { } /** - * Refresh key-values. + * Loads configuration settings from App Configuration, either key-value settings or feature flag settings. + * Additionally, updates the `pageEtags` property of the corresponding @see PagedSettingSelector after loading. + * + * @param loadFeatureFlag - Determines which type of configurationsettings to load: + * If true, loads feature flag using the feature flag selectors; + * If false, loads key-value using the key-value selectors. Defaults to false. + */ + async #loadConfigurationSettings(loadFeatureFlag: boolean = false): Promise { + const selectors = loadFeatureFlag ? this.#ffSelectors : this.#kvSelectors; + const funcToExecute = async (client) => { + const loadedSettings: ConfigurationSetting[] = []; + // deep copy selectors to avoid modification if current client fails + const selectorsToUpdate = JSON.parse( + JSON.stringify(selectors) + ); + + for (const selector of selectorsToUpdate) { + const listOptions: ListConfigurationSettingsOptions = { + keyFilter: selector.keyFilter, + labelFilter: selector.labelFilter + }; + + const pageEtags: string[] = []; + const pageIterator = listConfigurationSettingsWithTrace( + this.#requestTraceOptions, + client, + listOptions + ).byPage(); + for await (const page of pageIterator) { + pageEtags.push(page.etag ?? ""); + for (const setting of page.items) { + if (loadFeatureFlag === isFeatureFlag(setting)) { + loadedSettings.push(setting); + } + } + } + selector.pageEtags = pageEtags; + } + + if (loadFeatureFlag) { + this.#ffSelectors = selectorsToUpdate; + } else { + this.#kvSelectors = selectorsToUpdate; + } + return loadedSettings; + }; + + return await this.#executeWithFailoverPolicy(funcToExecute) as ConfigurationSetting[]; + } + + /** + * Loads selected key-values and watched settings (sentinels) for refresh from App Configuration to the local configuration. + */ + async #loadSelectedAndWatchedKeyValues() { + const keyValues: [key: string, value: unknown][] = []; + const loadedSettings = await this.#loadConfigurationSettings(); + if (this.#refreshEnabled && !this.#watchAll) { + await this.#updateWatchedKeyValuesEtag(loadedSettings); + } + + // process key-values, watched settings have higher priority + for (const setting of loadedSettings) { + const [key, value] = await this.#processKeyValues(setting); + keyValues.push([key, value]); + } + + this.#clearLoadedKeyValues(); // clear existing key-values in case of configuration setting deletion + for (const [k, v] of keyValues) { + this.#configMap.set(k, v); // reset the configuration + } + } + + /** + * Updates etag of watched settings from loaded data. If a watched setting is not covered by any selector, a request will be sent to retrieve it. + */ + async #updateWatchedKeyValuesEtag(existingSettings: ConfigurationSetting[]): Promise { + for (const sentinel of this.#sentinels) { + const matchedSetting = existingSettings.find(s => s.key === sentinel.key && s.label === sentinel.label); + if (matchedSetting) { + sentinel.etag = matchedSetting.etag; + } else { + // Send a request to retrieve key-value since it may be either not loaded or loaded with a different label or different casing + const { key, label } = sentinel; + const response = await this.#getConfigurationSetting({ key, label }); + if (response) { + sentinel.etag = response.etag; + } else { + sentinel.etag = undefined; + } + } + } + } + + /** + * Clears all existing key-values in the local configuration except feature flags. + */ + async #clearLoadedKeyValues() { + for (const key of this.#configMap.keys()) { + if (key !== FEATURE_MANAGEMENT_KEY_NAME) { + this.#configMap.delete(key); + } + } + } + + /** + * Loads feature flags from App Configuration to the local configuration. + */ + async #loadFeatureFlags() { + const loadFeatureFlag = true; + const featureFlagSettings = await this.#loadConfigurationSettings(loadFeatureFlag); + + // parse feature flags + const featureFlags = await Promise.all( + featureFlagSettings.map(setting => this.#parseFeatureFlag(setting)) + ); + + // feature_management is a reserved key, and feature_flags is an array of feature flags + this.#configMap.set(FEATURE_MANAGEMENT_KEY_NAME, { [FEATURE_FLAGS_KEY_NAME]: featureFlags }); + } + + /** + * Refreshes key-values. * @returns true if key-values are refreshed, false otherwise. */ async #refreshKeyValues(): Promise { // if still within refresh interval/backoff, return - if (!this.#refreshTimer.canRefresh()) { + if (!this.#kvRefreshTimer.canRefresh()) { return Promise.resolve(false); } // try refresh if any of watched settings is changed. let needRefresh = false; + if (this.#watchAll) { + needRefresh = await this.#checkConfigurationSettingsChange(this.#kvSelectors); + } for (const sentinel of this.#sentinels.values()) { const response = await this.#getConfigurationSetting(sentinel, { onlyIfChanged: true @@ -521,25 +499,39 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { await this.#loadSelectedAndWatchedKeyValues(); } - this.#refreshTimer.reset(); + this.#kvRefreshTimer.reset(); return Promise.resolve(needRefresh); } /** - * Refresh feature flags. + * Refreshes feature flags. * @returns true if feature flags are refreshed, false otherwise. */ async #refreshFeatureFlags(): Promise { // if still within refresh interval/backoff, return - if (!this.#featureFlagRefreshTimer.canRefresh()) { + if (!this.#ffRefreshTimer.canRefresh()) { return Promise.resolve(false); } - // check if any feature flag is changed + const needRefresh = await this.#checkConfigurationSettingsChange(this.#ffSelectors); + if (needRefresh) { + await this.#loadFeatureFlags(); + } + + this.#ffRefreshTimer.reset(); + return Promise.resolve(needRefresh); + } + + /** + * Checks whether the key-value collection has changed. + * @param selectors - The @see PagedSettingSelector of the kev-value collection. + * @returns true if key-value collection has changed, false otherwise. + */ + async #checkConfigurationSettingsChange(selectors: PagedSettingSelector[]): Promise { const funcToExecute = async (client) => { - for (const selector of this.#featureFlagSelectors) { + for (const selector of selectors) { const listOptions: ListConfigurationSettingsOptions = { - keyFilter: `${featureFlagPrefix}${selector.keyFilter}`, + keyFilter: selector.keyFilter, labelFilter: selector.labelFilter, pageEtags: selector.pageEtags }; @@ -559,30 +551,76 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return false; }; - const needRefresh: boolean = await this.#executeWithFailoverPolicy(funcToExecute); - if (needRefresh) { - await this.#loadFeatureFlags(); - } + const isChanged = await this.#executeWithFailoverPolicy(funcToExecute); + return isChanged; + } - this.#featureFlagRefreshTimer.reset(); - return Promise.resolve(needRefresh); + /** + * Gets a configuration setting by key and label.If the setting is not found, return undefine instead of throwing an error. + */ + async #getConfigurationSetting(configurationSettingId: ConfigurationSettingId, customOptions?: GetConfigurationSettingOptions): Promise { + const funcToExecute = async (client) => { + return getConfigurationSettingWithTrace( + this.#requestTraceOptions, + client, + configurationSettingId, + customOptions + ); + }; + + let response: GetConfigurationSettingResponse | undefined; + try { + response = await this.#executeWithFailoverPolicy(funcToExecute); + } catch (error) { + if (isRestError(error) && error.statusCode === 404) { + response = undefined; + } else { + throw error; + } + } + return response; } - onRefresh(listener: () => any, thisArg?: any): Disposable { - if (!this.#refreshEnabled && !this.#featureFlagRefreshEnabled) { - throw new Error("Refresh is not enabled for key-values or feature flags."); + async #executeWithFailoverPolicy(funcToExecute: (client: AppConfigurationClient) => Promise): Promise { + let clientWrappers = await this.#clientManager.getClients(); + if (this.#options?.loadBalancingEnabled && this.#lastSuccessfulEndpoint !== "" && clientWrappers.length > 1) { + let nextClientIndex = 0; + // Iterate through clients to find the index of the client with the last successful endpoint + for (const clientWrapper of clientWrappers) { + nextClientIndex++; + if (clientWrapper.endpoint === this.#lastSuccessfulEndpoint) { + break; + } + } + // If we found the last successful client, rotate the list so that the next client is at the beginning + if (nextClientIndex < clientWrappers.length) { + clientWrappers = [...clientWrappers.slice(nextClientIndex), ...clientWrappers.slice(0, nextClientIndex)]; + } } - const boundedListener = listener.bind(thisArg); - this.#onRefreshListeners.push(boundedListener); + let successful: boolean; + for (const clientWrapper of clientWrappers) { + successful = false; + try { + const result = await funcToExecute(clientWrapper.client); + this.#isFailoverRequest = false; + this.#lastSuccessfulEndpoint = clientWrapper.endpoint; + successful = true; + clientWrapper.updateBackoffStatus(successful); + return result; + } catch (error) { + if (isFailoverableError(error)) { + clientWrapper.updateBackoffStatus(successful); + this.#isFailoverRequest = true; + continue; + } - const remove = () => { - const index = this.#onRefreshListeners.indexOf(boundedListener); - if (index >= 0) { - this.#onRefreshListeners.splice(index, 1); + throw error; } - }; - return new Disposable(remove); + } + + this.#clientManager.refreshClients(); + throw new Error("All clients failed to get configuration settings."); } async #processKeyValues(setting: ConfigurationSetting): Promise<[string, unknown]> { @@ -611,32 +649,6 @@ export class AzureAppConfigurationImpl implements AzureAppConfiguration { return key; } - /** - * Get a configuration setting by key and label. If the setting is not found, return undefine instead of throwing an error. - */ - async #getConfigurationSetting(configurationSettingId: ConfigurationSettingId, customOptions?: GetConfigurationSettingOptions): Promise { - const funcToExecute = async (client) => { - return getConfigurationSettingWithTrace( - this.#requestTraceOptions, - client, - configurationSettingId, - customOptions - ); - }; - - let response: GetConfigurationSettingResponse | undefined; - try { - response = await this.#executeWithFailoverPolicy(funcToExecute); - } catch (error) { - if (isRestError(error) && error.statusCode === 404) { - response = undefined; - } else { - throw error; - } - } - return response; - } - async #parseFeatureFlag(setting: ConfigurationSetting): Promise { const rawFlag = setting.value; if (rawFlag === undefined) { @@ -877,7 +889,7 @@ function getValidSelectors(selectors: SettingSelector[]): SettingSelector[] { } function getValidKeyValueSelectors(selectors?: SettingSelector[]): SettingSelector[] { - if (!selectors || selectors.length === 0) { + if (selectors === undefined || selectors.length === 0) { // Default selector: key: *, label: \0 return [{ keyFilter: KeyFilter.Any, labelFilter: LabelFilter.Null }]; } @@ -885,10 +897,13 @@ function getValidKeyValueSelectors(selectors?: SettingSelector[]): SettingSelect } function getValidFeatureFlagSelectors(selectors?: SettingSelector[]): SettingSelector[] { - if (!selectors || selectors.length === 0) { + if (selectors === undefined || selectors.length === 0) { // selectors must be explicitly provided. throw new Error("Feature flag selectors must be provided."); } else { + selectors.forEach(selector => { + selector.keyFilter = `${featureFlagPrefix}${selector.keyFilter}`; + }); return getValidSelectors(selectors); } } diff --git a/src/RefreshOptions.ts b/src/RefreshOptions.ts index 3742511..d5e4da5 100644 --- a/src/RefreshOptions.ts +++ b/src/RefreshOptions.ts @@ -22,6 +22,9 @@ export interface RefreshOptions { /** * One or more configuration settings to be watched for changes on the server. * Any modifications to watched settings will refresh all settings loaded by the configuration provider when refresh() is called. + * + * @remarks + * If no watched setting is specified, all configuration settings will be watched. */ watchedSettings?: WatchedSetting[]; } diff --git a/test/refresh.test.ts b/test/refresh.test.ts index 6c537fb..6457cb1 100644 --- a/test/refresh.test.ts +++ b/test/refresh.test.ts @@ -58,25 +58,6 @@ describe("dynamic refresh", function () { return expect(refreshCall).eventually.rejectedWith("Refresh is not enabled for key-values or feature flags."); }); - it("should only allow non-empty list of watched settings when refresh is enabled", async () => { - const connectionString = createMockedConnectionString(); - const loadWithEmptyWatchedSettings = load(connectionString, { - refreshOptions: { - enabled: true, - watchedSettings: [] - } - }); - const loadWithUndefinedWatchedSettings = load(connectionString, { - refreshOptions: { - enabled: true - } - }); - return Promise.all([ - expect(loadWithEmptyWatchedSettings).eventually.rejectedWith("Refresh is enabled but no watched settings are specified."), - expect(loadWithUndefinedWatchedSettings).eventually.rejectedWith("Refresh is enabled but no watched settings are specified.") - ]); - }); - it("should not allow refresh interval less than 1 second", async () => { const connectionString = createMockedConnectionString(); const loadWithInvalidRefreshInterval = load(connectionString, { @@ -354,6 +335,73 @@ describe("dynamic refresh", function () { expect(settings.get("app.settings.fontColor")).eq("red"); }); + it("should refresh key value based on page eTag, if no watched setting is specified", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + refreshOptions: { + enabled: true, + refreshIntervalInMs: 2000 + } + }); + expect(listKvRequestCount).eq(1); + expect(getKvRequestCount).eq(0); + expect(settings).not.undefined; + expect(settings.get("app.settings.fontColor")).eq("red"); + expect(settings.get("app.settings.fontSize")).eq("40"); + + // change setting + updateSetting("app.settings.fontColor", "blue"); + + // after refreshInterval, should really refresh + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + expect(listKvRequestCount).eq(3); // 1 + 2 more requests: one conditional request to detect change and one request to reload all key values + expect(getKvRequestCount).eq(0); + expect(settings.get("app.settings.fontColor")).eq("blue"); + }); + + it("should refresh key value based on page Etag, only on change", async () => { + const connectionString = createMockedConnectionString(); + const settings = await load(connectionString, { + refreshOptions: { + enabled: true, + refreshIntervalInMs: 2000 + } + }); + expect(listKvRequestCount).eq(1); + expect(getKvRequestCount).eq(0); + + let refreshSuccessfulCount = 0; + settings.onRefresh(() => { + refreshSuccessfulCount++; + }); + + expect(settings).not.undefined; + expect(settings.get("app.settings.fontColor")).eq("red"); + + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + expect(listKvRequestCount).eq(2); // one more conditional request to detect change + expect(getKvRequestCount).eq(0); + expect(refreshSuccessfulCount).eq(0); // no change in key values, because page etags are the same. + + // change key value + restoreMocks(); + const changedKVs = [ + { value: "blue", key: "app.settings.fontColor" }, + { value: "40", key: "app.settings.fontSize" } + ].map(createMockedKeyValue); + mockAppConfigurationClientListConfigurationSettings([changedKVs], listKvCallback); + mockAppConfigurationClientGetConfigurationSetting(changedKVs, getKvCallback); + + await sleepInMs(2 * 1000 + 1); + await settings.refresh(); + expect(listKvRequestCount).eq(4); // 2 + 2 more requests: one conditional request to detect change and one request to reload all key values + expect(getKvRequestCount).eq(0); + expect(refreshSuccessfulCount).eq(1); // change in key values, because page etags are different. + expect(settings.get("app.settings.fontColor")).eq("blue"); + }); + it("should not refresh any more when there is refresh in progress", async () => { const connectionString = createMockedConnectionString(); const settings = await load(connectionString, { @@ -449,7 +497,7 @@ describe("dynamic refresh feature flags", function () { }); - it("should refresh feature flags only on change, based on page etags", async () => { + it("should refresh feature flags based on page etags, only on change", async () => { // mock multiple pages of feature flags const page1 = [ createMockedFeatureFlag("Alpha_1", { enabled: true }), From 8600654aed7974ae7bb0a5521ebb62bc1584635d Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> Date: Thu, 19 Dec 2024 18:47:11 +0800 Subject: [PATCH 14/15] fix timeout (#144) --- src/ConfigurationClientManager.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ConfigurationClientManager.ts b/src/ConfigurationClientManager.ts index b11e935..6510b7f 100644 --- a/src/ConfigurationClientManager.ts +++ b/src/ConfigurationClientManager.ts @@ -140,13 +140,16 @@ export class ConfigurationClientManager { async #discoverFallbackClients(host: string) { let result; + let timeout; try { result = await Promise.race([ - new Promise((_, reject) => setTimeout(() => reject(new Error("SRV record query timed out.")), SRV_QUERY_TIMEOUT)), + new Promise((_, reject) => timeout = setTimeout(() => reject(new Error("SRV record query timed out.")), SRV_QUERY_TIMEOUT)), this.#querySrvTargetHost(host) ]); } catch (error) { throw new Error(`Failed to build fallback clients, ${error.message}`); + } finally { + clearTimeout(timeout); } const srvTargetHosts = shuffleList(result) as string[]; From f5d48b24ed7fe12a201c9a30a6216b186840d2e0 Mon Sep 17 00:00:00 2001 From: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> Date: Tue, 7 Jan 2025 16:19:57 +0800 Subject: [PATCH 15/15] version bump 2.0.0-preview.2 (#146) --- package-lock.json | 4 ++-- package.json | 2 +- src/version.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 44b53a0..2b821f0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@azure/app-configuration-provider", - "version": "2.0.0-preview.1", + "version": "2.0.0-preview.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@azure/app-configuration-provider", - "version": "2.0.0-preview.1", + "version": "2.0.0-preview.2", "license": "MIT", "dependencies": { "@azure/app-configuration": "^1.6.1", diff --git a/package.json b/package.json index 0842f45..16148a7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@azure/app-configuration-provider", - "version": "2.0.0-preview.1", + "version": "2.0.0-preview.2", "description": "The JavaScript configuration provider for Azure App Configuration", "main": "dist/index.js", "module": "./dist-esm/index.js", diff --git a/src/version.ts b/src/version.ts index beea578..bb9c7aa 100644 --- a/src/version.ts +++ b/src/version.ts @@ -1,4 +1,4 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -export const VERSION = "2.0.0-preview.1"; +export const VERSION = "2.0.0-preview.2";