Skip to content

Commit c1e6600

Browse files
RubenCerna2079CopilotM4AlMu4allAniruddh25
authored
Cherry-Pick File Sink & Bug Fixes for 1.6 Release (#2852)
## Why make this change? - The 1.6 release is missing the implementation for the File Sink feature, as well as the bug fixes to fix the session context and fix implementation of row-level security when users use a JWT token. ## What is this change? Cherry-picked PRs: - File Sink: - #2752 - #2825 - #2818 - Bug Fix: - #2344 ## How was this tested? - [ ] Integration Tests - [X] Unit Tests --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: M4Al <[email protected]> Co-authored-by: KobeLenjou <[email protected]> Co-authored-by: Aniruddh Munde <[email protected]>
1 parent b032ff8 commit c1e6600

23 files changed

+1002
-43
lines changed

schemas/dab.draft.schema.json

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,50 @@
483483
"required": [ "auth" ]
484484
}
485485
},
486+
"file": {
487+
"type": "object",
488+
"additionalProperties": false,
489+
"properties": {
490+
"enabled": {
491+
"type": "boolean",
492+
"description": "Enable/disable file sink telemetry logging.",
493+
"default": false
494+
},
495+
"path": {
496+
"type": "string",
497+
"description": "File path for telemetry logs.",
498+
"default": "/logs/dab-log.txt"
499+
},
500+
"rolling-interval": {
501+
"type": "string",
502+
"description": "Rolling interval for log files.",
503+
"default": "Day",
504+
"enum": [ "Minute", "Hour", "Day", "Month", "Year", "Infinite" ]
505+
},
506+
"retained-file-count-limit": {
507+
"type": "integer",
508+
"description": "Maximum number of retained log files.",
509+
"default": 1,
510+
"minimum": 1
511+
},
512+
"file-size-limit-bytes": {
513+
"type": "integer",
514+
"description": "Maximum file size in bytes before rolling.",
515+
"default": 1048576,
516+
"minimum": 1
517+
}
518+
},
519+
"if": {
520+
"properties": {
521+
"enabled": {
522+
"const": true
523+
}
524+
}
525+
},
526+
"then": {
527+
"required": [ "path" ]
528+
}
529+
},
486530
"log-level": {
487531
"type": "object",
488532
"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",

src/Cli.Tests/ConfigureOptionsTests.cs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
using Serilog;
5+
46
namespace Cli.Tests
57
{
68
/// <summary>
@@ -188,6 +190,49 @@ public void TestAddAzureLogAnalyticsOptions()
188190
Assert.AreEqual("dce-endpoint-test", config.Runtime.Telemetry.AzureLogAnalytics.Auth.DceEndpoint);
189191
}
190192

193+
/// <summary>
194+
/// Tests that running the "configure --file" commands on a config without file sink properties results
195+
/// in a valid config being generated.
196+
/// </summary>
197+
[TestMethod]
198+
public void TestAddFileSinkOptions()
199+
{
200+
// Arrange
201+
string fileSinkPath = "/custom/log/path.txt";
202+
RollingInterval fileSinkRollingInterval = RollingInterval.Hour;
203+
int fileSinkRetainedFileCountLimit = 5;
204+
int fileSinkFileSizeLimitBytes = 2097152;
205+
206+
_fileSystem!.AddFile(TEST_RUNTIME_CONFIG_FILE, new MockFileData(INITIAL_CONFIG));
207+
208+
Assert.IsTrue(_fileSystem!.File.Exists(TEST_RUNTIME_CONFIG_FILE));
209+
210+
// Act: Attempts to add file options
211+
ConfigureOptions options = new(
212+
fileSinkEnabled: CliBool.True,
213+
fileSinkPath: fileSinkPath,
214+
fileSinkRollingInterval: fileSinkRollingInterval,
215+
fileSinkRetainedFileCountLimit: fileSinkRetainedFileCountLimit,
216+
fileSinkFileSizeLimitBytes: fileSinkFileSizeLimitBytes,
217+
config: TEST_RUNTIME_CONFIG_FILE
218+
);
219+
220+
bool isSuccess = TryConfigureSettings(options, _runtimeConfigLoader!, _fileSystem!);
221+
222+
// Assert: Validate the file options are added.
223+
Assert.IsTrue(isSuccess);
224+
string updatedConfig = _fileSystem!.File.ReadAllText(TEST_RUNTIME_CONFIG_FILE);
225+
Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(updatedConfig, out RuntimeConfig? config));
226+
Assert.IsNotNull(config.Runtime);
227+
Assert.IsNotNull(config.Runtime.Telemetry);
228+
Assert.IsNotNull(config.Runtime.Telemetry.File);
229+
Assert.AreEqual(true, config.Runtime.Telemetry.File.Enabled);
230+
Assert.AreEqual(fileSinkPath, config.Runtime.Telemetry.File.Path);
231+
Assert.AreEqual(fileSinkRollingInterval.ToString(), config.Runtime.Telemetry.File.RollingInterval);
232+
Assert.AreEqual(fileSinkRetainedFileCountLimit, config.Runtime.Telemetry.File.RetainedFileCountLimit);
233+
Assert.AreEqual(fileSinkFileSizeLimitBytes, config.Runtime.Telemetry.File.FileSizeLimitBytes);
234+
}
235+
191236
/// <summary>
192237
/// Tests that running "dab configure --runtime.graphql.enabled" on a config with various values results
193238
/// in runtime. Takes in updated value for graphql.enabled and

src/Cli.Tests/ValidateConfigTests.cs

Lines changed: 40 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using Azure.DataApiBuilder.Core.Configurations;
55
using Azure.DataApiBuilder.Core.Models;
6+
using Serilog;
67

78
namespace Cli.Tests;
89
/// <summary>
@@ -282,17 +283,6 @@ public void ValidateConfigSchemaWhereConfigReferencesEnvironmentVariables()
282283
public async Task TestValidateAKVOptionsWithoutEndpointFails()
283284
{
284285
// Arrange
285-
_fileSystem!.AddFile(TEST_RUNTIME_CONFIG_FILE, new MockFileData(INITIAL_CONFIG));
286-
Assert.IsTrue(_fileSystem!.File.Exists(TEST_RUNTIME_CONFIG_FILE));
287-
Mock<RuntimeConfigProvider> mockRuntimeConfigProvider = new(_runtimeConfigLoader);
288-
RuntimeConfigValidator validator = new(mockRuntimeConfigProvider.Object, _fileSystem, new Mock<ILogger<RuntimeConfigValidator>>().Object);
289-
Mock<ILoggerFactory> mockLoggerFactory = new();
290-
Mock<ILogger<JsonConfigSchemaValidator>> mockLogger = new();
291-
mockLoggerFactory
292-
.Setup(factory => factory.CreateLogger(typeof(JsonConfigSchemaValidator).FullName!))
293-
.Returns(mockLogger.Object);
294-
295-
// Act: Attempts to add AKV options
296286
ConfigureOptions options = new(
297287
azureKeyVaultRetryPolicyMaxCount: 1,
298288
azureKeyVaultRetryPolicyDelaySeconds: 1,
@@ -302,14 +292,8 @@ public async Task TestValidateAKVOptionsWithoutEndpointFails()
302292
config: TEST_RUNTIME_CONFIG_FILE
303293
);
304294

305-
bool isSuccess = TryConfigureSettings(options, _runtimeConfigLoader!, _fileSystem!);
306-
307-
// Assert: Settings are configured, config parses, validation fails.
308-
Assert.IsTrue(isSuccess);
309-
string updatedConfig = _fileSystem!.File.ReadAllText(TEST_RUNTIME_CONFIG_FILE);
310-
Assert.IsTrue(RuntimeConfigLoader.TryParseConfig(updatedConfig, out RuntimeConfig? config));
311-
JsonSchemaValidationResult result = await validator.ValidateConfigSchema(config, TEST_RUNTIME_CONFIG_FILE, mockLoggerFactory.Object);
312-
Assert.IsFalse(result.IsValid);
295+
// Act
296+
await ValidatePropertyOptionsFails(options);
313297
}
314298

315299
/// <summary>
@@ -319,24 +303,53 @@ public async Task TestValidateAKVOptionsWithoutEndpointFails()
319303
public async Task TestValidateAzureLogAnalyticsOptionsWithoutAuthFails()
320304
{
321305
// Arrange
306+
ConfigureOptions options = new(
307+
azureLogAnalyticsEnabled: CliBool.True,
308+
azureLogAnalyticsDabIdentifier: "dab-identifier-test",
309+
azureLogAnalyticsFlushIntervalSeconds: 1,
310+
config: TEST_RUNTIME_CONFIG_FILE
311+
);
312+
313+
// Act
314+
await ValidatePropertyOptionsFails(options);
315+
}
316+
317+
/// <summary>
318+
/// Tests that validation fails when File Sink options are configured without the 'path' property.
319+
/// </summary>
320+
[TestMethod]
321+
public async Task TestValidateFileSinkOptionsWithoutPathFails()
322+
{
323+
// Arrange
324+
ConfigureOptions options = new(
325+
fileSinkEnabled: CliBool.True,
326+
fileSinkRollingInterval: RollingInterval.Day,
327+
fileSinkRetainedFileCountLimit: 1,
328+
fileSinkFileSizeLimitBytes: 1024,
329+
config: TEST_RUNTIME_CONFIG_FILE
330+
);
331+
332+
// Act
333+
await ValidatePropertyOptionsFails(options);
334+
}
335+
336+
/// <summary>
337+
/// Helper function that ensures properties with missing options fail validation.
338+
/// </summary>
339+
private async Task ValidatePropertyOptionsFails(ConfigureOptions options)
340+
{
322341
_fileSystem!.AddFile(TEST_RUNTIME_CONFIG_FILE, new MockFileData(INITIAL_CONFIG));
323342
Assert.IsTrue(_fileSystem!.File.Exists(TEST_RUNTIME_CONFIG_FILE));
324343
Mock<RuntimeConfigProvider> mockRuntimeConfigProvider = new(_runtimeConfigLoader);
325344
RuntimeConfigValidator validator = new(mockRuntimeConfigProvider.Object, _fileSystem, new Mock<ILogger<RuntimeConfigValidator>>().Object);
345+
326346
Mock<ILoggerFactory> mockLoggerFactory = new();
327347
Mock<ILogger<JsonConfigSchemaValidator>> mockLogger = new();
328348
mockLoggerFactory
329349
.Setup(factory => factory.CreateLogger(typeof(JsonConfigSchemaValidator).FullName!))
330350
.Returns(mockLogger.Object);
331351

332-
// Act: Attempts to add Azure Log Analytics options without Auth options
333-
ConfigureOptions options = new(
334-
azureLogAnalyticsEnabled: CliBool.True,
335-
azureLogAnalyticsDabIdentifier: "dab-identifier-test",
336-
azureLogAnalyticsFlushIntervalSeconds: 1,
337-
config: TEST_RUNTIME_CONFIG_FILE
338-
);
339-
352+
// Act: Attempts to add File Sink options without empty path
340353
bool isSuccess = TryConfigureSettings(options, _runtimeConfigLoader!, _fileSystem!);
341354

342355
// Assert: Settings are configured, config parses, validation fails.

src/Cli/Commands/ConfigureOptions.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88
using Cli.Constants;
99
using CommandLine;
1010
using Microsoft.Extensions.Logging;
11+
using Serilog;
1112
using static Cli.Utils;
13+
using ILogger = Microsoft.Extensions.Logging.ILogger;
1214

1315
namespace Cli.Commands
1416
{
@@ -54,6 +56,11 @@ public ConfigureOptions(
5456
string? azureLogAnalyticsCustomTableName = null,
5557
string? azureLogAnalyticsDcrImmutableId = null,
5658
string? azureLogAnalyticsDceEndpoint = null,
59+
CliBool? fileSinkEnabled = null,
60+
string? fileSinkPath = null,
61+
RollingInterval? fileSinkRollingInterval = null,
62+
int? fileSinkRetainedFileCountLimit = null,
63+
long? fileSinkFileSizeLimitBytes = null,
5764
string? config = null)
5865
: base(config)
5966
{
@@ -98,6 +105,12 @@ public ConfigureOptions(
98105
AzureLogAnalyticsCustomTableName = azureLogAnalyticsCustomTableName;
99106
AzureLogAnalyticsDcrImmutableId = azureLogAnalyticsDcrImmutableId;
100107
AzureLogAnalyticsDceEndpoint = azureLogAnalyticsDceEndpoint;
108+
// File
109+
FileSinkEnabled = fileSinkEnabled;
110+
FileSinkPath = fileSinkPath;
111+
FileSinkRollingInterval = fileSinkRollingInterval;
112+
FileSinkRetainedFileCountLimit = fileSinkRetainedFileCountLimit;
113+
FileSinkFileSizeLimitBytes = fileSinkFileSizeLimitBytes;
101114
}
102115

103116
[Option("data-source.database-type", Required = false, HelpText = "Database type. Allowed values: MSSQL, PostgreSQL, CosmosDB_NoSQL, MySQL.")]
@@ -202,6 +215,21 @@ public ConfigureOptions(
202215
[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")]
203216
public string? AzureLogAnalyticsDceEndpoint { get; }
204217

218+
[Option("runtime.telemetry.file.enabled", Required = false, HelpText = "Enable/Disable File Sink logging. Default: False (boolean)")]
219+
public CliBool? FileSinkEnabled { get; }
220+
221+
[Option("runtime.telemetry.file.path", Required = false, HelpText = "Configure path for File Sink logging. Default: /logs/dab-log.txt")]
222+
public string? FileSinkPath { get; }
223+
224+
[Option("runtime.telemetry.file.rolling-interval", Required = false, HelpText = "Configure rolling interval for File Sink logging. Default: Day")]
225+
public RollingInterval? FileSinkRollingInterval { get; }
226+
227+
[Option("runtime.telemetry.file.retained-file-count-limit", Required = false, HelpText = "Configure maximum number of retained files. Default: 1")]
228+
public int? FileSinkRetainedFileCountLimit { get; }
229+
230+
[Option("runtime.telemetry.file.file-size-limit-bytes", Required = false, HelpText = "Configure maximum file size limit in bytes. Default: 1048576")]
231+
public long? FileSinkFileSizeLimitBytes { get; }
232+
205233
public int Handler(ILogger logger, FileSystemRuntimeConfigLoader loader, IFileSystem fileSystem)
206234
{
207235
logger.LogInformation("{productName} {version}", PRODUCT_NAME, ProductInfo.GetProductVersion());

src/Cli/ConfigGenerator.cs

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using Azure.DataApiBuilder.Service;
1414
using Cli.Commands;
1515
using Microsoft.Extensions.Logging;
16+
using Serilog;
1617
using static Cli.Utils;
1718

1819
namespace Cli
@@ -798,6 +799,25 @@ options.AzureLogAnalyticsDcrImmutableId is not null ||
798799
}
799800
}
800801

802+
// Telemetry: File Sink
803+
if (options.FileSinkEnabled is not null ||
804+
options.FileSinkPath is not null ||
805+
options.FileSinkRollingInterval is not null ||
806+
options.FileSinkRetainedFileCountLimit is not null ||
807+
options.FileSinkFileSizeLimitBytes is not null)
808+
{
809+
FileSinkOptions updatedFileSinkOptions = runtimeConfig?.Runtime?.Telemetry?.File ?? new();
810+
bool status = TryUpdateConfiguredFileOptions(options, ref updatedFileSinkOptions);
811+
if (status)
812+
{
813+
runtimeConfig = runtimeConfig! with { Runtime = runtimeConfig.Runtime! with { Telemetry = runtimeConfig.Runtime!.Telemetry is not null ? runtimeConfig.Runtime!.Telemetry with { File = updatedFileSinkOptions } : new TelemetryOptions(File: updatedFileSinkOptions) } };
814+
}
815+
else
816+
{
817+
return false;
818+
}
819+
}
820+
801821
return runtimeConfig != null;
802822
}
803823

@@ -1199,6 +1219,74 @@ private static bool TryUpdateConfiguredAzureLogAnalyticsOptions(
11991219
}
12001220
}
12011221

1222+
/// <summary>
1223+
/// Updates the file sink options in the configuration.
1224+
/// </summary>
1225+
/// <param name="options">The configuration options provided by the user.</param>
1226+
/// <param name="fileOptions">The file sink options to be updated.</param>
1227+
/// <returns>True if the options were successfully updated; otherwise, false.</returns>
1228+
private static bool TryUpdateConfiguredFileOptions(
1229+
ConfigureOptions options,
1230+
ref FileSinkOptions fileOptions)
1231+
{
1232+
try
1233+
{
1234+
// Runtime.Telemetry.File.Enabled
1235+
if (options.FileSinkEnabled is not null)
1236+
{
1237+
fileOptions = fileOptions with { Enabled = options.FileSinkEnabled is CliBool.True, UserProvidedEnabled = true };
1238+
_logger.LogInformation($"Updated configuration with runtime.telemetry.file.enabled as '{options.FileSinkEnabled}'");
1239+
}
1240+
1241+
// Runtime.Telemetry.File.Path
1242+
if (options.FileSinkPath is not null)
1243+
{
1244+
fileOptions = fileOptions with { Path = options.FileSinkPath, UserProvidedPath = true };
1245+
_logger.LogInformation($"Updated configuration with runtime.telemetry.file.path as '{options.FileSinkPath}'");
1246+
}
1247+
1248+
// Runtime.Telemetry.File.RollingInterval
1249+
if (options.FileSinkRollingInterval is not null)
1250+
{
1251+
fileOptions = fileOptions with { RollingInterval = ((RollingInterval)options.FileSinkRollingInterval).ToString(), UserProvidedRollingInterval = true };
1252+
_logger.LogInformation($"Updated configuration with runtime.telemetry.file.rolling-interval as '{options.FileSinkRollingInterval}'");
1253+
}
1254+
1255+
// Runtime.Telemetry.File.RetainedFileCountLimit
1256+
if (options.FileSinkRetainedFileCountLimit is not null)
1257+
{
1258+
if (options.FileSinkRetainedFileCountLimit <= 0)
1259+
{
1260+
_logger.LogError("Failed to update configuration with runtime.telemetry.file.retained-file-count-limit. Value must be a positive integer greater than 0.");
1261+
return false;
1262+
}
1263+
1264+
fileOptions = fileOptions with { RetainedFileCountLimit = (int)options.FileSinkRetainedFileCountLimit, UserProvidedRetainedFileCountLimit = true };
1265+
_logger.LogInformation($"Updated configuration with runtime.telemetry.file.retained-file-count-limit as '{options.FileSinkRetainedFileCountLimit}'");
1266+
}
1267+
1268+
// Runtime.Telemetry.File.FileSizeLimitBytes
1269+
if (options.FileSinkFileSizeLimitBytes is not null)
1270+
{
1271+
if (options.FileSinkFileSizeLimitBytes <= 0)
1272+
{
1273+
_logger.LogError("Failed to update configuration with runtime.telemetry.file.file-size-limit-bytes. Value must be a positive integer greater than 0.");
1274+
return false;
1275+
}
1276+
1277+
fileOptions = fileOptions with { FileSizeLimitBytes = (long)options.FileSinkFileSizeLimitBytes, UserProvidedFileSizeLimitBytes = true };
1278+
_logger.LogInformation($"Updated configuration with runtime.telemetry.file.file-size-limit-bytes as '{options.FileSinkFileSizeLimitBytes}'");
1279+
}
1280+
1281+
return true;
1282+
}
1283+
catch (Exception ex)
1284+
{
1285+
_logger.LogError($"Failed to update configuration with runtime.telemetry.file. Exception message: {ex.Message}.");
1286+
return false;
1287+
}
1288+
}
1289+
12021290
/// <summary>
12031291
/// Parse permission string to create PermissionSetting array.
12041292
/// </summary>

src/Config/Azure.DataApiBuilder.Config.csproj

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,15 @@
1818
<PackageReference Include="Microsoft.AspNetCore.Authorization" />
1919
<PackageReference Include="Microsoft.IdentityModel.Protocols" />
2020
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" />
21+
<PackageReference Include="Serilog.Sinks.File" />
2122
<PackageReference Include="System.IO.Abstractions" />
2223
<PackageReference Include="System.Drawing.Common" />
2324
<PackageReference Include="Microsoft.Data.SqlClient" />
2425
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" />
2526
<PackageReference Include="System.IdentityModel.Tokens.Jwt" />
2627
<PackageReference Include="Humanizer" />
2728
<PackageReference Include="Npgsql" />
28-
<PackageReference Include="OpenTelemetry.Exporter.OpenTelemetryProtocol" />
29+
<PackageReference Include="OpenTelemetry.Exporter.OpenTelemetryProtocol" />
2930
</ItemGroup>
3031

3132
<ItemGroup>

0 commit comments

Comments
 (0)