Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Azure blob in flat folder structure #25078

Closed
wants to merge 6 commits into from
Closed

Azure blob in flat folder structure #25078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d3110c0
Added an option to output Azure blob in flat folder structure with cu…
Aug 20, 2020
e89228d
Remove the commented code - not needed anymore
Aug 26, 2020
9f320f3
Changed approach
Oct 15, 2020
917a1d3
Fixed binary breaking change
Oct 16, 2020
88eb54f
Renamed parm
Oct 16, 2020
2adb379
Renamed the parm back
Oct 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions src/Logging.AzureAppServices/src/AzureAppServicesLoggerFactoryExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,30 @@ public static class AzureAppServicesLoggerFactoryExtensions
/// Adds an Azure Web Apps diagnostics logger.
/// </summary>
/// <param name="builder">The extension method argument</param>
/// <returns></returns>
public static ILoggingBuilder AddAzureWebAppDiagnostics(this ILoggingBuilder builder)
{
var context = WebAppContext.Default;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The custom prefix should be passed into the constructor.

Copy link
Author

@yogigrantz yogigrantz Oct 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidfowl
This is inside a static class, where there is no constructor.

AddAzureWebAppDiagnostics is an overload extension of ILoggingBuilder, which is the "entry point" to this API. The web application program injects this API to the IHostBuilder container like this:

ihostBuilder.ConfigureLogging(x =>
{
x.AddAzureWebAppDiagnostics(customPrefixName);
});

as opposed to the original version, where the DI is coded like this:

ihostBuilder.ConfigureLogging(x =>
{
x.AddAzureWebAppDiagnostics();
});

Notice that all I did was adding that customPrefixName option and store its value in a static field where later on the provider checks whether to use the custom prefixed flat blob filename logic. I didn't want to touch the original method if possible, out of respect to the original programmer. Please let me know if there is a more preferable approach to implement this logic

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a static class extension entry point to IHostBuilder ConfigureLogging Dependency Injection. It takes the responsibility of a constructor. I'm sure you are familiar with the use of Logging Provider Dependency Injection to IHostBuilder ConfigureLogging. Please revisit this code, I appreciate your time!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't design anything like this, statics are pretty much forbidden, especially mutable ones like this. This needs to be a constructor argument to the instance.


// Only add the provider if we're in Azure WebApp. That cannot change once the apps started
return AddAzureWebAppDiagnostics(builder, context);
return AddAzureWebAppDiagnostics(builder, context, null);
}

internal static ILoggingBuilder AddAzureWebAppDiagnostics(this ILoggingBuilder builder, IWebAppContext context)
/// <summary>
/// Adds an Azure Web Apps diagnostics logger.
/// </summary>
/// <param name="builder">The extension method argument</param>
/// <param name="customPrefix"></param>
/// <returns></returns>
public static ILoggingBuilder AddAzureWebAppDiagnostics(this ILoggingBuilder builder, string customPrefix)
{
var context = WebAppContext.Default;

// Only add the provider if we're in Azure WebApp. That cannot change once the apps started
return AddAzureWebAppDiagnostics(builder, context, customPrefix);
}

internal static ILoggingBuilder AddAzureWebAppDiagnostics(this ILoggingBuilder builder, IWebAppContext context, string customPrefix)
{
if (!context.IsRunningInAzureWebApp)
{
Expand Down Expand Up @@ -63,7 +78,7 @@ internal static ILoggingBuilder AddAzureWebAppDiagnostics(this ILoggingBuilder b
if (addedBlobLogger)
{
services.AddSingleton<IConfigureOptions<LoggerFilterOptions>>(CreateBlobFilterConfigureOptions(config));
services.AddSingleton<IConfigureOptions<AzureBlobLoggerOptions>>(new BlobLoggerConfigureOptions(config, context));
services.AddSingleton<IConfigureOptions<AzureBlobLoggerOptions>>(new BlobLoggerConfigureOptions(config, context, customPrefix));
services.AddSingleton<IOptionsChangeTokenSource<AzureBlobLoggerOptions>>(
new ConfigurationChangeTokenSource<AzureBlobLoggerOptions>(config));
LoggerProviderOptions.RegisterProviderOptions<AzureBlobLoggerOptions, BlobLoggerProvider>(builder.Services);
Expand Down
11 changes: 10 additions & 1 deletion src/Logging.AzureAppServices/src/AzureBlobLoggerOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.Extensions.Logging.AzureAppServices
public class AzureBlobLoggerOptions: BatchingLoggerOptions
{
private string _blobName = "applicationLog.txt";

private string _customPrefixFileName = null;
/// <summary>
/// Gets or sets the last section of log blob name.
/// Defaults to <c>"applicationLog.txt"</c>.
Expand All @@ -30,6 +30,15 @@ public string BlobName
}
}

internal string CustomPrefixFileName
{
get { return _customPrefixFileName; }
set
{
_customPrefixFileName = value;
}
}

internal string ContainerUrl { get; set; }

internal string ApplicationName { get; set; }
Expand Down
5 changes: 4 additions & 1 deletion src/Logging.AzureAppServices/src/BlobLoggerConfigureOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ internal class BlobLoggerConfigureOptions : BatchLoggerConfigureOptions, IConfig
{
private readonly IConfiguration _configuration;
private readonly IWebAppContext _context;
private readonly string _customPrefix;

public BlobLoggerConfigureOptions(IConfiguration configuration, IWebAppContext context)
public BlobLoggerConfigureOptions(IConfiguration configuration, IWebAppContext context, string customPrefix)
: base(configuration, "AzureBlobEnabled")
{
_configuration = configuration;
_context = context;
_customPrefix = customPrefix;
}

public void Configure(AzureBlobLoggerOptions options)
Expand All @@ -25,6 +27,7 @@ public void Configure(AzureBlobLoggerOptions options)
options.ContainerUrl = _configuration.GetSection("APPSETTING_DIAGNOSTICS_AZUREBLOBCONTAINERSASURL")?.Value;
options.ApplicationName = _context.SiteName;
options.ApplicationInstanceId = _context.SiteInstanceId;
options.CustomPrefixFileName = _customPrefix;
}
}
}
13 changes: 12 additions & 1 deletion src/Logging.AzureAppServices/src/BlobLoggerProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public class BlobLoggerProvider : BatchingLoggerProvider
private readonly string _fileName;
private readonly Func<string, ICloudAppendBlob> _blobReferenceFactory;
private readonly HttpClient _httpClient;
private readonly string _customPrefixFileName;

/// <summary>
/// Creates a new instance of <see cref="BlobLoggerProvider"/>
Expand Down Expand Up @@ -53,15 +54,25 @@ internal BlobLoggerProvider(
_fileName = value.ApplicationInstanceId + "_" + value.BlobName;
_blobReferenceFactory = blobReferenceFactory;
_httpClient = new HttpClient();
_customPrefixFileName = value.CustomPrefixFileName;
}

internal override async Task WriteMessagesAsync(IEnumerable<LogMessage> messages, CancellationToken cancellationToken)
{
var eventGroups = messages.GroupBy(GetBlobKey);
DateTime currDate = DateTime.Now;
foreach (var eventGroup in eventGroups)
{
var key = eventGroup.Key;
var blobName = $"{_appName}/{key.Year}/{key.Month:00}/{key.Day:00}/{key.Hour:00}/{_fileName}";
string blobName;

if (!string.IsNullOrEmpty(_customPrefixFileName))
{
string filename = $"{_customPrefixFileName}{currDate.Year}{currDate.Month:00}{currDate.Day:00}.txt";
blobName = $"{_appName}/{filename}";
}
else
blobName = $"{_appName}/{key.Year}/{key.Month:00}/{key.Day:00}/{key.Hour:00}/{_fileName}";

var blob = _blobReferenceFactory(blobName);

Expand Down