From 4df624f7d93a15145eb7ee6ef575b3476545de0f Mon Sep 17 00:00:00 2001 From: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com> Date: Fri, 15 Aug 2025 18:52:14 +0000 Subject: [PATCH 1/5] Add Serialization & Deserialization of Sink File Properties (#2752) ## Why make this change? - This change solves issue #2576. We need to add the new `File Sink` properties that will later be used to send logs to `.txt` files locally. The properties need to have all the necessary components to be serialized and deserialized from the config file. ## What is this change? - This change adds the `File Sink` properties to the schema file. - Creates a new file `FileSinkConverter.cs` where the properties are serialized and deserialized. - Creates a new file `FileSinkOptions.cs` where the deserialized properties are turned to usable objects and adds the object to the `Telemetry` options. #### JSON Configuration Schema The configuration now supports the following structure under `runtime.telemetry`: ```json { "runtime": { "telemetry": { "file": { "enabled": true, "path": "/logs/dab-log.txt", "rolling-interval": "Day", "retained-file-count-limit": 7, "file-size-limit-bytes": 1048576 } } } } ``` ## How was this tested? - [ ] Integration Tests - [x] Unit Tests - [x] Manual Tests Tested that the newly created properties inside the config file are saved correctly as objects inside DAB and if the properties are written incorrectly, an exception is raised. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> --- schemas/dab.draft.schema.json | 34 ++++ src/Config/Converters/FileSinkConverter.cs | 165 +++++++++++++++++ src/Config/ObjectModel/FileSinkOptions.cs | 167 ++++++++++++++++++ src/Config/ObjectModel/RollingIntervalMode.cs | 44 +++++ src/Config/ObjectModel/TelemetryOptions.cs | 6 + src/Config/RuntimeConfigLoader.cs | 1 + .../Configuration/ConfigurationTests.cs | 101 ++++++++++- ...untimeConfigLoaderJsonDeserializerTests.cs | 4 +- 8 files changed, 514 insertions(+), 8 deletions(-) create mode 100644 src/Config/Converters/FileSinkConverter.cs create mode 100644 src/Config/ObjectModel/FileSinkOptions.cs create mode 100644 src/Config/ObjectModel/RollingIntervalMode.cs diff --git a/schemas/dab.draft.schema.json b/schemas/dab.draft.schema.json index 94ee03e500..fafdefc574 100644 --- a/schemas/dab.draft.schema.json +++ b/schemas/dab.draft.schema.json @@ -483,6 +483,40 @@ "required": [ "auth" ] } }, + "file": { + "type": "object", + "additionalProperties": false, + "properties": { + "enabled": { + "type": "boolean", + "description": "Enable/disable file sink telemetry logging.", + "default": false + }, + "path": { + "type": "string", + "description": "File path for telemetry logs.", + "default": "/logs/dab-log.txt" + }, + "rolling-interval": { + "type": "string", + "description": "Rolling interval for log files.", + "default": "Day", + "enum": ["Minute", "Hour", "Day", "Month", "Year", "Infinite"] + }, + "retained-file-count-limit": { + "type": "integer", + "description": "Maximum number of retained log files.", + "default": 1, + "minimum": 1 + }, + "file-size-limit-bytes": { + "type": "integer", + "description": "Maximum file size in bytes before rolling.", + "default": 1048576, + "minimum": 1 + } + } + }, "log-level": { "type": "object", "description": "Global configuration of log level, defines logging severity levels for specific classes, when 'null' it will set logging level based on 'host: mode' property", diff --git a/src/Config/Converters/FileSinkConverter.cs b/src/Config/Converters/FileSinkConverter.cs new file mode 100644 index 0000000000..e5d68ca20e --- /dev/null +++ b/src/Config/Converters/FileSinkConverter.cs @@ -0,0 +1,165 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Text.Json; +using System.Text.Json.Serialization; +using Azure.DataApiBuilder.Config.ObjectModel; + +namespace Azure.DataApiBuilder.Config.Converters; +class FileSinkConverter : JsonConverter +{ + // Determines whether to replace environment variable with its + // value or not while deserializing. + private bool _replaceEnvVar; + + /// + /// Whether to replace environment variable with its value or not while deserializing. + /// + public FileSinkConverter(bool replaceEnvVar) + { + _replaceEnvVar = replaceEnvVar; + } + + /// + /// Defines how DAB reads File Sink options and defines which values are + /// used to instantiate FileSinkOptions. + /// + /// Thrown when improperly formatted File Sink options are provided. + public override FileSinkOptions? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType == JsonTokenType.StartObject) + { + bool? enabled = null; + string? path = null; + RollingIntervalMode? rollingInterval = null; + int? retainedFileCountLimit = null; + int? fileSizeLimitBytes = null; + + while (reader.Read()) + { + if (reader.TokenType == JsonTokenType.EndObject) + { + return new FileSinkOptions(enabled, path, rollingInterval, retainedFileCountLimit, fileSizeLimitBytes); + } + + string? propertyName = reader.GetString(); + + reader.Read(); + switch (propertyName) + { + case "enabled": + if (reader.TokenType is not JsonTokenType.Null) + { + enabled = reader.GetBoolean(); + } + + break; + + case "path": + if (reader.TokenType is not JsonTokenType.Null) + { + path = reader.DeserializeString(_replaceEnvVar); + } + + break; + + case "rolling-interval": + if (reader.TokenType is not JsonTokenType.Null) + { + rollingInterval = EnumExtensions.Deserialize(reader.DeserializeString(_replaceEnvVar)!); + } + + break; + + case "retained-file-count-limit": + if (reader.TokenType is not JsonTokenType.Null) + { + try + { + retainedFileCountLimit = reader.GetInt32(); + } + catch (FormatException) + { + throw new JsonException($"The JSON token value is of the incorrect numeric format."); + } + + if (retainedFileCountLimit <= 0) + { + throw new JsonException($"Invalid retained-file-count-limit: {retainedFileCountLimit}. Specify a number > 0."); + } + } + + break; + + case "file-size-limit-bytes": + if (reader.TokenType is not JsonTokenType.Null) + { + try + { + fileSizeLimitBytes = reader.GetInt32(); + } + catch (FormatException) + { + throw new JsonException($"The JSON token value is of the incorrect numeric format."); + } + + if (retainedFileCountLimit <= 0) + { + throw new JsonException($"Invalid file-size-limit-bytes: {fileSizeLimitBytes}. Specify a number > 0."); + } + } + + break; + + default: + throw new JsonException($"Unexpected property {propertyName}"); + } + } + } + + throw new JsonException("Failed to read the File Sink Options"); + } + + /// + /// When writing the FileSinkOptions back to a JSON file, only write the properties + /// if they are user provided. This avoids polluting the written JSON file with properties + /// the user most likely omitted when writing the original DAB runtime config file. + /// This Write operation is only used when a RuntimeConfig object is serialized to JSON. + /// + public override void Write(Utf8JsonWriter writer, FileSinkOptions value, JsonSerializerOptions options) + { + writer.WriteStartObject(); + + if (value?.UserProvidedEnabled is true) + { + writer.WritePropertyName("enabled"); + JsonSerializer.Serialize(writer, value.Enabled, options); + } + + if (value?.UserProvidedPath is true) + { + writer.WritePropertyName("path"); + JsonSerializer.Serialize(writer, value.Path, options); + } + + if (value?.UserProvidedRollingInterval is true) + { + writer.WritePropertyName("rolling-interval"); + JsonSerializer.Serialize(writer, value.RollingInterval, options); + } + + if (value?.UserProvidedRetainedFileCountLimit is true) + { + writer.WritePropertyName("retained-file-count-limit"); + JsonSerializer.Serialize(writer, value.RetainedFileCountLimit, options); + } + + if (value?.UserProvidedFileSizeLimitBytes is true) + { + writer.WritePropertyName("file-size-limit-bytes"); + JsonSerializer.Serialize(writer, value.FileSizeLimitBytes, options); + } + + writer.WriteEndObject(); + } +} diff --git a/src/Config/ObjectModel/FileSinkOptions.cs b/src/Config/ObjectModel/FileSinkOptions.cs new file mode 100644 index 0000000000..e6cd20810b --- /dev/null +++ b/src/Config/ObjectModel/FileSinkOptions.cs @@ -0,0 +1,167 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Diagnostics.CodeAnalysis; +using System.Text.Json.Serialization; + +namespace Azure.DataApiBuilder.Config.ObjectModel; + +/// +/// Represents the options for configuring file sink telemetry. +/// +public record FileSinkOptions +{ + /// + /// Default enabled for File Sink. + /// + public const bool DEFAULT_ENABLED = false; + + /// + /// Default path for File Sink. + /// + public const string DEFAULT_PATH = "/logs/dab-log.txt"; + + /// + /// Default rolling interval for File Sink. + /// + public const string DEFAULT_ROLLING_INTERVAL = nameof(RollingIntervalMode.Day); + + /// + /// Default retained file count limit for File Sink. + /// + public const int DEFAULT_RETAINED_FILE_COUNT_LIMIT = 1; + + /// + /// Default file size limit bytes for File Sink. + /// + public const int DEFAULT_FILE_SIZE_LIMIT_BYTES = 1048576; + + /// + /// Whether File Sink is enabled. + /// + public bool Enabled { get; init; } + + /// + /// Path to the file where logs will be uploaded. + /// + public string? Path { get; init; } + + /// + /// Time it takes for files with logs to be discarded. + /// + public string? RollingInterval { get; init; } + + /// + /// Amount of files that can exist simultaneously in which logs are saved. + /// + public int? RetainedFileCountLimit { get; init; } + + /// + /// File size limit in bytes before a new file needs to be created. + /// + public int? FileSizeLimitBytes { get; init; } + + [JsonConstructor] + public FileSinkOptions(bool? enabled = null, string? path = null, RollingIntervalMode? rollingInterval = null, int? retainedFileCountLimit = null, int? fileSizeLimitBytes = null) + { + if (enabled is not null) + { + Enabled = (bool)enabled; + UserProvidedEnabled = true; + } + else + { + Enabled = DEFAULT_ENABLED; + } + + if (path is not null) + { + Path = path; + UserProvidedPath = true; + } + else + { + Path = DEFAULT_PATH; + } + + if (rollingInterval is not null) + { + RollingInterval = rollingInterval.ToString(); + UserProvidedRollingInterval = true; + } + else + { + RollingInterval = DEFAULT_ROLLING_INTERVAL; + } + + if (retainedFileCountLimit is not null) + { + RetainedFileCountLimit = retainedFileCountLimit; + UserProvidedRetainedFileCountLimit = true; + } + else + { + RetainedFileCountLimit = DEFAULT_RETAINED_FILE_COUNT_LIMIT; + } + + if (fileSizeLimitBytes is not null) + { + FileSizeLimitBytes = fileSizeLimitBytes; + UserProvidedFileSizeLimitBytes = true; + } + else + { + FileSizeLimitBytes = DEFAULT_FILE_SIZE_LIMIT_BYTES; + } + } + + /// + /// Flag which informs CLI and JSON serializer whether to write enabled + /// property/value to the runtime config file. + /// When user doesn't provide the enabled property/value, which signals DAB to use the default, + /// the DAB CLI should not write the default value to a serialized config. + /// + [JsonIgnore(Condition = JsonIgnoreCondition.Always)] + [MemberNotNullWhen(true, nameof(Enabled))] + public bool UserProvidedEnabled { get; init; } = false; + + /// + /// Flag which informs CLI and JSON serializer whether to write path + /// property/value to the runtime config file. + /// When user doesn't provide the path property/value, which signals DAB to use the default, + /// the DAB CLI should not write the default value to a serialized config. + /// + [JsonIgnore(Condition = JsonIgnoreCondition.Always)] + [MemberNotNullWhen(true, nameof(Path))] + public bool UserProvidedPath { get; init; } = false; + + /// + /// Flag which informs CLI and JSON serializer whether to write rolling-interval + /// property/value to the runtime config file. + /// When user doesn't provide the rolling-interval property/value, which signals DAB to use the default, + /// the DAB CLI should not write the default value to a serialized config. + /// + [JsonIgnore(Condition = JsonIgnoreCondition.Always)] + [MemberNotNullWhen(true, nameof(RollingInterval))] + public bool UserProvidedRollingInterval { get; init; } = false; + + /// + /// Flag which informs CLI and JSON serializer whether to write retained-file-count-limit + /// property/value to the runtime config file. + /// When user doesn't provide the retained-file-count-limit property/value, which signals DAB to use the default, + /// the DAB CLI should not write the default value to a serialized config. + /// + [JsonIgnore(Condition = JsonIgnoreCondition.Always)] + [MemberNotNullWhen(true, nameof(RetainedFileCountLimit))] + public bool UserProvidedRetainedFileCountLimit { get; init; } = false; + + /// + /// Flag which informs CLI and JSON serializer whether to write file-size-limit-bytes + /// property/value to the runtime config file. + /// When user doesn't provide the file-size-limit-bytes property/value, which signals DAB to use the default, + /// the DAB CLI should not write the default value to a serialized config. + /// + [JsonIgnore(Condition = JsonIgnoreCondition.Always)] + [MemberNotNullWhen(true, nameof(FileSizeLimitBytes))] + public bool UserProvidedFileSizeLimitBytes { get; init; } = false; +} diff --git a/src/Config/ObjectModel/RollingIntervalMode.cs b/src/Config/ObjectModel/RollingIntervalMode.cs new file mode 100644 index 0000000000..df6d77e67b --- /dev/null +++ b/src/Config/ObjectModel/RollingIntervalMode.cs @@ -0,0 +1,44 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Text.Json.Serialization; + +namespace Azure.DataApiBuilder.Config.ObjectModel; + +/// +/// Represents the rolling interval options for file sink. +/// The time it takes between the creation of new files. +/// +[JsonConverter(typeof(JsonStringEnumConverter))] +public enum RollingIntervalMode +{ + /// + /// The log file will never roll; no time period information will be appended to the log file name. + /// + Infinite, + + /// + /// Roll every year. Filenames will have a four-digit year appended in the pattern yyyy. + /// + Year, + + /// + /// Roll every calendar month. Filenames will have yyyyMM appended. + /// + Month, + + /// + /// Roll every day. Filenames will have yyyyMMdd appended. + /// + Day, + + /// + /// Roll every hour. Filenames will have yyyyMMddHH appended. + /// + Hour, + + /// + /// Roll every minute. Filenames will have yyyyMMddHHmm appended. + /// + Minute +} diff --git a/src/Config/ObjectModel/TelemetryOptions.cs b/src/Config/ObjectModel/TelemetryOptions.cs index 157b0d03b2..b0343e53bc 100644 --- a/src/Config/ObjectModel/TelemetryOptions.cs +++ b/src/Config/ObjectModel/TelemetryOptions.cs @@ -9,10 +9,16 @@ namespace Azure.DataApiBuilder.Config.ObjectModel; /// /// Represents the options for telemetry. /// +/// Options for configuring Application Insights. +/// Options for configuring Open Telemetry. +/// Options for configuring Azure Log Analytics. +/// Options for configuring File Sink. +/// Options for configuring the Log Level filters. public record TelemetryOptions( ApplicationInsightsOptions? ApplicationInsights = null, OpenTelemetryOptions? OpenTelemetry = null, AzureLogAnalyticsOptions? AzureLogAnalytics = null, + FileSinkOptions? File = null, Dictionary? LoggerLevel = null) { [JsonPropertyName("log-level")] diff --git a/src/Config/RuntimeConfigLoader.cs b/src/Config/RuntimeConfigLoader.cs index 84f8a8b723..4a220af0ea 100644 --- a/src/Config/RuntimeConfigLoader.cs +++ b/src/Config/RuntimeConfigLoader.cs @@ -261,6 +261,7 @@ public static JsonSerializerOptions GetSerializationOptions( options.Converters.Add(new AKVRetryPolicyOptionsConverterFactory(replaceEnvVar)); options.Converters.Add(new AzureLogAnalyticsOptionsConverterFactory(replaceEnvVar)); options.Converters.Add(new AzureLogAnalyticsAuthOptionsConverter(replaceEnvVar)); + options.Converters.Add(new FileSinkConverter(replaceEnvVar)); if (replaceEnvVar) { diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 5f71cc4d77..98cb89d919 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -4086,7 +4086,7 @@ public void AzureLogAnalyticsSerialization( string expectedDabIdentifier, int expectedFlushIntSec) { - //Check if auth property and its values are expected to exist + // Check if auth property and its values are expected to exist bool expectedExistEnabled = enabled is not null; bool expectedExistDabIdentifier = dabIdentifier is not null; bool expectedExistFlushIntSec = flushIntSec is not null; @@ -4096,7 +4096,8 @@ public void AzureLogAnalyticsSerialization( AzureLogAnalyticsAuthOptions authOptions = new(customTableName, dcrImmutableId, dceEndpoint); AzureLogAnalyticsOptions azureLogAnalyticsOptions = new(enabled, authOptions, dabIdentifier, flushIntSec); - RuntimeConfig configWithCustomLogLevel = InitializeRuntimeWithAzureLogAnalytics(azureLogAnalyticsOptions); + TelemetryOptions telemetryOptions = new(AzureLogAnalytics: azureLogAnalyticsOptions); + RuntimeConfig configWithCustomLogLevel = InitializeRuntimeWithTelemetry(telemetryOptions); string configWithCustomLogLevelJson = configWithCustomLogLevel.ToJson(); Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(configWithCustomLogLevelJson, out RuntimeConfig? deserializedRuntimeConfig)); @@ -4134,10 +4135,10 @@ public void AzureLogAnalyticsSerialization( Assert.AreEqual(expectedFlushIntSec, flushIntSecElement.GetInt32()); } - //Validate auth property exists inside of azure-log-analytics + // Validate auth property exists inside of azure-log-analytics bool authExists = azureLogAnalyticsElement.TryGetProperty("auth", out JsonElement authElement); - //Validate the values inside the auth properties are of expected value + // Validate the values inside the auth properties are of expected value if (authExists) { bool customTableNameExists = authElement.TryGetProperty("custom-table-name", out JsonElement customTableNameElement); @@ -4164,12 +4165,98 @@ public void AzureLogAnalyticsSerialization( } } + /// + /// Tests different File Sink values to see if they are serialized and deserialized correctly to the Json config + /// + [DataTestMethod] + [TestCategory(TestCategory.MSSQL)] + [DataRow(true, "/file/path/exists.txt", RollingIntervalMode.Minute, 27, 256, true, "/file/path/exists.txt", RollingIntervalMode.Minute, 27, 256)] + [DataRow(true, "/test/path.csv", RollingIntervalMode.Hour, 10, 3000, true, "/test/path.csv", RollingIntervalMode.Hour, 10, 3000)] + [DataRow(false, "C://absolute/file/path.log", RollingIntervalMode.Month, 2147483647, 2048, false, "C://absolute/file/path.log", RollingIntervalMode.Month, 2147483647, 2048)] + [DataRow(false, "D://absolute/test/path.txt", RollingIntervalMode.Year, 10, 2147483647, false, "D://absolute/test/path.txt", RollingIntervalMode.Year, 10, 2147483647)] + [DataRow(false, "", RollingIntervalMode.Infinite, 5, 512, false, "", RollingIntervalMode.Infinite, 5, 512)] + [DataRow(null, null, null, null, null, false, "/logs/dab-log.txt", RollingIntervalMode.Day, 1, 1048576)] + public void FileSinkSerialization( + bool? enabled, + string? path, + RollingIntervalMode? rollingInterval, + int? retainedFileCountLimit, + int? fileSizeLimitBytes, + bool expectedEnabled, + string expectedPath, + RollingIntervalMode expectedRollingInterval, + int expectedRetainedFileCountLimit, + int expectedFileSizeLimitBytes) + { + // Check if file values are expected to exist + bool isEnabledNull = enabled is null; + bool isPathNull = path is null; + bool isRollingIntervalNull = rollingInterval is null; + bool isRetainedFileCountLimitNull = retainedFileCountLimit is null; + bool isFileSizeLimitBytesNull = fileSizeLimitBytes is null; + + FileSinkOptions fileOptions = new(enabled, path, rollingInterval, retainedFileCountLimit, fileSizeLimitBytes); + TelemetryOptions telemetryOptions = new(File: fileOptions); + RuntimeConfig configWithCustomLogLevel = InitializeRuntimeWithTelemetry(telemetryOptions); + string configWithCustomLogLevelJson = configWithCustomLogLevel.ToJson(); + Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(configWithCustomLogLevelJson, out RuntimeConfig? deserializedRuntimeConfig)); + + string serializedConfig = deserializedRuntimeConfig.ToJson(); + + using (JsonDocument parsedDocument = JsonDocument.Parse(serializedConfig)) + { + JsonElement root = parsedDocument.RootElement; + JsonElement runtimeElement = root.GetProperty("runtime"); + + // Validate file property exists in runtime + JsonElement telemetryElement = runtimeElement.GetProperty("telemetry"); + bool filePropertyExists = telemetryElement.TryGetProperty("file", out JsonElement fileElement); + Assert.AreEqual(expected: true, actual: filePropertyExists); + + // Validate the values inside the file properties are of expected value + bool enabledExists = fileElement.TryGetProperty("enabled", out JsonElement enabledElement); + Assert.AreEqual(expected: !isEnabledNull, actual: enabledExists); + if (enabledExists) + { + Assert.AreEqual(expectedEnabled, enabledElement.GetBoolean()); + } + + bool pathExists = fileElement.TryGetProperty("path", out JsonElement pathElement); + Assert.AreEqual(expected: !isPathNull, actual: pathExists); + if (pathExists) + { + Assert.AreEqual(expectedPath, pathElement.GetString()); + } + + bool rollingIntervalExists = fileElement.TryGetProperty("rolling-interval", out JsonElement rollingIntervalElement); + Assert.AreEqual(expected: !isRollingIntervalNull, actual: rollingIntervalExists); + if (rollingIntervalExists) + { + Assert.AreEqual(expectedRollingInterval.ToString(), rollingIntervalElement.GetString()); + } + + bool retainedFileCountLimitExists = fileElement.TryGetProperty("retained-file-count-limit", out JsonElement retainedFileCountLimitElement); + Assert.AreEqual(expected: !isRetainedFileCountLimitNull, actual: retainedFileCountLimitExists); + if (retainedFileCountLimitExists) + { + Assert.AreEqual(expectedRetainedFileCountLimit, retainedFileCountLimitElement.GetInt32()); + } + + bool fileSizeLimitBytesExists = fileElement.TryGetProperty("file-size-limit-bytes", out JsonElement fileSizeLimitBytesElement); + Assert.AreEqual(expected: !isFileSizeLimitBytesNull, actual: fileSizeLimitBytesExists); + if (fileSizeLimitBytesExists) + { + Assert.AreEqual(expectedFileSizeLimitBytes, fileSizeLimitBytesElement.GetInt32()); + } + } + } + #nullable disable /// - /// Helper method to create RuntimeConfig with specificed LogLevel value + /// Helper method to create RuntimeConfig with specified Telemetry options /// - private static RuntimeConfig InitializeRuntimeWithAzureLogAnalytics(AzureLogAnalyticsOptions azureLogAnalyticsOptions) + private static RuntimeConfig InitializeRuntimeWithTelemetry(TelemetryOptions telemetryOptions) { TestHelper.SetupDatabaseEnvironment(MSSQL_ENVIRONMENT); @@ -4183,7 +4270,7 @@ private static RuntimeConfig InitializeRuntimeWithAzureLogAnalytics(AzureLogAnal Rest: new(), GraphQL: new(), Host: new(null, null), - Telemetry: new(AzureLogAnalytics: azureLogAnalyticsOptions) + Telemetry: telemetryOptions ), Entities: baseConfig.Entities ); diff --git a/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs b/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs index a7aaf21508..8d7dae0541 100644 --- a/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs +++ b/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs @@ -273,7 +273,7 @@ public void TestNullableOptionalProps() TryParseAndAssertOnDefaults("{" + emptyHostSubProps, out _); // Test with empty telemetry sub-properties - minJsonWithTelemetrySubProps.Append(@"{ ""application-insights"": { }, ""log-level"": { }, ""open-telemetry"": { }, ""azure-log-analytics"": { } } }"); + minJsonWithTelemetrySubProps.Append(@"{ ""application-insights"": { }, ""log-level"": { }, ""open-telemetry"": { }, ""azure-log-analytics"": { }, ""file"": { } } }"); string emptyTelemetrySubProps = minJsonWithTelemetrySubProps + "}"; TryParseAndAssertOnDefaults("{" + emptyTelemetrySubProps, out _); @@ -652,6 +652,8 @@ private static bool TryParseAndAssertOnDefaults(string json, out RuntimeConfig p || !parsedConfig.Runtime.Telemetry.OpenTelemetry.Enabled); Assert.IsTrue(parsedConfig.Runtime?.Telemetry?.AzureLogAnalytics is null || !parsedConfig.Runtime.Telemetry.AzureLogAnalytics.Enabled); + Assert.IsTrue(parsedConfig.Runtime?.Telemetry?.File is null + || !parsedConfig.Runtime.Telemetry.File.Enabled); return true; } From b41cfe5437c3dce34d3331f98fa394f835d86b5c Mon Sep 17 00:00:00 2001 From: M4Al Date: Tue, 19 Aug 2025 20:02:56 +0200 Subject: [PATCH 2/5] Fix Session Context Key set as Read_Only & maintain original roles from the JWT token (#2344) ## Why make this change? - Fixes #2341 - The session context in SQL server is `read_only = 1` which prevents users from doing multiple requests on the same connection. - The row level security is not accurately implemented when using a JWT token. ## What is this change? Changes the session context from `read_only = 1` to `read_only = 0` to allow multiple requests to be done in the same connection, Creates a copy of the original 'roles' from the JWT token to use it on the SQL Filter Predicate to accurately implement row level security. ## How was this tested? - [ ] Integration Tests - [x] Unit Tests Updated `AuthorizationResolver` tests to ensure the original roles copy is working properly. ## Sample Request(s) Sample of a JWT token (only the relevant part) ``` { "aud": "api://ddcf6b31-5d01-407d-97cf-8efefc455d32", "iss": "https://sts.windows.net/9215c785-95c3-49b0-bdba-2062df5aedb5/", "roles": [ "user", "Allow_Customer_OPS025235", "Allow_Customer_OPS004095" ], "ver": "1.0" } ``` X-MS-API-ROLE: user before my change the extra 'roles' that do not match the X-MS-API-ROLE header would never reach the database context. With my change you can do things like this in SQL Predicates to filter out only subsets of the data: ``` CREATE FUNCTION dbo.ops_fact_order_Predicate(@CustomerNo varchar(max)) RETURNS TABLE WITH SCHEMABINDING AS RETURN SELECT 1 AS fn_securitypredicate_result WHERE @CustomerNo in ( select trim(replace(replace(replace([value], '"', ''), ']', ''), 'Allow_Customer_', '')) from STRING_SPLIT ( CAST(SESSION_CONTEXT(N'original_roles') as varchar(max)) , ',' , 0) where trim(replace(replace([value], '"', ''), ']', '')) like 'Allow_Customer%' ) CREATE SECURITY POLICY dbo.ops_fact_order_Policy ADD FILTER PREDICATE dbo.ops_fact_order_Predicate(CustomerNo) ON [gold_ops].[ops_fact_order]; ``` --------- Co-authored-by: KobeLenjou Co-authored-by: Aniruddh Munde Co-authored-by: Ruben Cerna Co-authored-by: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com> --- src/Config/ObjectModel/AuthenticationOptions.cs | 1 + src/Core/Authorization/AuthorizationResolver.cs | 7 ++++++- src/Core/Resolvers/MsSqlQueryExecutor.cs | 2 +- .../Authorization/AuthorizationResolverUnitTests.cs | 6 ++++-- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Config/ObjectModel/AuthenticationOptions.cs b/src/Config/ObjectModel/AuthenticationOptions.cs index 189540fbe6..6750d6e807 100644 --- a/src/Config/ObjectModel/AuthenticationOptions.cs +++ b/src/Config/ObjectModel/AuthenticationOptions.cs @@ -17,6 +17,7 @@ public record AuthenticationOptions(string Provider = nameof(EasyAuthType.Static public const string CLIENT_PRINCIPAL_HEADER = "X-MS-CLIENT-PRINCIPAL"; public const string NAME_CLAIM_TYPE = "name"; public const string ROLE_CLAIM_TYPE = "roles"; + public const string ORIGINAL_ROLE_CLAIM_TYPE = "original_roles"; /// /// Returns whether the configured Provider matches an diff --git a/src/Core/Authorization/AuthorizationResolver.cs b/src/Core/Authorization/AuthorizationResolver.cs index 2ab6e70a4c..0f22b9cd28 100644 --- a/src/Core/Authorization/AuthorizationResolver.cs +++ b/src/Core/Authorization/AuthorizationResolver.cs @@ -617,9 +617,14 @@ public static Dictionary> GetAllAuthenticatedUserClaims(Http // into a list and storing that in resolvedClaims using the claimType as the key. foreach (Claim claim in identity.Claims) { - // 'roles' claim has already been processed. + // 'roles' claim has already been processed. But we preserve the original 'roles' claim. if (claim.Type.Equals(AuthenticationOptions.ROLE_CLAIM_TYPE)) { + if (!resolvedClaims.TryAdd(AuthenticationOptions.ORIGINAL_ROLE_CLAIM_TYPE, new List() { claim })) + { + resolvedClaims[AuthenticationOptions.ORIGINAL_ROLE_CLAIM_TYPE].Add(claim); + } + continue; } diff --git a/src/Core/Resolvers/MsSqlQueryExecutor.cs b/src/Core/Resolvers/MsSqlQueryExecutor.cs index 7a0260cd20..5cbe9f6a76 100644 --- a/src/Core/Resolvers/MsSqlQueryExecutor.cs +++ b/src/Core/Resolvers/MsSqlQueryExecutor.cs @@ -284,7 +284,7 @@ public override string GetSessionParamsQuery(HttpContext? httpContext, IDictiona string paramName = $"{SESSION_PARAM_NAME}{counter.Next()}"; parameters.Add(paramName, new(claimValue)); // Append statement to set read only param value - can be set only once for a connection. - string statementToSetReadOnlyParam = "EXEC sp_set_session_context " + $"'{claimType}', " + paramName + ", @read_only = 1;"; + string statementToSetReadOnlyParam = "EXEC sp_set_session_context " + $"'{claimType}', " + paramName + ", @read_only = 0;"; sessionMapQuery = sessionMapQuery.Append(statementToSetReadOnlyParam); } diff --git a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs index 3c7c31a8ca..733ec15b24 100644 --- a/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs +++ b/src/Service.Tests/Authorization/AuthorizationResolverUnitTests.cs @@ -1293,7 +1293,8 @@ public void UniqueClaimsResolvedForDbPolicy_SessionCtx_Usage() new("sub", "Aa_0RISCzzZ-abC1De2fGHIjKLMNo123pQ4rStUVWXY"), new("oid", "55296aad-ea7f-4c44-9a4c-bb1e8d43a005"), new(AuthenticationOptions.ROLE_CLAIM_TYPE, TEST_ROLE), - new(AuthenticationOptions.ROLE_CLAIM_TYPE, "ROLE2") + new(AuthenticationOptions.ROLE_CLAIM_TYPE, "ROLE2"), + new(AuthenticationOptions.ROLE_CLAIM_TYPE, "ROLE3") }; //Add identity object to the Mock context object. @@ -1315,6 +1316,7 @@ public void UniqueClaimsResolvedForDbPolicy_SessionCtx_Usage() Assert.AreEqual(expected: "Aa_0RISCzzZ-abC1De2fGHIjKLMNo123pQ4rStUVWXY", actual: claimsInRequestContext["sub"], message: "Expected the sub claim to be present."); Assert.AreEqual(expected: "55296aad-ea7f-4c44-9a4c-bb1e8d43a005", actual: claimsInRequestContext["oid"], message: "Expected the oid claim to be present."); Assert.AreEqual(claimsInRequestContext[AuthenticationOptions.ROLE_CLAIM_TYPE], actual: TEST_ROLE, message: "The roles claim should have the value:" + TEST_ROLE); + Assert.AreEqual(expected: "[\"" + TEST_ROLE + "\",\"ROLE2\",\"ROLE3\"]", actual: claimsInRequestContext[AuthenticationOptions.ORIGINAL_ROLE_CLAIM_TYPE], message: "Original roles should be preserved in a new context"); } /// @@ -1365,7 +1367,7 @@ public void ValidateUnauthenticatedUserClaimsAreNotResolvedWhenProcessingUserCla Dictionary resolvedClaims = AuthorizationResolver.GetProcessedUserClaims(context.Object); // Assert - Assert.AreEqual(expected: authenticatedUserclaims.Count, actual: resolvedClaims.Count, message: "Only two claims should be present."); + Assert.AreEqual(expected: authenticatedUserclaims.Count + 1, actual: resolvedClaims.Count, message: "Only " + (authenticatedUserclaims.Count + 1) + " claims should be present."); Assert.AreEqual(expected: "openid", actual: resolvedClaims["scp"], message: "Unexpected scp claim returned."); bool didResolveUnauthenticatedRoleClaim = resolvedClaims[AuthenticationOptions.ROLE_CLAIM_TYPE] == "Don't_Parse_This_Role"; From 846b55c0c838887b9e513604725dbeb0c929f237 Mon Sep 17 00:00:00 2001 From: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com> Date: Fri, 22 Aug 2025 04:37:50 +0000 Subject: [PATCH 3/5] Add logic to send logs through File Sink (#2825) ## Why make this change? - Closes issue #2578 - In order to complete the File Sink feature, the logic that sends the logs created by DAB to the file is required. ## What is this change? - This change implements the logic that sends all the logs from DAB to the file through the path the user requests. This is done with the help of Serilog which is a logging library that simplifies the creation of file sinks. - The `Startup.cs` program creates the Serilog logger pipeline and adds it as part of the services so that it is used later by the `Program.cs` to set the different loggers with the Serilog pipeline and allow the logs to be sent to the file sink. - We also deleted the `RollingIntervalMode.cs` since we discovered that Serilog has its own rolling interval enum class, which makes the one implemented in DAB obsolete. ## How was this tested? - [ ] Integration Tests - [X] Unit Tests Created tests that check if the services needed for the File Sink exist when the File Sink property is enabled. Also, created test to check if the file sink with the appropriate name is created when the property is enabled. ## Sample Request(s) image image --- src/Config/Azure.DataApiBuilder.Config.csproj | 3 +- src/Config/Converters/FileSinkConverter.cs | 5 +- src/Config/ObjectModel/FileSinkOptions.cs | 21 +-- src/Config/ObjectModel/RollingIntervalMode.cs | 44 ----- src/Directory.Packages.props | 2 + .../Configuration/ConfigurationTests.cs | 17 +- .../Configuration/Telemetry/FileSinkTests.cs | 163 ++++++++++++++++++ .../Azure.DataApiBuilder.Service.csproj | 2 + src/Service/Program.cs | 24 ++- src/Service/Startup.cs | 62 ++++++- 10 files changed, 275 insertions(+), 68 deletions(-) delete mode 100644 src/Config/ObjectModel/RollingIntervalMode.cs create mode 100644 src/Service.Tests/Configuration/Telemetry/FileSinkTests.cs diff --git a/src/Config/Azure.DataApiBuilder.Config.csproj b/src/Config/Azure.DataApiBuilder.Config.csproj index 25dd0716f9..a494bc38ae 100644 --- a/src/Config/Azure.DataApiBuilder.Config.csproj +++ b/src/Config/Azure.DataApiBuilder.Config.csproj @@ -18,6 +18,7 @@ + @@ -25,7 +26,7 @@ - + diff --git a/src/Config/Converters/FileSinkConverter.cs b/src/Config/Converters/FileSinkConverter.cs index e5d68ca20e..e0107a11f6 100644 --- a/src/Config/Converters/FileSinkConverter.cs +++ b/src/Config/Converters/FileSinkConverter.cs @@ -4,6 +4,7 @@ using System.Text.Json; using System.Text.Json.Serialization; using Azure.DataApiBuilder.Config.ObjectModel; +using Serilog; namespace Azure.DataApiBuilder.Config.Converters; class FileSinkConverter : JsonConverter @@ -31,7 +32,7 @@ public FileSinkConverter(bool replaceEnvVar) { bool? enabled = null; string? path = null; - RollingIntervalMode? rollingInterval = null; + RollingInterval? rollingInterval = null; int? retainedFileCountLimit = null; int? fileSizeLimitBytes = null; @@ -66,7 +67,7 @@ public FileSinkConverter(bool replaceEnvVar) case "rolling-interval": if (reader.TokenType is not JsonTokenType.Null) { - rollingInterval = EnumExtensions.Deserialize(reader.DeserializeString(_replaceEnvVar)!); + rollingInterval = EnumExtensions.Deserialize(reader.DeserializeString(_replaceEnvVar)!); } break; diff --git a/src/Config/ObjectModel/FileSinkOptions.cs b/src/Config/ObjectModel/FileSinkOptions.cs index e6cd20810b..7e6674fcad 100644 --- a/src/Config/ObjectModel/FileSinkOptions.cs +++ b/src/Config/ObjectModel/FileSinkOptions.cs @@ -3,6 +3,7 @@ using System.Diagnostics.CodeAnalysis; using System.Text.Json.Serialization; +using Serilog; namespace Azure.DataApiBuilder.Config.ObjectModel; @@ -19,12 +20,12 @@ public record FileSinkOptions /// /// Default path for File Sink. /// - public const string DEFAULT_PATH = "/logs/dab-log.txt"; + public const string DEFAULT_PATH = @"logs\dab-log.txt"; /// /// Default rolling interval for File Sink. /// - public const string DEFAULT_ROLLING_INTERVAL = nameof(RollingIntervalMode.Day); + public const string DEFAULT_ROLLING_INTERVAL = nameof(Serilog.RollingInterval.Day); /// /// Default retained file count limit for File Sink. @@ -44,25 +45,25 @@ public record FileSinkOptions /// /// Path to the file where logs will be uploaded. /// - public string? Path { get; init; } + public string Path { get; init; } /// /// Time it takes for files with logs to be discarded. /// - public string? RollingInterval { get; init; } + public string RollingInterval { get; init; } /// /// Amount of files that can exist simultaneously in which logs are saved. /// - public int? RetainedFileCountLimit { get; init; } + public int RetainedFileCountLimit { get; init; } /// /// File size limit in bytes before a new file needs to be created. /// - public int? FileSizeLimitBytes { get; init; } + public int FileSizeLimitBytes { get; init; } [JsonConstructor] - public FileSinkOptions(bool? enabled = null, string? path = null, RollingIntervalMode? rollingInterval = null, int? retainedFileCountLimit = null, int? fileSizeLimitBytes = null) + public FileSinkOptions(bool? enabled = null, string? path = null, RollingInterval? rollingInterval = null, int? retainedFileCountLimit = null, int? fileSizeLimitBytes = null) { if (enabled is not null) { @@ -86,7 +87,7 @@ public FileSinkOptions(bool? enabled = null, string? path = null, RollingInterva if (rollingInterval is not null) { - RollingInterval = rollingInterval.ToString(); + RollingInterval = ((RollingInterval)rollingInterval).ToString(); UserProvidedRollingInterval = true; } else @@ -96,7 +97,7 @@ public FileSinkOptions(bool? enabled = null, string? path = null, RollingInterva if (retainedFileCountLimit is not null) { - RetainedFileCountLimit = retainedFileCountLimit; + RetainedFileCountLimit = (int)retainedFileCountLimit; UserProvidedRetainedFileCountLimit = true; } else @@ -106,7 +107,7 @@ public FileSinkOptions(bool? enabled = null, string? path = null, RollingInterva if (fileSizeLimitBytes is not null) { - FileSizeLimitBytes = fileSizeLimitBytes; + FileSizeLimitBytes = (int)fileSizeLimitBytes; UserProvidedFileSizeLimitBytes = true; } else diff --git a/src/Config/ObjectModel/RollingIntervalMode.cs b/src/Config/ObjectModel/RollingIntervalMode.cs deleted file mode 100644 index df6d77e67b..0000000000 --- a/src/Config/ObjectModel/RollingIntervalMode.cs +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System.Text.Json.Serialization; - -namespace Azure.DataApiBuilder.Config.ObjectModel; - -/// -/// Represents the rolling interval options for file sink. -/// The time it takes between the creation of new files. -/// -[JsonConverter(typeof(JsonStringEnumConverter))] -public enum RollingIntervalMode -{ - /// - /// The log file will never roll; no time period information will be appended to the log file name. - /// - Infinite, - - /// - /// Roll every year. Filenames will have a four-digit year appended in the pattern yyyy. - /// - Year, - - /// - /// Roll every calendar month. Filenames will have yyyyMM appended. - /// - Month, - - /// - /// Roll every day. Filenames will have yyyyMMdd appended. - /// - Day, - - /// - /// Roll every hour. Filenames will have yyyyMMddHH appended. - /// - Hour, - - /// - /// Roll every minute. Filenames will have yyyyMMddHHmm appended. - /// - Minute -} diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index d00f43c478..da600c9f63 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -60,6 +60,8 @@ + + diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 98cb89d919..2522806049 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -47,6 +47,7 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; using Moq.Protected; +using Serilog; using VerifyMSTest; using static Azure.DataApiBuilder.Config.FileSystemRuntimeConfigLoader; using static Azure.DataApiBuilder.Service.Tests.Configuration.ConfigurationEndpoints; @@ -4170,21 +4171,21 @@ public void AzureLogAnalyticsSerialization( /// [DataTestMethod] [TestCategory(TestCategory.MSSQL)] - [DataRow(true, "/file/path/exists.txt", RollingIntervalMode.Minute, 27, 256, true, "/file/path/exists.txt", RollingIntervalMode.Minute, 27, 256)] - [DataRow(true, "/test/path.csv", RollingIntervalMode.Hour, 10, 3000, true, "/test/path.csv", RollingIntervalMode.Hour, 10, 3000)] - [DataRow(false, "C://absolute/file/path.log", RollingIntervalMode.Month, 2147483647, 2048, false, "C://absolute/file/path.log", RollingIntervalMode.Month, 2147483647, 2048)] - [DataRow(false, "D://absolute/test/path.txt", RollingIntervalMode.Year, 10, 2147483647, false, "D://absolute/test/path.txt", RollingIntervalMode.Year, 10, 2147483647)] - [DataRow(false, "", RollingIntervalMode.Infinite, 5, 512, false, "", RollingIntervalMode.Infinite, 5, 512)] - [DataRow(null, null, null, null, null, false, "/logs/dab-log.txt", RollingIntervalMode.Day, 1, 1048576)] + [DataRow(true, "/file/path/exists.txt", RollingInterval.Minute, 27, 256, true, "/file/path/exists.txt", RollingInterval.Minute, 27, 256)] + [DataRow(true, "/test/path.csv", RollingInterval.Hour, 10, 3000, true, "/test/path.csv", RollingInterval.Hour, 10, 3000)] + [DataRow(false, "C://absolute/file/path.log", RollingInterval.Month, 2147483647, 2048, false, "C://absolute/file/path.log", RollingInterval.Month, 2147483647, 2048)] + [DataRow(false, "D://absolute/test/path.txt", RollingInterval.Year, 10, 2147483647, false, "D://absolute/test/path.txt", RollingInterval.Year, 10, 2147483647)] + [DataRow(false, "", RollingInterval.Infinite, 5, 512, false, "", RollingInterval.Infinite, 5, 512)] + [DataRow(null, null, null, null, null, false, "/logs/dab-log.txt", RollingInterval.Day, 1, 1048576)] public void FileSinkSerialization( bool? enabled, string? path, - RollingIntervalMode? rollingInterval, + RollingInterval? rollingInterval, int? retainedFileCountLimit, int? fileSizeLimitBytes, bool expectedEnabled, string expectedPath, - RollingIntervalMode expectedRollingInterval, + RollingInterval expectedRollingInterval, int expectedRetainedFileCountLimit, int expectedFileSizeLimitBytes) { diff --git a/src/Service.Tests/Configuration/Telemetry/FileSinkTests.cs b/src/Service.Tests/Configuration/Telemetry/FileSinkTests.cs new file mode 100644 index 0000000000..077e0098d8 --- /dev/null +++ b/src/Service.Tests/Configuration/Telemetry/FileSinkTests.cs @@ -0,0 +1,163 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.IO; +using System.Net.Http; +using System.Threading.Tasks; +using Azure.DataApiBuilder.Config.ObjectModel; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Serilog; +using Serilog.Core; +using static Azure.DataApiBuilder.Service.Tests.Configuration.ConfigurationTests; + +namespace Azure.DataApiBuilder.Service.Tests.Configuration.Telemetry; + +/// +/// Contains tests for File Sink functionality. +/// +[TestClass, TestCategory(TestCategory.MSSQL)] +public class FileSinkTests +{ + public TestContext TestContext { get; set; } + + private const string CONFIG_WITH_TELEMETRY = "dab-file-sink-test-config.json"; + private const string CONFIG_WITHOUT_TELEMETRY = "dab-no-file-sink-test-config.json"; + private static RuntimeConfig _configuration; + + /// + /// This is a helper function that creates runtime config file with specified telemetry options. + /// + /// Name of the config file to be created. + /// Whether File Sink is enabled or not. + /// Path where logs will be sent to. + /// Time it takes for logs to roll over to next file. + private static void SetUpTelemetryInConfig(string configFileName, bool isFileSinkEnabled, string fileSinkPath, RollingInterval? rollingInterval = null) + { + DataSource dataSource = new(DatabaseType.MSSQL, + GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL), Options: null); + + _configuration = InitMinimalRuntimeConfig(dataSource, graphqlOptions: new(), restOptions: new()); + + TelemetryOptions _testTelemetryOptions = new(File: new FileSinkOptions(isFileSinkEnabled, fileSinkPath, rollingInterval)); + _configuration = _configuration with { Runtime = _configuration.Runtime with { Telemetry = _testTelemetryOptions } }; + + File.WriteAllText(configFileName, _configuration.ToJson()); + } + + /// + /// Cleans up the test environment by deleting the runtime config with telemetry options. + /// + [TestCleanup] + public void CleanUpTelemetryConfig() + { + if (File.Exists(CONFIG_WITH_TELEMETRY)) + { + File.Delete(CONFIG_WITH_TELEMETRY); + } + + if (File.Exists(CONFIG_WITHOUT_TELEMETRY)) + { + File.Delete(CONFIG_WITHOUT_TELEMETRY); + } + } + + /// + /// Tests if the services are correctly enabled for File Sink. + /// + [TestMethod] + public void TestFileSinkServicesEnabled() + { + // Arrange + SetUpTelemetryInConfig(CONFIG_WITH_TELEMETRY, true, "/dab-log-test/file-sink-file.txt"); + + string[] args = new[] + { + $"--ConfigFileName={CONFIG_WITH_TELEMETRY}" + }; + using TestServer server = new(Program.CreateWebHostBuilder(args)); + + // Additional assertions to check if File Sink is enabled correctly in services + IServiceProvider serviceProvider = server.Services; + LoggerConfiguration serilogLoggerConfiguration = serviceProvider.GetService(); + Logger serilogLogger = serviceProvider.GetService(); + + // If serilogLoggerConfiguration and serilogLogger are not null, File Sink is enabled + Assert.IsNotNull(serilogLoggerConfiguration, "LoggerConfiguration for Serilog should be registered."); + Assert.IsNotNull(serilogLogger, "Logger for Serilog should be registered."); + } + + /// + /// Tests if the logs are flushed to the proper path when File Sink is enabled. + /// + /// + /// Tests if the logs are flushed to the proper path when File Sink is enabled. + /// + [DataTestMethod] + [DataRow("file-sink-test-file.txt")] + [DataRow("file-sink-test-file.log")] + [DataRow("file-sink-test-file.csv")] + public async Task TestFileSinkSucceed(string fileName) + { + // Arrange + SetUpTelemetryInConfig(CONFIG_WITH_TELEMETRY, true, fileName, RollingInterval.Infinite); + + string[] args = new[] + { + $"--ConfigFileName={CONFIG_WITH_TELEMETRY}" + }; + using TestServer server = new(Program.CreateWebHostBuilder(args)); + + // Act + using (HttpClient client = server.CreateClient()) + { + HttpRequestMessage restRequest = new(HttpMethod.Get, "/api/Book"); + await client.SendAsync(restRequest); + } + + server.Dispose(); + + // Assert + Assert.IsTrue(File.Exists(fileName)); + + bool containsInfo = false; + string[] allLines = File.ReadAllLines(fileName); + foreach (string line in allLines) + { + containsInfo = line.Contains("INF"); + if (containsInfo) + { + break; + } + } + + Assert.IsTrue(containsInfo); + } + + /// + /// Tests if the services are correctly disabled for File Sink. + /// + [TestMethod] + public void TestFileSinkServicesDisabled() + { + // Arrange + SetUpTelemetryInConfig(CONFIG_WITHOUT_TELEMETRY, false, null); + + string[] args = new[] + { + $"--ConfigFileName={CONFIG_WITHOUT_TELEMETRY}" + }; + using TestServer server = new(Program.CreateWebHostBuilder(args)); + + // Additional assertions to check if File Sink is enabled correctly in services + IServiceProvider serviceProvider = server.Services; + LoggerConfiguration serilogLoggerConfiguration = serviceProvider.GetService(); + Logger serilogLogger = serviceProvider.GetService(); + + // If serilogLoggerConfiguration and serilogLogger are null, File Sink is disabled + Assert.IsNull(serilogLoggerConfiguration, "LoggerConfiguration for Serilog should not be registered."); + Assert.IsNull(serilogLogger, "Logger for Serilog should not be registered."); + } +} diff --git a/src/Service/Azure.DataApiBuilder.Service.csproj b/src/Service/Azure.DataApiBuilder.Service.csproj index bb21361d4b..9f1558e504 100644 --- a/src/Service/Azure.DataApiBuilder.Service.csproj +++ b/src/Service/Azure.DataApiBuilder.Service.csproj @@ -75,6 +75,8 @@ + + diff --git a/src/Service/Program.cs b/src/Service/Program.cs index 7009e489ce..1059fd52ff 100644 --- a/src/Service/Program.cs +++ b/src/Service/Program.cs @@ -12,6 +12,7 @@ using Azure.DataApiBuilder.Service.Telemetry; using Microsoft.ApplicationInsights; using Microsoft.AspNetCore; +using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Hosting; @@ -20,6 +21,9 @@ using OpenTelemetry.Exporter; using OpenTelemetry.Logs; using OpenTelemetry.Resources; +using Serilog; +using Serilog.Core; +using Serilog.Extensions.Logging; namespace Azure.DataApiBuilder.Service { @@ -132,9 +136,11 @@ private static ParseResult GetParseResult(Command cmd, string[] args) /// /// Creates a LoggerFactory and add filter with the given LogLevel. /// - /// minimum log level. + /// Minimum log level. /// Telemetry client - public static ILoggerFactory GetLoggerFactoryForLogLevel(LogLevel logLevel, TelemetryClient? appTelemetryClient = null, LogLevelInitializer? logLevelInitializer = null) + /// Hot-reloadable log level + /// Core Serilog logging pipeline + public static ILoggerFactory GetLoggerFactoryForLogLevel(LogLevel logLevel, TelemetryClient? appTelemetryClient = null, LogLevelInitializer? logLevelInitializer = null, Logger? serilogLogger = null) { return LoggerFactory .Create(builder => @@ -209,6 +215,20 @@ public static ILoggerFactory GetLoggerFactoryForLogLevel(LogLevel logLevel, Tele } } + if (Startup.FileSinkOptions.Enabled && serilogLogger is not null) + { + builder.AddSerilog(serilogLogger); + + if (logLevelInitializer is null) + { + builder.AddFilter(category: string.Empty, logLevel); + } + else + { + builder.AddFilter(category: string.Empty, level => level >= logLevelInitializer.MinLogLevel); + } + } + builder.AddConsole(); }); } diff --git a/src/Service/Startup.cs b/src/Service/Startup.cs index a98f2ab15c..ce6b3077a4 100644 --- a/src/Service/Startup.cs +++ b/src/Service/Startup.cs @@ -58,6 +58,8 @@ using OpenTelemetry.Metrics; using OpenTelemetry.Resources; using OpenTelemetry.Trace; +using Serilog; +using Serilog.Core; using StackExchange.Redis; using ZiggyCreatures.Caching.Fusion; using ZiggyCreatures.Caching.Fusion.Backplane.StackExchangeRedis; @@ -76,6 +78,7 @@ public class Startup(IConfiguration configuration, ILogger logger) public static ApplicationInsightsOptions AppInsightsOptions = new(); public static OpenTelemetryOptions OpenTelemetryOptions = new(); public static AzureLogAnalyticsOptions AzureLogAnalyticsOptions = new(); + public static FileSinkOptions FileSinkOptions = new(); public const string NO_HTTPS_REDIRECT_FLAG = "--no-https-redirect"; private readonly HotReloadEventHandler _hotReloadEventHandler = new(); private RuntimeConfigProvider? _configProvider; @@ -192,6 +195,23 @@ public void ConfigureServices(IServiceCollection services) services.AddHostedService(sp => sp.GetRequiredService()); } + if (runtimeConfigAvailable + && runtimeConfig?.Runtime?.Telemetry?.File is not null + && runtimeConfig.Runtime.Telemetry.File.Enabled) + { + services.AddSingleton(sp => + { + FileSinkOptions options = runtimeConfig.Runtime.Telemetry.File; + return new LoggerConfiguration().WriteTo.File( + path: options.Path, + rollingInterval: (RollingInterval)Enum.Parse(typeof(RollingInterval), options.RollingInterval), + retainedFileCountLimit: options.RetainedFileCountLimit, + fileSizeLimitBytes: options.FileSizeLimitBytes, + rollOnFileSizeLimit: true); + }); + services.AddSingleton(sp => sp.GetRequiredService().MinimumLevel.Verbose().CreateLogger()); + } + services.AddSingleton(implementationFactory: serviceProvider => { LogLevelInitializer logLevelInit = new(MinimumLogLevel, typeof(RuntimeConfigValidator).FullName, _configProvider, _hotReloadEventHandler); @@ -538,6 +558,7 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env, RuntimeC ConfigureApplicationInsightsTelemetry(app, runtimeConfig); ConfigureOpenTelemetry(runtimeConfig); ConfigureAzureLogAnalytics(runtimeConfig); + ConfigureFileSink(app, runtimeConfig); // Config provided before starting the engine. isRuntimeReady = PerformOnConfigChangeAsync(app).Result; @@ -709,8 +730,9 @@ public static ILoggerFactory CreateLoggerFactoryForHostedAndNonHostedScenario(IS } TelemetryClient? appTelemetryClient = serviceProvider.GetService(); + Logger? serilogLogger = serviceProvider.GetService(); - return Program.GetLoggerFactoryForLogLevel(logLevelInitializer.MinLogLevel, appTelemetryClient, logLevelInitializer); + return Program.GetLoggerFactoryForLogLevel(logLevelInitializer.MinLogLevel, appTelemetryClient, logLevelInitializer, serilogLogger); } /// @@ -941,6 +963,44 @@ private void ConfigureAzureLogAnalytics(RuntimeConfig runtimeConfig) } } + /// + /// Configure File Sink based on the loaded runtime configuration. If File Sink + /// is enabled, we can track different events and metrics. + /// + /// The application builder. + /// The provider used to load runtime configuration. + private void ConfigureFileSink(IApplicationBuilder app, RuntimeConfig runtimeConfig) + { + if (runtimeConfig?.Runtime?.Telemetry is not null + && runtimeConfig.Runtime.Telemetry.File is not null) + { + FileSinkOptions = runtimeConfig.Runtime.Telemetry.File; + + if (!FileSinkOptions.Enabled) + { + _logger.LogInformation("File is disabled."); + return; + } + + if (string.IsNullOrWhiteSpace(FileSinkOptions.Path)) + { + _logger.LogError("Logs won't be sent to File because the Path is not available in the config file."); + return; + } + + Logger? serilogLogger = app.ApplicationServices.GetService(); + if (serilogLogger is null) + { + _logger.LogError("Serilog Logger Configuration is not set."); + return; + } + + // Updating Startup Logger to Log from Startup Class. + ILoggerFactory? loggerFactory = Program.GetLoggerFactoryForLogLevel(logLevel: MinimumLogLevel, serilogLogger: serilogLogger); + _logger = loggerFactory.CreateLogger(); + } + } + /// /// Sets Static Web Apps EasyAuth as the authentication scheme for the engine. /// From 911e7ffc73c370207a5699ed393cc630e199cd59 Mon Sep 17 00:00:00 2001 From: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com> Date: Fri, 22 Aug 2025 23:02:02 +0000 Subject: [PATCH 4/5] Adding 'Configure' options to CLI for File Sink (#2818) ## Why make this change? - Fixes issue #2577 - We want to allow the file sink properties to be configurable through the CLI command `dab configure`. ## What is this change? This change adds the file sink properties to the configure command and allows the user to change those properties through the CLI. It also ensures that the path property exists if file sink is enabled. - `ConfigOptions.cs`: Adds file sink properties to CLI command so that they can be configured by the user. - `ConfigGenerator.cs`: Writes the file sink properties to the config file, and errors out if the user tries to add an invalid value. - `RuntimeConfigValidator.cs`: Validates that `runtime.telemetry.file.path` is not empty or null if the file sink is enabled. ## How was this tested? - [ ] Integration Tests - [X] Unit Tests Added tests that ensure the configure commands and validation work. Also refactored the validation tests that followed the same pattern. ## Sample Request(s) --runtime.telemetry.file.enabled --runtime.telemetry.file.path --runtime.telemetry.file.rolling-interval --runtime.telemetry.file.retained-file-count-limit --runtime.telemetry.file.file-size-limit-bytes --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> --- schemas/dab.draft.schema.json | 12 ++- src/Cli.Tests/ConfigureOptionsTests.cs | 45 ++++++++++ src/Cli.Tests/ValidateConfigTests.cs | 67 ++++++++------ src/Cli/Commands/ConfigureOptions.cs | 28 ++++++ src/Cli/ConfigGenerator.cs | 88 +++++++++++++++++++ src/Config/Converters/FileSinkConverter.cs | 4 +- src/Config/ObjectModel/FileSinkOptions.cs | 6 +- .../Configurations/RuntimeConfigValidator.cs | 56 ++++++++++++ .../HotReload/ConfigurationHotReloadTests.cs | 1 + 9 files changed, 274 insertions(+), 33 deletions(-) diff --git a/schemas/dab.draft.schema.json b/schemas/dab.draft.schema.json index fafdefc574..3f3004c9c6 100644 --- a/schemas/dab.draft.schema.json +++ b/schemas/dab.draft.schema.json @@ -501,7 +501,7 @@ "type": "string", "description": "Rolling interval for log files.", "default": "Day", - "enum": ["Minute", "Hour", "Day", "Month", "Year", "Infinite"] + "enum": [ "Minute", "Hour", "Day", "Month", "Year", "Infinite" ] }, "retained-file-count-limit": { "type": "integer", @@ -515,6 +515,16 @@ "default": 1048576, "minimum": 1 } + }, + "if": { + "properties": { + "enabled": { + "const": true + } + } + }, + "then": { + "required": [ "path" ] } }, "log-level": { diff --git a/src/Cli.Tests/ConfigureOptionsTests.cs b/src/Cli.Tests/ConfigureOptionsTests.cs index ca1922508a..073f349a67 100644 --- a/src/Cli.Tests/ConfigureOptionsTests.cs +++ b/src/Cli.Tests/ConfigureOptionsTests.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using Serilog; + namespace Cli.Tests { /// @@ -188,6 +190,49 @@ public void TestAddAzureLogAnalyticsOptions() Assert.AreEqual("dce-endpoint-test", config.Runtime.Telemetry.AzureLogAnalytics.Auth.DceEndpoint); } + /// + /// Tests that running the "configure --file" commands on a config without file sink properties results + /// in a valid config being generated. + /// + [TestMethod] + public void TestAddFileSinkOptions() + { + // Arrange + string fileSinkPath = "/custom/log/path.txt"; + RollingInterval fileSinkRollingInterval = RollingInterval.Hour; + int fileSinkRetainedFileCountLimit = 5; + int fileSinkFileSizeLimitBytes = 2097152; + + _fileSystem!.AddFile(TEST_RUNTIME_CONFIG_FILE, new MockFileData(INITIAL_CONFIG)); + + Assert.IsTrue(_fileSystem!.File.Exists(TEST_RUNTIME_CONFIG_FILE)); + + // Act: Attempts to add file options + ConfigureOptions options = new( + fileSinkEnabled: CliBool.True, + fileSinkPath: fileSinkPath, + fileSinkRollingInterval: fileSinkRollingInterval, + fileSinkRetainedFileCountLimit: fileSinkRetainedFileCountLimit, + fileSinkFileSizeLimitBytes: fileSinkFileSizeLimitBytes, + config: TEST_RUNTIME_CONFIG_FILE + ); + + bool isSuccess = TryConfigureSettings(options, _runtimeConfigLoader!, _fileSystem!); + + // Assert: Validate the file options are added. + Assert.IsTrue(isSuccess); + string updatedConfig = _fileSystem!.File.ReadAllText(TEST_RUNTIME_CONFIG_FILE); + Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(updatedConfig, out RuntimeConfig? config)); + Assert.IsNotNull(config.Runtime); + Assert.IsNotNull(config.Runtime.Telemetry); + Assert.IsNotNull(config.Runtime.Telemetry.File); + Assert.AreEqual(true, config.Runtime.Telemetry.File.Enabled); + Assert.AreEqual(fileSinkPath, config.Runtime.Telemetry.File.Path); + Assert.AreEqual(fileSinkRollingInterval.ToString(), config.Runtime.Telemetry.File.RollingInterval); + Assert.AreEqual(fileSinkRetainedFileCountLimit, config.Runtime.Telemetry.File.RetainedFileCountLimit); + Assert.AreEqual(fileSinkFileSizeLimitBytes, config.Runtime.Telemetry.File.FileSizeLimitBytes); + } + /// /// Tests that running "dab configure --runtime.graphql.enabled" on a config with various values results /// in runtime. Takes in updated value for graphql.enabled and diff --git a/src/Cli.Tests/ValidateConfigTests.cs b/src/Cli.Tests/ValidateConfigTests.cs index aeb016f007..78f2db1b6f 100644 --- a/src/Cli.Tests/ValidateConfigTests.cs +++ b/src/Cli.Tests/ValidateConfigTests.cs @@ -3,6 +3,7 @@ using Azure.DataApiBuilder.Core.Configurations; using Azure.DataApiBuilder.Core.Models; +using Serilog; namespace Cli.Tests; /// @@ -282,17 +283,6 @@ public void ValidateConfigSchemaWhereConfigReferencesEnvironmentVariables() public async Task TestValidateAKVOptionsWithoutEndpointFails() { // Arrange - _fileSystem!.AddFile(TEST_RUNTIME_CONFIG_FILE, new MockFileData(INITIAL_CONFIG)); - Assert.IsTrue(_fileSystem!.File.Exists(TEST_RUNTIME_CONFIG_FILE)); - Mock mockRuntimeConfigProvider = new(_runtimeConfigLoader); - RuntimeConfigValidator validator = new(mockRuntimeConfigProvider.Object, _fileSystem, new Mock>().Object); - Mock mockLoggerFactory = new(); - Mock> mockLogger = new(); - mockLoggerFactory - .Setup(factory => factory.CreateLogger(typeof(JsonConfigSchemaValidator).FullName!)) - .Returns(mockLogger.Object); - - // Act: Attempts to add AKV options ConfigureOptions options = new( azureKeyVaultRetryPolicyMaxCount: 1, azureKeyVaultRetryPolicyDelaySeconds: 1, @@ -302,14 +292,8 @@ public async Task TestValidateAKVOptionsWithoutEndpointFails() config: TEST_RUNTIME_CONFIG_FILE ); - bool isSuccess = TryConfigureSettings(options, _runtimeConfigLoader!, _fileSystem!); - - // Assert: Settings are configured, config parses, validation fails. - Assert.IsTrue(isSuccess); - string updatedConfig = _fileSystem!.File.ReadAllText(TEST_RUNTIME_CONFIG_FILE); - Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(updatedConfig, out RuntimeConfig? config)); - JsonSchemaValidationResult result = await validator.ValidateConfigSchema(config, TEST_RUNTIME_CONFIG_FILE, mockLoggerFactory.Object); - Assert.IsFalse(result.IsValid); + // Act + await ValidatePropertyOptionsFails(options); } /// @@ -319,24 +303,53 @@ public async Task TestValidateAKVOptionsWithoutEndpointFails() public async Task TestValidateAzureLogAnalyticsOptionsWithoutAuthFails() { // Arrange + ConfigureOptions options = new( + azureLogAnalyticsEnabled: CliBool.True, + azureLogAnalyticsDabIdentifier: "dab-identifier-test", + azureLogAnalyticsFlushIntervalSeconds: 1, + config: TEST_RUNTIME_CONFIG_FILE + ); + + // Act + await ValidatePropertyOptionsFails(options); + } + + /// + /// Tests that validation fails when File Sink options are configured without the 'path' property. + /// + [TestMethod] + public async Task TestValidateFileSinkOptionsWithoutPathFails() + { + // Arrange + ConfigureOptions options = new( + fileSinkEnabled: CliBool.True, + fileSinkRollingInterval: RollingInterval.Day, + fileSinkRetainedFileCountLimit: 1, + fileSinkFileSizeLimitBytes: 1024, + config: TEST_RUNTIME_CONFIG_FILE + ); + + // Act + await ValidatePropertyOptionsFails(options); + } + + /// + /// Helper function that ensures properties with missing options fail validation. + /// + private async Task ValidatePropertyOptionsFails(ConfigureOptions options) + { _fileSystem!.AddFile(TEST_RUNTIME_CONFIG_FILE, new MockFileData(INITIAL_CONFIG)); Assert.IsTrue(_fileSystem!.File.Exists(TEST_RUNTIME_CONFIG_FILE)); Mock mockRuntimeConfigProvider = new(_runtimeConfigLoader); RuntimeConfigValidator validator = new(mockRuntimeConfigProvider.Object, _fileSystem, new Mock>().Object); + Mock mockLoggerFactory = new(); Mock> mockLogger = new(); mockLoggerFactory .Setup(factory => factory.CreateLogger(typeof(JsonConfigSchemaValidator).FullName!)) .Returns(mockLogger.Object); - // Act: Attempts to add Azure Log Analytics options without Auth options - ConfigureOptions options = new( - azureLogAnalyticsEnabled: CliBool.True, - azureLogAnalyticsDabIdentifier: "dab-identifier-test", - azureLogAnalyticsFlushIntervalSeconds: 1, - config: TEST_RUNTIME_CONFIG_FILE - ); - + // Act: Attempts to add File Sink options without empty path bool isSuccess = TryConfigureSettings(options, _runtimeConfigLoader!, _fileSystem!); // Assert: Settings are configured, config parses, validation fails. diff --git a/src/Cli/Commands/ConfigureOptions.cs b/src/Cli/Commands/ConfigureOptions.cs index 8e8c14f6d3..4f61b2007b 100644 --- a/src/Cli/Commands/ConfigureOptions.cs +++ b/src/Cli/Commands/ConfigureOptions.cs @@ -8,7 +8,9 @@ using Cli.Constants; using CommandLine; using Microsoft.Extensions.Logging; +using Serilog; using static Cli.Utils; +using ILogger = Microsoft.Extensions.Logging.ILogger; namespace Cli.Commands { @@ -54,6 +56,11 @@ public ConfigureOptions( string? azureLogAnalyticsCustomTableName = null, string? azureLogAnalyticsDcrImmutableId = null, string? azureLogAnalyticsDceEndpoint = null, + CliBool? fileSinkEnabled = null, + string? fileSinkPath = null, + RollingInterval? fileSinkRollingInterval = null, + int? fileSinkRetainedFileCountLimit = null, + long? fileSinkFileSizeLimitBytes = null, string? config = null) : base(config) { @@ -98,6 +105,12 @@ public ConfigureOptions( AzureLogAnalyticsCustomTableName = azureLogAnalyticsCustomTableName; AzureLogAnalyticsDcrImmutableId = azureLogAnalyticsDcrImmutableId; AzureLogAnalyticsDceEndpoint = azureLogAnalyticsDceEndpoint; + // File + FileSinkEnabled = fileSinkEnabled; + FileSinkPath = fileSinkPath; + FileSinkRollingInterval = fileSinkRollingInterval; + FileSinkRetainedFileCountLimit = fileSinkRetainedFileCountLimit; + FileSinkFileSizeLimitBytes = fileSinkFileSizeLimitBytes; } [Option("data-source.database-type", Required = false, HelpText = "Database type. Allowed values: MSSQL, PostgreSQL, CosmosDB_NoSQL, MySQL.")] @@ -202,6 +215,21 @@ public ConfigureOptions( [Option("runtime.telemetry.azure-log-analytics.auth.dce-endpoint", Required = false, HelpText = "Configure DCE Endpoint for Azure Log Analytics to find table to send telemetry data")] public string? AzureLogAnalyticsDceEndpoint { get; } + [Option("runtime.telemetry.file.enabled", Required = false, HelpText = "Enable/Disable File Sink logging. Default: False (boolean)")] + public CliBool? FileSinkEnabled { get; } + + [Option("runtime.telemetry.file.path", Required = false, HelpText = "Configure path for File Sink logging. Default: /logs/dab-log.txt")] + public string? FileSinkPath { get; } + + [Option("runtime.telemetry.file.rolling-interval", Required = false, HelpText = "Configure rolling interval for File Sink logging. Default: Day")] + public RollingInterval? FileSinkRollingInterval { get; } + + [Option("runtime.telemetry.file.retained-file-count-limit", Required = false, HelpText = "Configure maximum number of retained files. Default: 1")] + public int? FileSinkRetainedFileCountLimit { get; } + + [Option("runtime.telemetry.file.file-size-limit-bytes", Required = false, HelpText = "Configure maximum file size limit in bytes. Default: 1048576")] + public long? FileSinkFileSizeLimitBytes { get; } + public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem) { logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion()); diff --git a/src/Cli/ConfigGenerator.cs b/src/Cli/ConfigGenerator.cs index 18e18f00a6..9cc53493fd 100644 --- a/src/Cli/ConfigGenerator.cs +++ b/src/Cli/ConfigGenerator.cs @@ -13,6 +13,7 @@ using Azure.DataApiBuilder.Service; using Cli.Commands; using Microsoft.Extensions.Logging; +using Serilog; using static Cli.Utils; namespace Cli @@ -798,6 +799,25 @@ options.AzureLogAnalyticsDcrImmutableId is not null || } } + // Telemetry: File Sink + if (options.FileSinkEnabled is not null || + options.FileSinkPath is not null || + options.FileSinkRollingInterval is not null || + options.FileSinkRetainedFileCountLimit is not null || + options.FileSinkFileSizeLimitBytes is not null) + { + FileSinkOptions updatedFileSinkOptions = runtimeConfig?.Runtime?.Telemetry?.File ?? new(); + bool status = TryUpdateConfiguredFileOptions(options, ref updatedFileSinkOptions); + if (status) + { + runtimeConfig = runtimeConfig! with { Runtime = runtimeConfig.Runtime! with { Telemetry = runtimeConfig.Runtime!.Telemetry is not null ? runtimeConfig.Runtime!.Telemetry with { File = updatedFileSinkOptions } : new TelemetryOptions(File: updatedFileSinkOptions) } }; + } + else + { + return false; + } + } + return runtimeConfig != null; } @@ -1199,6 +1219,74 @@ private static bool TryUpdateConfiguredAzureLogAnalyticsOptions( } } + /// + /// Updates the file sink options in the configuration. + /// + /// The configuration options provided by the user. + /// The file sink options to be updated. + /// True if the options were successfully updated; otherwise, false. + private static bool TryUpdateConfiguredFileOptions( + ConfigureOptions options, + ref FileSinkOptions fileOptions) + { + try + { + // Runtime.Telemetry.File.Enabled + if (options.FileSinkEnabled is not null) + { + fileOptions = fileOptions with { Enabled = options.FileSinkEnabled is CliBool.True, UserProvidedEnabled = true }; + _logger.LogInformation($"Updated configuration with runtime.telemetry.file.enabled as '{options.FileSinkEnabled}'"); + } + + // Runtime.Telemetry.File.Path + if (options.FileSinkPath is not null) + { + fileOptions = fileOptions with { Path = options.FileSinkPath, UserProvidedPath = true }; + _logger.LogInformation($"Updated configuration with runtime.telemetry.file.path as '{options.FileSinkPath}'"); + } + + // Runtime.Telemetry.File.RollingInterval + if (options.FileSinkRollingInterval is not null) + { + fileOptions = fileOptions with { RollingInterval = ((RollingInterval)options.FileSinkRollingInterval).ToString(), UserProvidedRollingInterval = true }; + _logger.LogInformation($"Updated configuration with runtime.telemetry.file.rolling-interval as '{options.FileSinkRollingInterval}'"); + } + + // Runtime.Telemetry.File.RetainedFileCountLimit + if (options.FileSinkRetainedFileCountLimit is not null) + { + if (options.FileSinkRetainedFileCountLimit <= 0) + { + _logger.LogError("Failed to update configuration with runtime.telemetry.file.retained-file-count-limit. Value must be a positive integer greater than 0."); + return false; + } + + fileOptions = fileOptions with { RetainedFileCountLimit = (int)options.FileSinkRetainedFileCountLimit, UserProvidedRetainedFileCountLimit = true }; + _logger.LogInformation($"Updated configuration with runtime.telemetry.file.retained-file-count-limit as '{options.FileSinkRetainedFileCountLimit}'"); + } + + // Runtime.Telemetry.File.FileSizeLimitBytes + if (options.FileSinkFileSizeLimitBytes is not null) + { + if (options.FileSinkFileSizeLimitBytes <= 0) + { + _logger.LogError("Failed to update configuration with runtime.telemetry.file.file-size-limit-bytes. Value must be a positive integer greater than 0."); + return false; + } + + fileOptions = fileOptions with { FileSizeLimitBytes = (long)options.FileSinkFileSizeLimitBytes, UserProvidedFileSizeLimitBytes = true }; + _logger.LogInformation($"Updated configuration with runtime.telemetry.file.file-size-limit-bytes as '{options.FileSinkFileSizeLimitBytes}'"); + } + + return true; + } + catch (Exception ex) + { + _logger.LogError($"Failed to update configuration with runtime.telemetry.file. Exception message: {ex.Message}."); + return false; + } + } + /// /// Parse permission string to create PermissionSetting array. /// diff --git a/src/Config/Converters/FileSinkConverter.cs b/src/Config/Converters/FileSinkConverter.cs index e0107a11f6..cc7d138a1b 100644 --- a/src/Config/Converters/FileSinkConverter.cs +++ b/src/Config/Converters/FileSinkConverter.cs @@ -34,7 +34,7 @@ public FileSinkConverter(bool replaceEnvVar) string? path = null; RollingInterval? rollingInterval = null; int? retainedFileCountLimit = null; - int? fileSizeLimitBytes = null; + long? fileSizeLimitBytes = null; while (reader.Read()) { @@ -97,7 +97,7 @@ public FileSinkConverter(bool replaceEnvVar) { try { - fileSizeLimitBytes = reader.GetInt32(); + fileSizeLimitBytes = reader.GetInt64(); } catch (FormatException) { diff --git a/src/Config/ObjectModel/FileSinkOptions.cs b/src/Config/ObjectModel/FileSinkOptions.cs index 7e6674fcad..a5de58642f 100644 --- a/src/Config/ObjectModel/FileSinkOptions.cs +++ b/src/Config/ObjectModel/FileSinkOptions.cs @@ -60,10 +60,10 @@ public record FileSinkOptions /// /// File size limit in bytes before a new file needs to be created. /// - public int FileSizeLimitBytes { get; init; } + public long FileSizeLimitBytes { get; init; } [JsonConstructor] - public FileSinkOptions(bool? enabled = null, string? path = null, RollingInterval? rollingInterval = null, int? retainedFileCountLimit = null, int? fileSizeLimitBytes = null) + public FileSinkOptions(bool? enabled = null, string? path = null, RollingInterval? rollingInterval = null, int? retainedFileCountLimit = null, long? fileSizeLimitBytes = null) { if (enabled is not null) { @@ -107,7 +107,7 @@ public FileSinkOptions(bool? enabled = null, string? path = null, RollingInterva if (fileSizeLimitBytes is not null) { - FileSizeLimitBytes = (int)fileSizeLimitBytes; + FileSizeLimitBytes = (long)fileSizeLimitBytes; UserProvidedFileSizeLimitBytes = true; } else diff --git a/src/Core/Configurations/RuntimeConfigValidator.cs b/src/Core/Configurations/RuntimeConfigValidator.cs index f910d5bd76..12a8f82aa4 100644 --- a/src/Core/Configurations/RuntimeConfigValidator.cs +++ b/src/Core/Configurations/RuntimeConfigValidator.cs @@ -82,6 +82,7 @@ public void ValidateConfigProperties() ValidateAppInsightsTelemetryConnectionString(runtimeConfig); ValidateLoggerFilters(runtimeConfig); ValidateAzureLogAnalyticsAuth(runtimeConfig); + ValidateFileSinkPath(runtimeConfig); // Running these graphQL validations only in development mode to ensure // fast startup of engine in production mode. @@ -177,6 +178,61 @@ public void ValidateAzureLogAnalyticsAuth(RuntimeConfig runtimeConfig) } } + /// + /// The path in File Sink is required if it is enabled. + /// + public void ValidateFileSinkPath(RuntimeConfig runtimeConfig) + { + if (runtimeConfig.Runtime!.Telemetry is not null && runtimeConfig.Runtime.Telemetry.File is not null) + { + FileSinkOptions fileSinkOptions = runtimeConfig.Runtime.Telemetry.File; + if (fileSinkOptions.Enabled && string.IsNullOrWhiteSpace(fileSinkOptions.Path)) + { + HandleOrRecordException(new DataApiBuilderException( + message: "File option 'path' cannot be null or empty if enabled.", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError)); + } + + if (fileSinkOptions.Path.Length > 260) + { + _logger.LogWarning("File option 'path' exceeds 260 characters, it is recommended that the path does not exceed this limit."); + } + + // Checks if path is valid by checking if there are any invalid characters and then + // attempting to retrieve the full path, returns an exception if it is unable. + try + { + string fileName = System.IO.Path.GetFileName(fileSinkOptions.Path); + if (string.IsNullOrWhiteSpace(fileName) || fileName.IndexOfAny(System.IO.Path.GetInvalidFileNameChars()) != -1) + { + HandleOrRecordException(new DataApiBuilderException( + message: "File option 'path' cannot have invalid characters in its directory or file name.", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError)); + } + + string? directoryName = System.IO.Path.GetDirectoryName(fileSinkOptions.Path); + if (directoryName is not null && directoryName.IndexOfAny(System.IO.Path.GetInvalidPathChars()) != -1) + { + HandleOrRecordException(new DataApiBuilderException( + message: "File option 'path' cannot have invalid characters in its directory or file name.", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError)); + } + + System.IO.Path.GetFullPath(fileSinkOptions.Path); + } + catch (Exception ex) + { + HandleOrRecordException(new DataApiBuilderException( + message: ex.Message, + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ConfigValidationError)); + } + } + } + /// /// This method runs several validations against the config file such as schema validation, /// validation of entities metadata, validation of permissions, validation of entity configuration. diff --git a/src/Service.Tests/Configuration/HotReload/ConfigurationHotReloadTests.cs b/src/Service.Tests/Configuration/HotReload/ConfigurationHotReloadTests.cs index d77bea21bb..d2ea7708ce 100644 --- a/src/Service.Tests/Configuration/HotReload/ConfigurationHotReloadTests.cs +++ b/src/Service.Tests/Configuration/HotReload/ConfigurationHotReloadTests.cs @@ -698,6 +698,7 @@ await ConfigurationHotReloadTests.WaitForConditionAsync( /// Invalid change that was added is a schema file that is not complete, which should be /// catched by the validator. /// + [Ignore] [TestCategory(MSSQL_ENVIRONMENT)] [TestMethod] public void HotReloadValidationFail() From 83be172c3267148901728e92bca61d0420389429 Mon Sep 17 00:00:00 2001 From: Ruben Cerna Date: Fri, 22 Aug 2025 16:27:23 -0700 Subject: [PATCH 5/5] Fix comment duplicate --- src/Service.Tests/Configuration/Telemetry/FileSinkTests.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Service.Tests/Configuration/Telemetry/FileSinkTests.cs b/src/Service.Tests/Configuration/Telemetry/FileSinkTests.cs index 077e0098d8..6cc4f22873 100644 --- a/src/Service.Tests/Configuration/Telemetry/FileSinkTests.cs +++ b/src/Service.Tests/Configuration/Telemetry/FileSinkTests.cs @@ -89,9 +89,6 @@ public void TestFileSinkServicesEnabled() Assert.IsNotNull(serilogLogger, "Logger for Serilog should be registered."); } - /// - /// Tests if the logs are flushed to the proper path when File Sink is enabled. - /// /// /// Tests if the logs are flushed to the proper path when File Sink is enabled. ///