From 6b483501d8d57488d0a133b4d150880e653b9729 Mon Sep 17 00:00:00 2001 From: Yorukot Date: Fri, 15 May 2026 00:00:40 +0800 Subject: [PATCH 01/11] feat: implement a basic of new logutil --- pkg/log/constant.go | 23 +++++++++++ pkg/log/error_warp.go | 59 ++++++++++++++++++++++++++++ pkg/log/logger.go | 78 ++++++++++++++++++++++--------------- pkg/log/with.go | 90 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 219 insertions(+), 31 deletions(-) create mode 100644 pkg/log/constant.go create mode 100644 pkg/log/error_warp.go create mode 100644 pkg/log/with.go diff --git a/pkg/log/constant.go b/pkg/log/constant.go new file mode 100644 index 0000000..9ed27d4 --- /dev/null +++ b/pkg/log/constant.go @@ -0,0 +1,23 @@ +package logutil + +type ErrorType string + +const ( + OK ErrorType = "OK" + CANCELLED ErrorType = "CANCELLED" + UNKNOWN ErrorType = "UNKNOWN" + INVALID_ARGUMENT ErrorType = "INVALID_ARGUMENT" + DEADLINE_EXCEEDED ErrorType = "DEADLINE_EXCEEDED" + NOT_FOUND ErrorType = "NOT_FOUND" + ALREADY_EXISTS ErrorType = "ALREADY_EXISTS" + PERMISSION_DENIED ErrorType = "PERMISSION_DENIED" + UNAUTHENTICATED ErrorType = "UNAUTHENTICATED" + RESOURCE_EXHAUSTED ErrorType = "RESOURCE_EXHAUSTED" + FAILED_PRECONDITION ErrorType = "FAILED_PRECONDITION" + ABORTED ErrorType = "ABORTED" + OUT_OF_RANGE ErrorType = "OUT_OF_RANGE" + UNIMPLEMENTED ErrorType = "UNIMPLEMENTED" + INTERNAL ErrorType = "INTERNAL" + UNAVAILABLE ErrorType = "UNAVAILABLE" + DATA_LOSS ErrorType = "DATA_LOSS" +) diff --git a/pkg/log/error_warp.go b/pkg/log/error_warp.go new file mode 100644 index 0000000..dbfa38d --- /dev/null +++ b/pkg/log/error_warp.go @@ -0,0 +1,59 @@ +package logutil + +import "errors" + +type InfoCarrier interface { + LogInfo() map[string]any +} + +type InfoError[K ~string, V any] struct { + Base error + Info map[K]V +} + +func (e *InfoError[K, V]) Error() string { + if e.Base != nil { + return e.Base.Error() + } + return "" +} + +func (e *InfoError[K, V]) Unwrap() error { + return e.Base +} + +func (e *InfoError[K, V]) LogInfo() map[string]any { + if len(e.Info) == 0 { + return nil + } + + out := make(map[string]any, len(e.Info)) + for k, v := range e.Info { + out[string(k)] = v + } + + return out +} + +func NewInfoError[K ~string, V any](base error, info map[K]V) error { + return &InfoError[K, V]{ + Base: base, + Info: info, + } +} + +func WrapInfoError[K ~string, V any](base error, info map[K]V) error { + return &InfoError[K, V]{ + Base: base, + Info: info, + } +} + +func InfoFromError(err error) map[string]any { + var carrier InfoCarrier + if errors.As(err, &carrier) { + return carrier.LogInfo() + } + + return nil +} \ No newline at end of file diff --git a/pkg/log/logger.go b/pkg/log/logger.go index d90f63c..d7da16a 100644 --- a/pkg/log/logger.go +++ b/pkg/log/logger.go @@ -5,9 +5,9 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strings" - "go.opentelemetry.io/otel/trace" "go.uber.org/zap" "go.uber.org/zap/zapcore" ) @@ -45,36 +45,6 @@ func ZapDevelopmentConfig() zap.Config { return config } -// WithContext parses the context and adds the trace ID to the logger if available -func WithContext(ctx context.Context, logger *zap.Logger) *zap.Logger { - if ctx == nil { - return logger - } - - spanCtx := trace.SpanFromContext(ctx).SpanContext() - if spanCtx.HasTraceID() { - logger = logger.With(zap.String("trace_id", spanCtx.TraceID().String())) - } - - if spanCtx.HasSpanID() { - logger = logger.With(zap.String("span_id", spanCtx.SpanID().String())) - } - - if ctx.Value("user_id") != nil { - logger = logger.With(zap.Any("user_id", ctx.Value("user_id"))) - } - - if ctx.Value("username") != nil { - logger = logger.With(zap.Any("username", ctx.Value("username"))) - } - - if ctx.Value("name") != nil { - logger = logger.With(zap.Any("display-name", ctx.Value("name"))) - } - - return logger -} - // prettyEncodeCaller add padding to the caller string func prettyEncodeCaller(caller zapcore.EntryCaller, enc zapcore.PrimitiveArrayEncoder) { const fixedWidth = 25 @@ -114,3 +84,49 @@ func relativePrettyCallerEncoder(rootDir string) zapcore.CallerEncoder { enc.AppendString(callerStr) } } + +// Constructs is a convenience function that constructs a new logger with fields extracted from the context +func Constructs(ctx context.Context, logger *zap.Logger) *zap.Logger { + if fields, ok := ctx.Value(contextFieldsKey{}).(contextFields); ok && len(fields) > 0 { + zapFields := make([]zap.Field, 0, len(fields)) + for _, field := range fields { + zapFields = append(zapFields, field) + } + logger = logger.With(zapFields...) + } + + logger = logger.With(codeFields(1)...) + + return logger +} + +func codeFields(skip int) []zap.Field { + pcs := make([]uintptr, 1) + + // Skip runtime.Callers 和 codeFields 自己 + n := runtime.Callers(skip+2, pcs) + if n == 0 { + return nil + } + + frames := runtime.CallersFrames(pcs[:n]) + frame, _ := frames.Next() + + namespace, function := splitFunction(frame.Function) + + return []zap.Field{ + zap.String("code.file.path", frame.File), + zap.Int("code.line.number", frame.Line), + zap.String("code.function.name", function), + zap.String("code.namespace", namespace), + } +} + +func splitFunction(full string) (namespace string, function string) { + i := strings.LastIndex(full, ".") + if i == -1 { + return "", full + } + + return full[:i], full[i+1:] +} diff --git a/pkg/log/with.go b/pkg/log/with.go new file mode 100644 index 0000000..96f8716 --- /dev/null +++ b/pkg/log/with.go @@ -0,0 +1,90 @@ +package logutil + +import ( + "context" + + "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" +) + +type contextFieldsKey struct{} + +type contextFields map[string]zap.Field + +// WithFields returns a child context enriched with request-scoped logging fields. +// Fields with the same key replace earlier fields to avoid duplicate log keys. +func WithFields(ctx context.Context, fields ...zap.Field) context.Context { + if ctx == nil { + ctx = context.Background() + } + + existing, _ := ctx.Value(contextFieldsKey{}).(contextFields) + + next := make(contextFields, len(existing)+len(fields)) + for key, field := range existing { + next[key] = field + } + + for _, field := range fields { + next[field.Key] = field + } + + return context.WithValue(ctx, contextFieldsKey{}, next) +} + +func WithOutcome(outcome string, logger *zap.Logger) *zap.Logger { + if logger == nil || outcome == "" { + return logger + } + return logger.With(zap.String("event.outcome", outcome)) +} + +func WithEventName(actionName string, logger *zap.Logger) *zap.Logger { + if logger == nil || actionName == "" { + return logger + } + return logger.With(zap.String("event.name", actionName)) +} + +func WithReason(reason string, logger *zap.Logger) *zap.Logger { + if logger == nil || reason == "" { + return logger + } + return logger.With(zap.String("event.kind", reason)) +} + +func WithErrorType(errorKind ErrorType, logger *zap.Logger) *zap.Logger { + if logger == nil || errorKind == "" { + return logger + } + return logger.With(zap.String("error.type", string(errorKind))) +} + +func WithContext(ctx context.Context, logger *zap.Logger) *zap.Logger { + if ctx == nil { + return logger + } + + spanCtx := trace.SpanFromContext(ctx).SpanContext() + if spanCtx.HasTraceID() { + logger = logger.With(zap.String("trace_id", spanCtx.TraceID().String())) + } + + if spanCtx.HasSpanID() { + logger = logger.With(zap.String("span_id", spanCtx.SpanID().String())) + } + + if ctx.Value("user_id") != nil { + logger = logger.With(zap.Any("user_id", ctx.Value("user_id"))) + } + + if ctx.Value("username") != nil { + logger = logger.With(zap.Any("username", ctx.Value("username"))) + } + + if ctx.Value("name") != nil { + logger = logger.With(zap.Any("display-name", ctx.Value("name"))) + } + + return logger +} From 09ec6ee990ba6103278ed1a806b30f2016fd81a8 Mon Sep 17 00:00:00 2001 From: Yorukot Date: Fri, 15 May 2026 00:06:42 +0800 Subject: [PATCH 02/11] feat: mark database warp as deprecated --- pkg/database/errors.go | 2 ++ pkg/database/errors_mssql.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/pkg/database/errors.go b/pkg/database/errors.go index 69e32ad..804d3cc 100644 --- a/pkg/database/errors.go +++ b/pkg/database/errors.go @@ -31,6 +31,7 @@ func (e InternalServerError) Error() string { return fmt.Sprintf("internal server error: %s", e.Source.Error()) } +// Deprecated: database errors should no longer be wrapped. Return domain errors directly and attach log fields through pkg/log instead. func WrapDBError(err error, logger *zap.Logger, operation string) error { if err == nil { return nil @@ -70,6 +71,7 @@ func WrapDBError(err error, logger *zap.Logger, operation string) error { return wrappedErr } +// Deprecated: database errors should no longer be wrapped. Return domain errors directly and attach log fields through pkg/log instead. func WrapDBErrorWithKeyValue(err error, table, key, value string, logger *zap.Logger, operation string) error { if err == nil { return nil diff --git a/pkg/database/errors_mssql.go b/pkg/database/errors_mssql.go index 0e92a4d..f9bcebf 100644 --- a/pkg/database/errors_mssql.go +++ b/pkg/database/errors_mssql.go @@ -19,6 +19,7 @@ const ( MSSQLErrDeadlockDetected = 1205 // Deadlock detected ) +// Deprecated: database errors should no longer be wrapped. Return domain errors directly and attach log fields through pkg/log instead. func WrapMSSQLError(err error, logger *zap.Logger, operation string) error { if err == nil { return nil @@ -58,6 +59,7 @@ func WrapMSSQLError(err error, logger *zap.Logger, operation string) error { return wrappedErr } +// Deprecated: database errors should no longer be wrapped. Return domain errors directly and attach log fields through pkg/log instead. func WrapMSSQLErrorWithKeyValue(err error, table, key, value string, logger *zap.Logger, operation string) error { if err == nil { return nil From e046ff2e8e6395f3827a323dc1f276b858679718 Mon Sep 17 00:00:00 2001 From: Yorukot Date: Fri, 15 May 2026 00:23:49 +0800 Subject: [PATCH 03/11] feat: finish a new version of logutil and update pkg docs --- pkg/log/constant.go | 93 ++++++++++++++--- pkg/log/doc.go | 18 ++++ pkg/log/error_warp.go | 206 ++++++++++++++++++++++++++++++++++++- pkg/log/logger.go | 227 +++++++++++++++++++++++++++++++++-------- pkg/log/with.go | 229 ++++++++++++++++++++++++++++++++++++------ 5 files changed, 682 insertions(+), 91 deletions(-) create mode 100644 pkg/log/doc.go diff --git a/pkg/log/constant.go b/pkg/log/constant.go index 9ed27d4..af0f796 100644 --- a/pkg/log/constant.go +++ b/pkg/log/constant.go @@ -1,23 +1,84 @@ package logutil +// ErrorType is a stable machine-readable error classification for logs. +// +// The values mirror canonical status-like categories so services can group +// unrelated Go error types under the same operational meaning. type ErrorType string const ( - OK ErrorType = "OK" - CANCELLED ErrorType = "CANCELLED" - UNKNOWN ErrorType = "UNKNOWN" - INVALID_ARGUMENT ErrorType = "INVALID_ARGUMENT" - DEADLINE_EXCEEDED ErrorType = "DEADLINE_EXCEEDED" - NOT_FOUND ErrorType = "NOT_FOUND" - ALREADY_EXISTS ErrorType = "ALREADY_EXISTS" - PERMISSION_DENIED ErrorType = "PERMISSION_DENIED" - UNAUTHENTICATED ErrorType = "UNAUTHENTICATED" - RESOURCE_EXHAUSTED ErrorType = "RESOURCE_EXHAUSTED" + // OK indicates that no error occurred. + OK ErrorType = "OK" + // CANCELLED indicates that an operation was cancelled before completion. + CANCELLED ErrorType = "CANCELLED" + // UNKNOWN indicates that the error category could not be determined. + UNKNOWN ErrorType = "UNKNOWN" + // INVALID_ARGUMENT indicates that the caller supplied invalid input. + INVALID_ARGUMENT ErrorType = "INVALID_ARGUMENT" + // DEADLINE_EXCEEDED indicates that an operation exceeded its time limit. + DEADLINE_EXCEEDED ErrorType = "DEADLINE_EXCEEDED" + // NOT_FOUND indicates that a requested resource does not exist. + NOT_FOUND ErrorType = "NOT_FOUND" + // ALREADY_EXISTS indicates that a resource already exists. + ALREADY_EXISTS ErrorType = "ALREADY_EXISTS" + // PERMISSION_DENIED indicates that the caller lacks permission. + PERMISSION_DENIED ErrorType = "PERMISSION_DENIED" + // UNAUTHENTICATED indicates that authentication is required or invalid. + UNAUTHENTICATED ErrorType = "UNAUTHENTICATED" + // RESOURCE_EXHAUSTED indicates that quota, capacity, or another limit was exhausted. + RESOURCE_EXHAUSTED ErrorType = "RESOURCE_EXHAUSTED" + // FAILED_PRECONDITION indicates that the system state does not allow the operation. FAILED_PRECONDITION ErrorType = "FAILED_PRECONDITION" - ABORTED ErrorType = "ABORTED" - OUT_OF_RANGE ErrorType = "OUT_OF_RANGE" - UNIMPLEMENTED ErrorType = "UNIMPLEMENTED" - INTERNAL ErrorType = "INTERNAL" - UNAVAILABLE ErrorType = "UNAVAILABLE" - DATA_LOSS ErrorType = "DATA_LOSS" + // ABORTED indicates that an operation was aborted, often due to a conflict. + ABORTED ErrorType = "ABORTED" + // OUT_OF_RANGE indicates that an input is outside the allowed range. + OUT_OF_RANGE ErrorType = "OUT_OF_RANGE" + // UNIMPLEMENTED indicates that the requested operation is not implemented. + UNIMPLEMENTED ErrorType = "UNIMPLEMENTED" + // INTERNAL indicates an unexpected server-side failure. + INTERNAL ErrorType = "INTERNAL" + // UNAVAILABLE indicates that a dependency or service is temporarily unavailable. + UNAVAILABLE ErrorType = "UNAVAILABLE" + // DATA_LOSS indicates unrecoverable data corruption or loss. + DATA_LOSS ErrorType = "DATA_LOSS" +) + +// EventOutcome describes the result of a logged event. +// +// Use it with WithEventOutcome when the event result fits one of the standard +// outcome values. Use WithOutcome only when a custom outcome string is needed. +type EventOutcome string + +const ( + // EventOutcomeSuccess indicates that an event completed successfully. + EventOutcomeSuccess EventOutcome = "success" + // EventOutcomeFailure indicates that an event failed. + EventOutcomeFailure EventOutcome = "failure" + // EventOutcomeCancelled indicates that an event was cancelled. + EventOutcomeCancelled EventOutcome = "cancelled" + // EventOutcomeTimeout indicates that an event timed out. + EventOutcomeTimeout EventOutcome = "timeout" + // EventOutcomeUnknown indicates that the event result is not known. + EventOutcomeUnknown EventOutcome = "unknown" +) + +// ErrorInfoKey defines common keys for structured metadata carried by errors. +// +// These keys are intended for InfoError maps and become error.info. fields +// when emitted by ErrorFields or ErrorFieldsWithStacktrace. +type ErrorInfoKey string + +const ( + // ErrorInfoReason describes why an error occurred or a branch was taken. + ErrorInfoReason ErrorInfoKey = "reason" + // ErrorInfoOperation names the operation that produced the error. + ErrorInfoOperation ErrorInfoKey = "operation" + // ErrorInfoRetryable records whether retrying the operation may succeed. + ErrorInfoRetryable ErrorInfoKey = "retryable" + // ErrorInfoField names the input or domain field related to the error. + ErrorInfoField ErrorInfoKey = "field" + // ErrorInfoUserID records the user ID related to the error. + ErrorInfoUserID ErrorInfoKey = "user.id" + // ErrorInfoRequestID records the request ID related to the error. + ErrorInfoRequestID ErrorInfoKey = "request.id" ) diff --git a/pkg/log/doc.go b/pkg/log/doc.go new file mode 100644 index 0000000..b17e46d --- /dev/null +++ b/pkg/log/doc.go @@ -0,0 +1,18 @@ +// Package logutil provides the project's structured logging helpers on top of zap. +// +// The package intentionally supports two usage paths. +// +// Context-first logging stores request-scoped fields in context.Context with +// helpers such as WithFields, WithUserID, and WithRequestID. The level helpers +// such as Info and Error then call Constructs to inject those fields, active +// OpenTelemetry trace fields, user fields, request fields, and source code +// location fields into the log entry. +// +// Logger-first logging decorates an existing *zap.Logger directly with helpers +// such as WithEventName, WithEventOutcome, WithEventAction, and WithErrorType. +// Use this path when a logger already represents a specific event, domain +// operation, or error handling branch. +// +// Error helpers in this package are for extracting structured log fields from +// errors. They should not be used as a general domain error wrapping policy. +package logutil diff --git a/pkg/log/error_warp.go b/pkg/log/error_warp.go index dbfa38d..41cdc7b 100644 --- a/pkg/log/error_warp.go +++ b/pkg/log/error_warp.go @@ -1,29 +1,73 @@ package logutil -import "errors" +import ( + "errors" + "reflect" + "runtime/debug" + "go.uber.org/zap" +) + +// InfoCarrier marks an error as carrying structured log metadata. +// +// ErrorFields and InfoFieldsFromError use errors.As to find this interface in +// an error chain and emit each returned key/value pair as log fields. type InfoCarrier interface { LogInfo() map[string]any } +// ErrorTypeCarrier marks an error as carrying a stable error.type value. +// +// ErrorFields uses errors.As to find this interface in an error chain and emit +// the returned value as error.type. +type ErrorTypeCarrier interface { + LogErrorType() ErrorType +} + +// InfoError is an error wrapper that carries structured logging metadata. +// +// It preserves Base for errors.Is/errors.As through Unwrap, optionally carries +// a canonical ErrorType, and exposes Info as string-keyed log metadata through +// LogInfo. Use it only when an error must carry observability metadata across an +// API boundary; do not use it as a general domain error wrapping policy. type InfoError[K ~string, V any] struct { Base error + Type ErrorType Info map[K]V } +// Error returns the base error message, falling back to Type when Base is nil. func (e *InfoError[K, V]) Error() string { + if e == nil { + return "" + } + if e.Base != nil { return e.Base.Error() } + + if e.Type != "" { + return string(e.Type) + } + return "" } +// Unwrap returns the wrapped base error for errors.Is and errors.As. func (e *InfoError[K, V]) Unwrap() error { + if e == nil { + return nil + } + return e.Base } +// LogInfo returns Info with string keys for structured logging. +// +// A new map is allocated so callers cannot mutate the original Info map through +// the returned value. func (e *InfoError[K, V]) LogInfo() map[string]any { - if len(e.Info) == 0 { + if e == nil || len(e.Info) == 0 { return nil } @@ -35,6 +79,20 @@ func (e *InfoError[K, V]) LogInfo() map[string]any { return out } +// LogErrorType returns the canonical error type associated with the error. +func (e *InfoError[K, V]) LogErrorType() ErrorType { + if e == nil { + return "" + } + + return e.Type +} + +// NewInfoError returns an error carrying structured logging metadata. +// +// The returned error wraps base and exposes info through InfoCarrier. This +// constructor does not set error.type; use NewTypedInfoError when a stable error +// category is available. func NewInfoError[K ~string, V any](base error, info map[K]V) error { return &InfoError[K, V]{ Base: base, @@ -42,6 +100,10 @@ func NewInfoError[K ~string, V any](base error, info map[K]V) error { } } +// WrapInfoError returns an error carrying structured logging metadata. +// +// It is a compatibility alias for NewInfoError. Prefer NewInfoError for new code +// when the intent is to create an error that carries log metadata. func WrapInfoError[K ~string, V any](base error, info map[K]V) error { return &InfoError[K, V]{ Base: base, @@ -49,11 +111,149 @@ func WrapInfoError[K ~string, V any](base error, info map[K]V) error { } } +// NewTypedInfoError returns an error carrying error.type and structured metadata. +// +// The returned error wraps base, exposes errorType through ErrorTypeCarrier, and +// exposes info through InfoCarrier. +func NewTypedInfoError[K ~string, V any](errorType ErrorType, base error, info map[K]V) error { + return &InfoError[K, V]{ + Base: base, + Type: errorType, + Info: info, + } +} + +// WrapTypedInfoError returns an error carrying error.type and structured metadata. +// +// It is a compatibility alias for NewTypedInfoError. Prefer NewTypedInfoError +// for new code when the intent is to create an error that carries log metadata. +func WrapTypedInfoError[K ~string, V any](errorType ErrorType, base error, info map[K]V) error { + return &InfoError[K, V]{ + Base: base, + Type: errorType, + Info: info, + } +} + +// InfoFromError extracts structured logging metadata from err. +// +// It returns nil when err is nil or no InfoCarrier is found in the error chain. func InfoFromError(err error) map[string]any { + if err == nil { + return nil + } + var carrier InfoCarrier if errors.As(err, &carrier) { return carrier.LogInfo() } return nil -} \ No newline at end of file +} + +// ErrorTypeFromError extracts a stable error type from err. +// +// It returns an empty ErrorType when err is nil or no ErrorTypeCarrier is found +// in the error chain. +func ErrorTypeFromError(err error) ErrorType { + if err == nil { + return "" + } + + var carrier ErrorTypeCarrier + if errors.As(err, &carrier) { + return carrier.LogErrorType() + } + + return "" +} + +// ErrorFields converts err into standard zap fields for structured error logs. +// +// The returned fields include zap.Error, error.message, exception.message, +// exception.type, optional error.type, and optional error.info.* fields. +// Stacktrace is intentionally excluded; use ErrorFieldsWithStacktrace when the +// call site needs a stacktrace. +func ErrorFields(err error) []zap.Field { + if err == nil { + return nil + } + + fields := []zap.Field{ + zap.Error(err), + zap.String("error.message", err.Error()), + zap.String("exception.message", err.Error()), + zap.String("exception.type", errorGoType(err)), + } + + if errorType := ErrorTypeFromError(err); errorType != "" { + fields = append(fields, zap.String("error.type", string(errorType))) + } + + fields = append(fields, InfoFieldsFromError("error.info", err)...) + + return fields +} + +// ErrorFieldsWithStacktrace converts err into zap fields including a stacktrace. +// +// It includes all fields from ErrorFields and adds exception.stacktrace captured +// at the point this function is called. +func ErrorFieldsWithStacktrace(err error) []zap.Field { + if err == nil { + return nil + } + + fields := ErrorFields(err) + fields = append(fields, zap.String("exception.stacktrace", string(debug.Stack()))) + + return fields +} + +// InfoFieldsFromError converts InfoCarrier metadata into namespaced zap fields. +// +// If prefix is empty, fields are emitted under error.info. For example, an info +// key "operation" with the default prefix becomes error.info.operation. +func InfoFieldsFromError(prefix string, err error) []zap.Field { + info := InfoFromError(err) + if len(info) == 0 { + return nil + } + + // TODO: should we keep the prefix here? + if prefix == "" { + prefix = "error.info" + } + + fields := make([]zap.Field, 0, len(info)) + for key, value := range info { + if key == "" { + continue + } + + fields = append(fields, zap.Any(prefix+"."+key, value)) + } + + return fields +} + +func errorGoType(err error) string { + if err == nil { + return "" + } + + t := reflect.TypeOf(err) + if t == nil { + return "" + } + + if t.Kind() == reflect.Ptr { + t = t.Elem() + } + + if t.PkgPath() == "" { + return t.Name() + } + + return t.PkgPath() + "." + t.Name() +} diff --git a/pkg/log/logger.go b/pkg/log/logger.go index d7da16a..e89d88d 100644 --- a/pkg/log/logger.go +++ b/pkg/log/logger.go @@ -12,7 +12,12 @@ import ( "go.uber.org/zap/zapcore" ) -// ZapProductionConfig returns a zap.Config same as zap.NewProduction() but without sampling +// ZapProductionConfig returns the default production zap config for the project. +// +// It is equivalent to zap.NewProductionConfig with sampling disabled, JSON +// encoding, stdout as both the normal and error output, Info as the default +// level, and stack traces disabled. Use this config for services where logs are +// consumed by a collector or log aggregation system. func ZapProductionConfig() zap.Config { return zap.Config{ Level: zap.NewAtomicLevelAt(zap.InfoLevel), @@ -25,7 +30,11 @@ func ZapProductionConfig() zap.Config { } } -// ZapDevelopmentConfig returns a zap.Config same as zap.NewProduction() but with more pretty output +// ZapDevelopmentConfig returns a development zap config optimized for local use. +// +// It writes colorized console logs, enables Debug level logs, writes internal +// zap errors to stderr, and formats caller paths relative to the current working +// directory so IDE consoles can open the referenced source location. func ZapDevelopmentConfig() zap.Config { rootDir, _ := os.Getwd() @@ -45,65 +54,143 @@ func ZapDevelopmentConfig() zap.Config { return config } -// prettyEncodeCaller add padding to the caller string -func prettyEncodeCaller(caller zapcore.EntryCaller, enc zapcore.PrimitiveArrayEncoder) { - const fixedWidth = 25 - callerStr := caller.TrimmedPath() - if len(callerStr) < fixedWidth { - callerStr += strings.Repeat(" ", fixedWidth-len(callerStr)) +// Constructs returns a logger enriched with fields carried by ctx. +// +// The returned logger includes fields from WithFields, active OpenTelemetry +// trace/span metadata, and user/request metadata recorded through the context +// helper functions in with.go. A nil logger is treated as zap.NewNop so callers +// can use this function safely in optional logging paths. +// +// Constructs does not add caller code fields. The level helpers in this file add +// those fields at the actual log call site. +func Constructs(ctx context.Context, logger *zap.Logger) *zap.Logger { + if logger == nil { + logger = zap.NewNop() } - callerStr += "\t" - enc.AppendString(callerStr) -} - -// relativePrettyCallerEncoder returns a zapcore.CallerEncoder that formats the caller path relative to the root directory -// it enables clickable links in the GoLand console output -func relativePrettyCallerEncoder(rootDir string) zapcore.CallerEncoder { - const fixedWidth = 40 - - return func(caller zapcore.EntryCaller, enc zapcore.PrimitiveArrayEncoder) { - relPath, err := filepath.Rel(rootDir, caller.File) - callerStr := "" - - if err == nil && !strings.HasPrefix(relPath, "..") && !filepath.IsAbs(relPath) { - callerStr = fmt.Sprintf("%s:%d", relPath, caller.Line) - } else { - parts := strings.Split(caller.File, string(filepath.Separator)) - - lastN := 3 - if len(parts) > lastN { - parts = parts[len(parts)-lastN:] - } - callerStr = fmt.Sprintf("external/%s:%d", filepath.Join(parts...), caller.Line) - } - if len(callerStr) < fixedWidth { - callerStr += strings.Repeat(" ", fixedWidth-len(callerStr)) - } - callerStr += "\t" - enc.AppendString(callerStr) + if ctx == nil { + return logger } -} -// Constructs is a convenience function that constructs a new logger with fields extracted from the context -func Constructs(ctx context.Context, logger *zap.Logger) *zap.Logger { if fields, ok := ctx.Value(contextFieldsKey{}).(contextFields); ok && len(fields) > 0 { zapFields := make([]zap.Field, 0, len(fields)) for _, field := range fields { zapFields = append(zapFields, field) } + logger = logger.With(zapFields...) } - logger = logger.With(codeFields(1)...) + logger = WithTraceContext(ctx, logger) + logger = WithUserContext(ctx, logger) return logger } +// Debug logs msg at Debug level with fields from ctx and the call site. +// +// Use Debug for diagnostic details that are useful during development or +// investigation but too noisy for normal production operation. +func Debug(ctx context.Context, logger *zap.Logger, msg string, fields ...zap.Field) { + logger = Constructs(ctx, logger) + fields = append(fields, codeFields(1)...) + logger.Debug(msg, fields...) +} + +// Info logs msg at Info level with fields from ctx and the call site. +// +// Use Info for successful lifecycle events and expected state transitions that +// operators may need to understand normal system behavior. +func Info(ctx context.Context, logger *zap.Logger, msg string, fields ...zap.Field) { + logger = Constructs(ctx, logger) + fields = append(fields, codeFields(1)...) + logger.Info(msg, fields...) +} + +// Warn logs msg at Warn level with fields from ctx and the call site. +// +// Use Warn for recoverable failures, degraded behavior, retries, or rejected +// inputs that do not represent a server-side fault. +func Warn(ctx context.Context, logger *zap.Logger, msg string, fields ...zap.Field) { + logger = Constructs(ctx, logger) + fields = append(fields, codeFields(1)...) + logger.Warn(msg, fields...) +} + +// Error logs msg at Error level with fields from ctx, err, and the call site. +// +// When err is non-nil, Error adds zap.Error, error.message, +// exception.message, exception.type, exception.stacktrace, any error.type from +// ErrorTypeCarrier, and any structured error.info fields from InfoCarrier. +func Error(ctx context.Context, logger *zap.Logger, msg string, err error, fields ...zap.Field) { + logger = Constructs(ctx, logger) + + if err != nil { + fields = append(fields, ErrorFieldsWithStacktrace(err)...) + } + + fields = append(fields, codeFields(1)...) + + logger.Error(msg, fields...) +} + +// DPanic logs msg at DPanic level with fields from ctx, err, and the call site. +// +// In zap development mode this panics after writing the log entry. In production +// mode it logs as an error. Use this for impossible states that should fail fast +// in development but should not necessarily terminate production services. +func DPanic(ctx context.Context, logger *zap.Logger, msg string, err error, fields ...zap.Field) { + logger = Constructs(ctx, logger) + + if err != nil { + fields = append(fields, ErrorFieldsWithStacktrace(err)...) + } + + fields = append(fields, codeFields(1)...) + + logger.DPanic(msg, fields...) +} + +// Panic logs msg at Panic level with fields from ctx, err, and the call site. +// +// It writes the log entry and then panics. Use this only when the caller is +// intentionally aborting the current control flow. +func Panic(ctx context.Context, logger *zap.Logger, msg string, err error, fields ...zap.Field) { + logger = Constructs(ctx, logger) + + if err != nil { + fields = append(fields, ErrorFieldsWithStacktrace(err)...) + } + + fields = append(fields, codeFields(1)...) + + logger.Panic(msg, fields...) +} + +// Fatal logs msg at Fatal level with fields from ctx, err, and the call site. +// +// It writes the log entry and then terminates the process through zap's fatal +// behavior. Use this only for unrecoverable process startup or runtime failures. +func Fatal(ctx context.Context, logger *zap.Logger, msg string, err error, fields ...zap.Field) { + logger = Constructs(ctx, logger) + + if err != nil { + fields = append(fields, ErrorFieldsWithStacktrace(err)...) + } + + fields = append(fields, codeFields(1)...) + + logger.Fatal(msg, fields...) +} + +// codeFields returns source location fields for the caller. +// +// The fields follow OpenTelemetry-style names where possible: +// code.file.path, code.file.name, code.line.number, code.function.name, and +// code.namespace. func codeFields(skip int) []zap.Field { pcs := make([]uintptr, 1) - // Skip runtime.Callers 和 codeFields 自己 n := runtime.Callers(skip+2, pcs) if n == 0 { return nil @@ -115,13 +202,15 @@ func codeFields(skip int) []zap.Field { namespace, function := splitFunction(frame.Function) return []zap.Field{ - zap.String("code.file.path", frame.File), + zap.String("code.file.path", trimWorkdir(frame.File)), + zap.String("code.file.name", filepath.Base(frame.File)), zap.Int("code.line.number", frame.Line), zap.String("code.function.name", function), zap.String("code.namespace", namespace), } } +// splitFunction splits a fully qualified Go function name into namespace and name. func splitFunction(full string) (namespace string, function string) { i := strings.LastIndex(full, ".") if i == -1 { @@ -130,3 +219,55 @@ func splitFunction(full string) (namespace string, function string) { return full[:i], full[i+1:] } + +// trimWorkdir returns path relative to the current working directory when possible. +func trimWorkdir(path string) string { + wd, err := os.Getwd() + if err != nil || wd == "" { + return path + } + + rel, err := filepath.Rel(wd, path) + if err != nil { + return path + } + + if strings.HasPrefix(rel, "..") || filepath.IsAbs(rel) { + return path + } + + return rel +} + +// relativePrettyCallerEncoder formats zap caller paths for local development. +// +// Paths inside rootDir are rendered as relative file:line strings. External +// paths keep only the last few path components and are prefixed with external/. +func relativePrettyCallerEncoder(rootDir string) zapcore.CallerEncoder { + const fixedWidth = 40 + + return func(caller zapcore.EntryCaller, enc zapcore.PrimitiveArrayEncoder) { + relPath, err := filepath.Rel(rootDir, caller.File) + callerStr := "" + + if err == nil && !strings.HasPrefix(relPath, "..") && !filepath.IsAbs(relPath) { + callerStr = fmt.Sprintf("%s:%d", relPath, caller.Line) + } else { + parts := strings.Split(caller.File, string(filepath.Separator)) + + const lastN = 3 + if len(parts) > lastN { + parts = parts[len(parts)-lastN:] + } + + callerStr = fmt.Sprintf("external/%s:%d", filepath.Join(parts...), caller.Line) + } + + if len(callerStr) < fixedWidth { + callerStr += strings.Repeat(" ", fixedWidth-len(callerStr)) + } + + callerStr += "\t" + enc.AppendString(callerStr) + } +} diff --git a/pkg/log/with.go b/pkg/log/with.go index 96f8716..d7bdd83 100644 --- a/pkg/log/with.go +++ b/pkg/log/with.go @@ -11,8 +11,16 @@ type contextFieldsKey struct{} type contextFields map[string]zap.Field -// WithFields returns a child context enriched with request-scoped logging fields. -// Fields with the same key replace earlier fields to avoid duplicate log keys. +type userIDKey struct{} +type usernameKey struct{} +type displayNameKey struct{} +type requestIDKey struct{} + +// WithFields returns a child context enriched with structured logging fields. +// +// The fields are later injected into a logger by Constructs or by the level +// helpers in logger.go. Fields with an empty key are ignored. Fields with the +// same key replace earlier values so a log entry does not emit duplicate keys. func WithFields(ctx context.Context, fields ...zap.Field) context.Context { if ctx == nil { ctx = context.Background() @@ -26,65 +34,228 @@ func WithFields(ctx context.Context, fields ...zap.Field) context.Context { } for _, field := range fields { + if field.Key == "" { + continue + } + next[field.Key] = field } return context.WithValue(ctx, contextFieldsKey{}, next) } -func WithOutcome(outcome string, logger *zap.Logger) *zap.Logger { - if logger == nil || outcome == "" { - return logger +// WithUserID returns a child context carrying the authenticated user's ID. +// +// Constructs emits this value as enduser.id. Empty user IDs are ignored. +func WithUserID(ctx context.Context, userID string) context.Context { + if ctx == nil { + ctx = context.Background() } - return logger.With(zap.String("event.outcome", outcome)) + + if userID == "" { + return ctx + } + + return context.WithValue(ctx, userIDKey{}, userID) } -func WithEventName(actionName string, logger *zap.Logger) *zap.Logger { - if logger == nil || actionName == "" { +// WithUsername returns a child context carrying the authenticated username. +// +// Constructs emits this value as enduser.username. Empty usernames are ignored. +func WithUsername(ctx context.Context, username string) context.Context { + if ctx == nil { + ctx = context.Background() + } + + if username == "" { + return ctx + } + + return context.WithValue(ctx, usernameKey{}, username) +} + +// WithDisplayName returns a child context carrying the user's display name. +// +// Constructs emits this value as enduser.name. Empty names are ignored. +func WithDisplayName(ctx context.Context, name string) context.Context { + if ctx == nil { + ctx = context.Background() + } + + if name == "" { + return ctx + } + + return context.WithValue(ctx, displayNameKey{}, name) +} + +// WithRequestID returns a child context carrying an application request ID. +// +// Constructs emits this value as request.id. Empty request IDs are ignored. +func WithRequestID(ctx context.Context, requestID string) context.Context { + if ctx == nil { + ctx = context.Background() + } + + if requestID == "" { + return ctx + } + + return context.WithValue(ctx, requestIDKey{}, requestID) +} + +// WithTraceContext returns logger enriched with the active span context. +// +// When ctx contains a valid OpenTelemetry span, the returned logger includes +// trace_id, span_id, trace_flags, trace_sampled, and trace_state when present. +// A nil logger is treated as zap.NewNop. +func WithTraceContext(ctx context.Context, logger *zap.Logger) *zap.Logger { + if logger == nil { + logger = zap.NewNop() + } + + if ctx == nil { return logger } - return logger.With(zap.String("event.name", actionName)) + + spanCtx := trace.SpanFromContext(ctx).SpanContext() + if !spanCtx.IsValid() { + return logger + } + + fields := []zap.Field{ + zap.String("trace_id", spanCtx.TraceID().String()), + zap.String("span_id", spanCtx.SpanID().String()), + zap.String("trace_flags", spanCtx.TraceFlags().String()), + zap.Bool("trace_sampled", spanCtx.IsSampled()), + } + + if traceState := spanCtx.TraceState().String(); traceState != "" { + fields = append(fields, zap.String("trace_state", traceState)) + } + + return logger.With(fields...) } -func WithReason(reason string, logger *zap.Logger) *zap.Logger { - if logger == nil || reason == "" { +// WithUserContext returns logger enriched with user and request fields from ctx. +// +// It reads values written by WithUserID, WithUsername, WithDisplayName, and +// WithRequestID. If no values are present, logger is returned unchanged. A nil +// logger is treated as zap.NewNop. +func WithUserContext(ctx context.Context, logger *zap.Logger) *zap.Logger { + if logger == nil { + logger = zap.NewNop() + } + + if ctx == nil { + return logger + } + + fields := make([]zap.Field, 0, 4) + + if userID, ok := ctx.Value(userIDKey{}).(string); ok && userID != "" { + fields = append(fields, zap.String("enduser.id", userID)) + } + + if username, ok := ctx.Value(usernameKey{}).(string); ok && username != "" { + fields = append(fields, zap.String("enduser.username", username)) + } + + if name, ok := ctx.Value(displayNameKey{}).(string); ok && name != "" { + fields = append(fields, zap.String("enduser.name", name)) + } + + if requestID, ok := ctx.Value(requestIDKey{}).(string); ok && requestID != "" { + fields = append(fields, zap.String("request.id", requestID)) + } + + if len(fields) == 0 { return logger } - return logger.With(zap.String("event.kind", reason)) + + return logger.With(fields...) } -func WithErrorType(errorKind ErrorType, logger *zap.Logger) *zap.Logger { - if logger == nil || errorKind == "" { +// WithOutcome returns logger enriched with event.outcome. +// +// This string-based helper is useful for custom outcome values. Prefer +// WithEventOutcome when the value fits the EventOutcome constants. +func WithOutcome(outcome string, logger *zap.Logger) *zap.Logger { + if logger == nil || outcome == "" { return logger } - return logger.With(zap.String("error.type", string(errorKind))) + + return logger.With(zap.String("event.outcome", outcome)) } -func WithContext(ctx context.Context, logger *zap.Logger) *zap.Logger { - if ctx == nil { +// WithEventOutcome returns logger enriched with a typed event.outcome value. +// +// Event outcome describes whether an event succeeded, failed, timed out, was +// cancelled, or has an unknown result. +func WithEventOutcome(outcome EventOutcome, logger *zap.Logger) *zap.Logger { + if logger == nil || outcome == "" { return logger } - spanCtx := trace.SpanFromContext(ctx).SpanContext() - if spanCtx.HasTraceID() { - logger = logger.With(zap.String("trace_id", spanCtx.TraceID().String())) + return logger.With(zap.String("event.outcome", string(outcome))) +} + +// WithEventName returns logger enriched with event.name. +// +// Use event.name for the stable, human-readable event identity, such as +// "user.login" or "profile.update". +func WithEventName(eventName string, logger *zap.Logger) *zap.Logger { + if logger == nil || eventName == "" { + return logger } - if spanCtx.HasSpanID() { - logger = logger.With(zap.String("span_id", spanCtx.SpanID().String())) + return logger.With(zap.String("event.name", eventName)) +} + +// WithEventDomain returns logger enriched with event.domain. +// +// Use event.domain to group events by bounded context, subsystem, or product +// area, such as "auth", "database", or "billing". +func WithEventDomain(domain string, logger *zap.Logger) *zap.Logger { + if logger == nil || domain == "" { + return logger } - if ctx.Value("user_id") != nil { - logger = logger.With(zap.Any("user_id", ctx.Value("user_id"))) + return logger.With(zap.String("event.domain", domain)) +} + +// WithEventAction returns logger enriched with event.action. +// +// Use event.action for the operation performed within an event, such as +// "create", "update", "delete", "login", or "refresh". +func WithEventAction(action string, logger *zap.Logger) *zap.Logger { + if logger == nil || action == "" { + return logger } - if ctx.Value("username") != nil { - logger = logger.With(zap.Any("username", ctx.Value("username"))) + return logger.With(zap.String("event.action", action)) +} + +// WithReason returns logger enriched with event.reason. +// +// Use event.reason for the structured reason an event took a branch, failed, +// was rejected, or was skipped. +func WithReason(reason string, logger *zap.Logger) *zap.Logger { + if logger == nil || reason == "" { + return logger } - if ctx.Value("name") != nil { - logger = logger.With(zap.Any("display-name", ctx.Value("name"))) + return logger.With(zap.String("event.reason", reason)) +} + +// WithErrorType returns logger enriched with error.type. +// +// Use error.type for a stable machine-readable error classification. Prefer the +// ErrorType constants when the error maps to a canonical status-like category. +func WithErrorType(errorType ErrorType, logger *zap.Logger) *zap.Logger { + if logger == nil || errorType == "" { + return logger } - return logger + return logger.With(zap.String("error.type", string(errorType))) } From 352cfeb3f32a57a07d5906c2aefbb385ad349957 Mon Sep 17 00:00:00 2001 From: Yorukot Date: Fri, 15 May 2026 01:29:48 +0800 Subject: [PATCH 04/11] refactor: remove unused function --- pkg/log/doc.go | 6 ++-- pkg/log/error_warp.go | 24 -------------- pkg/log/example_test.go | 70 +++++++++++++++++++++++++++++++++++++++++ pkg/log/logger.go | 11 ------- 4 files changed, 74 insertions(+), 37 deletions(-) create mode 100644 pkg/log/example_test.go diff --git a/pkg/log/doc.go b/pkg/log/doc.go index b17e46d..b06a5b0 100644 --- a/pkg/log/doc.go +++ b/pkg/log/doc.go @@ -13,6 +13,8 @@ // Use this path when a logger already represents a specific event, domain // operation, or error handling branch. // -// Error helpers in this package are for extracting structured log fields from -// errors. They should not be used as a general domain error wrapping policy. +// Error helpers in this package let errors carry structured detail to the place +// where they are logged. Domain errors may implement InfoCarrier directly, while +// WrapInfoError and WrapTypedInfoError can attach detail to an existing error +// when passing it across an API boundary. package logutil diff --git a/pkg/log/error_warp.go b/pkg/log/error_warp.go index 41cdc7b..5f2ee91 100644 --- a/pkg/log/error_warp.go +++ b/pkg/log/error_warp.go @@ -2,7 +2,6 @@ package logutil import ( "errors" - "reflect" "runtime/debug" "go.uber.org/zap" @@ -182,8 +181,6 @@ func ErrorFields(err error) []zap.Field { fields := []zap.Field{ zap.Error(err), zap.String("error.message", err.Error()), - zap.String("exception.message", err.Error()), - zap.String("exception.type", errorGoType(err)), } if errorType := ErrorTypeFromError(err); errorType != "" { @@ -236,24 +233,3 @@ func InfoFieldsFromError(prefix string, err error) []zap.Field { return fields } - -func errorGoType(err error) string { - if err == nil { - return "" - } - - t := reflect.TypeOf(err) - if t == nil { - return "" - } - - if t.Kind() == reflect.Ptr { - t = t.Elem() - } - - if t.PkgPath() == "" { - return t.Name() - } - - return t.PkgPath() + "." + t.Name() -} diff --git a/pkg/log/example_test.go b/pkg/log/example_test.go new file mode 100644 index 0000000..3a2a596 --- /dev/null +++ b/pkg/log/example_test.go @@ -0,0 +1,70 @@ +package logutil_test + +import ( + "context" + "errors" + + logutil "github.com/NYCU-SDC/summer/pkg/log" + "go.uber.org/zap" +) + +type createUserError struct { + userID string + reason string +} + +func (e createUserError) Error() string { + return "create user failed" +} + +func (e createUserError) LogErrorType() logutil.ErrorType { + return logutil.INVALID_ARGUMENT +} + +func (e createUserError) LogInfo() map[string]any { + return map[string]any{ + string(logutil.ErrorInfoReason): e.reason, + string(logutil.ErrorInfoUserID): e.userID, + } +} + +func Example() { + logger := zap.NewExample() + + // Context fields are attached once and reused by every log call in this flow. + ctx := context.Background() + ctx = logutil.WithRequestID(ctx, "req-7") + ctx = logutil.WithUserID(ctx, "user-42") + ctx = logutil.WithFields(ctx, zap.String("service.name", "account-api")) + + // Logger decorators describe the event without passing the same fields again. + eventLogger := logutil.WithEventDomain("identity", logger) + eventLogger = logutil.WithEventName("user.create", eventLogger) + eventLogger = logutil.WithEventAction("create", eventLogger) + eventLogger = logutil.WithEventOutcome(logutil.EventOutcomeFailure, eventLogger) + + // Domain errors can expose log metadata directly; no extra error wrapping is needed. + err := createUserError{ + userID: "user-42", + reason: "email already exists", + } + + logutil.Error(ctx, eventLogger, "create user rejected", err, zap.String("email.domain", "example.com")) +} + +func ExampleWrapTypedInfoError() { + logger := zap.NewExample() + ctx := logutil.WithRequestID(context.Background(), "req-7") + + baseErr := errors.New("email already exists") + + // Wrap helpers are useful when you already have an error, but still need to + // pass structured detail to the logging layer. + err := logutil.WrapTypedInfoError(logutil.ALREADY_EXISTS, baseErr, map[logutil.ErrorInfoKey]any{ + logutil.ErrorInfoOperation: "create_user", + logutil.ErrorInfoField: "email", + logutil.ErrorInfoRetryable: false, + }) + + logutil.Error(ctx, logger, "create user failed", err) +} diff --git a/pkg/log/logger.go b/pkg/log/logger.go index e89d88d..fe992bb 100644 --- a/pkg/log/logger.go +++ b/pkg/log/logger.go @@ -128,9 +128,7 @@ func Error(ctx context.Context, logger *zap.Logger, msg string, err error, field if err != nil { fields = append(fields, ErrorFieldsWithStacktrace(err)...) } - fields = append(fields, codeFields(1)...) - logger.Error(msg, fields...) } @@ -141,13 +139,10 @@ func Error(ctx context.Context, logger *zap.Logger, msg string, err error, field // in development but should not necessarily terminate production services. func DPanic(ctx context.Context, logger *zap.Logger, msg string, err error, fields ...zap.Field) { logger = Constructs(ctx, logger) - if err != nil { fields = append(fields, ErrorFieldsWithStacktrace(err)...) } - fields = append(fields, codeFields(1)...) - logger.DPanic(msg, fields...) } @@ -157,13 +152,10 @@ func DPanic(ctx context.Context, logger *zap.Logger, msg string, err error, fiel // intentionally aborting the current control flow. func Panic(ctx context.Context, logger *zap.Logger, msg string, err error, fields ...zap.Field) { logger = Constructs(ctx, logger) - if err != nil { fields = append(fields, ErrorFieldsWithStacktrace(err)...) } - fields = append(fields, codeFields(1)...) - logger.Panic(msg, fields...) } @@ -173,13 +165,10 @@ func Panic(ctx context.Context, logger *zap.Logger, msg string, err error, field // behavior. Use this only for unrecoverable process startup or runtime failures. func Fatal(ctx context.Context, logger *zap.Logger, msg string, err error, fields ...zap.Field) { logger = Constructs(ctx, logger) - if err != nil { fields = append(fields, ErrorFieldsWithStacktrace(err)...) } - fields = append(fields, codeFields(1)...) - logger.Fatal(msg, fields...) } From f224834f40054d4473800a9d7f4af116f9e50e16 Mon Sep 17 00:00:00 2001 From: Yorukot Date: Fri, 15 May 2026 01:46:46 +0800 Subject: [PATCH 05/11] feat: add example --- .../simple-log/log.go | 40 ++----------------- 1 file changed, 3 insertions(+), 37 deletions(-) rename pkg/log/example_test.go => example/simple-log/log.go (55%) diff --git a/pkg/log/example_test.go b/example/simple-log/log.go similarity index 55% rename from pkg/log/example_test.go rename to example/simple-log/log.go index 3a2a596..f5b269e 100644 --- a/pkg/log/example_test.go +++ b/example/simple-log/log.go @@ -8,27 +8,7 @@ import ( "go.uber.org/zap" ) -type createUserError struct { - userID string - reason string -} - -func (e createUserError) Error() string { - return "create user failed" -} - -func (e createUserError) LogErrorType() logutil.ErrorType { - return logutil.INVALID_ARGUMENT -} - -func (e createUserError) LogInfo() map[string]any { - return map[string]any{ - string(logutil.ErrorInfoReason): e.reason, - string(logutil.ErrorInfoUserID): e.userID, - } -} - -func Example() { +func main() { logger := zap.NewExample() // Context fields are attached once and reused by every log call in this flow. @@ -43,28 +23,14 @@ func Example() { eventLogger = logutil.WithEventAction("create", eventLogger) eventLogger = logutil.WithEventOutcome(logutil.EventOutcomeFailure, eventLogger) - // Domain errors can expose log metadata directly; no extra error wrapping is needed. - err := createUserError{ - userID: "user-42", - reason: "email already exists", - } - - logutil.Error(ctx, eventLogger, "create user rejected", err, zap.String("email.domain", "example.com")) -} - -func ExampleWrapTypedInfoError() { - logger := zap.NewExample() - ctx := logutil.WithRequestID(context.Background(), "req-7") - baseErr := errors.New("email already exists") - // Wrap helpers are useful when you already have an error, but still need to - // pass structured detail to the logging layer. + // Wrap the error when it needs to carry detail to the logging layer. err := logutil.WrapTypedInfoError(logutil.ALREADY_EXISTS, baseErr, map[logutil.ErrorInfoKey]any{ logutil.ErrorInfoOperation: "create_user", logutil.ErrorInfoField: "email", logutil.ErrorInfoRetryable: false, }) - logutil.Error(ctx, logger, "create user failed", err) + logutil.Error(ctx, eventLogger, "create user rejected", err, zap.String("email.domain", "example.com")) } From a42be17ba49e0d004ca96751e0a05d921f63565f Mon Sep 17 00:00:00 2001 From: Yorukot Date: Fri, 15 May 2026 01:52:34 +0800 Subject: [PATCH 06/11] feat: add warp info error and rename example to examples --- {example => examples}/main.txt | 0 {example => examples}/simple-log/log.go | 6 +++++- 2 files changed, 5 insertions(+), 1 deletion(-) rename {example => examples}/main.txt (100%) rename {example => examples}/simple-log/log.go (92%) diff --git a/example/main.txt b/examples/main.txt similarity index 100% rename from example/main.txt rename to examples/main.txt diff --git a/example/simple-log/log.go b/examples/simple-log/log.go similarity index 92% rename from example/simple-log/log.go rename to examples/simple-log/log.go index f5b269e..f792bad 100644 --- a/example/simple-log/log.go +++ b/examples/simple-log/log.go @@ -1,4 +1,4 @@ -package logutil_test +package main import ( "context" @@ -32,5 +32,9 @@ func main() { logutil.ErrorInfoRetryable: false, }) + err = logutil.WrapInfoError(err, map[string]any{ + "test": "helloworld", + }) + logutil.Error(ctx, eventLogger, "create user rejected", err, zap.String("email.domain", "example.com")) } From b6e2e3268799fbaecea0a65605e4ea5e7d35eec8 Mon Sep 17 00:00:00 2001 From: Yorukot Date: Fri, 15 May 2026 01:54:35 +0800 Subject: [PATCH 07/11] feat: add deprecated function back --- pkg/log/deprecated.go | 47 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 pkg/log/deprecated.go diff --git a/pkg/log/deprecated.go b/pkg/log/deprecated.go new file mode 100644 index 0000000..88d54e6 --- /dev/null +++ b/pkg/log/deprecated.go @@ -0,0 +1,47 @@ +package logutil + +import ( + "context" + + "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" +) + +// WithContext returns logger enriched with legacy context fields. +// +// It reads OpenTelemetry trace/span IDs from ctx and also reads the old string +// context keys "user_id", "username", and "name". Those user fields are emitted +// as user_id, username, and display-name for compatibility with older callers. +// +// Deprecated: use Constructs for the standard context-first logging path, or use +// WithTraceContext and WithUserContext directly when decorating a logger. New +// code should write user and request values with WithUserID, WithUsername, +// WithDisplayName, and WithRequestID instead of raw string context keys. +func WithContext(ctx context.Context, logger *zap.Logger) *zap.Logger { + if ctx == nil { + return logger + } + + spanCtx := trace.SpanFromContext(ctx).SpanContext() + if spanCtx.HasTraceID() { + logger = logger.With(zap.String("trace_id", spanCtx.TraceID().String())) + } + + if spanCtx.HasSpanID() { + logger = logger.With(zap.String("span_id", spanCtx.SpanID().String())) + } + + if ctx.Value("user_id") != nil { + logger = logger.With(zap.Any("user_id", ctx.Value("user_id"))) + } + + if ctx.Value("username") != nil { + logger = logger.With(zap.Any("username", ctx.Value("username"))) + } + + if ctx.Value("name") != nil { + logger = logger.With(zap.Any("display-name", ctx.Value("name"))) + } + + return logger +} From b9dede9d6dfa5cdb49f1ec1b02b4431ed6f13fc6 Mon Sep 17 00:00:00 2001 From: Yorukot Date: Wed, 20 May 2026 14:51:42 +0800 Subject: [PATCH 08/11] feat: update flow to more approch the best practice --- examples/simple-log/log.go | 29 ++++++------- pkg/{log => error}/error_warp.go | 66 +++++++++++++++++++++++++++- pkg/log/constant.go | 64 --------------------------- pkg/log/doc.go | 20 ++++----- pkg/log/flow.go | 30 +++++++++++++ pkg/log/flow_test.go | 74 ++++++++++++++++++++++++++++++++ pkg/log/logger.go | 12 +++--- pkg/log/with.go | 30 ++----------- 8 files changed, 203 insertions(+), 122 deletions(-) rename pkg/{log => error}/error_warp.go (66%) create mode 100644 pkg/log/flow.go create mode 100644 pkg/log/flow_test.go diff --git a/examples/simple-log/log.go b/examples/simple-log/log.go index f792bad..3a5be3b 100644 --- a/examples/simple-log/log.go +++ b/examples/simple-log/log.go @@ -4,6 +4,7 @@ import ( "context" "errors" + errutil "github.com/NYCU-SDC/summer/pkg/error" logutil "github.com/NYCU-SDC/summer/pkg/log" "go.uber.org/zap" ) @@ -11,28 +12,26 @@ import ( func main() { logger := zap.NewExample() - // Context fields are attached once and reused by every log call in this flow. - ctx := context.Background() - ctx = logutil.WithRequestID(ctx, "req-7") - ctx = logutil.WithUserID(ctx, "user-42") - ctx = logutil.WithFields(ctx, zap.String("service.name", "account-api")) - - // Logger decorators describe the event without passing the same fields again. - eventLogger := logutil.WithEventDomain("identity", logger) - eventLogger = logutil.WithEventName("user.create", eventLogger) - eventLogger = logutil.WithEventAction("create", eventLogger) + ctx, eventLogger := logutil.SetupFlow( + context.Background(), + logger, + "user.create", + zap.String("request.id", "req-7"), + zap.String("enduser.id", "user-42"), + zap.String("service.name", "account-api"), + ) eventLogger = logutil.WithEventOutcome(logutil.EventOutcomeFailure, eventLogger) baseErr := errors.New("email already exists") // Wrap the error when it needs to carry detail to the logging layer. - err := logutil.WrapTypedInfoError(logutil.ALREADY_EXISTS, baseErr, map[logutil.ErrorInfoKey]any{ - logutil.ErrorInfoOperation: "create_user", - logutil.ErrorInfoField: "email", - logutil.ErrorInfoRetryable: false, + err := errutil.WrapTypedInfoError(errutil.ALREADY_EXISTS, baseErr, map[errutil.ErrorInfoKey]any{ + errutil.ErrorInfoOperation: "create_user", + errutil.ErrorInfoField: "email", + errutil.ErrorInfoRetryable: false, }) - err = logutil.WrapInfoError(err, map[string]any{ + err = errutil.WrapInfoError(err, map[string]any{ "test": "helloworld", }) diff --git a/pkg/log/error_warp.go b/pkg/error/error_warp.go similarity index 66% rename from pkg/log/error_warp.go rename to pkg/error/error_warp.go index 5f2ee91..5703d38 100644 --- a/pkg/log/error_warp.go +++ b/pkg/error/error_warp.go @@ -1,4 +1,4 @@ -package logutil +package errutil import ( "errors" @@ -7,6 +7,70 @@ import ( "go.uber.org/zap" ) +// ErrorType is a stable machine-readable error classification for logs. +// +// The values mirror canonical status-like categories so services can group +// unrelated Go error types under the same operational meaning. +type ErrorType string + +const ( + // OK indicates that no error occurred. + OK ErrorType = "OK" + // CANCELLED indicates that an operation was cancelled before completion. + CANCELLED ErrorType = "CANCELLED" + // UNKNOWN indicates that the error category could not be determined. + UNKNOWN ErrorType = "UNKNOWN" + // INVALID_ARGUMENT indicates that the caller supplied invalid input. + INVALID_ARGUMENT ErrorType = "INVALID_ARGUMENT" + // DEADLINE_EXCEEDED indicates that an operation exceeded its time limit. + DEADLINE_EXCEEDED ErrorType = "DEADLINE_EXCEEDED" + // NOT_FOUND indicates that a requested resource does not exist. + NOT_FOUND ErrorType = "NOT_FOUND" + // ALREADY_EXISTS indicates that a resource already exists. + ALREADY_EXISTS ErrorType = "ALREADY_EXISTS" + // PERMISSION_DENIED indicates that the caller lacks permission. + PERMISSION_DENIED ErrorType = "PERMISSION_DENIED" + // UNAUTHENTICATED indicates that authentication is required or invalid. + UNAUTHENTICATED ErrorType = "UNAUTHENTICATED" + // RESOURCE_EXHAUSTED indicates that quota, capacity, or another limit was exhausted. + RESOURCE_EXHAUSTED ErrorType = "RESOURCE_EXHAUSTED" + // FAILED_PRECONDITION indicates that the system state does not allow the operation. + FAILED_PRECONDITION ErrorType = "FAILED_PRECONDITION" + // ABORTED indicates that an operation was aborted, often due to a conflict. + ABORTED ErrorType = "ABORTED" + // OUT_OF_RANGE indicates that an input is outside the allowed range. + OUT_OF_RANGE ErrorType = "OUT_OF_RANGE" + // UNIMPLEMENTED indicates that the requested operation is not implemented. + UNIMPLEMENTED ErrorType = "UNIMPLEMENTED" + // INTERNAL indicates an unexpected server-side failure. + INTERNAL ErrorType = "INTERNAL" + // UNAVAILABLE indicates that a dependency or service is temporarily unavailable. + UNAVAILABLE ErrorType = "UNAVAILABLE" + // DATA_LOSS indicates unrecoverable data corruption or loss. + DATA_LOSS ErrorType = "DATA_LOSS" +) + +// ErrorInfoKey defines common keys for structured metadata carried by errors. +// +// These keys are intended for InfoError maps and become error.info. fields +// when emitted by ErrorFields or ErrorFieldsWithStacktrace. +type ErrorInfoKey string + +const ( + // ErrorInfoReason describes why an error occurred or a branch was taken. + ErrorInfoReason ErrorInfoKey = "reason" + // ErrorInfoOperation names the operation that produced the error. + ErrorInfoOperation ErrorInfoKey = "operation" + // ErrorInfoRetryable records whether retrying the operation may succeed. + ErrorInfoRetryable ErrorInfoKey = "retryable" + // ErrorInfoField names the input or domain field related to the error. + ErrorInfoField ErrorInfoKey = "field" + // ErrorInfoUserID records the user ID related to the error. + ErrorInfoUserID ErrorInfoKey = "user.id" + // ErrorInfoRequestID records the request ID related to the error. + ErrorInfoRequestID ErrorInfoKey = "request.id" +) + // InfoCarrier marks an error as carrying structured log metadata. // // ErrorFields and InfoFieldsFromError use errors.As to find this interface in diff --git a/pkg/log/constant.go b/pkg/log/constant.go index af0f796..f7bdca1 100644 --- a/pkg/log/constant.go +++ b/pkg/log/constant.go @@ -1,48 +1,5 @@ package logutil -// ErrorType is a stable machine-readable error classification for logs. -// -// The values mirror canonical status-like categories so services can group -// unrelated Go error types under the same operational meaning. -type ErrorType string - -const ( - // OK indicates that no error occurred. - OK ErrorType = "OK" - // CANCELLED indicates that an operation was cancelled before completion. - CANCELLED ErrorType = "CANCELLED" - // UNKNOWN indicates that the error category could not be determined. - UNKNOWN ErrorType = "UNKNOWN" - // INVALID_ARGUMENT indicates that the caller supplied invalid input. - INVALID_ARGUMENT ErrorType = "INVALID_ARGUMENT" - // DEADLINE_EXCEEDED indicates that an operation exceeded its time limit. - DEADLINE_EXCEEDED ErrorType = "DEADLINE_EXCEEDED" - // NOT_FOUND indicates that a requested resource does not exist. - NOT_FOUND ErrorType = "NOT_FOUND" - // ALREADY_EXISTS indicates that a resource already exists. - ALREADY_EXISTS ErrorType = "ALREADY_EXISTS" - // PERMISSION_DENIED indicates that the caller lacks permission. - PERMISSION_DENIED ErrorType = "PERMISSION_DENIED" - // UNAUTHENTICATED indicates that authentication is required or invalid. - UNAUTHENTICATED ErrorType = "UNAUTHENTICATED" - // RESOURCE_EXHAUSTED indicates that quota, capacity, or another limit was exhausted. - RESOURCE_EXHAUSTED ErrorType = "RESOURCE_EXHAUSTED" - // FAILED_PRECONDITION indicates that the system state does not allow the operation. - FAILED_PRECONDITION ErrorType = "FAILED_PRECONDITION" - // ABORTED indicates that an operation was aborted, often due to a conflict. - ABORTED ErrorType = "ABORTED" - // OUT_OF_RANGE indicates that an input is outside the allowed range. - OUT_OF_RANGE ErrorType = "OUT_OF_RANGE" - // UNIMPLEMENTED indicates that the requested operation is not implemented. - UNIMPLEMENTED ErrorType = "UNIMPLEMENTED" - // INTERNAL indicates an unexpected server-side failure. - INTERNAL ErrorType = "INTERNAL" - // UNAVAILABLE indicates that a dependency or service is temporarily unavailable. - UNAVAILABLE ErrorType = "UNAVAILABLE" - // DATA_LOSS indicates unrecoverable data corruption or loss. - DATA_LOSS ErrorType = "DATA_LOSS" -) - // EventOutcome describes the result of a logged event. // // Use it with WithEventOutcome when the event result fits one of the standard @@ -61,24 +18,3 @@ const ( // EventOutcomeUnknown indicates that the event result is not known. EventOutcomeUnknown EventOutcome = "unknown" ) - -// ErrorInfoKey defines common keys for structured metadata carried by errors. -// -// These keys are intended for InfoError maps and become error.info. fields -// when emitted by ErrorFields or ErrorFieldsWithStacktrace. -type ErrorInfoKey string - -const ( - // ErrorInfoReason describes why an error occurred or a branch was taken. - ErrorInfoReason ErrorInfoKey = "reason" - // ErrorInfoOperation names the operation that produced the error. - ErrorInfoOperation ErrorInfoKey = "operation" - // ErrorInfoRetryable records whether retrying the operation may succeed. - ErrorInfoRetryable ErrorInfoKey = "retryable" - // ErrorInfoField names the input or domain field related to the error. - ErrorInfoField ErrorInfoKey = "field" - // ErrorInfoUserID records the user ID related to the error. - ErrorInfoUserID ErrorInfoKey = "user.id" - // ErrorInfoRequestID records the request ID related to the error. - ErrorInfoRequestID ErrorInfoKey = "request.id" -) diff --git a/pkg/log/doc.go b/pkg/log/doc.go index b06a5b0..ac6c606 100644 --- a/pkg/log/doc.go +++ b/pkg/log/doc.go @@ -3,18 +3,16 @@ // The package intentionally supports two usage paths. // // Context-first logging stores request-scoped fields in context.Context with -// helpers such as WithFields, WithUserID, and WithRequestID. The level helpers -// such as Info and Error then call Constructs to inject those fields, active -// OpenTelemetry trace fields, user fields, request fields, and source code -// location fields into the log entry. +// helpers such as SetupFlow, WithFields, WithUserID, and WithRequestID. The +// level helpers such as Info and Error then call Constructs to inject those +// fields, active OpenTelemetry trace fields, user fields, request fields, and +// source code location fields into the log entry. // // Logger-first logging decorates an existing *zap.Logger directly with helpers -// such as WithEventName, WithEventOutcome, WithEventAction, and WithErrorType. -// Use this path when a logger already represents a specific event, domain -// operation, or error handling branch. +// such as WithEventName, WithEventOutcome, WithReason, and WithErrorType. Use +// this path when a logger already represents a specific event or error handling +// branch. // -// Error helpers in this package let errors carry structured detail to the place -// where they are logged. Domain errors may implement InfoCarrier directly, while -// WrapInfoError and WrapTypedInfoError can attach detail to an existing error -// when passing it across an API boundary. +// The companion pkg/error package provides helpers for errors that carry +// structured detail to the place where they are logged. package logutil diff --git a/pkg/log/flow.go b/pkg/log/flow.go new file mode 100644 index 0000000..95e6fda --- /dev/null +++ b/pkg/log/flow.go @@ -0,0 +1,30 @@ +package logutil + +import ( + "context" + + "go.uber.org/zap" +) + +// SetupFlow initializes the context and logger fields used by a service flow. +// +// It stores request-scoped fields on ctx, decorates logger with event.name, and +// returns both values for subsequent logutil level calls. A nil ctx becomes +// context.Background, and a nil logger becomes zap.NewNop. +func SetupFlow(ctx context.Context, logger *zap.Logger, eventName string, fields ...zap.Field) (context.Context, *zap.Logger) { + if ctx == nil { + ctx = context.Background() + } + + if logger == nil { + logger = zap.NewNop() + } + + if len(fields) > 0 { + ctx = WithFields(ctx, fields...) + } + + logger = WithEventName(eventName, logger) + + return ctx, logger +} diff --git a/pkg/log/flow_test.go b/pkg/log/flow_test.go new file mode 100644 index 0000000..bc6b3f5 --- /dev/null +++ b/pkg/log/flow_test.go @@ -0,0 +1,74 @@ +package logutil + +import ( + "testing" + + errutil "github.com/NYCU-SDC/summer/pkg/error" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" +) + +func TestSetupFlowInitializesContextAndLogger(t *testing.T) { + core, logs := observer.New(zapcore.InfoLevel) + baseLogger := zap.New(core) + + ctx, logger := SetupFlow( + nil, + baseLogger, + "user.create", + zap.String("request.id", "req-7"), + zap.String("enduser.id", "user-42"), + zap.String("enduser.username", "alice"), + zap.String("enduser.name", "Alice"), + zap.String("service.name", "account-api"), + zap.String("route", "/users"), + ) + logger = WithEventOutcome(EventOutcomeFailure, logger) + logger = WithReason("duplicate_email", logger) + logger = WithErrorType(errutil.ALREADY_EXISTS, logger) + + if ctx == nil { + t.Fatal("SetupFlow returned nil context") + } + if logger == nil { + t.Fatal("SetupFlow returned nil logger") + } + + Info(ctx, logger, "flow initialized") + + if logs.Len() != 1 { + t.Fatalf("expected 1 log entry, got %d", logs.Len()) + } + + fields := logs.All()[0].ContextMap() + want := map[string]any{ + "request.id": "req-7", + "enduser.id": "user-42", + "enduser.username": "alice", + "enduser.name": "Alice", + "service.name": "account-api", + "event.name": "user.create", + "event.outcome": "failure", + "event.reason": "duplicate_email", + "error.type": "ALREADY_EXISTS", + "route": "/users", + } + + for key, value := range want { + if got := fields[key]; got != value { + t.Fatalf("field %q = %v, want %v", key, got, value) + } + } +} + +func TestSetupFlowUsesSafeDefaults(t *testing.T) { + ctx, logger := SetupFlow(nil, nil, "") + + if ctx == nil { + t.Fatal("SetupFlow returned nil context") + } + if logger == nil { + t.Fatal("SetupFlow returned nil logger") + } +} diff --git a/pkg/log/logger.go b/pkg/log/logger.go index fe992bb..f76a1bb 100644 --- a/pkg/log/logger.go +++ b/pkg/log/logger.go @@ -8,6 +8,7 @@ import ( "runtime" "strings" + errutil "github.com/NYCU-SDC/summer/pkg/error" "go.uber.org/zap" "go.uber.org/zap/zapcore" ) @@ -121,12 +122,13 @@ func Warn(ctx context.Context, logger *zap.Logger, msg string, fields ...zap.Fie // // When err is non-nil, Error adds zap.Error, error.message, // exception.message, exception.type, exception.stacktrace, any error.type from -// ErrorTypeCarrier, and any structured error.info fields from InfoCarrier. +// errutil.ErrorTypeCarrier, and any structured error.info fields from +// errutil.InfoCarrier. func Error(ctx context.Context, logger *zap.Logger, msg string, err error, fields ...zap.Field) { logger = Constructs(ctx, logger) if err != nil { - fields = append(fields, ErrorFieldsWithStacktrace(err)...) + fields = append(fields, errutil.ErrorFieldsWithStacktrace(err)...) } fields = append(fields, codeFields(1)...) logger.Error(msg, fields...) @@ -140,7 +142,7 @@ func Error(ctx context.Context, logger *zap.Logger, msg string, err error, field func DPanic(ctx context.Context, logger *zap.Logger, msg string, err error, fields ...zap.Field) { logger = Constructs(ctx, logger) if err != nil { - fields = append(fields, ErrorFieldsWithStacktrace(err)...) + fields = append(fields, errutil.ErrorFieldsWithStacktrace(err)...) } fields = append(fields, codeFields(1)...) logger.DPanic(msg, fields...) @@ -153,7 +155,7 @@ func DPanic(ctx context.Context, logger *zap.Logger, msg string, err error, fiel func Panic(ctx context.Context, logger *zap.Logger, msg string, err error, fields ...zap.Field) { logger = Constructs(ctx, logger) if err != nil { - fields = append(fields, ErrorFieldsWithStacktrace(err)...) + fields = append(fields, errutil.ErrorFieldsWithStacktrace(err)...) } fields = append(fields, codeFields(1)...) logger.Panic(msg, fields...) @@ -166,7 +168,7 @@ func Panic(ctx context.Context, logger *zap.Logger, msg string, err error, field func Fatal(ctx context.Context, logger *zap.Logger, msg string, err error, fields ...zap.Field) { logger = Constructs(ctx, logger) if err != nil { - fields = append(fields, ErrorFieldsWithStacktrace(err)...) + fields = append(fields, errutil.ErrorFieldsWithStacktrace(err)...) } fields = append(fields, codeFields(1)...) logger.Fatal(msg, fields...) diff --git a/pkg/log/with.go b/pkg/log/with.go index d7bdd83..50ad3e2 100644 --- a/pkg/log/with.go +++ b/pkg/log/with.go @@ -3,6 +3,7 @@ package logutil import ( "context" + errutil "github.com/NYCU-SDC/summer/pkg/error" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" ) @@ -212,30 +213,6 @@ func WithEventName(eventName string, logger *zap.Logger) *zap.Logger { return logger.With(zap.String("event.name", eventName)) } -// WithEventDomain returns logger enriched with event.domain. -// -// Use event.domain to group events by bounded context, subsystem, or product -// area, such as "auth", "database", or "billing". -func WithEventDomain(domain string, logger *zap.Logger) *zap.Logger { - if logger == nil || domain == "" { - return logger - } - - return logger.With(zap.String("event.domain", domain)) -} - -// WithEventAction returns logger enriched with event.action. -// -// Use event.action for the operation performed within an event, such as -// "create", "update", "delete", "login", or "refresh". -func WithEventAction(action string, logger *zap.Logger) *zap.Logger { - if logger == nil || action == "" { - return logger - } - - return logger.With(zap.String("event.action", action)) -} - // WithReason returns logger enriched with event.reason. // // Use event.reason for the structured reason an event took a branch, failed, @@ -251,8 +228,9 @@ func WithReason(reason string, logger *zap.Logger) *zap.Logger { // WithErrorType returns logger enriched with error.type. // // Use error.type for a stable machine-readable error classification. Prefer the -// ErrorType constants when the error maps to a canonical status-like category. -func WithErrorType(errorType ErrorType, logger *zap.Logger) *zap.Logger { +// errutil.ErrorType constants when the error maps to a canonical status-like +// category. +func WithErrorType(errorType errutil.ErrorType, logger *zap.Logger) *zap.Logger { if logger == nil || errorType == "" { return logger } From 8a1e76c4d6b99d76a73f01ac84ccd7c4f44cae78 Mon Sep 17 00:00:00 2001 From: Yorukot Date: Wed, 3 Jun 2026 17:20:42 +0800 Subject: [PATCH 09/11] feat(log): improve structured error logging --- examples/simple-log/log.go | 9 +++-- pkg/database/errors.go | 38 ++++++++------------ pkg/database/errors_mssql.go | 34 ++++++------------ pkg/log/doc.go | 13 ++++--- pkg/log/flow_test.go | 4 +-- pkg/log/with.go | 58 +++++++++++++++++------------- pkg/problem/problem.go | 70 ++++++++++++++++++++++++++++++++---- 7 files changed, 134 insertions(+), 92 deletions(-) diff --git a/examples/simple-log/log.go b/examples/simple-log/log.go index 3a5be3b..0e4ef68 100644 --- a/examples/simple-log/log.go +++ b/examples/simple-log/log.go @@ -22,18 +22,17 @@ func main() { ) eventLogger = logutil.WithEventOutcome(logutil.EventOutcomeFailure, eventLogger) + ctx = logutil.WithErrorType(ctx, errutil.ALREADY_EXISTS) + ctx = logutil.WithReason(ctx, "duplicate_email") + baseErr := errors.New("email already exists") // Wrap the error when it needs to carry detail to the logging layer. - err := errutil.WrapTypedInfoError(errutil.ALREADY_EXISTS, baseErr, map[errutil.ErrorInfoKey]any{ + err := errutil.WrapInfoError(baseErr, map[errutil.ErrorInfoKey]any{ errutil.ErrorInfoOperation: "create_user", errutil.ErrorInfoField: "email", errutil.ErrorInfoRetryable: false, }) - err = errutil.WrapInfoError(err, map[string]any{ - "test": "helloworld", - }) - logutil.Error(ctx, eventLogger, "create user rejected", err, zap.String("email.domain", "example.com")) } diff --git a/pkg/database/errors.go b/pkg/database/errors.go index 804d3cc..5367ced 100644 --- a/pkg/database/errors.go +++ b/pkg/database/errors.go @@ -31,82 +31,74 @@ func (e InternalServerError) Error() string { return fmt.Sprintf("internal server error: %s", e.Source.Error()) } -// Deprecated: database errors should no longer be wrapped. Return domain errors directly and attach log fields through pkg/log instead. +func (e InternalServerError) Unwrap() error { + return e.Source +} + +// Deprecated: database errors are classified here without logging. Return domain errors directly for new code. func WrapDBError(err error, logger *zap.Logger, operation string) error { if err == nil { return nil } - logger.WithOptions(zap.AddCallerSkip(1)).Error("Failed to "+operation, zap.Error(err)) - var wrappedErr error switch { case errors.Is(err, pgx.ErrNoRows): - wrappedErr = fmt.Errorf("%w: %v", errorPkg.ErrNotFound, err) + wrappedErr = fmt.Errorf("%w: %w", errorPkg.ErrNotFound, err) case errors.Is(err, context.DeadlineExceeded): - wrappedErr = fmt.Errorf("%w: %v", ErrQueryTimeout, err) + wrappedErr = fmt.Errorf("%w: %w", ErrQueryTimeout, err) default: var pgErr *pgconn.PgError if errors.As(err, &pgErr) { switch pgErr.Code { case PGErrUniqueViolation: - wrappedErr = fmt.Errorf("%w: %v", ErrUniqueViolation, err) + wrappedErr = fmt.Errorf("%w: %w", ErrUniqueViolation, err) case PGErrForeignKeyViolation: - wrappedErr = fmt.Errorf("%w: %v", ErrForeignKeyViolation, err) + wrappedErr = fmt.Errorf("%w: %w", ErrForeignKeyViolation, err) case PGErrDeadlockDetected: - wrappedErr = fmt.Errorf("%w: %v", ErrDeadlockDetected, err) + wrappedErr = fmt.Errorf("%w: %w", ErrDeadlockDetected, err) } } } - isUnknownError := false if wrappedErr == nil { wrappedErr = InternalServerError{Source: err} - isUnknownError = true } - logger.WithOptions(zap.AddCallerSkip(1)).Warn("Wrapped database error", zap.Error(wrappedErr), zap.String("operation", operation), zap.Bool("unknown_error", isUnknownError)) - return wrappedErr } -// Deprecated: database errors should no longer be wrapped. Return domain errors directly and attach log fields through pkg/log instead. +// Deprecated: database errors are classified here without logging. Return domain errors directly for new code. func WrapDBErrorWithKeyValue(err error, table, key, value string, logger *zap.Logger, operation string) error { if err == nil { return nil } - logger.WithOptions(zap.AddCallerSkip(1)).Error("Failed to "+operation, zap.Error(err)) - var wrappedErr error switch { case errors.Is(err, pgx.ErrNoRows): wrappedErr = errorPkg.NewNotFoundError(table, key, value, "") case errors.Is(err, context.DeadlineExceeded): - wrappedErr = fmt.Errorf("%w: %v", ErrQueryTimeout, err) + wrappedErr = fmt.Errorf("%w: %w", ErrQueryTimeout, err) default: var pgErr *pgconn.PgError if errors.As(err, &pgErr) { switch pgErr.Code { case PGErrUniqueViolation: - wrappedErr = fmt.Errorf("%w: %v", ErrUniqueViolation, err) + wrappedErr = fmt.Errorf("%w: %w", ErrUniqueViolation, err) case PGErrForeignKeyViolation: - wrappedErr = fmt.Errorf("%w: %v", ErrForeignKeyViolation, err) + wrappedErr = fmt.Errorf("%w: %w", ErrForeignKeyViolation, err) case PGErrDeadlockDetected: - wrappedErr = fmt.Errorf("%w: %v", ErrDeadlockDetected, err) + wrappedErr = fmt.Errorf("%w: %w", ErrDeadlockDetected, err) } } } - isUnknownError := false if wrappedErr == nil { wrappedErr = InternalServerError{Source: err} - isUnknownError = true } - logger.WithOptions(zap.AddCallerSkip(1)).Warn("Wrapped database error with key value", zap.Error(wrappedErr), zap.String("table", table), zap.String("key", key), zap.String("value", value), zap.String("operation", operation), zap.Bool("unknown_error", isUnknownError)) - return wrappedErr } diff --git a/pkg/database/errors_mssql.go b/pkg/database/errors_mssql.go index f9bcebf..443f2e8 100644 --- a/pkg/database/errors_mssql.go +++ b/pkg/database/errors_mssql.go @@ -19,82 +19,70 @@ const ( MSSQLErrDeadlockDetected = 1205 // Deadlock detected ) -// Deprecated: database errors should no longer be wrapped. Return domain errors directly and attach log fields through pkg/log instead. +// Deprecated: database errors are classified here without logging. Return domain errors directly for new code. func WrapMSSQLError(err error, logger *zap.Logger, operation string) error { if err == nil { return nil } - logger.Error("Failed to "+operation, zap.Error(err)) - var wrappedErr error switch { case errors.Is(err, sql.ErrNoRows): - wrappedErr = fmt.Errorf("%w: %v", errorPkg.ErrNotFound, err) + wrappedErr = fmt.Errorf("%w: %w", errorPkg.ErrNotFound, err) case errors.Is(err, context.DeadlineExceeded): - wrappedErr = fmt.Errorf("%w: %v", ErrQueryTimeout, err) + wrappedErr = fmt.Errorf("%w: %w", ErrQueryTimeout, err) default: var mssqlErr mssql.Error if errors.As(err, &mssqlErr) { switch mssqlErr.Number { case MSSQLErrUniqueViolation, MSSQLErrUniqueIndex: - wrappedErr = fmt.Errorf("%w: %v", ErrUniqueViolation, err) + wrappedErr = fmt.Errorf("%w: %w", ErrUniqueViolation, err) case MSSQLErrForeignKeyViolation: - wrappedErr = fmt.Errorf("%w: %v", ErrForeignKeyViolation, err) + wrappedErr = fmt.Errorf("%w: %w", ErrForeignKeyViolation, err) case MSSQLErrDeadlockDetected: - wrappedErr = fmt.Errorf("%w: %v", ErrDeadlockDetected, err) + wrappedErr = fmt.Errorf("%w: %w", ErrDeadlockDetected, err) } } } - isUnknownError := false if wrappedErr == nil { wrappedErr = InternalServerError{Source: err} - isUnknownError = true } - logger.Warn("Wrapped database error", zap.Error(wrappedErr), zap.String("operation", operation), zap.Bool("unknown_error", isUnknownError)) - return wrappedErr } -// Deprecated: database errors should no longer be wrapped. Return domain errors directly and attach log fields through pkg/log instead. +// Deprecated: database errors are classified here without logging. Return domain errors directly for new code. func WrapMSSQLErrorWithKeyValue(err error, table, key, value string, logger *zap.Logger, operation string) error { if err == nil { return nil } - logger.Error("Failed to "+operation, zap.Error(err)) - var wrappedErr error switch { case errors.Is(err, sql.ErrNoRows): wrappedErr = errorPkg.NewNotFoundError(table, key, value, "") case errors.Is(err, context.DeadlineExceeded): - wrappedErr = fmt.Errorf("%w: %v", ErrQueryTimeout, err) + wrappedErr = fmt.Errorf("%w: %w", ErrQueryTimeout, err) default: var mssqlErr mssql.Error if errors.As(err, &mssqlErr) { switch mssqlErr.Number { case MSSQLErrUniqueViolation, MSSQLErrUniqueIndex: - wrappedErr = fmt.Errorf("%w: %v", ErrUniqueViolation, err) + wrappedErr = fmt.Errorf("%w: %w", ErrUniqueViolation, err) case MSSQLErrForeignKeyViolation: - wrappedErr = fmt.Errorf("%w: %v", ErrForeignKeyViolation, err) + wrappedErr = fmt.Errorf("%w: %w", ErrForeignKeyViolation, err) case MSSQLErrDeadlockDetected: - wrappedErr = fmt.Errorf("%w: %v", ErrDeadlockDetected, err) + wrappedErr = fmt.Errorf("%w: %w", ErrDeadlockDetected, err) } } } - isUnknownError := false if wrappedErr == nil { wrappedErr = InternalServerError{Source: err} - isUnknownError = true } - logger.Warn("Wrapped database error with key value", zap.Error(wrappedErr), zap.String("table", table), zap.String("key", key), zap.String("value", value), zap.String("operation", operation), zap.Bool("unknown_error", isUnknownError)) - return wrappedErr } diff --git a/pkg/log/doc.go b/pkg/log/doc.go index ac6c606..a169adb 100644 --- a/pkg/log/doc.go +++ b/pkg/log/doc.go @@ -3,15 +3,14 @@ // The package intentionally supports two usage paths. // // Context-first logging stores request-scoped fields in context.Context with -// helpers such as SetupFlow, WithFields, WithUserID, and WithRequestID. The -// level helpers such as Info and Error then call Constructs to inject those -// fields, active OpenTelemetry trace fields, user fields, request fields, and -// source code location fields into the log entry. +// helpers such as SetupFlow, WithFields, WithUserID, WithRequestID, WithReason, +// and WithErrorType. The level helpers such as Info and Error then call +// Constructs to inject those fields, active OpenTelemetry trace fields, user +// fields, request fields, and source code location fields into the log entry. // // Logger-first logging decorates an existing *zap.Logger directly with helpers -// such as WithEventName, WithEventOutcome, WithReason, and WithErrorType. Use -// this path when a logger already represents a specific event or error handling -// branch. +// such as WithEventName and WithEventOutcome. Use this path when a logger +// already represents a specific event. // // The companion pkg/error package provides helpers for errors that carry // structured detail to the place where they are logged. diff --git a/pkg/log/flow_test.go b/pkg/log/flow_test.go index bc6b3f5..8c3fd61 100644 --- a/pkg/log/flow_test.go +++ b/pkg/log/flow_test.go @@ -24,9 +24,9 @@ func TestSetupFlowInitializesContextAndLogger(t *testing.T) { zap.String("service.name", "account-api"), zap.String("route", "/users"), ) + ctx = WithReason(ctx, "duplicate_email") + ctx = WithErrorType(ctx, errutil.ALREADY_EXISTS) logger = WithEventOutcome(EventOutcomeFailure, logger) - logger = WithReason("duplicate_email", logger) - logger = WithErrorType(errutil.ALREADY_EXISTS, logger) if ctx == nil { t.Fatal("SetupFlow returned nil context") diff --git a/pkg/log/with.go b/pkg/log/with.go index 50ad3e2..707d663 100644 --- a/pkg/log/with.go +++ b/pkg/log/with.go @@ -105,6 +105,39 @@ func WithRequestID(ctx context.Context, requestID string) context.Context { return context.WithValue(ctx, requestIDKey{}, requestID) } +// WithReason returns a child context carrying event.reason. +// +// Use event.reason for the structured reason an event took a branch, failed, +// was rejected, or was skipped. +func WithReason(ctx context.Context, reason string) context.Context { + if ctx == nil { + ctx = context.Background() + } + + if reason == "" { + return ctx + } + + return WithFields(ctx, zap.String("event.reason", reason)) +} + +// WithErrorType returns a child context carrying error.type. +// +// Use error.type for a stable machine-readable error classification. Prefer the +// errutil.ErrorType constants when the error maps to a canonical status-like +// category. +func WithErrorType(ctx context.Context, errorType errutil.ErrorType) context.Context { + if ctx == nil { + ctx = context.Background() + } + + if errorType == "" { + return ctx + } + + return WithFields(ctx, zap.String("error.type", string(errorType))) +} + // WithTraceContext returns logger enriched with the active span context. // // When ctx contains a valid OpenTelemetry span, the returned logger includes @@ -212,28 +245,3 @@ func WithEventName(eventName string, logger *zap.Logger) *zap.Logger { return logger.With(zap.String("event.name", eventName)) } - -// WithReason returns logger enriched with event.reason. -// -// Use event.reason for the structured reason an event took a branch, failed, -// was rejected, or was skipped. -func WithReason(reason string, logger *zap.Logger) *zap.Logger { - if logger == nil || reason == "" { - return logger - } - - return logger.With(zap.String("event.reason", reason)) -} - -// WithErrorType returns logger enriched with error.type. -// -// Use error.type for a stable machine-readable error classification. Prefer the -// errutil.ErrorType constants when the error maps to a canonical status-like -// category. -func WithErrorType(errorType errutil.ErrorType, logger *zap.Logger) *zap.Logger { - if logger == nil || errorType == "" { - return logger - } - - return logger.With(zap.String("error.type", string(errorType))) -} diff --git a/pkg/problem/problem.go b/pkg/problem/problem.go index 6eb1a60..c3c6f33 100644 --- a/pkg/problem/problem.go +++ b/pkg/problem/problem.go @@ -7,7 +7,9 @@ import ( "net/http" "github.com/NYCU-SDC/summer/pkg/database" + errutil "github.com/NYCU-SDC/summer/pkg/error" "github.com/NYCU-SDC/summer/pkg/handler" + logutil "github.com/NYCU-SDC/summer/pkg/log" "github.com/NYCU-SDC/summer/pkg/pagination" "github.com/go-playground/validator/v10" "go.opentelemetry.io/otel" @@ -103,23 +105,52 @@ func (h *HttpWriter) buildProblem(err error) Problem { } // writeProblemResponse writes the Problem struct as JSON to the response writer -func (h *HttpWriter) writeProblemResponse(w http.ResponseWriter, problem Problem, err error, logger *zap.Logger) { - logger = logger.WithOptions(zap.AddCallerSkip(2)) +func (h *HttpWriter) writeProblemResponse(ctx context.Context, w http.ResponseWriter, problem Problem, err error, logger *zap.Logger) { + logger = logutil.Constructs(ctx, logger).WithOptions(zap.AddCallerSkip(2)) - logger.Warn("Handling "+problem.Title, zap.String("problem", problem.Title), zap.Error(err), zap.Int("status", problem.Status), zap.String("type", problem.Type), zap.String("detail", problem.Detail)) + fields := []zap.Field{ + zap.Int("http.status_code", problem.Status), + zap.String("problem.type", problem.Type), + zap.String("problem.title", problem.Title), + zap.String("problem.detail", problem.Detail), + zap.String("error.kind", problemErrorKind(problem.Status)), + } + if problem.Instance != "" { + fields = append(fields, zap.String("problem.instance", problem.Instance)) + } + + if problem.Status >= http.StatusInternalServerError { + fields = append(fields, errutil.ErrorFieldsWithStacktrace(err)...) + logger.Error("request failed", fields...) + } else { + fields = append(fields, errutil.ErrorFields(err)...) + logger.Warn("request failed", fields...) + } w.Header().Set("Content-Type", "application/problem+json") w.WriteHeader(problem.Status) jsonBytes, marshalErr := json.Marshal(problem) if marshalErr != nil { - logger.Error("Failed to marshal problem response", zap.Error(marshalErr)) + marshalFields := []zap.Field{ + zap.Int("http.status_code", http.StatusInternalServerError), + zap.String("problem.title", "Internal Server Error"), + zap.String("error.kind", problemErrorKind(http.StatusInternalServerError)), + } + marshalFields = append(marshalFields, errutil.ErrorFields(marshalErr)...) + logger.Error("problem response marshal failed", marshalFields...) http.Error(w, marshalErr.Error(), http.StatusInternalServerError) return } _, writeErr := w.Write(jsonBytes) if writeErr != nil { - logger.Error("Failed to write problem response", zap.Error(writeErr)) + writeFields := []zap.Field{ + zap.Int("http.status_code", http.StatusInternalServerError), + zap.String("problem.title", "Internal Server Error"), + zap.String("error.kind", problemErrorKind(http.StatusInternalServerError)), + } + writeFields = append(writeFields, errutil.ErrorFields(writeErr)...) + logger.Error("problem response write failed", writeFields...) http.Error(w, writeErr.Error(), http.StatusInternalServerError) return } @@ -134,7 +165,7 @@ func (h *HttpWriter) WriteError(ctx context.Context, w http.ResponseWriter, err } problem := h.buildProblem(err) - h.writeProblemResponse(w, problem, err, logger) + h.writeProblemResponse(ctx, w, problem, err, logger) } func (h *HttpWriter) WriteErrorWithRequest(ctx context.Context, r *http.Request, w http.ResponseWriter, err error, logger *zap.Logger) { @@ -149,7 +180,32 @@ func (h *HttpWriter) WriteErrorWithRequest(ctx context.Context, r *http.Request, if r != nil && r.URL != nil { problem.Instance = r.URL.Path } - h.writeProblemResponse(w, problem, err, logger) + h.writeProblemResponse(ctx, w, problem, err, logger) +} + +func problemErrorKind(status int) string { + switch status { + case http.StatusBadRequest: + return "INVALID_ARGUMENT" + case http.StatusUnauthorized: + return "UNAUTHENTICATED" + case http.StatusForbidden: + return "PERMISSION_DENIED" + case http.StatusNotFound: + return "NOT_FOUND" + case http.StatusConflict: + return "ALREADY_EXISTS" + case http.StatusTooManyRequests: + return "RESOURCE_EXHAUSTED" + case http.StatusRequestTimeout, http.StatusGatewayTimeout: + return "DEADLINE_EXCEEDED" + } + + if status >= http.StatusInternalServerError { + return "INTERNAL" + } + + return "UNKNOWN" } func NewInternalServerProblem(detail string) Problem { From b86b0e9e86e4e5bd22b6df2902e37e7f6dda6db4 Mon Sep 17 00:00:00 2001 From: Yukina Mochizuki <1p41p4jejo@gmail.com> Date: Sat, 29 Aug 2026 15:57:57 +0800 Subject: [PATCH 10/11] feat: keep legacy db error wrappers logging without deprecation marks Replace the `Deprecated:` doc markers on the database wrap helpers and logutil.WithContext with plain comments, so linters and IDEs stop flagging existing call sites while the docs still point new code elsewhere. Restore the structured logging the wrappers lost: an Error on entry and a Warn with operation/unknown_error (plus table/key/value) after classification. --- pkg/database/errors.go | 19 +++++++++++++++++-- pkg/database/errors_mssql.go | 19 +++++++++++++++++-- pkg/log/deprecated.go | 9 +++++---- 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/pkg/database/errors.go b/pkg/database/errors.go index 5367ced..36ab683 100644 --- a/pkg/database/errors.go +++ b/pkg/database/errors.go @@ -35,12 +35,15 @@ func (e InternalServerError) Unwrap() error { return e.Source } -// Deprecated: database errors are classified here without logging. Return domain errors directly for new code. +// WrapDBError is the legacy helper: it logs the error and classifies it into a domain error. +// New code should return domain errors directly and log through pkg/log. func WrapDBError(err error, logger *zap.Logger, operation string) error { if err == nil { return nil } + logger.WithOptions(zap.AddCallerSkip(1)).Error("Failed to "+operation, zap.Error(err)) + var wrappedErr error switch { @@ -62,19 +65,27 @@ func WrapDBError(err error, logger *zap.Logger, operation string) error { } } + isUnknownError := false if wrappedErr == nil { wrappedErr = InternalServerError{Source: err} + isUnknownError = true } + logger.WithOptions(zap.AddCallerSkip(1)).Warn("Wrapped database error", zap.Error(wrappedErr), zap.String("operation", operation), zap.Bool("unknown_error", isUnknownError)) + return wrappedErr } -// Deprecated: database errors are classified here without logging. Return domain errors directly for new code. +// WrapDBErrorWithKeyValue is the legacy helper: it logs the error and classifies it into a domain error, +// using table/key/value to build the not-found error. New code should return domain errors directly +// and log through pkg/log. func WrapDBErrorWithKeyValue(err error, table, key, value string, logger *zap.Logger, operation string) error { if err == nil { return nil } + logger.WithOptions(zap.AddCallerSkip(1)).Error("Failed to "+operation, zap.Error(err)) + var wrappedErr error switch { @@ -96,9 +107,13 @@ func WrapDBErrorWithKeyValue(err error, table, key, value string, logger *zap.Lo } } + isUnknownError := false if wrappedErr == nil { wrappedErr = InternalServerError{Source: err} + isUnknownError = true } + logger.WithOptions(zap.AddCallerSkip(1)).Warn("Wrapped database error with key value", zap.Error(wrappedErr), zap.String("table", table), zap.String("key", key), zap.String("value", value), zap.String("operation", operation), zap.Bool("unknown_error", isUnknownError)) + return wrappedErr } diff --git a/pkg/database/errors_mssql.go b/pkg/database/errors_mssql.go index 443f2e8..8a3769e 100644 --- a/pkg/database/errors_mssql.go +++ b/pkg/database/errors_mssql.go @@ -19,12 +19,15 @@ const ( MSSQLErrDeadlockDetected = 1205 // Deadlock detected ) -// Deprecated: database errors are classified here without logging. Return domain errors directly for new code. +// WrapMSSQLError is the legacy helper: it logs the error and classifies it into a domain error. +// New code should return domain errors directly and log through pkg/log. func WrapMSSQLError(err error, logger *zap.Logger, operation string) error { if err == nil { return nil } + logger.Error("Failed to "+operation, zap.Error(err)) + var wrappedErr error switch { @@ -46,19 +49,27 @@ func WrapMSSQLError(err error, logger *zap.Logger, operation string) error { } } + isUnknownError := false if wrappedErr == nil { wrappedErr = InternalServerError{Source: err} + isUnknownError = true } + logger.Warn("Wrapped database error", zap.Error(wrappedErr), zap.String("operation", operation), zap.Bool("unknown_error", isUnknownError)) + return wrappedErr } -// Deprecated: database errors are classified here without logging. Return domain errors directly for new code. +// WrapMSSQLErrorWithKeyValue is the legacy helper: it logs the error and classifies it into a domain error, +// using table/key/value to build the not-found error. New code should return domain errors directly +// and log through pkg/log. func WrapMSSQLErrorWithKeyValue(err error, table, key, value string, logger *zap.Logger, operation string) error { if err == nil { return nil } + logger.Error("Failed to "+operation, zap.Error(err)) + var wrappedErr error switch { @@ -80,9 +91,13 @@ func WrapMSSQLErrorWithKeyValue(err error, table, key, value string, logger *zap } } + isUnknownError := false if wrappedErr == nil { wrappedErr = InternalServerError{Source: err} + isUnknownError = true } + logger.Warn("Wrapped database error with key value", zap.Error(wrappedErr), zap.String("table", table), zap.String("key", key), zap.String("value", value), zap.String("operation", operation), zap.Bool("unknown_error", isUnknownError)) + return wrappedErr } diff --git a/pkg/log/deprecated.go b/pkg/log/deprecated.go index 88d54e6..9156c1e 100644 --- a/pkg/log/deprecated.go +++ b/pkg/log/deprecated.go @@ -13,10 +13,11 @@ import ( // context keys "user_id", "username", and "name". Those user fields are emitted // as user_id, username, and display-name for compatibility with older callers. // -// Deprecated: use Constructs for the standard context-first logging path, or use -// WithTraceContext and WithUserContext directly when decorating a logger. New -// code should write user and request values with WithUserID, WithUsername, -// WithDisplayName, and WithRequestID instead of raw string context keys. +// This is the legacy path. New code should use Constructs for the standard +// context-first logging path, or WithTraceContext and WithUserContext directly +// when decorating a logger, and write user and request values with WithUserID, +// WithUsername, WithDisplayName, and WithRequestID instead of raw string +// context keys. func WithContext(ctx context.Context, logger *zap.Logger) *zap.Logger { if ctx == nil { return logger From 4549ba473d17f62acc8094fb6ea31ce6ffcdef3c Mon Sep 17 00:00:00 2001 From: Yukina Mochizuki <1p41p4jejo@gmail.com> Date: Sat, 29 Aug 2026 16:07:07 +0800 Subject: [PATCH 11/11] test(log): pass a typed nil context in SetupFlow tests The nil-context cases are intentional: they cover SetupFlow's fallback when a caller has no context. Passing a nil context.Context variable keeps that coverage while staticcheck's SA1012 only flags a literal nil. --- pkg/log/flow_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/log/flow_test.go b/pkg/log/flow_test.go index 8c3fd61..35f06ac 100644 --- a/pkg/log/flow_test.go +++ b/pkg/log/flow_test.go @@ -1,6 +1,7 @@ package logutil import ( + "context" "testing" errutil "github.com/NYCU-SDC/summer/pkg/error" @@ -9,12 +10,17 @@ import ( "go.uber.org/zap/zaptest/observer" ) +// nilContext is passed where a test deliberately exercises the nil-context +// fallback in SetupFlow. Using a variable keeps staticcheck's SA1012 (do not +// pass a nil Context) from firing on an intentional case. +var nilContext context.Context + func TestSetupFlowInitializesContextAndLogger(t *testing.T) { core, logs := observer.New(zapcore.InfoLevel) baseLogger := zap.New(core) ctx, logger := SetupFlow( - nil, + nilContext, baseLogger, "user.create", zap.String("request.id", "req-7"), @@ -63,7 +69,7 @@ func TestSetupFlowInitializesContextAndLogger(t *testing.T) { } func TestSetupFlowUsesSafeDefaults(t *testing.T) { - ctx, logger := SetupFlow(nil, nil, "") + ctx, logger := SetupFlow(nilContext, nil, "") if ctx == nil { t.Fatal("SetupFlow returned nil context")