Skip to content

Commit 576f4c5

Browse files
RubenCerna2079Copilot
RubenCerna2079
and
Copilot
authored
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] <[email protected]>
1 parent 51c2dcd commit 576f4c5

9 files changed

Lines changed: 274 additions & 33 deletions

File tree

‎schemas/dab.draft.schema.json‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@
501501
"type": "string",
502502
"description": "Rolling interval for log files.",
503503
"default": "Day",
504-
"enum": ["Minute", "Hour", "Day", "Month", "Year", "Infinite"]
504+
"enum": [ "Minute", "Hour", "Day", "Month", "Year", "Infinite" ]
505505
},
506506
"retained-file-count-limit": {
507507
"type": "integer",
@@ -515,6 +515,16 @@
515515
"default": 1048576,
516516
"minimum": 1
517517
}
518+
},
519+
"if": {
520+
"properties": {
521+
"enabled": {
522+
"const": true
523+
}
524+
}
525+
},
526+
"then": {
527+
"required": [ "path" ]
518528
}
519529
},
520530
"log-level": {

‎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/Converters/FileSinkConverter.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public FileSinkConverter(bool replaceEnvVar)
3434
string? path = null;
3535
RollingInterval? rollingInterval = null;
3636
int? retainedFileCountLimit = null;
37-
int? fileSizeLimitBytes = null;
37+
long? fileSizeLimitBytes = null;
3838

3939
while (reader.Read())
4040
{
@@ -97,7 +97,7 @@ public FileSinkConverter(bool replaceEnvVar)
9797
{
9898
try
9999
{
100-
fileSizeLimitBytes = reader.GetInt32();
100+
fileSizeLimitBytes = reader.GetInt64();
101101
}
102102
catch (FormatException)
103103
{

‎src/Config/ObjectModel/FileSinkOptions.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,10 @@ public record FileSinkOptions
6060
/// <summary>
6161
/// File size limit in bytes before a new file needs to be created.
6262
/// </summary>
63-
public int FileSizeLimitBytes { get; init; }
63+
public long FileSizeLimitBytes { get; init; }
6464

6565
[JsonConstructor]
66-
public FileSinkOptions(bool? enabled = null, string? path = null, RollingInterval? rollingInterval = null, int? retainedFileCountLimit = null, int? fileSizeLimitBytes = null)
66+
public FileSinkOptions(bool? enabled = null, string? path = null, RollingInterval? rollingInterval = null, int? retainedFileCountLimit = null, long? fileSizeLimitBytes = null)
6767
{
6868
if (enabled is not null)
6969
{
@@ -107,7 +107,7 @@ public FileSinkOptions(bool? enabled = null, string? path = null, RollingInterva
107107

108108
if (fileSizeLimitBytes is not null)
109109
{
110-
FileSizeLimitBytes = (int)fileSizeLimitBytes;
110+
FileSizeLimitBytes = (long)fileSizeLimitBytes;
111111
UserProvidedFileSizeLimitBytes = true;
112112
}
113113
else

0 commit comments

Comments
 (0)