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

feat(logging): automatic project detection in logging.NewClient() #9006

Merged
merged 4 commits into from Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 15 additions & 4 deletions logging/logging.go
Expand Up @@ -102,8 +102,9 @@ var (
ErrRedirectProtoPayloadNotSupported = errors.New("printEntryToStdout: cannot find valid payload")

// For testing:
now = time.Now
toLogEntryInternal = toLogEntryInternalImpl
now = time.Now
toLogEntryInternal = toLogEntryInternalImpl
detectResourceInternal = detectResource

// ErrOverflow signals that the number of buffered entries for a Logger
// exceeds its BufferLimit.
Expand Down Expand Up @@ -147,9 +148,12 @@ type Client struct {
// billingAccounts/ACCOUNT_ID
// organizations/ORG_ID
//
// for backwards compatibility, a string with no '/' is also allowed and is interpreted
// For backwards compatibility, a string with no '/' is also allowed and is interpreted
// as a project ID.
//
// If an empty string is provided as the parent, the parent will be interpreted as a project
// ID, and its value will be inferred from the environment.
//
// By default NewClient uses WriteScope. To use a different scope, call
// NewClient using a WithScopes option (see https://godoc.org/google.golang.org/api/option#WithScopes).
func NewClient(ctx context.Context, parent string, opts ...option.ClientOption) (*Client, error) {
Expand Down Expand Up @@ -191,6 +195,13 @@ func NewClient(ctx context.Context, parent string, opts ...option.ClientOption)

func makeParent(parent string) (string, error) {
if !strings.ContainsRune(parent, '/') {
if parent == "" {
gkevinzheng marked this conversation as resolved.
Show resolved Hide resolved
resource := detectResourceInternal()
if resource == nil {
return parent, fmt.Errorf("could not determine project ID from environment")
}
parent = resource.Labels["project_id"]
}
return "projects/" + parent, nil
}
prefix := strings.Split(parent, "/")[0]
Expand Down Expand Up @@ -300,7 +311,7 @@ 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 := detectResource()
r := detectResourceInternal()
if r == nil {
r = monitoredResource(c.parent)
}
Expand Down
68 changes: 68 additions & 0 deletions logging/logging_test.go
Expand Up @@ -1033,6 +1033,74 @@ func TestNonProjectParent(t *testing.T) {
}
}

func TestEmptyStringParent(t *testing.T) {
ctx := context.Background()
initLogs()
addr, err := ltesting.NewServer()
if err != nil {
t.Fatalf("creating fake server: %v", err)
}
conn, err := grpc.Dial(addr, grpc.WithInsecure())
if err != nil {
t.Fatalf("dialing %q: %v", addr, err)
}

tests := []struct {
name string
resource *mrpb.MonitoredResource
want string
wantError error
}{
{
name: "Test empty parent properly set up resource detection",
resource: &mrpb.MonitoredResource{
Labels: map[string]string{"project_id": testProjectID},
},
want: "projects/" + testProjectID,
},
{
name: "Test empty parent no resource detected",
resource: nil,
wantError: errors.New("could not determine project ID from environment"),
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
// Check if toLogEntryInternal was called with the right parent
toLogEntryInternalMock := func(got logging.Entry, l *logging.Logger, parent string, skipLevels int) (*logpb.LogEntry, error) {
if parent != test.want {
t.Errorf("toLogEntryInternal called with wrong parent. got: %s want: %s", parent, test.want)
}
return &logpb.LogEntry{}, nil
}

detectResourceMock := func() *mrpb.MonitoredResource {
return test.resource
}

realToLogEntryInternal := logging.SetToLogEntryInternal(toLogEntryInternalMock)
defer func() { logging.SetToLogEntryInternal(realToLogEntryInternal) }()

realDetectResourceInternal := logging.SetDetectResourceInternal(detectResourceMock)
defer func() { logging.SetDetectResourceInternal(realDetectResourceInternal) }()

cli, err := logging.NewClient(ctx, "", option.WithGRPCConn(conn))
if err != nil && test.wantError == nil {
t.Fatalf("Unexpected error: %+v: %v", test.resource, err)
}
if err == nil && test.wantError != nil {
t.Fatalf("Error is expected: %+v: %v", test.resource, test.wantError)
}
if test.wantError != nil {
return
}

cli.Logger(testLogID).LogSync(ctx, logging.Entry{Payload: "hello"})
})
}
}

// waitFor calls f repeatedly with exponential backoff, blocking until it returns true.
// It returns false after a while (if it times out).
func waitFor(f func() bool) bool {
Expand Down
5 changes: 5 additions & 0 deletions logging/logging_unexported_test.go
Expand Up @@ -397,3 +397,8 @@ func SetToLogEntryInternal(f func(Entry, *Logger, string, int) (*logpb.LogEntry,
toLogEntryInternal, f = f, toLogEntryInternal
return f
}

func SetDetectResourceInternal(f func() *mrpb.MonitoredResource) func() *mrpb.MonitoredResource {
detectResourceInternal, f = f, detectResourceInternal
return f
}