Skip to content

Commit

Permalink
feat(logging): automatic project detection in logging.NewClient() (#9006
Browse files Browse the repository at this point in the history
)

* feat(logging): automatic project detection in logging.NewClient()

* fixed testcase

* Used DetectProjectId as a sentinel value for automatic project detection
  • Loading branch information
gkevinzheng committed Dec 6, 2023
1 parent 69215e3 commit bc13e6a
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 4 deletions.
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
}

0 comments on commit bc13e6a

Please sign in to comment.