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

TimeZoneInfo の取得方法を統一する #554

Closed
b-wind opened this issue Dec 4, 2021 · 7 comments · Fixed by #531
Closed

TimeZoneInfo の取得方法を統一する #554

b-wind opened this issue Dec 4, 2021 · 7 comments · Fixed by #531
Labels
confirmed 開発内部管理用 enhancement 新しい機能や改善のリクエスト

Comments

@b-wind
Copy link

b-wind commented Dec 4, 2021

その機能リクエストは何らかの問題に関連しますか / Is your feature request related to a problem?

LogFile に記録する TimeZone はJSTに固定されている。
この為 JST を表すTimeZoneInfoが必要だが、 TimeZoneInfo.FindSystemTimeZoneById で取得する形式となっている。

該当処理

private static TimeZoneInfo JstTimeZoneInfo()
{
// iOS/Android/Unix
try
{
return TimeZoneInfo.FindSystemTimeZoneById("Asia/Tokyo");
}
catch (TimeZoneNotFoundException)
{
// Not iOS/Android/Unix
}
// Windows
try
{
return TimeZoneInfo.FindSystemTimeZoneById("Tokyo Standard Time");
}
catch (TimeZoneNotFoundException)
{
// Not Windows
}
// Emergency fallback
return TimeZoneInfo.CreateCustomTimeZone("JST", new TimeSpan(9, 0, 0), "(GMT+09:00) JST", "JST");

呼び出し元

(2021/12/11 追記) #578 に関連して

TimeSpam での演算はやめよう。

public DateTime UpdateDateTimeUtc
=> DateTime.SpecifyKind(UpdateDateTimeJst - TIME_DIFFERENCIAL_JST_UTC, DateTimeKind.Utc);

解決策についてお書きください / Describe the solution you'd like

Issue #523 で検討した手法に統一する。

あなたが考える代替案についてご説明ください / Describe alternatives you've considered

動いて居るので変えない。

ただし、同様の処理をバラバラの方法で行う事になる。

その他 / Additional context

個人的にはログに記録する日時もUTCにしたいところだけれど、強い要望では無い。


Internal IDs:

  • NFR 5077
@b-wind b-wind changed the title TimeZoneInfo の取得方法の統一 TimeZoneInfo の取得方法を統一する Dec 4, 2021
@keiji keiji added enhancement 新しい機能や改善のリクエスト waiting-for-confirmation 関係者に確認中のもの labels Dec 5, 2021
@keiji
Copy link
Collaborator

keiji commented Dec 5, 2021

人間が見る表示はローカルタイムの方がいいですね。
むしろJST/UTC固定より、こういうときこそLocalTimeを使った方がいいのでは(JST固定だと動作端末のタイムゾーン情報が落ちてしまう)と思います。

ギリギリまでUTCで取り扱って出力時にLocalTimeに変換(日時文字列にタイムゾーンを含める)くらいでしょうか。

@keiji
Copy link
Collaborator

keiji commented Dec 5, 2021

むしろ、ここにタイムゾーン関係の処理があったのか…LoggerServiceからしか使われないないから気づいてなかった。
JstNowについては、使われている箇所がLoggerServiceの1箇所しかない+ログを出力するたびにタイムゾーンを取得する処理が走っているので、staticで持って一回で済むようにしたいですね。

Issueにある解決策と代替案でどちらかという意味では解決策の方「Timezone関係は1つのクラスにまとめる(#523 で統一する)」方向で行きたいですね。
一方、処理をすでにあるUtilsクラスにまとめるかは検討の余地があると考えています。TimezoneUtilsとか、もう少し用途がわかる名前にしたい。

(そういえば DateTimeUtility というクラスもあるんだった。けど、これはServiceとして取り扱う提案されているので、ここへの移行は良くないかも)。

@b-wind
Copy link
Author

b-wind commented Dec 6, 2021

人間が見る表示はローカルタイムの方がいいですね。

人間が見る…… というよりは本来の目的からすると開発チームがトラブルシューティングのために見るのが存在の第一目的なのかなと。

端末自体のタイムゾーン情報は言語設定(CultureInfo)と一緒に必要な場所でログ出力する方が適切かなと。
端末自体の設定を変えることで「ログファイル内の日時表示方法」が変わるのは嬉しく無さそう。

ギリギリまでUTCで取り扱って出力時にLocalTimeに変換(日時文字列にタイムゾーンを含める)くらいでしょうか。
このあたりは出来れば Viewer側(COCOAの外)に任せたいのはあります。
専用のViewerでは無く Excel 等で開いている人も多いのかな?

@b-wind
Copy link
Author

b-wind commented Dec 6, 2021

一方、処理をすでにあるUtilsクラスにまとめるかは検討の余地があると考えています。TimezoneUtilsとか、もう少し用途がわかる名前にしたい。

Utils クラス。使ってしまう気持ちは分かりますが、本件の様に見落としやすくなる一因だとは思います。

@b-wind
Copy link
Author

b-wind commented Dec 6, 2021

私自身の運用経験から来る一般論ですが、クライアントとサーバーのログを突合等するケースを考えて同一のタイムゾーン・日時フォーマットの方が嬉しい等はありますね。

@b-wind
Copy link
Author

b-wind commented Dec 6, 2021

話を広げてしまっていてなんなのですが、当 Issue の目的はあくまで処理方法の統一までとさせて戴きたいと考えます。
(検討の上 TimezoneUtils クラスなどに纏める・インスタンスの生成を効率化する所までは含む)

動作ログに出力する内容の変更は必要に応じて別 Issue にした方が良いかと思います。

@keiji
Copy link
Collaborator

keiji commented Dec 28, 2021

AppConstantsに定数として定義して、アプリ内からは常に1つのインスタンスを使う(テストは別)ようにしました。

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
confirmed 開発内部管理用 enhancement 新しい機能や改善のリクエスト
Projects
None yet
3 participants