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.

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

V14: Refactor LogViewerService #14511

Merged
merged 20 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@
factory.GetRequiredService<IEventMessagesFactory>(),
factory.GetRequiredService<IExternalLoginWithKeyRepository>()
));
Services.AddUnique<ILogViewerService, LogViewerService>();

Check warning on line 331 in src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

❌ Getting worse: Large Method

AddCoreServices increases from 149 to 150 lines of code, threshold = 70. Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.
Services.AddUnique<IExternalLoginWithKeyService>(factory => factory.GetRequiredService<ExternalLoginService>());
Services.AddUnique<ILocalizedTextService>(factory => new LocalizedTextService(
factory.GetRequiredService<Lazy<LocalizedTextServiceFileSources>>(),
Expand Down
32 changes: 32 additions & 0 deletions src/Umbraco.Core/Services/ILogViewerRepository.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using Umbraco.Cms.Core.Logging;
using Umbraco.Cms.Core.Logging.Viewer;

namespace Umbraco.Cms.Core.Services;

public interface ILogViewerRepository
{
/// <summary>
/// Returns the collection of log entries.
/// </summary>
IEnumerable<ILogEntry> GetLogs(LogTimePeriod logTimePeriod, string? filterExpression = null);

/// <summary>
/// Returns the number of the different log level entries.
/// </summary>
LogLevelCounts GetLogCount(LogTimePeriod logTimePeriod);

/// <summary>
/// Returns a list of all unique message templates and their counts.
/// </summary>
LogTemplate[] GetMessageTemplates(LogTimePeriod logTimePeriod);

/// <summary>
/// Gets the minimum-level log value from the config file.
/// </summary>
LogLevel GetGlobalMinLogLevel();

/// <summary>
/// Get the minimum-level log value from the config file.
/// </summary>
LogLevel RestrictedToMinimumLevel();
}
Original file line number Diff line number Diff line change
@@ -1,37 +1,33 @@
using System.Collections.ObjectModel;

Check notice on line 1 in src/Umbraco.Core/Services/LogViewerService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

ℹ Getting worse: Primitive Obsession

The ratio of primitive types in function arguments increases from 35.48% to 40.00%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.

Check notice on line 1 in src/Umbraco.Core/Services/LogViewerService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

✅ No longer an issue: Overall Code Complexity

The mean cyclomatic complexity in this module is no longer above the threshold
using Serilog.Events;
using Umbraco.Cms.Core.Logging;
using Umbraco.Cms.Core.Logging.Viewer;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Core.Services.OperationStatus;
using Umbraco.Cms.Core.Logging;
using Umbraco.Cms.Core.Serialization;
using Umbraco.Extensions;
using LogLevel = Umbraco.Cms.Core.Logging.LogLevel;

namespace Umbraco.Cms.Core.Services.Implement;
namespace Umbraco.Cms.Core.Services;

// FIXME: Get rid of ILogViewer and ILogLevelLoader dependencies (as they are obsolete)
// and fix the implementation of the methods using it
public class LogViewerService : ILogViewerService
{
private const int FileSizeCap = 100;
private readonly ILogViewerQueryRepository _logViewerQueryRepository;
private readonly ILogViewer _logViewer;
private readonly ILogLevelLoader _logLevelLoader;
private readonly ICoreScopeProvider _provider;
private readonly IJsonSerializer _jsonSerializer;
private readonly ILoggingConfiguration _loggingConfiguration;
private readonly ILogViewerRepository _logViewerRepository;

public LogViewerService(
ILogViewerQueryRepository logViewerQueryRepository,
ILogViewer logViewer,
ILogLevelLoader logLevelLoader,
ICoreScopeProvider provider,
IJsonSerializer jsonSerializer)
ILoggingConfiguration loggingConfiguration,
ILogViewerRepository logViewerRepository)
{
_logViewerQueryRepository = logViewerQueryRepository;
_logViewer = logViewer;
_logLevelLoader = logLevelLoader;
_provider = provider;
_jsonSerializer = jsonSerializer;
_loggingConfiguration = loggingConfiguration;
_logViewerRepository = logViewerRepository;
}

/// <inheritdoc/>
Expand All @@ -54,14 +50,12 @@
null);
}

PagedModel<LogMessage> logMessages =
_logViewer.GetLogsAsPagedModel(logTimePeriod, skip, take, orderDirection, filterExpression, logLevels);

var logEntries = new PagedModel<ILogEntry>(logMessages.Total, logMessages.Items.Select(x => ToLogEntry(x)));
PagedModel<ILogEntry> filteredLogs = GetFilteredLogs(logTimePeriod, filterExpression, logLevels, orderDirection, skip, take);

return Attempt.SucceedWithStatus<PagedModel<ILogEntry>?, LogViewerOperationStatus>(
LogViewerOperationStatus.Success,
logEntries);
filteredLogs);
}

/// <inheritdoc/>
Expand Down Expand Up @@ -141,9 +135,11 @@
null);
}

LogLevelCounts counter = _logViewerRepository.GetLogCount(logTimePeriod);

return Attempt.SucceedWithStatus<LogLevelCounts?, LogViewerOperationStatus>(
LogViewerOperationStatus.Success,
_logViewer.GetLogLevelCounts(logTimePeriod));
counter);
}

/// <inheritdoc/>
Expand All @@ -159,7 +155,7 @@
null!);
}

LogTemplate[] messageTemplates = _logViewer.GetMessageTemplates(logTimePeriod).ToArray();
LogTemplate[] messageTemplates = _logViewerRepository.GetMessageTemplates(logTimePeriod);

return Attempt.SucceedWithStatus(
LogViewerOperationStatus.Success,
Expand All @@ -169,18 +165,17 @@
/// <inheritdoc/>
public ReadOnlyDictionary<string, LogLevel> GetLogLevelsFromSinks()
{
ReadOnlyDictionary<string, LogEventLevel?> configuredLogLevels = _logLevelLoader.GetLogLevelsFromSinks();
var configuredLogLevels = new Dictionary<string, LogLevel>
{
{ "Global", GetGlobalMinLogLevel() },
{ "UmbracoFile", _logViewerRepository.RestrictedToMinimumLevel() },
};

return configuredLogLevels.ToDictionary(logLevel => logLevel.Key, logLevel => Enum.Parse<LogLevel>(logLevel.Value!.ToString()!)).AsReadOnly();
return configuredLogLevels.AsReadOnly();
}

/// <inheritdoc/>
public LogLevel GetGlobalMinLogLevel()
{
LogEventLevel? serilogLogLevel = _logLevelLoader.GetGlobalMinLogLevel();

return Enum.Parse<LogLevel>(serilogLogLevel!.ToString()!);
}
public LogLevel GetGlobalMinLogLevel() => _logViewerRepository.GetGlobalMinLogLevel();

/// <summary>
/// Returns a <see cref="LogTimePeriod" /> representation from a start and end date for filtering log files.
Expand Down Expand Up @@ -214,58 +209,73 @@
/// <returns>The value whether or not you are able to view the logs.</returns>
private bool CanViewLogs(LogTimePeriod logTimePeriod)
{
// Check if the interface can deal with large files
if (_logViewer.CanHandleLargeLogs)
// Number of entries
long fileSizeCount = 0;

// foreach full day in the range - see if we can find one or more filenames that end with
// yyyyMMdd.json - Ends with due to MachineName in filenames - could be 1 or more due to load balancing
for (DateTime day = logTimePeriod.StartTime.Date; day.Date <= logTimePeriod.EndTime.Date; day = day.AddDays(1))
{
return true;
}
// Filename ending to search for (As could be multiple)
var filesToFind = GetSearchPattern(day);

return _logViewer.CheckCanOpenLogs(logTimePeriod);
}
var filesForCurrentDay = Directory.GetFiles(_loggingConfiguration.LogDirectory, filesToFind);

private ILogEntry ToLogEntry(LogMessage logMessage)
{
return new LogEntry()
{
Timestamp = logMessage.Timestamp,
Level = Enum.Parse<LogLevel>(logMessage.Level.ToString()),
MessageTemplateText = logMessage.MessageTemplateText,
RenderedMessage = logMessage.RenderedMessage,
Properties = MapLogMessageProperties(logMessage.Properties),
Exception = logMessage.Exception
};
fileSizeCount += filesForCurrentDay.Sum(x => new FileInfo(x).Length);
}

// The GetLogSize call on JsonLogViewer returns the total file size in bytes
// Check if the log size is not greater than 100Mb (FileSizeCap)
var logSizeAsMegabytes = fileSizeCount / 1024 / 1024;
return logSizeAsMegabytes <= FileSizeCap;
}

private IReadOnlyDictionary<string, string?> MapLogMessageProperties(IReadOnlyDictionary<string, LogEventPropertyValue>? properties)
private PagedModel<ILogEntry> GetFilteredLogs(
LogTimePeriod logTimePeriod,
string? filterExpression,
string[]? logLevels,
Direction orderDirection,
int skip,
int take)
{
var result = new Dictionary<string, string?>();
IEnumerable<ILogEntry> logs = _logViewerRepository.GetLogs(logTimePeriod, filterExpression).ToArray();

if (properties is not null)
// This is user used the checkbox UI to toggle which log levels they wish to see
// If an empty array or null - its implied all levels to be viewed
if (logLevels?.Length > 0)
{
foreach (KeyValuePair<string, LogEventPropertyValue> property in properties)
var logsAfterLevelFilters = new List<ILogEntry>();
var validLogType = true;
foreach (var level in logLevels)
{
string? value;


if (property.Value is ScalarValue scalarValue)
// Check if level string is part of the LogEventLevel enum
if (Enum.IsDefined(typeof(LogLevel), level))
{
value = scalarValue.Value?.ToString();
}
else if (property.Value is StructureValue structureValue)
{
var textWriter = new StringWriter();
structureValue.Render(textWriter);
value = textWriter.ToString();
validLogType = true;
logsAfterLevelFilters.AddRange(logs.Where(x =>
string.Equals(x.Level.ToString(), level, StringComparison.InvariantCultureIgnoreCase)));
}
else
{
value = _jsonSerializer.Serialize(property.Value);
validLogType = false;
}
}

result.Add(property.Key, value);
if (validLogType)
{
logs = logsAfterLevelFilters;
}
}

return result.AsReadOnly();
return new PagedModel<ILogEntry>
{
Total = logs.Count(),
Items = logs
.OrderBy(l => l.Timestamp, orderDirection)
.Skip(skip)
.Take(take),
};

Check notice on line 277 in src/Umbraco.Core/Services/LogViewerService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

ℹ New issue: Excess Number of Function Arguments

GetFilteredLogs has 6 arguments, threshold = 4. This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments.
}

private string GetSearchPattern(DateTime day) => $"*{day:yyyyMMdd}*.json";

Check notice on line 280 in src/Umbraco.Core/Services/LogViewerService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v14/dev)

✅ No longer an issue: Complex Method

MapLogMessageProperties is no longer above the threshold for cyclomatic complexity. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
}
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,6 @@ public static IUmbracoBuilder AddLogViewer(this IUmbracoBuilder builder)
factory.GetRequiredService<ILoggingConfiguration>(),
factory.GetRequiredService<ILogLevelLoader>(),
Log.Logger));
builder.Services.AddSingleton<ILogViewerService, LogViewerService>();

return builder;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
using Microsoft.Extensions.DependencyInjection;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Infrastructure.Persistence.Repositories;
using Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement;
using Umbraco.Cms.Infrastructure.Services.Implement;
using Umbraco.Extensions;

namespace Umbraco.Cms.Infrastructure.DependencyInjection;
Expand Down Expand Up @@ -67,8 +69,9 @@ internal static IUmbracoBuilder AddRepositories(this IUmbracoBuilder builder)
builder.Services.AddUnique<ILogViewerQueryRepository, LogViewerQueryRepository>();
builder.Services.AddUnique<INodeCountRepository, NodeCountRepository>();
builder.Services.AddUnique<IIdKeyMapRepository, IdKeyMapRepository>();
builder.Services.AddUnique<IPropertyTypeUsageRepository, PropertyTypeUsageRepository>();
builder.Services.AddUnique<IDataTypeUsageRepository, DataTypeUsageRepository>();
builder.Services.AddUnique<IPropertyTypeUsageRepository, PropertyTypeUsageRepository>();
builder.Services.AddUnique<IDataTypeUsageRepository, DataTypeUsageRepository>();
builder.Services.AddUnique<ILogViewerRepository, LogViewerRepository>();

return builder;
}
Expand Down
Loading
Loading