Skip to content

Commit

Permalink
fix(logging): Skip automatic resource detection if a CommonResource (#…
Browse files Browse the repository at this point in the history
…10441)

LoggerOption is passed in
  • Loading branch information
gkevinzheng authored Jun 27, 2024
1 parent eec7a3b commit fc4c910
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 14 deletions.
19 changes: 14 additions & 5 deletions logging/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ type Logger struct {

// Options
commonResource *mrpb.MonitoredResource
commonResourceSet bool
commonLabels map[string]string
ctxFunc func() (context.Context, func())
populateSourceLocation int
Expand Down Expand Up @@ -323,14 +324,11 @@ func (r *loggerRetryer) Retry(err error) (pause time.Duration, shouldRetry bool)
// characters: [A-Za-z0-9]; and punctuation characters: forward-slash,
// underscore, hyphen, and period.
func (c *Client) Logger(logID string, opts ...LoggerOption) *Logger {
r := detectResourceInternal()
if r == nil {
r = monitoredResource(c.parent)
}
l := &Logger{
client: c,
logName: internal.LogPath(c.parent, logID),
commonResource: r,
commonResource: nil,
commonResourceSet: false,
ctxFunc: func() (context.Context, func()) { return context.Background(), nil },
populateSourceLocation: DoNotPopulateSourceLocation,
partialSuccess: false,
Expand All @@ -344,9 +342,20 @@ func (c *Client) Logger(logID string, opts ...LoggerOption) *Logger {
l.bundler.BundleByteThreshold = DefaultEntryByteThreshold
l.bundler.BundleByteLimit = DefaultBundleByteLimit
l.bundler.BufferedByteLimit = DefaultBufferedByteLimit

for _, opt := range opts {
opt.set(l)
}

// Set common resource here so that we skip automatic resource detection
// if a user has provided a common resource.
if !l.commonResourceSet {
l.commonResource = detectResourceInternal()
if l.commonResource == nil {
l.commonResource = monitoredResource(c.parent)
}
}

l.stdLoggers = map[Severity]*log.Logger{}
for s := range severityName {
e := Entry{Severity: s}
Expand Down
33 changes: 25 additions & 8 deletions logging/logging_unexported_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,11 @@ func TestLoggerCreation(t *testing.T) {
BufferedByteLimit: DefaultBufferedByteLimit,
}
for _, test := range []struct {
options []LoggerOption
wantLogger *Logger
defaultResource bool
wantBundler *bundler.Bundler
options []LoggerOption
wantLogger *Logger
defaultResource bool
wantBundler *bundler.Bundler
testNoDetectResource bool
}{
{
options: nil,
Expand All @@ -106,12 +107,14 @@ func TestLoggerCreation(t *testing.T) {
commonResource: nil,
commonLabels: map[string]string{"a": "1"},
},
wantBundler: defaultBundler,
wantBundler: defaultBundler,
testNoDetectResource: true,
},
{
options: []LoggerOption{CommonResource(customResource)},
wantLogger: &Logger{commonResource: customResource},
wantBundler: defaultBundler,
options: []LoggerOption{CommonResource(customResource)},
wantLogger: &Logger{commonResource: customResource},
wantBundler: defaultBundler,
testNoDetectResource: true,
},
{
options: []LoggerOption{
Expand All @@ -132,6 +135,16 @@ func TestLoggerCreation(t *testing.T) {
},
},
} {
detectResourceMock := func() *mrpb.MonitoredResource {
t.Errorf("%v: detectResource was called when it shouldn't be", test.options)
return nil
}
realDetectResourceInternal := detectResourceInternal

if test.testNoDetectResource {
SetDetectResourceInternal(detectResourceMock)
}

gotLogger := c.Logger(logID, test.options...)
if got, want := gotLogger.commonResource, test.wantLogger.commonResource; !test.defaultResource && !proto.Equal(got, want) {
t.Errorf("%v: resource: got %v, want %v", test.options, got, want)
Expand All @@ -154,6 +167,10 @@ func TestLoggerCreation(t *testing.T) {
if got, want := gotLogger.bundler.BufferedByteLimit, test.wantBundler.BufferedByteLimit; got != want {
t.Errorf("%v: BufferedByteLimit: got %v, want %v", test.options, got, want)
}

if test.testNoDetectResource {
SetDetectResourceInternal(realDetectResourceInternal)
}
}
}

Expand Down
5 changes: 4 additions & 1 deletion logging/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ func CommonResource(r *mrpb.MonitoredResource) LoggerOption { return commonResou

type commonResource struct{ *mrpb.MonitoredResource }

func (r commonResource) set(l *Logger) { l.commonResource = r.MonitoredResource }
func (r commonResource) set(l *Logger) {
l.commonResourceSet = true
l.commonResource = r.MonitoredResource
}

type resource struct {
pb *mrpb.MonitoredResource
Expand Down

0 comments on commit fc4c910

Please sign in to comment.