Skip to content

Commit 0e45d22

Browse files
authored
Merge pull request #76 from codeGROOVE-dev/reliable
decrease logging spam
2 parents 333df54 + 520bdc6 commit 0e45d22

File tree

4 files changed

+75
-49
lines changed

4 files changed

+75
-49
lines changed

cmd/server/main.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,10 @@ func main() {
9090
AddSource: true,
9191
Level: slog.LevelInfo,
9292
ReplaceAttr: func(_ []string, a slog.Attr) slog.Attr {
93-
// Shorten source paths to relative paths for cleaner logs
93+
// Use basename for source file for cleaner logs
9494
if a.Key == slog.SourceKey {
9595
if source, ok := a.Value.Any().(*slog.Source); ok {
96-
// Find project root by looking for /slacker/ in path
97-
if idx := strings.LastIndex(source.File, "/slacker/"); idx >= 0 {
98-
source.File = source.File[idx+9:] // Skip "/slacker/"
99-
}
96+
source.File = filepath.Base(source.File)
10097
}
10198
}
10299
return a

pkg/bot/bot.go

Lines changed: 70 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -59,19 +59,21 @@ type messageUpdateParams struct {
5959
//
6060
//nolint:govet // Field order optimized for logical grouping over memory alignment
6161
type Coordinator struct {
62-
processingEvents sync.WaitGroup // Tracks in-flight event processing for graceful shutdown
63-
stateStore StateStore // Persistent state across restarts
64-
sprinklerURL string
65-
workspaceName string // Track workspace name for better logging
66-
slack SlackClient
67-
github GitHubClient
68-
configManager ConfigManager
69-
notifier *notify.Manager
70-
userMapper UserMapper
71-
threadCache *cache.ThreadCache // In-memory cache for fast lookups
72-
commitPRCache *cache.CommitPRCache // Maps commit SHAs to PR numbers for check events
73-
eventSemaphore chan struct{} // Limits concurrent event processing (prevents overwhelming APIs)
74-
dmLocks sync.Map // Per-user-PR locks to prevent duplicate DMs (key: "userID:prURL")
62+
processingEvents sync.WaitGroup // Tracks in-flight event processing for graceful shutdown
63+
stateStore StateStore // Persistent state across restarts
64+
sprinklerURL string
65+
workspaceName string // Track workspace name for better logging
66+
slack SlackClient
67+
github GitHubClient
68+
configManager ConfigManager
69+
notifier *notify.Manager
70+
userMapper UserMapper
71+
threadCache *cache.ThreadCache // In-memory cache for fast lookups
72+
commitPRCache *cache.CommitPRCache // Maps commit SHAs to PR numbers for check events
73+
eventSemaphore chan struct{} // Limits concurrent event processing (prevents overwhelming APIs)
74+
dmLocks sync.Map // Per-user-PR locks to prevent duplicate DMs (key: "userID:prURL")
75+
recentUpdateLogsMu sync.Mutex // Protects recentUpdateLogs map
76+
recentUpdateLogs map[string]time.Time // Tracks recent "updating message" logs to prevent spam (key: "channelID:threadTS:state")
7577
}
7678

7779
// StateStore interface for persistent state - allows dependency injection for testing.
@@ -111,16 +113,17 @@ func New(
111113
stateStore StateStore,
112114
) *Coordinator {
113115
c := &Coordinator{
114-
slack: slackClient,
115-
github: githubClient,
116-
configManager: configManager,
117-
notifier: notifier,
118-
userMapper: usermapping.New(slackClient.API(), githubClient.InstallationToken(ctx)),
119-
sprinklerURL: sprinklerURL,
120-
stateStore: stateStore,
121-
threadCache: cache.New(),
122-
commitPRCache: cache.NewCommitPRCache(),
123-
eventSemaphore: make(chan struct{}, 10), // Allow 10 concurrent events per org
116+
slack: slackClient,
117+
github: githubClient,
118+
configManager: configManager,
119+
notifier: notifier,
120+
userMapper: usermapping.New(slackClient.API(), githubClient.InstallationToken(ctx)),
121+
sprinklerURL: sprinklerURL,
122+
stateStore: stateStore,
123+
threadCache: cache.New(),
124+
commitPRCache: cache.NewCommitPRCache(),
125+
eventSemaphore: make(chan struct{}, 10), // Allow 10 concurrent events per org
126+
recentUpdateLogs: make(map[string]time.Time),
124127
}
125128

126129
// Set GitHub client in config manager for this org.
@@ -569,7 +572,7 @@ func (c *Coordinator) processEvent(ctx context.Context, msg SprinklerMessage) er
569572
case "check_run", "check_suite":
570573
// Check events update PR test status - handle like pull_request events
571574
if msg.PRNumber > 0 {
572-
slog.Info("received check event for PR, refreshing state",
575+
slog.Debug("received check event for PR, refreshing state",
573576
"owner", owner,
574577
"repo", repo,
575578
"pr", msg.PRNumber,
@@ -616,7 +619,7 @@ func (c *Coordinator) handlePullRequestEventWithData(ctx context.Context, owner,
616619
) {
617620
prNumber := event.Number
618621

619-
slog.Info("PR event with pre-fetched data",
622+
slog.Debug("PR event with pre-fetched data",
620623
logFieldOwner, owner,
621624
logFieldRepo, repo,
622625
"number", prNumber,
@@ -997,12 +1000,10 @@ func (c *Coordinator) processChannelsInParallel(
9971000
if !exists {
9981001
// First time seeing this user
9991002
taggedUsers[userID] = info
1000-
} else {
1003+
} else if info.IsInAnyChannel {
10011004
// User already tagged in another channel - update IsInAnyChannel if this channel has them
1002-
if info.IsInAnyChannel {
1003-
existing.IsInAnyChannel = true
1004-
taggedUsers[userID] = existing
1005-
}
1005+
existing.IsInAnyChannel = true
1006+
taggedUsers[userID] = existing
10061007
}
10071008
}
10081009
taggedUsersMu.Unlock()
@@ -1270,16 +1271,44 @@ func (c *Coordinator) updateMessageIfNeeded(ctx context.Context, params messageU
12701271
return
12711272
}
12721273

1273-
slog.Info("updating message - content changed",
1274-
"workspace", c.workspaceName,
1275-
logFieldPR, fmt.Sprintf(prFormatString, params.Owner, params.Repo, params.PRNumber),
1276-
"channel", params.ChannelDisplay,
1277-
"channel_id", params.ChannelID,
1278-
"thread_ts", params.ThreadTS,
1279-
"pr_state", params.PRState,
1280-
"old_state", params.OldState,
1281-
"current_message_preview", params.CurrentText[:min(100, len(params.CurrentText))],
1282-
"expected_message_preview", expectedText[:min(100, len(expectedText))])
1274+
// Deduplicate "updating message" logs within 1 second to reduce noise from concurrent events
1275+
logKey := fmt.Sprintf("%s:%s:%s", params.ChannelID, params.ThreadTS, params.PRState)
1276+
shouldLog := true // Default to logging if map not initialized
1277+
if c.recentUpdateLogs != nil {
1278+
c.recentUpdateLogsMu.Lock()
1279+
if lastLogged, exists := c.recentUpdateLogs[logKey]; !exists || time.Since(lastLogged) > time.Second {
1280+
c.recentUpdateLogs[logKey] = time.Now()
1281+
shouldLog = true
1282+
// Clean up old entries (older than 5 seconds) to prevent memory leak
1283+
for key, timestamp := range c.recentUpdateLogs {
1284+
if time.Since(timestamp) > 5*time.Second {
1285+
delete(c.recentUpdateLogs, key)
1286+
}
1287+
}
1288+
} else {
1289+
shouldLog = false
1290+
}
1291+
c.recentUpdateLogsMu.Unlock()
1292+
}
1293+
1294+
if shouldLog {
1295+
slog.Info("updating message - content changed",
1296+
"workspace", c.workspaceName,
1297+
logFieldPR, fmt.Sprintf(prFormatString, params.Owner, params.Repo, params.PRNumber),
1298+
"channel", params.ChannelDisplay,
1299+
"channel_id", params.ChannelID,
1300+
"thread_ts", params.ThreadTS,
1301+
"pr_state", params.PRState,
1302+
"old_state", params.OldState,
1303+
"current_message_preview", params.CurrentText[:min(100, len(params.CurrentText))],
1304+
"expected_message_preview", expectedText[:min(100, len(expectedText))])
1305+
} else {
1306+
slog.Debug("suppressing duplicate 'updating message' log (within 1s)",
1307+
"workspace", c.workspaceName,
1308+
logFieldPR, fmt.Sprintf(prFormatString, params.Owner, params.Repo, params.PRNumber),
1309+
"channel", params.ChannelDisplay,
1310+
"pr_state", params.PRState)
1311+
}
12831312

12841313
if err := c.slack.UpdateMessage(ctx, params.ChannelID, params.ThreadTS, expectedText); err != nil {
12851314
slog.Error("failed to update PR message",
@@ -1326,7 +1355,7 @@ func (c *Coordinator) updateMessageIfNeeded(ctx context.Context, params messageU
13261355
func (c *Coordinator) handlePullRequestFromSprinkler(
13271356
ctx context.Context, owner, repo string, prNumber int, sprinklerURL string, eventTimestamp time.Time,
13281357
) {
1329-
slog.Info("handling PR event from sprinkler using turnclient",
1358+
slog.Debug("handling PR event from sprinkler using turnclient",
13301359
logFieldOwner, owner,
13311360
logFieldRepo, repo,
13321361
"pr_number", prNumber,

pkg/bot/bot_sprinkler.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ func (c *Coordinator) handleSprinklerEvent(ctx context.Context, event client.Eve
300300
return
301301
}
302302

303-
slog.Info("accepted event for async processing",
303+
slog.Debug("accepted event for async processing",
304304
"organization", organization,
305305
"type", event.Type,
306306
"url", event.URL,

pkg/bot/dm.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ func (c *Coordinator) findDMInHistory(ctx context.Context, userID, prURL string)
346346
// Returns (shouldQueue bool, sendAfter time.Time).
347347
// Channel membership is determined by caller - if channelID is non-empty, user was in at least one channel.
348348
func (c *Coordinator) shouldDelayNewDM(
349-
ctx context.Context,
349+
_ context.Context,
350350
userID, channelID, channelName string,
351351
owner, _ string,
352352
) (bool, time.Time) {
@@ -526,7 +526,7 @@ func (c *Coordinator) sendDMNotificationsToTaggedUsers(
526526
err := c.sendPRNotification(ctx, dmNotificationRequest{
527527
UserID: userInfo.UserID,
528528
ChannelID: channelID, // "delay" or empty based on channel membership
529-
ChannelName: "", // not used
529+
ChannelName: "", // not used
530530
Owner: owner,
531531
Repo: repo,
532532
PRNumber: prNumber,

0 commit comments

Comments
 (0)