Skip to content

Commit f7f5021

Browse files
committed
P2: Logging abstraction + file adapter
New files: - internal/telegram/log.go — Logger interface + FileLogger adapter (LogLevel, Logger interface, file-backed + stderr writer, NopLogger) - internal/telegram/log_test.go — 280 lines, all levels + concurrency Modified: - config.go — Added log_level, log_file fields + env parsing - bot.go — Logger field, SetLogger, all API errors now logged - handler.go — Logger field, SetLogger, all fmt.Fprintf/OnError → log - poller.go — Logger field, SetLogger, poll errors → log - approver.go — Replaced OnError callback with Logger - telegram.go — Logger creation + wiring to all components - handler_test.go — testBot gets NopLogger - poller_test.go — all Bot literals get NopLogger Format: 2006-01-02T15:04:05.000Z [LEVEL] telegram: message key=value Config: "log_level": "info", "log_file": "/var/log/odek-telegram.log" Env: ODEK_TELEGRAM_LOG_LEVEL, ODEK_TELEGRAM_LOG_FILE All 15 packages pass, go vet clean.
1 parent 6a70d23 commit f7f5021

11 files changed

Lines changed: 680 additions & 15 deletions

File tree

.hermes/plans/telegram-logging.md

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# P2: Proper logging abstraction + file adapter
2+
3+
## Design
4+
5+
### Logger interface (internal/telegram/log.go)
6+
```go
7+
type LogLevel int
8+
const ( LogDebug, LogInfo, LogWarn, LogError )
9+
10+
type Logger interface {
11+
Debug(msg string, fields ...any)
12+
Info(msg string, fields ...any)
13+
Warn(msg string, fields ...any)
14+
Error(msg string, fields ...any)
15+
With(fields ...any) Logger // returns child with extra fields
16+
}
17+
```
18+
19+
Fields are alternating key-value pairs: `log.Info("started", "chat_id", chatID)`.
20+
21+
### FileLogger (internal/telegram/log.go)
22+
- Writes to a file or stderr
23+
- Format: `2006-01-02T15:04:05.000Z [LEVEL] telegram: <msg> [key=value ...]`
24+
- File is opened at creation, append mode
25+
- If no log_file in config, defaults to stderr using the same format
26+
- Thread-safe (mutex on writes)
27+
28+
### Config additions (existing TelegramConfig)
29+
- `log_level` string → "debug", "info", "warn", "error" (default: "info")
30+
- `log_file` string → path (empty = stderr)
31+
32+
### Wiring changes
33+
Each component that currently logs gets a Logger field:
34+
35+
- **Bot**: `Logger` field, logs API errors
36+
- **Handler**: `Logger` field, replaces all `OnError` + `fmt.Fprintf` + `reportError`
37+
- **Poller**: `Logger` field, replaces `fmt.Fprintf`
38+
- **Approver**: `Logger` field, replaces `a.OnError`
39+
40+
**telegram.go entry point**:
41+
1. Resolve log level + log file from config
42+
2. Create the logger (file or stderr)
43+
3. Pass it to Bot, Handler, Poller via constructors/setters
44+
4. Handler's OnError can still exist as a callback (for chat notifications), but logging goes through Logger
45+
46+
### Files to modify
47+
- `internal/telegram/log.go` — NEW: interface + file adapter
48+
- `internal/telegram/config.go` — Add log_level, log_file fields
49+
- `internal/telegram/bot.go` — Add Logger field
50+
- `internal/telegram/handler.go` — Add Logger field, replace all logging
51+
- `internal/telegram/poller.go` — Add Logger field
52+
- `internal/telegram/approver.go` — Replace a.OnError with Logger
53+
- `cmd/odek/telegram.go` — Wire logger creation + pass to components

cmd/odek/telegram.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,16 @@ func telegramCmd(args []string) error {
5050
bot := telegram.NewBot(cfg.Token)
5151
bot.SetDailyTokenBudget(cfg.DailyTokenBudget)
5252

53-
// 4b. Configure fallback Telegram API endpoints if provided.
53+
// 4b. Create logger.
54+
level := telegram.ParseLogLevel(cfg.LogLevel)
55+
rootLog := telegram.NewFileLogger(level, cfg.LogFile)
56+
botLog := rootLog.With("component", "bot")
57+
handlerLog := rootLog.With("component", "handler")
58+
pollerLog := rootLog.With("component", "poller")
59+
60+
bot.SetLogger(botLog)
61+
62+
// 4c. Configure fallback Telegram API endpoints if provided.
5463
if len(cfg.FallbackURLs) > 0 {
5564
bot.SetFallbackURLs(cfg.FallbackURLs)
5665
}
@@ -68,6 +77,7 @@ func telegramCmd(args []string) error {
6877

6978
// 7. Create handler.
7079
handler := telegram.NewHandler(bot)
80+
handler.SetLogger(handlerLog)
7181

7282
// 8. Set handler config from cfg.
7383
handler.Config = telegram.HandlerConfig{
@@ -129,19 +139,20 @@ func telegramCmd(args []string) error {
129139
}
130140

131141
handler.OnError = func(chatID int64, err error) {
132-
fmt.Fprintf(os.Stderr, "odek telegram: error for chat %d: %v\n", chatID, err)
142+
handlerLog.Error("handler error", "chat_id", chatID, "error", err)
133143
}
134144

135145
// 11. Set command list via Telegram API.
136146
if err := bot.SetMyCommands(telegram.CommandDescriptors()); err != nil {
137-
fmt.Fprintf(os.Stderr, "odek telegram: warning: set commands failed: %v\n", err)
147+
handlerLog.Warn("set commands failed", "error", err)
138148
}
139149

140150
// 12. Print startup banner.
141-
fmt.Fprintf(os.Stderr, "odek telegram bot started\n")
151+
handlerLog.Info("telegram bot started")
142152

143153
// 13. Create poller.
144154
poller := telegram.NewPoller(bot)
155+
poller.SetLogger(pollerLog)
145156
poller.Interval = time.Duration(cfg.PollInterval) * time.Second
146157
poller.Timeout = cfg.PollTimeout
147158

internal/telegram/approver.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,10 @@ type TelegramApprover struct {
4444
pending map[string]chan string // requestID -> response channel
4545
mu sync.Mutex
4646
trusted map[danger.RiskClass]bool
47+
log Logger
4748

4849
// ChatID is the Telegram chat where approval prompts are sent.
4950
ChatID int64
50-
51-
// OnError logs errors (if nil, errors are silently ignored).
52-
OnError func(err error)
5351
}
5452

5553
// NewTelegramApprover creates a TelegramApprover for the given chat.
@@ -59,7 +57,17 @@ func NewTelegramApprover(bot *Bot, chatID int64) *TelegramApprover {
5957
ChatID: chatID,
6058
pending: make(map[string]chan string),
6159
trusted: make(map[danger.RiskClass]bool),
60+
log: NewNopLogger(),
61+
}
62+
}
63+
64+
// SetLogger sets the logger for this approver. If nil, a NopLogger is used.
65+
func (a *TelegramApprover) SetLogger(l Logger) {
66+
if l == nil {
67+
a.log = NewNopLogger()
68+
return
6269
}
70+
a.log = l
6371
}
6472

6573
// PromptCommand sends an approval request with inline keyboard and waits
@@ -141,9 +149,7 @@ func (a *TelegramApprover) PromptCommand(cls danger.RiskClass, cmd, description
141149
if _, err := a.bot.SendMessage(a.ChatID,
142150
fmt.Sprintf("🔒 Class `%s` trusted for this session.", cls),
143151
&SendOpts{ParseMode: ParseModeMarkdownV2}); err != nil {
144-
if a.OnError != nil {
145-
a.OnError(fmt.Errorf("telegram approver: confirm trust: %w", err))
146-
}
152+
a.log.Error("confirm trust message failed", "chat_id", a.ChatID, "error", err)
147153
}
148154
return nil
149155
case "deny":

internal/telegram/bot.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ type Bot struct {
2020
FileBaseURL string
2121
Client *http.Client
2222
DailyTokenBudget int64
23+
log Logger
2324
}
2425

2526
// NewBot creates a new Bot with the given token and a default HTTP client
@@ -32,9 +33,19 @@ func NewBot(token string) *Bot {
3233
Client: &http.Client{
3334
Timeout: 30 * time.Second,
3435
},
36+
log: NewNopLogger(),
3537
}
3638
}
3739

40+
// SetLogger sets the logger for this bot. If nil, a NopLogger is used (no-op).
41+
func (b *Bot) SetLogger(l Logger) {
42+
if l == nil {
43+
b.log = NewNopLogger()
44+
return
45+
}
46+
b.log = l
47+
}
48+
3849
// url builds the full API endpoint URL for the given method.
3950
func (b *Bot) url(method string) string {
4051
return fmt.Sprintf("%s/%s", b.BaseURL, method)
@@ -48,19 +59,22 @@ func (b *Bot) doJSON(method string, body any, dest any) error {
4859
var err error
4960
reqBody, err = json.Marshal(body)
5061
if err != nil {
62+
b.log.Error("marshal request failed", "method", method, "error", err)
5163
return fmt.Errorf("telegram: marshal request: %w", err)
5264
}
5365
}
5466

5567
url := b.url(method)
5668
resp, err := b.Client.Post(url, "application/json", bytes.NewReader(reqBody))
5769
if err != nil {
70+
b.log.Error("http post failed", "method", method, "error", err)
5871
return fmt.Errorf("telegram: post %s: %w", method, err)
5972
}
6073
defer resp.Body.Close()
6174

6275
respBody, err := io.ReadAll(resp.Body)
6376
if err != nil {
77+
b.log.Error("read response body failed", "method", method, "error", err)
6478
return fmt.Errorf("telegram: read response: %w", err)
6579
}
6680

@@ -71,15 +85,18 @@ func (b *Bot) doJSON(method string, body any, dest any) error {
7185
ErrorCode int `json:"error_code"`
7286
}
7387
if err := json.Unmarshal(respBody, &apiResp); err != nil {
88+
b.log.Error("unmarshal response failed", "method", method, "error", err)
7489
return fmt.Errorf("telegram: unmarshal response: %w", err)
7590
}
7691

7792
if !apiResp.OK {
93+
b.log.Error("api error", "method", method, "description", apiResp.Description, "error_code", apiResp.ErrorCode)
7894
return fmt.Errorf("telegram: %s failed: %s (code %d)", method, apiResp.Description, apiResp.ErrorCode)
7995
}
8096

8197
if dest != nil && len(apiResp.Result) > 0 {
8298
if err := json.Unmarshal(apiResp.Result, dest); err != nil {
99+
b.log.Error("unmarshal result failed", "method", method, "error", err)
83100
return fmt.Errorf("telegram: unmarshal result: %w", err)
84101
}
85102
}
@@ -91,6 +108,7 @@ func (b *Bot) doJSON(method string, body any, dest any) error {
91108
func (b *Bot) doUpload(method string, field string, path string, params map[string]any, dest any) error {
92109
file, err := os.Open(path)
93110
if err != nil {
111+
b.log.Error("open file failed", "method", method, "path", path, "error", err)
94112
return fmt.Errorf("telegram: open file %s: %w", path, err)
95113
}
96114
defer file.Close()
@@ -101,42 +119,50 @@ func (b *Bot) doUpload(method string, field string, path string, params map[stri
101119
// Write the file part.
102120
part, err := writer.CreateFormFile(field, filepath.Base(path))
103121
if err != nil {
122+
b.log.Error("create form file failed", "method", method, "field", field, "error", err)
104123
return fmt.Errorf("telegram: create form file: %w", err)
105124
}
106125
if _, err := io.Copy(part, file); err != nil {
126+
b.log.Error("copy file content failed", "method", method, "path", path, "error", err)
107127
return fmt.Errorf("telegram: copy file content: %w", err)
108128
}
109129

110130
// Write extra parameters as JSON parts.
111131
for key, val := range params {
112132
jsonVal, err := json.Marshal(val)
113133
if err != nil {
134+
b.log.Error("marshal param failed", "method", method, "key", key, "error", err)
114135
return fmt.Errorf("telegram: marshal param %s: %w", key, err)
115136
}
116137
if err := writer.WriteField(key, string(jsonVal)); err != nil {
138+
b.log.Error("write field failed", "method", method, "key", key, "error", err)
117139
return fmt.Errorf("telegram: write field %s: %w", key, err)
118140
}
119141
}
120142

121143
if err := writer.Close(); err != nil {
144+
b.log.Error("close multipart writer failed", "method", method, "error", err)
122145
return fmt.Errorf("telegram: close multipart writer: %w", err)
123146
}
124147

125148
url := b.url(method)
126149
req, err := http.NewRequest(http.MethodPost, url, &buf)
127150
if err != nil {
151+
b.log.Error("create request failed", "method", method, "error", err)
128152
return fmt.Errorf("telegram: create request: %w", err)
129153
}
130154
req.Header.Set("Content-Type", writer.FormDataContentType())
131155

132156
resp, err := b.Client.Do(req)
133157
if err != nil {
158+
b.log.Error("http post failed", "method", method, "error", err)
134159
return fmt.Errorf("telegram: post %s: %w", method, err)
135160
}
136161
defer resp.Body.Close()
137162

138163
respBody, err := io.ReadAll(resp.Body)
139164
if err != nil {
165+
b.log.Error("read response body failed", "method", method, "error", err)
140166
return fmt.Errorf("telegram: read response: %w", err)
141167
}
142168

@@ -147,15 +173,18 @@ func (b *Bot) doUpload(method string, field string, path string, params map[stri
147173
ErrorCode int `json:"error_code"`
148174
}
149175
if err := json.Unmarshal(respBody, &apiResp); err != nil {
176+
b.log.Error("unmarshal response failed", "method", method, "error", err)
150177
return fmt.Errorf("telegram: unmarshal response: %w", err)
151178
}
152179

153180
if !apiResp.OK {
181+
b.log.Error("api error", "method", method, "description", apiResp.Description, "error_code", apiResp.ErrorCode)
154182
return fmt.Errorf("telegram: %s failed: %s (code %d)", method, apiResp.Description, apiResp.ErrorCode)
155183
}
156184

157185
if dest != nil && len(apiResp.Result) > 0 {
158186
if err := json.Unmarshal(apiResp.Result, dest); err != nil {
187+
b.log.Error("unmarshal result failed", "method", method, "error", err)
159188
return fmt.Errorf("telegram: unmarshal result: %w", err)
160189
}
161190
}

internal/telegram/config.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ type TelegramConfig struct {
1919
DailyTokenBudget int64 `json:"daily_token_budget"` // default 1000000
2020
SessionTTL int `json:"session_ttl_hours"` // hours, default 24
2121
FallbackURLs []string `json:"fallback_urls"`
22+
LogLevel string `json:"log_level"` // "debug","info","warn","error" (default "info")
23+
LogFile string `json:"log_file"` // path or empty for stderr
2224
}
2325

2426
// DefaultConfig returns a TelegramConfig with sensible defaults.
@@ -77,6 +79,12 @@ func ConfigFromEnv(base TelegramConfig) TelegramConfig {
7779
if v := os.Getenv("ODEK_TELEGRAM_FALLBACK_URLS"); v != "" {
7880
cfg.FallbackURLs = splitAndTrim(v)
7981
}
82+
if v := os.Getenv("ODEK_TELEGRAM_LOG_LEVEL"); v != "" {
83+
cfg.LogLevel = v
84+
}
85+
if v := os.Getenv("ODEK_TELEGRAM_LOG_FILE"); v != "" {
86+
cfg.LogFile = v
87+
}
8088

8189
return cfg
8290
}

0 commit comments

Comments
 (0)