chore: deprecate cloudzap package and zap-based logging API - #844
Conversation
366030d to
e0f7e66
Compare
|
|
||
| // WithLogger adds a logger to the current context. | ||
| // | ||
| // Deprecated: Use slog.SetDefault with cloudslog.NewHandler instead. |
There was a problem hiding this comment.
Cloudrunner-go sets its own default with Google Cloud Logging support so using slog.SetDefault is probably not what one wants to do (generally)
There was a problem hiding this comment.
I updated it to
// WithLogger adds a logger to the current context.
//
// Deprecated: cloudrunner.Run configures the default slog logger with cloudslog.Handler.
// Use slog.InfoContext, slog.WarnContext, etc. instead of a context-based zap logger.
func WithLogger(ctx context.Context, logger *zap.Logger) context.Context {
return context.WithValue(ctx, loggerContextKey{}, logger)
}| // Deprecated: As per https://docs.cloud.google.com/trace/docs/trace-log-integration the | ||
| // preferred trace format is now just the trace ID, not projects/PROJECT_ID/traces/TRACE_ID. | ||
| // The cloudslog.Handler injects trace fields automatically from the OpenTelemetry span | ||
| // context without requiring a project ID. |
There was a problem hiding this comment.
This is actually not used and was mistakenly missed to be deleted in the last PR.
There was a problem hiding this comment.
Ok. I guess we shouldn't remove it- or do you think we should?
I changed it now into
// ProjectID of the project the service is running in.
//
// Deprecated: No longer used. As per https://docs.cloud.google.com/trace/docs/trace-log-integration
// the preferred trace format is now just the trace ID, not projects/PROJECT_ID/traces/TRACE_ID.
ProjectID string| // | ||
| // Deprecated: Use slog.Any with slog.Attr instead of zap.Field. The zapcore.Field support | ||
| // in AdditionalFields.Add will be removed in a future version. |
There was a problem hiding this comment.
There is actually no equivalent function at the moment. I did a quick search internally and I don't believe anybody is using this so let's just make the message clearer.
There was a problem hiding this comment.
Good catch, changed it:
// ErrorDetails creates a zap.Field that logs the gRPC error details of the provided error.
//
// Deprecated: Returns a zap.Field which ties consumers to the deprecated zap dependency.
// There is no drop-in slog replacement yet. This function will be removed in a future version.
func ErrorDetails(err error) zap.Field {
...
}| // | ||
| // Deprecated: As per https://docs.cloud.google.com/trace/docs/trace-log-integration the | ||
| // preferred trace format is now just the trace ID, not projects/PROJECT_ID/traces/TRACE_ID. | ||
| // The cloudslog.Handler injects trace fields automatically from the OpenTelemetry span | ||
| // context without requiring a project ID. |
There was a problem hiding this comment.
This is actually not used and was mistakenly missed to be removed from last PR
There was a problem hiding this comment.
Changed it like in the previous instance:
// ProjectID of the project the service is running in.
//
// Deprecated: No longer used. As per https://docs.cloud.google.com/trace/docs/trace-log-integration
// the preferred trace format is now just the trace ID, not projects/PROJECT_ID/traces/TRACE_ID.
ProjectID string| // WithOtelTraceHook configures the run context with a trace hook. | ||
| // | ||
| // Deprecated: The cloudslog.Handler automatically injects trace fields from the | ||
| // OpenTelemetry span context, making custom trace hooks for log enrichment redundant. |
There was a problem hiding this comment.
WithOtelTraceHook allows selecting the new opentelemetry trace features (instead of the old custom one). The reason it should be deprecated is that we will remove the old features completely and there will be no reason to select the one since it will be the only one. (I feel the comment was already correct in this one which was already marked as deprecated except for the function name which was wrong)
There was a problem hiding this comment.
Makes sense, I reverted this change into the deprecation warning we already had.
e0f7e66 to
dc5b68e
Compare
33232ef to
61faadb
Compare
0d6a1e3 to
40abb3d
Compare
The cloudslog.Handler configured by cloudrunner.Run provides the same capabilities as the cloudzap package — Cloud Logging field formatting, error reporting, and trace correlation — using standard log/slog. This makes the zap logger, its context middleware, and explicit trace field injection redundant. Deprecate ahead of removal in a future version: - cloudzap package and all exported symbols - cloudrunner.Logger and cloudrunner.WithLoggerFields - cloudrequestlog.ErrorDetails (returns zap.Field) - WithOtelTraceHook, TraceIDHook, IDHook - TraceHook fields on trace middlewares Also remove unused ProjectID fields from cloudotel.TraceMiddleware and cloudtrace.Middleware (dead code after the trace format change in #840), and fix incorrect doc comment on WithOtelTraceHook (said WithTraceHook).
5e12b54 to
adb0445
Compare
Why?
The
cloudslog.Handlerconfigured bycloudrunner.Runprovides the same capabilities as thecloudzappackage — Cloud Logging field formatting, error reporting, and trace correlation — using standardlog/sloginstead ofgo.uber.org/zap. The zap logger, its context middleware, and explicit trace field injection are all redundant now.Rather than removing the zap path outright (as in #814), this deprecates it first to give consumers time to migrate to
log/slogwithcloudslog.What?
cloudzappackage and all its exported symbolscloudrunner.Loggerandcloudrunner.WithLoggerFieldscloudrequestlog.ErrorDetails(returnszap.Field)WithOtelTraceHook,TraceIDHook,IDHookProjectIDandTraceHookfields on bothcloudotel.TraceMiddlewareandcloudtrace.MiddlewareWithOtelTraceHook(saidWithTraceHook)Migration guide
Logging
Replace
cloudrunner.Logger(ctx)with standardlog/slogcontext functions. Custom fields can be passed directly as arguments:Context enrichment
Both
cloudrunner.WithLoggerFieldsandcloudslog.Withenrich all subsequent logs within the returned context. The replacement is a direct swap:Trace correlation
No action needed. The
cloudslog.Handler(configured bycloudrunner.Run) automatically extracts trace/span IDs from the OpenTelemetry span context on every log call. Remove anyWithOtelTraceHookorWithTraceHookoptions from yourcloudrunner.Runcall — they are no longer needed.Error helpers
Use
cloudslog.Errorsfor logging multiple errors:Request logging (
cloudrequestlog)The
cloudrequestlogpackage already usessloginternally and is not deprecated. However,cloudrequestlog.ErrorDetailsreturns azap.Fieldand is now deprecated.AdditionalFields.Addstill accepts bothzap.Fieldandslog.Attrfor backwards compatibility, but preferslog.Attrwhen adding custom fields to request logs.Notes