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 all 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
28 changes: 24 additions & 4 deletions logging/logging.go
Expand Up @@ -95,6 +95,15 @@ const (

// Part of the error message when the payload contains invalid UTF-8 characters.
utfErrorString = "string field contains invalid UTF-8"

// DetectProjectID is a sentinel value that instructs NewClient to detect the
// project ID. It is given in place of the projectID argument. NewClient will
// use the project ID from the given credentials or the default credentials
// (https://developers.google.com/accounts/docs/application-default-credentials)
// if no credentials were provided. When providing credentials, not all
// options will allow NewClient to extract the project ID. Specifically a JWT
// does not have the project ID encoded.
DetectProjectID = "*detect-project-id*"
)

var (
Expand All @@ -103,8 +112,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 @@ -148,9 +158,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 logging.DetectProjectId 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 @@ -192,6 +205,13 @@ func NewClient(ctx context.Context, parent string, opts ...option.ClientOption)

func makeParent(parent string) (string, error) {
if !strings.ContainsRune(parent, '/') {
if parent == DetectProjectID {
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 @@ -301,7 +321,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 TestDetectProjectIdParent(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 DetectProjectId parent properly set up resource detection",
resource: &mrpb.MonitoredResource{
Labels: map[string]string{"project_id": testProjectID},
},
want: "projects/" + testProjectID,
},
{
name: "Test DetectProjectId 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, logging.DetectProjectID, 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
}