Skip to content

Commit 437c85b

Browse files
authored
fix(go): prevent swallowing metrics errors (#3600)
1 parent 27a95c4 commit 437c85b

File tree

1 file changed

+21
-19
lines changed

1 file changed

+21
-19
lines changed

go/plugins/googlecloud/googlecloud.go

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,13 @@ import (
4949
// Global sync.Once for showing logging setup instructions only once across all recovery attempts
5050
var showLoggingInstructionsOnce sync.Once
5151

52+
// stderrLogger is a stable logger that always writes to stderr and is never
53+
// replaced by calls to slog.SetDefault. Use this for error paths to ensure
54+
// messages are visible even if the default logger is misconfigured.
55+
var stderrLogger = slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{
56+
Level: slog.LevelInfo,
57+
}))
58+
5259
// EnableGoogleCloudTelemetry enables comprehensive telemetry export to Google Cloud Observability suite.
5360
// This directly initializes telemetry without requiring plugin registration.
5461
//
@@ -241,25 +248,25 @@ func setupGCPLogger(projectID string, level slog.Leveler, credentials *google.Cr
241248
}
242249
// Set up error handling for async logging failures with recursive recovery
243250
c.OnError = func(err error) {
244-
fallbackHandler := slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{
245-
Level: level,
246-
})
247-
slog.SetDefault(slog.New(fallbackHandler))
248-
slog.Warn("Switched to stderr logging due to Google Cloud logging failure", "error", err)
249-
slog.Error("Unable to send logs to Google Cloud", "error", err)
251+
slog.SetDefault(stderrLogger)
252+
stderrLogger.Warn("Unable to send logs to Google Cloud", "error", err)
250253
if loggingDenied(err) {
251254
showLoggingInstructionsOnce.Do(func() {
252255
fmt.Fprint(os.Stderr, loggingDeniedHelpText(projectID))
253256
})
254257
}
258+
// TODO: Reinitialize logger if possible
259+
// For now, we left the code below commented out because re-initialization was causing the
260+
// logger to swallow errors for some reason.
261+
/*
262+
stderrLogger.Error("Unable to send logs to Google Cloud. Re-initializing logger.")
263+
// Assume the logger is compromised, and we need a new one
264+
// Reinitialize the logger with a new instance with the same config
255265
256-
// Assume the logger is compromised, and we need a new one
257-
// Reinitialize the logger with a new instance with the same config
258-
if setupErr := setupGCPLogger(projectID, level, credentials); setupErr == nil {
259-
slog.Info("Initialized a new GcpLogger")
260-
} else {
261-
slog.Error("Failed to reinitialize GCP logger", "error", setupErr)
262-
}
266+
if setupErr := setupGCPLogger(projectID, level, credentials); setupErr != nil {
267+
stderrLogger.Error("Failed to reinitialize GCP logger", "error", setupErr)
268+
}
269+
*/
263270
}
264271
logger := c.Logger("genkit_log")
265272
slog.SetDefault(slog.New(newHandler(level, logger.Log, projectID)))
@@ -279,17 +286,12 @@ func (e *AdjustingTraceExporter) ExportSpans(ctx context.Context, spans []sdktra
279286
adjustedSpans := e.adjust(spans)
280287
err := e.exporter.ExportSpans(ctx, adjustedSpans)
281288
if err != nil {
282-
slog.Error("Failed to export spans to Google Cloud Trace",
289+
slog.Error("Unable to send telemetry to Google Cloud",
283290
"error", err,
284291
"span_count", len(adjustedSpans),
285292
"project_id", e.projectId,
286293
"error_type", fmt.Sprintf("%T", err))
287-
} else {
288-
slog.Debug("Successfully exported spans to Google Cloud Trace",
289-
"span_count", len(adjustedSpans),
290-
"project_id", e.projectId)
291294
}
292-
293295
return err
294296
}
295297

0 commit comments

Comments
 (0)