Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

logging: Reversed policy of fields - from immutable to fresh winning. #616

Merged
merged 2 commits into from
Aug 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions interceptors/logging/interceptors.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,16 @@ func reportable(logger Logger, opts *options) interceptors.CommonReportableFunc
kind = KindClientFieldValue
}

fields := ExtractFields(ctx).WithUnique(newCommonFields(kind, c))
// Field dups from context override the common fields.
fields := newCommonFields(kind, c).WithUnique(ExtractFields(ctx))
bwplotka marked this conversation as resolved.
Show resolved Hide resolved
if !c.IsClient {
if peer, ok := peer.FromContext(ctx); ok {
fields = append(fields, "peer.address", peer.Addr.String())
}
}
if opts.fieldsFromCtxFn != nil {
fields = fields.AppendUnique(opts.fieldsFromCtxFn(ctx))
// fieldsFromCtxFn dups override the existing fields.
fields = opts.fieldsFromCtxFn(ctx).AppendUnique(fields)
}

singleUseFields := Fields{"grpc.start_time", time.Now().Format(opts.timestampFormat)}
Expand Down
18 changes: 10 additions & 8 deletions interceptors/logging/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,9 @@ NextAddField:
}

// ExtractFields returns logging fields from the context.
// Logging interceptor adds fields into context when used.
// If there are no fields in the context, returns an empty Fields value.
// Fields can be added from the context using InjectFields. For example logging interceptor adds some (base) fields
// into context when used.
// If there are no fields in the context, it returns an empty Fields value.
// Extracted fields are useful to construct your own logger that has fields from gRPC interceptors.
func ExtractFields(ctx context.Context) Fields {
t, ok := ctx.Value(fieldsCtxMarkerKey).(Fields)
Expand All @@ -142,13 +143,14 @@ func ExtractFields(ctx context.Context) Fields {
return n
}

// InjectFields allows adding fields to any existing Fields that will be used by the logging interceptor.
// For explicitness, in case of duplicates, first field occurrence wins (immutability of fields). This also
// applies to all fields created by logging middleware. It uses labels from this context as a base, so fields like "grpc.service"
// can be overridden if your you add custom middleware that injects "grpc.service" before logging middleware injects those.
// Don't overuse overriding to avoid surprises.
// InjectFields allows adding fields to any existing Fields that will be used by the logging interceptor or can be
// extracted further in ExtractFields.
// For explicitness, in case of duplicates, the newest field occurrence wins. This allows nested components to update
// popular fields like grpc.component (e.g. server invoking gRPC client).
//
// Don't overuse mutation of fields to avoid surprises.
func InjectFields(ctx context.Context, f Fields) context.Context {
return context.WithValue(ctx, fieldsCtxMarkerKey, ExtractFields(ctx).WithUnique(f))
return context.WithValue(ctx, fieldsCtxMarkerKey, f.WithUnique(ExtractFields(ctx)))
}

// InjectLogField is like InjectFields, just for one field.
Expand Down
41 changes: 41 additions & 0 deletions interceptors/logging/logging_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (c) The go-grpc-middleware Authors.
// Licensed under the Apache License 2.0.

package logging

import (
"context"
"testing"

"github.com/stretchr/testify/require"
)

func TestFieldsInjectExtractFromContext(t *testing.T) {
c := context.Background()
f := ExtractFields(c)
require.Equal(t, Fields(nil), f)

f = f.AppendUnique([]any{"a", "1", "b", "2"})
require.Equal(t, Fields{"a", "1", "b", "2"}, f)

c2 := InjectFields(c, f)

// First context should be untouched.
f = ExtractFields(c)
require.Equal(t, Fields(nil), f)
f = ExtractFields(c2)
require.Equal(t, Fields{"a", "1", "b", "2"}, f)

f = Fields{"a", "changed"}.WithUnique(f)
require.Equal(t, Fields{"a", "changed", "b", "2"}, f)

c3 := InjectFields(c, f)

// Old contexts should be untouched.
f = ExtractFields(c)
require.Equal(t, Fields(nil), f)
f = ExtractFields(c2)
require.Equal(t, Fields{"a", "1", "b", "2"}, f)
f = ExtractFields(c3)
require.Equal(t, Fields{"a", "changed", "b", "2"}, f)
}
2 changes: 1 addition & 1 deletion interceptors/logging/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func DefaultClientCodeToLevel(code codes.Code) Level {

type fieldsFromCtxFn func(ctx context.Context) Fields

// WithFieldsFromContext allows adding extra fields to all log messages per given request.
// WithFieldsFromContext allows overriding existing or adding extra fields to all log messages per given request.
func WithFieldsFromContext(f fieldsFromCtxFn) Option {
return func(o *options) {
o.fieldsFromCtxFn = f
Expand Down
Loading