From 245c054a7a9c3cdc7ce313d8cd78c8ae686ca075 Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Tue, 15 Apr 2025 15:21:25 -0700 Subject: [PATCH 1/7] Fall back to using Host ID if WEBSITE_SITE_NAME doesn't exist --- docs/BindingsOverview.md | 4 +- docs/TriggerBinding.md | 9 +-- src/SqlBindingUtilities.cs | 15 ----- src/TriggerBinding/SqlTriggerBinding.cs | 23 ++++--- src/TriggerBinding/SqlTriggerListener.cs | 66 +++++++++++-------- .../SqlTriggerMetricsProvider.cs | 2 +- src/TriggerBinding/SqlTriggerUtils.cs | 2 +- .../SqlTriggerBindingIntegrationTests.cs | 26 +++++++- 8 files changed, 86 insertions(+), 61 deletions(-) diff --git a/docs/BindingsOverview.md b/docs/BindingsOverview.md index 0d9df90e1..1c629fde5 100644 --- a/docs/BindingsOverview.md +++ b/docs/BindingsOverview.md @@ -20,6 +20,7 @@ - [Sql\_Trigger\_MaxBatchSize](#sql_trigger_maxbatchsize) - [Sql\_Trigger\_PollingIntervalMs](#sql_trigger_pollingintervalms) - [Sql\_Trigger\_MaxChangesPerWorker](#sql_trigger_maxchangesperworker) + - [WEBSITE\_SITE\_NAME](#website_site_name) - [Scaling for Trigger Bindings](#scaling-for-trigger-bindings) - [Retry support for Trigger Bindings](#retry-support-for-trigger-bindings) - [Startup retries](#startup-retries) @@ -154,11 +155,10 @@ The upper limit on the number of pending changes in the user table that are allo #### WEBSITE_SITE_NAME -The unique name used in creating the lease tables. The local apps depend on this setting for creating unique leases tables, please give a unique name for each app. +If this setting exists, it will be used to generate a unique identifier for the function that is used for tracking function state. If not specified, this unique identifier will be generated from the [IHostIdProvider.GetHostIdAsync](https://github.com/Azure/azure-webjobs-sdk/blob/dev/src/Microsoft.Azure.WebJobs.Host/Executors/IHostIdProvider.cs#L14). > **NOTE:** > * If the setting is re-used across apps, having the same function name could cause the functions to use the same lease tables and the function runs to not work as expected. -> * If you have 2 different SQL trigger functions with same functionName locally, not having WEBSITE_SITE_NAME would mean that the same leasees table would be used for both triggers resulting in only one of the functions being triggered. > * This is a read-only variable that is provided by the Azure App service for deployed functions and the user provided value will be overridden. Refer to [Environment variables](https://learn.microsoft.com/azure/app-service/reference-app-settings?tabs=kudu%2Cdotnet#app-environment) for apps. ### Scaling for Trigger Bindings diff --git a/docs/TriggerBinding.md b/docs/TriggerBinding.md index 3b1b798bc..4fb7be380 100644 --- a/docs/TriggerBinding.md +++ b/docs/TriggerBinding.md @@ -96,11 +96,12 @@ To find the name of the leases table associated with your function, look in the This log message is at the `Information` level, so make sure your log level is set correctly. -NOTE: `FunctionId` is generated from a couple of inputs: - - The [WEBSITE_SITE_NAME](https://github.com/Azure/azure-functions-sql-extension/blob/main/docs/BindingsOverview.md#website_site_name) setting - - The name of the function +NOTE: `FunctionId` is generated from the name of the function and either -If either of these values are changed then a new FunctionId will be generated and result in the function starting over from the beginning, including creating a new Leases table. +* The [WEBSITE_SITE_NAME](https://github.com/Azure/azure-functions-sql-extension/blob/main/docs/BindingsOverview.md#website_site_name) setting +* [IHostIdProvider.GetHostIdAsync](https://github.com/Azure/azure-webjobs-sdk/blob/dev/src/Microsoft.Azure.WebJobs.Host/Executors/IHostIdProvider.cs#L14) as a fallback if the WEBSITE_SITE_NAME setting doesn't exist + +If either the name of the function or the ID value are changed then a new FunctionId will be generated and result in the function starting over from the beginning, including creating a new Leases table. This table is used to ensure that all changes are processed and that no change is processed more than once. This table consists of two groups of columns: diff --git a/src/SqlBindingUtilities.cs b/src/SqlBindingUtilities.cs index 0a7553bf6..f8c0eaa9b 100644 --- a/src/SqlBindingUtilities.cs +++ b/src/SqlBindingUtilities.cs @@ -54,21 +54,6 @@ public static string GetConnectionString(string connectionStringSetting, IConfig return connectionString; } - public static string GetWebSiteName(IConfiguration configuration) - { - if (configuration == null) - { - throw new ArgumentNullException(nameof(configuration)); - } - string websitename = configuration.GetConnectionStringOrSetting(SqlBindingConstants.WEBSITENAME); - // We require a WEBSITE_SITE_NAME for avoiding duplicates if users use the same function name accross apps. - if (string.IsNullOrEmpty(websitename)) - { - throw new ArgumentException($"WEBSITE_SITE_NAME cannot be null or empty in your function app settings, please update the setting with a string value. Please refer to https://github.com/Azure/azure-functions-sql-extension/blob/main/docs/BindingsOverview.md#website_site_name for more information."); - } - return websitename; - } - /// /// Parses the parameter string into a list of parameters, where each parameter is separated by "," and has the form /// "@param1=param2". "@param1" is the parameter name to be used in the query or stored procedure, and param1 is the diff --git a/src/TriggerBinding/SqlTriggerBinding.cs b/src/TriggerBinding/SqlTriggerBinding.cs index 326a21276..eb3f5d6d6 100644 --- a/src/TriggerBinding/SqlTriggerBinding.cs +++ b/src/TriggerBinding/SqlTriggerBinding.cs @@ -78,9 +78,9 @@ public async Task CreateListenerAsync(ListenerFactoryContext context) { _ = context ?? throw new ArgumentNullException(nameof(context), "Missing listener context"); - string userFunctionId = this.GetUserFunctionId(); - string oldUserFunctionId = await this.GetOldUserFunctionIdAsync(); - return new SqlTriggerListener(this._connectionString, this._tableName, this._leasesTableName, userFunctionId, oldUserFunctionId, context.Executor, this._sqlOptions, this._logger, this._configuration); + string websiteSiteNameFunctionId = this.GetWebsiteSiteNameFunctionId(); + string hostIdFunctionId = await this.GetHostIdFunctionIdAsync(); + return new SqlTriggerListener(this._connectionString, this._tableName, this._leasesTableName, websiteSiteNameFunctionId, hostIdFunctionId, context.Executor, this._sqlOptions, this._logger, this._configuration); } public ParameterDescriptor ToParameterDescriptor() @@ -94,18 +94,23 @@ public ParameterDescriptor ToParameterDescriptor() } /// - /// Returns an ID that uniquely identifies the user function. + /// Returns an ID that uniquely identifies the user function, based on the WEBSITE_SITE_NAME configuration value. /// /// We call the WEBSITE_SITE_NAME from the configuration and use that to create the hash of the /// user function id. Appending another hash of class+method in here ensures that if there /// are multiple user functions within the same process and tracking the same SQL table, then each one of them /// gets a separate view of the table changes. /// - private string GetUserFunctionId() + /// The function ID, or NULL if there isn't a config value for WEBSITE_SITE_NAME + private string GetWebsiteSiteNameFunctionId() { // Using read-only App name for the hash https://learn.microsoft.com/en-us/azure/app-service/reference-app-settings?tabs=kudu%2Cdotnet#app-environment - string websiteName = SqlBindingUtilities.GetWebSiteName(this._configuration); - + string websiteName = this._configuration.GetConnectionStringOrSetting(SqlBindingConstants.WEBSITENAME); + if (string.IsNullOrEmpty(websiteName)) + { + this._logger.LogWarning("WEBSITE_SITE_NAME configuration is not set, will fall back to using function ID based on the host ID. This will mean consumption plan scaling will not work as intended."); + return null; + } var methodInfo = (MethodInfo)this._parameter.Member; // Get the function name from FunctionName attribute for .NET functions and methodInfo.Name for non .Net string functionName = ((FunctionNameAttribute)methodInfo.GetCustomAttribute(typeof(FunctionNameAttribute)))?.Name ?? $"{methodInfo.Name}"; @@ -118,7 +123,7 @@ private string GetUserFunctionId() } /// - /// Returns the deprecated ID that was used to identify the user function. + /// Returns an ID that uniquely identifies the user function, based on the host ID. /// /// We call the WebJobs SDK library method to generate the host ID. The host ID is essentially a hash of the /// assembly name containing the user function(s). This ensures that if the user ever updates their application, @@ -127,7 +132,7 @@ private string GetUserFunctionId() /// are multiple user functions within the same process and tracking the same SQL table, then each one of them /// gets a separate view of the table changes. /// - private async Task GetOldUserFunctionIdAsync() + private async Task GetHostIdFunctionIdAsync() { string hostId = await this._hostIdProvider.GetHostIdAsync(CancellationToken.None); diff --git a/src/TriggerBinding/SqlTriggerListener.cs b/src/TriggerBinding/SqlTriggerListener.cs index 114ad67e2..a59527eff 100644 --- a/src/TriggerBinding/SqlTriggerListener.cs +++ b/src/TriggerBinding/SqlTriggerListener.cs @@ -35,8 +35,15 @@ internal sealed class SqlTriggerListener : IListener, IScaleMonitorProvider, private readonly SqlObject _userTable; private readonly string _connectionString; private readonly string _userDefinedLeasesTableName; + /// + /// The unique ID we'll use to identify this function in our global state tables + /// private readonly string _userFunctionId; - private readonly string _oldUserFunctionId; + /// + /// The unique function ID based on the host ID - this is used for backwards compatibility to + /// ensure that users upgrading to the new WEBSITE_SITE_NAME based ID don't lose their state + /// + private readonly string _hostIdFunctionId; private readonly ITriggeredFunctionExecutor _executor; private readonly SqlOptions _sqlOptions; private readonly ILogger _logger; @@ -59,19 +66,21 @@ internal sealed class SqlTriggerListener : IListener, IScaleMonitorProvider, /// SQL connection string used to connect to user database /// Name of the user table /// Optional - Name of the leases table - /// Unique identifier for the user function - /// deprecated user function id value created using hostId for the user function + /// Unique identifier for the user function based on the WEBSITE_SITE_NAME configuration value + /// Unique identifier for the user function based on the hostId for the function /// Defines contract for triggering user function /// /// Facilitates logging of messages /// Provides configuration values - public SqlTriggerListener(string connectionString, string tableName, string userDefinedLeasesTableName, string userFunctionId, string oldUserFunctionId, ITriggeredFunctionExecutor executor, SqlOptions sqlOptions, ILogger logger, IConfiguration configuration) + public SqlTriggerListener(string connectionString, string tableName, string userDefinedLeasesTableName, string websiteSiteNameFunctionId, string hostIdFunctionId, ITriggeredFunctionExecutor executor, SqlOptions sqlOptions, ILogger logger, IConfiguration configuration) { this._connectionString = !string.IsNullOrEmpty(connectionString) ? connectionString : throw new ArgumentNullException(nameof(connectionString)); this._userTable = !string.IsNullOrEmpty(tableName) ? new SqlObject(tableName) : throw new ArgumentNullException(nameof(tableName)); this._userDefinedLeasesTableName = userDefinedLeasesTableName; - this._userFunctionId = !string.IsNullOrEmpty(userFunctionId) ? userFunctionId : throw new ArgumentNullException(nameof(userFunctionId)); - this._oldUserFunctionId = oldUserFunctionId; + // We'll use the WEBSITE_SITE_NAME based ID if we have it, but some environments (like running locally) may not have it + // so we'll just fall back to the host ID version instead + this._userFunctionId = string.IsNullOrEmpty(websiteSiteNameFunctionId) ? hostIdFunctionId : websiteSiteNameFunctionId; + this._hostIdFunctionId = hostIdFunctionId; this._executor = executor ?? throw new ArgumentNullException(nameof(executor)); this._sqlOptions = sqlOptions ?? throw new ArgumentNullException(nameof(sqlOptions)); this._logger = logger ?? throw new ArgumentNullException(nameof(logger)); @@ -124,7 +133,7 @@ public async Task StartAsync(CancellationToken cancellationToken) await VerifyDatabaseSupported(connection, this._logger, cancellationToken); int userTableId = await GetUserTableIdAsync(connection, this._userTable, this._logger, cancellationToken); - IReadOnlyList<(string name, string type)> primaryKeyColumns = GetPrimaryKeyColumnsAsync(connection, userTableId, this._logger, this._userTable.FullName, cancellationToken); + IReadOnlyList<(string name, string type)> primaryKeyColumns = GetPrimaryKeyColumns(connection, userTableId, this._logger, this._userTable.FullName, cancellationToken); IReadOnlyList userTableColumns = this.GetUserTableColumns(connection, userTableId, cancellationToken); string bracketedLeasesTableName = GetBracketedLeasesTableName(this._userDefinedLeasesTableName, this._userFunctionId, userTableId); @@ -132,7 +141,7 @@ public async Task StartAsync(CancellationToken cancellationToken) var transactionSw = Stopwatch.StartNew(); long createdSchemaDurationMs = 0L, createGlobalStateTableDurationMs = 0L, insertGlobalStateTableRowDurationMs = 0L, createLeasesTableDurationMs = 0L; - + this._logger.LogInformation("Creating stuff"); using (SqlTransaction transaction = connection.BeginTransaction(System.Data.IsolationLevel.RepeatableRead)) { createdSchemaDurationMs = await this.CreateSchemaAsync(connection, transaction, cancellationToken); @@ -394,7 +403,7 @@ private async Task InsertGlobalStateTableRowAsync(SqlConnection connection string insertRowGlobalStateTableQuery = $@" {AppLockStatements} - -- For back compatibility copy the lastSyncVersion from _oldUserFunctionId if it exists. + -- For back compatibility copy the lastSyncVersion from _hostIdFunctionId if it exists. IF NOT EXISTS ( SELECT * FROM {GlobalStateTableName} WHERE UserFunctionID = '{this._userFunctionId}' AND UserTableID = {userTableId} @@ -402,12 +411,12 @@ IF NOT EXISTS ( BEGIN -- Migrate LastSyncVersion from oldUserFunctionId if it exists and delete the record DECLARE @lastSyncVersion bigint; - SELECT @lastSyncVersion = LastSyncVersion from az_func.GlobalState where UserFunctionID = '{this._oldUserFunctionId}' AND UserTableID = {userTableId} + SELECT @lastSyncVersion = LastSyncVersion from az_func.GlobalState where UserFunctionID = '{this._hostIdFunctionId}' AND UserTableID = {userTableId} IF @lastSyncVersion IS NULL SET @lastSyncVersion = {(long)minValidVersion}; ELSE - DELETE FROM az_func.GlobalState WHERE UserFunctionID = '{this._oldUserFunctionId}' AND UserTableID = {userTableId} - + DELETE FROM az_func.GlobalState WHERE UserFunctionID = '{this._hostIdFunctionId}' AND UserTableID = {userTableId} + INSERT INTO {GlobalStateTableName} VALUES ('{this._userFunctionId}', {userTableId}, @lastSyncVersion, GETUTCDATE()); END @@ -443,20 +452,11 @@ private async Task CreateLeasesTableAsync( { string primaryKeysWithTypes = string.Join(", ", primaryKeyColumns.Select(col => $"{col.name.AsBracketQuotedString()} {col.type}")); string primaryKeys = string.Join(", ", primaryKeyColumns.Select(col => col.name.AsBracketQuotedString())); - string oldLeasesTableName = leasesTableName.Contains(this._userFunctionId) ? leasesTableName.Replace(this._userFunctionId, this._oldUserFunctionId) : string.Empty; - - string createLeasesTableQuery = string.IsNullOrEmpty(oldLeasesTableName) ? $@" - {AppLockStatements} - - IF OBJECT_ID(N'{leasesTableName}', 'U') IS NULL - CREATE TABLE {leasesTableName} ( - {primaryKeysWithTypes}, - {LeasesTableChangeVersionColumnName} bigint NOT NULL, - {LeasesTableAttemptCountColumnName} int NOT NULL, - {LeasesTableLeaseExpirationTimeColumnName} datetime2, - PRIMARY KEY ({primaryKeys}) - ); - " : $@" + string oldLeasesTableName = leasesTableName.Contains(this._userFunctionId) ? leasesTableName.Replace(this._userFunctionId, this._hostIdFunctionId) : string.Empty; + // We should only migrate the lease table from the old hostId based one to the newer WEBSITE_SITE_NAME one if + // we're actually using the WEBSITE_SITE_NAME one (e.g. leasesTableName is different) + bool shouldMigrateOldLeasesTable = !string.IsNullOrEmpty(oldLeasesTableName) && oldLeasesTableName != leasesTableName; + string createLeasesTableQuery = shouldMigrateOldLeasesTable ? $@" {AppLockStatements} IF OBJECT_ID(N'{leasesTableName}', 'U') IS NULL @@ -478,6 +478,18 @@ INSERT INTO {leasesTableName} DROP TABLE {oldLeasesTableName}; END End + " : + $@" + {AppLockStatements} + + IF OBJECT_ID(N'{leasesTableName}', 'U') IS NULL + CREATE TABLE {leasesTableName} ( + {primaryKeysWithTypes}, + {LeasesTableChangeVersionColumnName} bigint NOT NULL, + {LeasesTableAttemptCountColumnName} int NOT NULL, + {LeasesTableLeaseExpirationTimeColumnName} datetime2, + PRIMARY KEY ({primaryKeys}) + ); "; using (var createLeasesTableCommand = new SqlCommand(createLeasesTableQuery, connection, transaction)) @@ -485,7 +497,7 @@ INSERT INTO {leasesTableName} var stopwatch = Stopwatch.StartNew(); try { - await createLeasesTableCommand.ExecuteNonQueryAsyncWithLogging(this._logger, cancellationToken); + await createLeasesTableCommand.ExecuteNonQueryAsyncWithLogging(this._logger, cancellationToken, true); } catch (Exception ex) { diff --git a/src/TriggerBinding/SqlTriggerMetricsProvider.cs b/src/TriggerBinding/SqlTriggerMetricsProvider.cs index ccc1076c1..4411e31d2 100644 --- a/src/TriggerBinding/SqlTriggerMetricsProvider.cs +++ b/src/TriggerBinding/SqlTriggerMetricsProvider.cs @@ -56,7 +56,7 @@ private async Task GetUnprocessedChangeCountAsync() await connection.OpenAsync(); int userTableId = await GetUserTableIdAsync(connection, this._userTable, this._logger, CancellationToken.None); - IReadOnlyList<(string name, string type)> primaryKeyColumns = GetPrimaryKeyColumnsAsync(connection, userTableId, this._logger, this._userTable.FullName, CancellationToken.None); + IReadOnlyList<(string name, string type)> primaryKeyColumns = GetPrimaryKeyColumns(connection, userTableId, this._logger, this._userTable.FullName, CancellationToken.None); // Use a transaction to automatically release the app lock when we're done executing the query using (SqlTransaction transaction = connection.BeginTransaction(IsolationLevel.RepeatableRead)) diff --git a/src/TriggerBinding/SqlTriggerUtils.cs b/src/TriggerBinding/SqlTriggerUtils.cs index 98637bd44..059376050 100644 --- a/src/TriggerBinding/SqlTriggerUtils.cs +++ b/src/TriggerBinding/SqlTriggerUtils.cs @@ -27,7 +27,7 @@ public static class SqlTriggerUtils /// /// Thrown if there are no primary key columns present in the user table or if their names conflict with columns in leases table. /// - public static IReadOnlyList<(string name, string type)> GetPrimaryKeyColumnsAsync(SqlConnection connection, int userTableId, ILogger logger, string userTableName, CancellationToken cancellationToken) + public static IReadOnlyList<(string name, string type)> GetPrimaryKeyColumns(SqlConnection connection, int userTableId, ILogger logger, string userTableName, CancellationToken cancellationToken) { const int NameIndex = 0, TypeIndex = 1, LengthIndex = 2, PrecisionIndex = 3, ScaleIndex = 4; string getPrimaryKeyColumnsQuery = $@" diff --git a/test/Integration/SqlTriggerBindingIntegrationTests.cs b/test/Integration/SqlTriggerBindingIntegrationTests.cs index df0350456..a56495e34 100644 --- a/test/Integration/SqlTriggerBindingIntegrationTests.cs +++ b/test/Integration/SqlTriggerBindingIntegrationTests.cs @@ -40,8 +40,28 @@ public SqlTriggerBindingIntegrationTests(ITestOutputHelper output = null) : base [RetryTheory] [SqlInlineData()] public async Task SingleOperationTriggerTest(SupportedLanguages lang) + { + await this.SingleOperationTriggerTestImpl(lang, clearWebsiteSiteName: false); + } + + /// + /// Ensures that the user function gets invoked for each of the insert, update and delete operation. + /// Does not set a WEBSITE_SITE_NAME to verify functionality without that + /// + [RetryTheory] + [SqlInlineData()] + public async Task SingleOperationTriggerTest_NoWebsiteSiteName(SupportedLanguages lang) + { + await this.SingleOperationTriggerTestImpl(lang, clearWebsiteSiteName: true); + } + + private async Task SingleOperationTriggerTestImpl(SupportedLanguages lang, bool clearWebsiteSiteName) { this.SetChangeTrackingForTable("Products"); + if (clearWebsiteSiteName) + { + Environment.SetEnvironmentVariable("WEBSITE_SITE_NAME", string.Empty); + } this.StartFunctionHost(nameof(ProductsTrigger), lang); int firstId = 1; @@ -81,6 +101,8 @@ await this.WaitForProductChanges( this.GetBatchProcessingTimeout(firstId, lastId)); } + + /// /// Verifies that manually settings of the batch size and polling interval from the app settings overrides settings from the host options /// @@ -596,7 +618,7 @@ public async Task ScaleHostEndToEndTest() ""MaxChangesPerWorker"" : 10 } } - } + } }"; string sqlTriggerJson = $@"{{ ""name"": ""{TestFunctionName}"", @@ -609,7 +631,7 @@ public async Task ScaleHostEndToEndTest() this.SetChangeTrackingForTable("Products"); - // Initializing the listener is needed to create relevant lease table to get unprocessed changes. + // Initializing the listener is needed to create relevant lease table to get unprocessed changes. // We would be using the scale host methods to get the scale status so the configuration values are not needed here. var listener = new SqlTriggerListener(this.DbConnectionString, "dbo.Products", "", "testFunctionId", "testOldFunctionId", Mock.Of(), Mock.Of(), Mock.Of(), configuration); await listener.StartAsync(CancellationToken.None); From d56f301cf52a6fee267bee4d920a5927d8d91cef Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Tue, 6 May 2025 10:43:40 -0700 Subject: [PATCH 2/7] cleanup --- src/TriggerBinding/SqlTriggerListener.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/TriggerBinding/SqlTriggerListener.cs b/src/TriggerBinding/SqlTriggerListener.cs index a59527eff..74dd850f1 100644 --- a/src/TriggerBinding/SqlTriggerListener.cs +++ b/src/TriggerBinding/SqlTriggerListener.cs @@ -141,7 +141,6 @@ public async Task StartAsync(CancellationToken cancellationToken) var transactionSw = Stopwatch.StartNew(); long createdSchemaDurationMs = 0L, createGlobalStateTableDurationMs = 0L, insertGlobalStateTableRowDurationMs = 0L, createLeasesTableDurationMs = 0L; - this._logger.LogInformation("Creating stuff"); using (SqlTransaction transaction = connection.BeginTransaction(System.Data.IsolationLevel.RepeatableRead)) { createdSchemaDurationMs = await this.CreateSchemaAsync(connection, transaction, cancellationToken); From f3df6e3efc085a038fe04a0a617c39352012577f Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Tue, 6 May 2025 10:44:04 -0700 Subject: [PATCH 3/7] Pass in env vars to startup --- .../Integration/SqlTriggerBindingIntegrationTests.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/test/Integration/SqlTriggerBindingIntegrationTests.cs b/test/Integration/SqlTriggerBindingIntegrationTests.cs index a56495e34..41822ff35 100644 --- a/test/Integration/SqlTriggerBindingIntegrationTests.cs +++ b/test/Integration/SqlTriggerBindingIntegrationTests.cs @@ -41,7 +41,7 @@ public SqlTriggerBindingIntegrationTests(ITestOutputHelper output = null) : base [SqlInlineData()] public async Task SingleOperationTriggerTest(SupportedLanguages lang) { - await this.SingleOperationTriggerTestImpl(lang, clearWebsiteSiteName: false); + await this.SingleOperationTriggerTestImpl(lang); } /// @@ -52,17 +52,13 @@ public async Task SingleOperationTriggerTest(SupportedLanguages lang) [SqlInlineData()] public async Task SingleOperationTriggerTest_NoWebsiteSiteName(SupportedLanguages lang) { - await this.SingleOperationTriggerTestImpl(lang, clearWebsiteSiteName: true); + await this.SingleOperationTriggerTestImpl(lang, new Dictionary() { { "WEBSITE_SITE_NAME", string.Empty } }); } - private async Task SingleOperationTriggerTestImpl(SupportedLanguages lang, bool clearWebsiteSiteName) + private async Task SingleOperationTriggerTestImpl(SupportedLanguages lang, IDictionary environmentVariables = null) { this.SetChangeTrackingForTable("Products"); - if (clearWebsiteSiteName) - { - Environment.SetEnvironmentVariable("WEBSITE_SITE_NAME", string.Empty); - } - this.StartFunctionHost(nameof(ProductsTrigger), lang); + this.StartFunctionHost(nameof(ProductsTrigger), lang, environmentVariables: environmentVariables); int firstId = 1; int lastId = 30; From 15a9affae7fe53c8908b60fced81457deeb1f3bb Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Tue, 6 May 2025 10:44:36 -0700 Subject: [PATCH 4/7] verify test fails --- src/TriggerBinding/SqlTriggerBinding.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/TriggerBinding/SqlTriggerBinding.cs b/src/TriggerBinding/SqlTriggerBinding.cs index eb3f5d6d6..b2db6cf38 100644 --- a/src/TriggerBinding/SqlTriggerBinding.cs +++ b/src/TriggerBinding/SqlTriggerBinding.cs @@ -108,8 +108,10 @@ private string GetWebsiteSiteNameFunctionId() string websiteName = this._configuration.GetConnectionStringOrSetting(SqlBindingConstants.WEBSITENAME); if (string.IsNullOrEmpty(websiteName)) { + // TODO REVERT this._logger.LogWarning("WEBSITE_SITE_NAME configuration is not set, will fall back to using function ID based on the host ID. This will mean consumption plan scaling will not work as intended."); - return null; + throw new ArgumentException($"WEBSITE_SITE_NAME cannot be null or empty in your function app settings, please update the setting with a string value. Please refer to https://github.com/Azure/azure-functions-sql-extension/blob/main/docs/BindingsOverview.md#website_site_name for more information."); + // return null; } var methodInfo = (MethodInfo)this._parameter.Member; // Get the function name from FunctionName attribute for .NET functions and methodInfo.Name for non .Net From e55589721bb34a8fd3552ade76f85723936e8dbb Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Tue, 6 May 2025 12:49:22 -0700 Subject: [PATCH 5/7] try --- src/TriggerBinding/SqlTriggerBinding.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/TriggerBinding/SqlTriggerBinding.cs b/src/TriggerBinding/SqlTriggerBinding.cs index b2db6cf38..0892cf2cd 100644 --- a/src/TriggerBinding/SqlTriggerBinding.cs +++ b/src/TriggerBinding/SqlTriggerBinding.cs @@ -102,11 +102,15 @@ public ParameterDescriptor ToParameterDescriptor() /// gets a separate view of the table changes. /// /// The function ID, or NULL if there isn't a config value for WEBSITE_SITE_NAME +#pragma warning disable CA1822 // Mark members as static private string GetWebsiteSiteNameFunctionId() +#pragma warning restore CA1822 // Mark members as static { // Using read-only App name for the hash https://learn.microsoft.com/en-us/azure/app-service/reference-app-settings?tabs=kudu%2Cdotnet#app-environment - string websiteName = this._configuration.GetConnectionStringOrSetting(SqlBindingConstants.WEBSITENAME); - if (string.IsNullOrEmpty(websiteName)) + // string websiteName = this._configuration.GetConnectionStringOrSetting(SqlBindingConstants.WEBSITENAME); + throw new ArgumentException($"WEBSITE_SITE_NAME cannot be null or empty in your function app settings, please update the setting with a string value. Please refer to https://github.com/Azure/azure-functions-sql-extension/blob/main/docs/BindingsOverview.md#website_site_name for more information."); + /* + * if (string.IsNullOrEmpty(websiteName)) { // TODO REVERT this._logger.LogWarning("WEBSITE_SITE_NAME configuration is not set, will fall back to using function ID based on the host ID. This will mean consumption plan scaling will not work as intended."); @@ -122,6 +126,7 @@ private string GetWebsiteSiteNameFunctionId() byte[] hash = sha256.ComputeHash(Encoding.UTF8.GetBytes(websiteName + functionName)); return new Guid(hash.Take(16).ToArray()).ToString("N").Substring(0, 16); } + */ } /// From d303aff01d246f5398a2418f406a6b5356cd9595 Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Tue, 6 May 2025 13:52:49 -0700 Subject: [PATCH 6/7] remove default site name settings --- samples/samples-csharp/local.settings.json | 1 - samples/samples-csx/local.settings.json | 1 - samples/samples-java/local.settings.json | 1 - samples/samples-js-v4/local.settings.json | 3 +-- samples/samples-js/local.settings.json | 1 - samples/samples-outofproc/local.settings.json | 1 - samples/samples-powershell/local.settings.json | 1 - samples/samples-python-v2/local.settings.json | 1 - samples/samples-python/local.settings.json | 1 - src/TriggerBinding/SqlTriggerBinding.cs | 13 +++---------- .../SqlTriggerBindingIntegrationTests.cs | 4 ++-- 11 files changed, 6 insertions(+), 22 deletions(-) diff --git a/samples/samples-csharp/local.settings.json b/samples/samples-csharp/local.settings.json index 11a573a93..2ee8fdfec 100644 --- a/samples/samples-csharp/local.settings.json +++ b/samples/samples-csharp/local.settings.json @@ -4,7 +4,6 @@ "AzureWebJobsStorage": "UseDevelopmentStorage=true", "FUNCTIONS_WORKER_RUNTIME": "dotnet", "SqlConnectionString": "", - "WEBSITE_SITE_NAME": "SamplesCSharp", "Sp_SelectCost": "SelectProductsCost", "ProductCost": 100 } diff --git a/samples/samples-csx/local.settings.json b/samples/samples-csx/local.settings.json index 2780f8f8d..2ee8fdfec 100644 --- a/samples/samples-csx/local.settings.json +++ b/samples/samples-csx/local.settings.json @@ -4,7 +4,6 @@ "AzureWebJobsStorage": "UseDevelopmentStorage=true", "FUNCTIONS_WORKER_RUNTIME": "dotnet", "SqlConnectionString": "", - "WEBSITE_SITE_NAME": "SamplesCsx", "Sp_SelectCost": "SelectProductsCost", "ProductCost": 100 } diff --git a/samples/samples-java/local.settings.json b/samples/samples-java/local.settings.json index 49701c41c..520adf666 100644 --- a/samples/samples-java/local.settings.json +++ b/samples/samples-java/local.settings.json @@ -4,7 +4,6 @@ "AzureWebJobsStorage": "UseDevelopmentStorage=true", "FUNCTIONS_WORKER_RUNTIME": "java", "SqlConnectionString": "", - "WEBSITE_SITE_NAME": "SamplesJava", "Sp_SelectCost": "SelectProductsCost", "ProductCost": 100 } diff --git a/samples/samples-js-v4/local.settings.json b/samples/samples-js-v4/local.settings.json index 9765f08a6..2609bea7d 100644 --- a/samples/samples-js-v4/local.settings.json +++ b/samples/samples-js-v4/local.settings.json @@ -4,7 +4,6 @@ "AzureWebJobsStorage": "UseDevelopmentStorage=true", "FUNCTIONS_WORKER_RUNTIME": "node", "AzureWebJobsFeatureFlags": "EnableWorkerIndexing", - "SqlConnectionString": "", - "WEBSITE_SITE_NAME": "SamplesNodeV4" + "SqlConnectionString": "" } } \ No newline at end of file diff --git a/samples/samples-js/local.settings.json b/samples/samples-js/local.settings.json index 419f9a0fc..d420cc65e 100644 --- a/samples/samples-js/local.settings.json +++ b/samples/samples-js/local.settings.json @@ -4,7 +4,6 @@ "AzureWebJobsStorage": "UseDevelopmentStorage=true", "FUNCTIONS_WORKER_RUNTIME": "node", "SqlConnectionString": "", - "WEBSITE_SITE_NAME": "SamplesJavascript", "Sp_SelectCost": "SelectProductsCost", "ProductCost": 100 } diff --git a/samples/samples-outofproc/local.settings.json b/samples/samples-outofproc/local.settings.json index b941acc76..fda4854dd 100644 --- a/samples/samples-outofproc/local.settings.json +++ b/samples/samples-outofproc/local.settings.json @@ -4,7 +4,6 @@ "AzureWebJobsStorage": "UseDevelopmentStorage=true", "FUNCTIONS_WORKER_RUNTIME": "dotnet-isolated", "SqlConnectionString": "", - "WEBSITE_SITE_NAME": "SamplesOOP", "Sp_SelectCost": "SelectProductsCost", "ProductCost": 100 } diff --git a/samples/samples-powershell/local.settings.json b/samples/samples-powershell/local.settings.json index 6e4d9ba9b..aac210146 100644 --- a/samples/samples-powershell/local.settings.json +++ b/samples/samples-powershell/local.settings.json @@ -5,7 +5,6 @@ "FUNCTIONS_WORKER_RUNTIME": "powershell", "FUNCTIONS_WORKER_RUNTIME_VERSION" : "~7.2", "SqlConnectionString": "", - "WEBSITE_SITE_NAME": "SamplesPowershell", "Sp_SelectCost": "SelectProductsCost", "ProductCost": 100 } diff --git a/samples/samples-python-v2/local.settings.json b/samples/samples-python-v2/local.settings.json index 881aeeed5..9f6f238ce 100644 --- a/samples/samples-python-v2/local.settings.json +++ b/samples/samples-python-v2/local.settings.json @@ -5,7 +5,6 @@ "AzureWebJobsStorage": "UseDevelopmentStorage=true", "AzureWebJobsFeatureFlags": "EnableWorkerIndexing", "SqlConnectionString": "", - "WEBSITE_SITE_NAME": "SamplesPythonV2", "PYTHON_ISOLATE_WORKER_DEPENDENCIES": "1" } } \ No newline at end of file diff --git a/samples/samples-python/local.settings.json b/samples/samples-python/local.settings.json index 38d7dff61..687701584 100644 --- a/samples/samples-python/local.settings.json +++ b/samples/samples-python/local.settings.json @@ -4,7 +4,6 @@ "AzureWebJobsStorage": "UseDevelopmentStorage=true", "FUNCTIONS_WORKER_RUNTIME": "python", "SqlConnectionString": "", - "WEBSITE_SITE_NAME": "SamplesPython", "Sp_SelectCost": "SelectProductsCost", "ProductCost": 100 } diff --git a/src/TriggerBinding/SqlTriggerBinding.cs b/src/TriggerBinding/SqlTriggerBinding.cs index 0892cf2cd..eb3f5d6d6 100644 --- a/src/TriggerBinding/SqlTriggerBinding.cs +++ b/src/TriggerBinding/SqlTriggerBinding.cs @@ -102,20 +102,14 @@ public ParameterDescriptor ToParameterDescriptor() /// gets a separate view of the table changes. /// /// The function ID, or NULL if there isn't a config value for WEBSITE_SITE_NAME -#pragma warning disable CA1822 // Mark members as static private string GetWebsiteSiteNameFunctionId() -#pragma warning restore CA1822 // Mark members as static { // Using read-only App name for the hash https://learn.microsoft.com/en-us/azure/app-service/reference-app-settings?tabs=kudu%2Cdotnet#app-environment - // string websiteName = this._configuration.GetConnectionStringOrSetting(SqlBindingConstants.WEBSITENAME); - throw new ArgumentException($"WEBSITE_SITE_NAME cannot be null or empty in your function app settings, please update the setting with a string value. Please refer to https://github.com/Azure/azure-functions-sql-extension/blob/main/docs/BindingsOverview.md#website_site_name for more information."); - /* - * if (string.IsNullOrEmpty(websiteName)) + string websiteName = this._configuration.GetConnectionStringOrSetting(SqlBindingConstants.WEBSITENAME); + if (string.IsNullOrEmpty(websiteName)) { - // TODO REVERT this._logger.LogWarning("WEBSITE_SITE_NAME configuration is not set, will fall back to using function ID based on the host ID. This will mean consumption plan scaling will not work as intended."); - throw new ArgumentException($"WEBSITE_SITE_NAME cannot be null or empty in your function app settings, please update the setting with a string value. Please refer to https://github.com/Azure/azure-functions-sql-extension/blob/main/docs/BindingsOverview.md#website_site_name for more information."); - // return null; + return null; } var methodInfo = (MethodInfo)this._parameter.Member; // Get the function name from FunctionName attribute for .NET functions and methodInfo.Name for non .Net @@ -126,7 +120,6 @@ private string GetWebsiteSiteNameFunctionId() byte[] hash = sha256.ComputeHash(Encoding.UTF8.GetBytes(websiteName + functionName)); return new Guid(hash.Take(16).ToArray()).ToString("N").Substring(0, 16); } - */ } /// diff --git a/test/Integration/SqlTriggerBindingIntegrationTests.cs b/test/Integration/SqlTriggerBindingIntegrationTests.cs index 41822ff35..1c8c7514b 100644 --- a/test/Integration/SqlTriggerBindingIntegrationTests.cs +++ b/test/Integration/SqlTriggerBindingIntegrationTests.cs @@ -50,9 +50,9 @@ public async Task SingleOperationTriggerTest(SupportedLanguages lang) /// [RetryTheory] [SqlInlineData()] - public async Task SingleOperationTriggerTest_NoWebsiteSiteName(SupportedLanguages lang) + public async Task SingleOperationTriggerTest_WithWebsiteSiteName(SupportedLanguages lang) { - await this.SingleOperationTriggerTestImpl(lang, new Dictionary() { { "WEBSITE_SITE_NAME", string.Empty } }); + await this.SingleOperationTriggerTestImpl(lang, new Dictionary() { { "WEBSITE_SITE_NAME", "SqlBindingsTriggerTest" } }); } private async Task SingleOperationTriggerTestImpl(SupportedLanguages lang, IDictionary environmentVariables = null) From f003dd4b832af36bcc577417a60d4225ee565d50 Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Tue, 6 May 2025 14:04:43 -0700 Subject: [PATCH 7/7] Update comment --- test/Integration/SqlTriggerBindingIntegrationTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Integration/SqlTriggerBindingIntegrationTests.cs b/test/Integration/SqlTriggerBindingIntegrationTests.cs index 1c8c7514b..930be8207 100644 --- a/test/Integration/SqlTriggerBindingIntegrationTests.cs +++ b/test/Integration/SqlTriggerBindingIntegrationTests.cs @@ -46,7 +46,7 @@ public async Task SingleOperationTriggerTest(SupportedLanguages lang) /// /// Ensures that the user function gets invoked for each of the insert, update and delete operation. - /// Does not set a WEBSITE_SITE_NAME to verify functionality without that + /// Sets a WEBSITE_SITE_NAME to verify functionality with that being set. /// [RetryTheory] [SqlInlineData()]