Skip to content
This repository has been archived by the owner on Apr 12, 2023. It is now read-only.

「ログ出力時に毎回ファイルを開いている」の修正 #67

Open
wants to merge 27 commits into
base: feature/log_improvement
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
de947fc
LogWriter 作成
Takym Mar 19, 2021
94b7d96
LoggerService のコンストラクタ変更に関する修正
Takym Mar 19, 2021
d0b4d44
CONTRIBUTORS.md 更新
Takym Mar 19, 2021
36b68b0
CreateLogRow を修正
Takym May 3, 2021
082a089
lock を Interlocked に変更
Takym May 3, 2021
016beb2
Merge branch 'master' into logging
Takym May 3, 2021
a9b4b62
Merge branch 'logging' of https://github.com/Takym/cocoa into logging
Takym May 3, 2021
e72f0da
Update Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs
Takym May 11, 2021
b459354
ソースファイルの書式を修正
Takym May 11, 2021
cfc16a0
Merge branch 'logging' of https://github.com/Takym/cocoa into logging
Takym May 11, 2021
a00563a
`CreateLogRow` を微修正
Takym May 11, 2021
03f7fab
フィールド名修正
Takym May 16, 2021
a68cac6
`_file` フィールドの書き換え処理を改善
Takym May 18, 2021
b4f15d8
`WriteLine(DateTime, string)` を改善
Takym May 18, 2021
2f6002d
`WriteLine(DateTime, string)` を微修正
Takym May 18, 2021
68b7d9f
`HEADER` の出力タイミングを修正
Takym May 18, 2021
f4986d2
テスト処理修正
Takym May 18, 2021
186a323
CreateLogRow、可読性&性能向上
Takym May 22, 2021
72a4f73
可読性&性能向上2
Takym May 22, 2021
c4bec33
`LogWriter.File` クラス改名
Takym Jun 5, 2021
09eaa47
LogWriter、フィールド変数の名前を変更
Takym Jun 5, 2021
c00b730
`LogWriter.LogFile` の仕組みを変更
Takym Jun 5, 2021
a4b32c0
`LogWriter.WriteLine` のローカル変数の名前を変更
Takym Jun 5, 2021
bc848ce
`LogWriter.LogFile.FileName` を **Path** に改名
Takym Jun 5, 2021
af59231
Merge branch 'cocoa-mhlw:develop' into logging
Takym Jun 11, 2021
b0a617c
Merge branch 'cocoa-mhlw:develop' into logging
Takym Jul 1, 2021
ac1f49f
Update unit tests for the logging system. (Takym/cocoa#4)
Takym Aug 20, 2021
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
2 changes: 1 addition & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,4 @@ jobs:
- name: Execute Unit Tests
run: dotnet test ${{ github.workspace }}/Covid19Radar/Tests/Covid19Radar.UnitTests/
env:
DOTNET_CLI_TELEMETRY_OPTOUT: true
DOTNET_CLI_TELEMETRY_OPTOUT: true
1 change: 1 addition & 0 deletions CONTRIBUTORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
- Tassana Thaveeteeratham (Thai Translation)
- Kotaro Sakamoto
- Koichi Yokota (Documentation)
- Takym (LogWriter)
Copy link
Collaborator

Choose a reason for hiding this comment

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

CONTRIBUTORに関しては新しく項目を設けますね。


# Original Covid19Radar Beta Testers
- Nagahata Kenji
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public LogPeriodicDeleteReceiver()
{
var essensialService = new EssentialsService();
var logPathService = new LogPathService(new LogPathServiceAndroid());
loggerService = new LoggerService(logPathService, essensialService);
loggerService = new LoggerService(new LogWriter(logPathService, essensialService));
logFileService = new Covid19Radar.Services.Logs.LogFileService(loggerService, logPathService);
Comment on lines 62 to 65
Copy link
Collaborator

Choose a reason for hiding this comment

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

この部分だけServiceLocatorから外れていて、Singleton保てていないんですね。
話がややこしくなりそうなので、ここをServiceLocator化するPull Request出すことにします。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LogPeriodicDeleteServiceAndroid の様に DependencyService.Resolve<T>() を使えば良いと思います。

Copy link
Contributor Author

@Takym Takym Jun 7, 2021

Choose a reason for hiding this comment

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

似た問題として #185#189 を確認しました。

}

Expand Down
1 change: 1 addition & 0 deletions Covid19Radar/Covid19Radar/App.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ private static void RegisterCommonTypes(IContainer container)
container.Register<ILogPathService, LogPathService>(Reuse.Singleton);
container.Register<ILogPeriodicDeleteService, LogPeriodicDeleteService>(Reuse.Singleton);
container.Register<ILogUploadService, LogUploadService>(Reuse.Singleton);
container.Register<ILogWriter, LogWriter>(Reuse.Singleton);
container.Register<IEssentialsService, EssentialsService>(Reuse.Singleton);
container.Register<IUserDataService, UserDataService>(Reuse.Singleton);
container.Register<IExposureNotificationService, ExposureNotificationService>(Reuse.Singleton);
Expand Down
241 changes: 241 additions & 0 deletions Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */

#nullable enable

using System;
using System.Diagnostics;
using System.IO;
using System.Runtime.CompilerServices;
using System.Text;
using System.Threading;
using Covid19Radar.Common;

namespace Covid19Radar.Services.Logs
{
/// <summary>
/// ログの書き込みを行う機能を提供します。
/// </summary>
public interface ILogWriter
{
/// <summary>
/// ログ出力を行います。
/// </summary>
/// <param name="message">出力するメッセージを指定します。</param>
/// <param name="method">実行中の関数の名前を指定します。</param>
/// <param name="filePath">実行中の関数を格納しているソースファイルの名前を指定します。</param>
/// <param name="lineNumber">実行中の関数のソースファイル内での行番号を指定します。</param>
/// <param name="logLevel">ログレベルを指定します。</param>
public void Write(string? message, string? method, string? filePath, int lineNumber, LogLevel logLevel);
}

/// <summary>
/// <see cref="Covid19Radar.Services.Logs.ILogWriter"/>の実装です。
/// </summary>
public sealed class LogWriter : ILogWriter
{
[ThreadStatic()]
private static StringBuilder? _sb_cache;
private const string DATETIME_FORMAT = "yyyy/MM/dd HH:mm:ss.fffffff";
private static readonly string HEADER = CreateLogHeaderRow();
private readonly ILogPathService _log_path;
Copy link
Collaborator

Choose a reason for hiding this comment

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

名前がここだけがスネークケースなので、これはキャメルケースにした上でServiceをつけて_logPathServiceとする方が読みやすいと思います。

private readonly IEssentialsService _essentials;
Copy link
Collaborator

Choose a reason for hiding this comment

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

こちらも_essentialServiceとした方が、利用箇所からServiceとして登録されているものだとわかりやすいと思います。

private readonly Encoding _encoding;
private LogFile? _log_file;

/// <summary>
/// 型'<see cref="Covid19Radar.Services.Logs.LogWriter"/>'の新しいインスタンスを生成します。
/// </summary>
/// <param name="logPath">ログファイルのパスを提供するサービスを指定します。</param>
/// <param name="essentials">環境情報を提供するサービスを指定します。</param>
public LogWriter(ILogPathService logPath, IEssentialsService essentials)
{
_log_path = logPath ?? throw new ArgumentNullException(nameof(logPath));
_essentials = essentials ?? throw new ArgumentNullException(nameof(essentials));
_encoding = Encoding.UTF8; // 文字コードを変更する必要性が出た場合は、外部から注入できる様にする。
}

/// <inheritdoc/>
public void Write(string? message, string? method, string? sourceFile, int lineNumber, LogLevel logLevel)
{
#if !DEBUG
if (logLevel == LogLevel.Verbose || logLevel == LogLevel.Debug) {
return;
}
#endif
try {
// null は空文字として扱う。
message ??= string.Empty;
method ??= string.Empty;
sourceFile ??= string.Empty;

// ローカル変数初期化。
var time = Utils.JstNow();
var file = _log_file;
string path = _log_path.LogFilePath(time);

// ファイルを更新する必要がある場合。
if (file is null || file.Path != path) {
// 新しいファイルを生成する。
var newFile = new LogFile(path, _encoding);

do {
if (Interlocked.CompareExchange(ref _log_file, newFile, file) == file) {
// _log_file を新しいファイルに置き換える事ができた場合、
// 元のファイルを破棄しループから抜ける。
file?.Dispose();
file = newFile;
break;
}

// 別スレッドに制御を渡す。
Thread.Yield();

// ローカル変数を更新する。
time = Utils.JstNow();
file = _log_file;
path = _log_path.LogFilePath(time);

if (newFile.Path != path) {
// 日付が更新された場合は新しいファイルを作り直す。
newFile.Dispose();
newFile = new LogFile(path, _encoding);
}
} while (file is null || file.Path != path);
}

// ログをファイルに書き込む。
string line = CreateLogContentRow(message, method, sourceFile, lineNumber, logLevel, time, _essentials);
file .WriteLine(line);
Debug.WriteLine(line);
} catch (Exception e) {
Debug.WriteLine(e.ToString());
}
}

private static string CreateLogHeaderRow()
{
return CreateLogRow(
"output_date",
"log_level",
"message",
"method",
"file_path",
"line_number",
"platform",
"platform_version",
"model",
"device_type",
"app_version",
"build_number"
);
}

private static string CreateLogContentRow(
string message,
string method,
string filePath,
int lineNumber,
LogLevel logLevel,
DateTime jstDateTime,
IEssentialsService essentials)
{
return CreateLogRow(
jstDateTime.ToString(DATETIME_FORMAT),
logLevel.ToString(),
message,
method,
filePath,
lineNumber.ToString(),
essentials.Platform,
essentials.PlatformVersion,
essentials.Model,
essentials.DeviceType,
essentials.AppVersion,
essentials.BuildNumber
);
}

private static string CreateLogRow(params string[] cols)
{
var sb = _sb_cache is null ? _sb_cache = new StringBuilder()
: _sb_cache.Clear();
foreach (string col in cols) {
sb.Append(",\"");
foreach (char ch in col) {
string? escaped = ch switch {
'\t' => "\\t", '\v' => "\\v",
'\r' => "\\r", '\n' => "\\n",
'\\' => "\\\\", '\"' => "\"\"",
_ => null
};
Comment on lines +167 to +172
Copy link
Collaborator

Choose a reason for hiding this comment

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

以前もお伝えしたとおり、CreateLogRow周りの変更について、現状(LINQ)から置き換えない方が良いと考えています。

本PRの主題は『「ログ出力時に毎回ファイルを開いている」の修正』なので、この変更を含めてしまうのは主題から外れるという認識です。性能面での改善は別PRにしてもらい、そこで検討するのが適切と考えます。

_ = escaped is null ? sb.Append(ch)
: sb.Append(escaped);
}
sb.Append('\"');
}
sb.Remove(0, 1);
return sb.ToString();
}

private static FileStream OpenLogFile(string path, bool useWriterMode)
{
return new FileStream(
path,
useWriterMode ? FileMode.Append : FileMode.Open,
useWriterMode ? FileAccess.Write : FileAccess.Read,
FileShare.ReadWrite | FileShare.Delete);
}

public static StreamReader OpenReader(string path)
{
return new StreamReader(OpenLogFile(path, false));
}

private sealed class LogFile : IDisposable
{
private readonly Encoding _encoding;
private readonly Lazy<TextWriter> _writer;

internal string Path { get; }

internal LogFile(string path, Encoding enc)
{
string dir = System.IO.Path.GetDirectoryName(path);
if (!Directory.Exists(dir)) {
Directory.CreateDirectory(dir);
}

this.Path = path;
_encoding = enc;
_writer = new Lazy<TextWriter>(this.OpenFile, LazyThreadSafetyMode.ExecutionAndPublication);
}

private TextWriter OpenFile()
{
var fs = OpenLogFile(this.Path, true);
var sw = TextWriter.Synchronized(new StreamWriter(fs, _encoding) {
AutoFlush = true
});
if (fs.Length <= _encoding.Preamble.Length) {
sw.WriteLine(HEADER);
}
return sw;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void WriteLine(string line)
{
_writer.Value.WriteLine(line);
}

public void Dispose()
{
if (_writer.IsValueCreated) {
_writer.Value.Dispose();
}
}
}
}
}
Loading