-
Notifications
You must be signed in to change notification settings - Fork 0
Develop To Main #88
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
Develop To Main #88
Changes from all commits
9c6ae54
13f0256
85b7e9f
7526013
d76910a
2c93fb3
de60aed
1d18eda
afb1029
9ff448b
dc2e854
d834883
3c28d41
95435ea
f23d530
a9c450e
9f29e62
f5f86c6
d4bf57c
a947592
7b6805d
b97e366
e18a30f
169634f
872197a
76c0279
eb5892e
d261319
102bfc9
db98466
4d30a99
ee9e42e
3b61562
12f9307
e39866b
a94647a
2cac241
1248d41
bd5c0fb
b03e099
c69b7a7
116b8da
e863b9a
037b6a0
d807d8c
9b1e3fb
0694c22
972036b
620ecd3
4cb243d
04c1e22
92085bd
8878673
2b2ad36
5130c29
4e1350d
3aa7769
36b89c0
524658e
528f22c
f769209
8e17c6f
2456786
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,7 @@ import ( | |||||||||||||||||||||||||
| "context" | ||||||||||||||||||||||||||
| "log/slog" | ||||||||||||||||||||||||||
| "os" | ||||||||||||||||||||||||||
| "time" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| "github.com/aws/aws-sdk-go-v2/aws" | ||||||||||||||||||||||||||
| "github.com/aws/aws-sdk-go-v2/config" | ||||||||||||||||||||||||||
|
|
@@ -14,14 +15,37 @@ import ( | |||||||||||||||||||||||||
| "github.com/umekikazuya/me/internal/infra/db" | ||||||||||||||||||||||||||
| "github.com/umekikazuya/me/internal/infra/fetcher" | ||||||||||||||||||||||||||
| "github.com/umekikazuya/me/internal/infra/tokenizer" | ||||||||||||||||||||||||||
| "github.com/umekikazuya/me/pkg/obs" | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| var targetPlatforms = []string{"qiita", "zenn"} | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| func main() { | ||||||||||||||||||||||||||
| slog.SetDefault(slog.New(slog.NewJSONHandler(os.Stdout, nil))) | ||||||||||||||||||||||||||
| ctx := context.Background() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| prov, shutdown, err := obs.Bootstrap(ctx, obs.Config{ | ||||||||||||||||||||||||||
| ServiceName: "batch", | ||||||||||||||||||||||||||
| Level: obs.ParseLevel(os.Getenv("LOG_LEVEL")), | ||||||||||||||||||||||||||
| AddSource: true, | ||||||||||||||||||||||||||
| EnableTraces: true, | ||||||||||||||||||||||||||
| EnableMetrics: true, | ||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||
| slog.Error("観測性基盤の初期化に失敗しました", "error", err) | ||||||||||||||||||||||||||
| os.Exit(1) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| // shutdown は bounded な context で呼ぶ。呼び出し時点で ctx が cancel | ||||||||||||||||||||||||||
| // されていると tracer/meter の flush が即座に諦められてしまう。 | ||||||||||||||||||||||||||
| defer func() { | ||||||||||||||||||||||||||
| shutdownCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | ||||||||||||||||||||||||||
| defer cancel() | ||||||||||||||||||||||||||
| _ = shutdown(shutdownCtx) | ||||||||||||||||||||||||||
| }() | ||||||||||||||||||||||||||
|
Comment on lines
+39
to
+43
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial シャットダウンエラーのログ出力を検討
♻️ エラーログ追加案 defer func() {
shutdownCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
- _ = shutdown(shutdownCtx)
+ if err := shutdown(shutdownCtx); err != nil {
+ slog.Error("observability shutdown failed", "error", err)
+ }
}()📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| slog.SetDefault(prov.Logger) | ||||||||||||||||||||||||||
| // RecoverProcess はログ出力後に re-panic するので、batch のプロセス終了は | ||||||||||||||||||||||||||
| // 非ゼロ exit になる (runtime のデフォルト panic ハンドラが exit 2 を返す)。 | ||||||||||||||||||||||||||
| defer obs.RecoverProcess(ctx, "batch.main") | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
Comment on lines
+45
to
+48
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🐛 修正案- defer obs.RecoverProcess(ctx, "batch.main")
+ defer func() {
+ obs.RecoverProcess(ctx, "batch.main")
+ }()または 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| // TODO: パラメータを注入する機構を考える(実行環境も) | ||||||||||||||||||||||||||
| endpoint := os.Getenv("DYNAMODB_ENDPOINT") | ||||||||||||||||||||||||||
| tableName := os.Getenv("DYNAMODB_TABLE_NAME") | ||||||||||||||||||||||||||
|
|
@@ -93,3 +117,4 @@ func main() { | |||||||||||||||||||||||||
| os.Exit(1) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
シャットダウンタイムアウトの不整合
Line 31 で
shutdownTimeout = 30 * time.Secondと定義されていますが、Line 65 のフォールバックでは5*time.Secondがハードコードされています。一貫性のため、定数を使用するか、フォールバック用に別の定数を定義することを推奨します。🐛 修正案
if shutdownCtx == nil { - shutdownCtx, shutdownCancel = context.WithTimeout(context.Background(), 5*time.Second) + shutdownCtx, shutdownCancel = context.WithTimeout(context.Background(), shutdownTimeout) }または、早期終了時の短いタイムアウトが意図的であれば、コメントでその理由を説明してください。
📝 Committable suggestion
🤖 Prompt for AI Agents