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 3 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
10 changes: 6 additions & 4 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,17 @@ public static class AzureAppServicesLoggerFactoryExtensions
/// Adds an Azure Web Apps diagnostics logger.
/// </summary>
/// <param name="builder">The extension method argument</param>
public static ILoggingBuilder AddAzureWebAppDiagnostics(this ILoggingBuilder builder)
/// <param name="customPrefix"></param>
/// <returns></returns>
public static ILoggingBuilder AddAzureWebAppDiagnostics(this ILoggingBuilder builder, string customPrefix = null)
Copy link
Member

Choose a reason for hiding this comment

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

@Pilchie is this a binary breaking change?

Copy link
Member

Choose a reason for hiding this comment

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

Absolutely.

Copy link
Member

Choose a reason for hiding this comment

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

Overloads are binary breaking safe, optional parameters are not - think about how they work, the method always receives two arguments, it's just that if you don't specify it, the compiler burns in the default value at the call-site.

Copy link
Author

Choose a reason for hiding this comment

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

My bad, I removed the optional parm and went ahead and created an overload instead. But CI still shows this optional parm error, even though I renamed the parameter.
@Pilchie @davidfowl
[SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters"

{
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);
return AddAzureWebAppDiagnostics(builder, context, customPrefix);
}

internal static ILoggingBuilder AddAzureWebAppDiagnostics(this ILoggingBuilder builder, IWebAppContext context)
internal static ILoggingBuilder AddAzureWebAppDiagnostics(this ILoggingBuilder builder, IWebAppContext context, string customPrefix)
{
if (!context.IsRunningInAzureWebApp)
{
Expand Down Expand Up @@ -63,7 +65,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