feat(logging): make toLogEntry function public

This will allows clients of the library to easily migrate from using
Logger abstraction to calling WriteLogEntries directly.

logging/examples_test.go

@@ -22,7 +22,9 @@ import (
 	"os"
 
 	"cloud.google.com/go/logging"
+	vkit "cloud.google.com/go/logging/apiv2"
 	"go.opencensus.io/trace"
+	logpb "google.golang.org/genproto/googleapis/logging/v2"
 )
 
 func ExampleNewClient() {
@@ -106,6 +108,27 @@ func ExampleHTTPRequest() {
 	lg.Log(httpEntry)
 }
 
+func ExampleToLogEntry() {
+	e := logging.Entry{
+		Payload: "Message",
+	}
+	le, err := logging.ToLogEntry(e, "my-project")
+	if err != nil {
+		// TODO: Handle error.
+	}
+	client, err := vkit.NewClient(context.Background())
+	if err != nil {
+		// TODO: Handle error.
+	}
+	_, err = client.WriteLogEntries(context.Background(), &logpb.WriteLogEntriesRequest{
+		Entries: []*logpb.LogEntry{le},
+		LogName: "stdout",
+	})
+	if err != nil {
+		// TODO: Handle error.
+	}
+}
+
 func ExampleLogger_LogSync() {
 	ctx := context.Background()
 	client, err := logging.NewClient(ctx, "my-project")

logging/logging.go

@@ -137,9 +137,7 @@ type Client struct {
 // 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) {
-	if !strings.ContainsRune(parent, '/') {
-		parent = "projects/" + parent
-	}
+	parent = makeParent(parent)
 	opts = append([]option.ClientOption{
 		option.WithScopes(WriteScope),
 	}, opts...)
@@ -172,6 +170,13 @@ func NewClient(ctx context.Context, parent string, opts ...option.ClientOption)
 	return client, nil
 }
 
+func makeParent(parent string) string {
+	if !strings.ContainsRune(parent, '/') {
+		return "projects/" + parent
+	}
+	return parent
+}
+
 // Ping reports whether the client's connection to the logging service and the
 // authentication configuration are valid. To accomplish this, Ping writes a
 // log entry "ping" to a log named "ping".
@@ -803,7 +808,7 @@ func jsonValueToStructValue(v interface{}) *structpb.Value {
 // and will block, it is intended primarily for debugging or critical errors.
 // Prefer Log for most uses.
 func (l *Logger) LogSync(ctx context.Context, e Entry) error {
-	ent, err := l.toLogEntry(e)
+	ent, err := toLogEntryInternal(e, l.client, l.client.parent)
 	if err != nil {
 		return err
 	}
@@ -818,7 +823,7 @@ func (l *Logger) LogSync(ctx context.Context, e Entry) error {
 
 // Log buffers the Entry for output to the logging service. It never blocks.
 func (l *Logger) Log(e Entry) {
-	ent, err := l.toLogEntry(e)
+	ent, err := toLogEntryInternal(e, l.client, l.client.parent)
 	if err != nil {
 		l.client.error(err)
 		return
@@ -894,7 +899,26 @@ func deconstructXCloudTraceContext(s string) (traceID, spanID string, traceSampl
 	return
 }
 
-func (l *Logger) toLogEntry(e Entry) (*logpb.LogEntry, error) {
+// ToLogEntry takes an Entry structure and converts it to the LogEntry proto.
+// A parent can take any of the following forms:
+//    projects/PROJECT_ID
+//    folders/FOLDER_ID
+//    billingAccounts/ACCOUNT_ID
+//    organizations/ORG_ID
+// for backwards compatibility, a string with no '/' is also allowed and is interpreted
+// as a project ID.
+//
+// ToLogEntry is implied when users invoke Logger.Log or Logger.LogSync,
+// but its exported as a pub function here to give users additional flexibility
+// when using the library. Don't call this method manually if Logger.Log or
+// Logger.LogSync are used, it is intended to be used together with direct call
+// to WriteLogEntries method.
+func ToLogEntry(e Entry, parent string) (*logpb.LogEntry, error) {
+	// We have this method to support logging agents that need a bigger flexibility.
+	return toLogEntryInternal(e, nil, makeParent(parent))
+}
+
+func toLogEntryInternal(e Entry, client *Client, parent string) (*logpb.LogEntry, error) {
 	if e.LogName != "" {
 		return nil, errors.New("logging: Entry.LogName should be not be set when writing")
 	}
@@ -913,7 +937,7 @@ func (l *Logger) toLogEntry(e Entry) (*logpb.LogEntry, error) {
 			// https://cloud.google.com/appengine/docs/flexible/go/writing-application-logs.
 			traceID, spanID, traceSampled := deconstructXCloudTraceContext(traceHeader)
 			if traceID != "" {
-				e.Trace = fmt.Sprintf("%s/traces/%s", l.client.parent, traceID)
+				e.Trace = fmt.Sprintf("%s/traces/%s", parent, traceID)
 			}
 			if e.SpanID == "" {
 				e.SpanID = spanID
@@ -927,7 +951,11 @@ func (l *Logger) toLogEntry(e Entry) (*logpb.LogEntry, error) {
 	}
 	req, err := fromHTTPRequest(e.HTTPRequest)
 	if err != nil {
-		l.client.error(err)
+		if client != nil {
+			client.error(err)
+		} else {
+			return nil, err
+		}
 	}
 	ent := &logpb.LogEntry{
 		Timestamp:      ts,

logging/logging.iml

@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="WEB_MODULE" version="4">
+  <component name="Go" enabled="true" />
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$" />
+    <orderEntry type="sourceFolder" forTests="false" />
+  </component>
+</module>

logging/logging_test.go

@@ -19,10 +19,13 @@ package logging_test
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"flag"
 	"fmt"
 	"log"
 	"math/rand"
+	"net/http"
+	"net/url"
 	"os"
 	"strings"
 	"sync"
@@ -41,6 +44,7 @@ import (
 	"google.golang.org/api/iterator"
 	"google.golang.org/api/option"
 	mrpb "google.golang.org/genproto/googleapis/api/monitoredres"
+	logpb "google.golang.org/genproto/googleapis/logging/v2"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -247,6 +251,183 @@ func TestContextFunc(t *testing.T) {
 	}
 }
 
+func TestToLogEntry(t *testing.T) {
+	u := &url.URL{Scheme: "http"}
+	tests := []struct {
+		name      string
+		in        logging.Entry
+		want      logpb.LogEntry
+		wantError error
+	}{
+		{
+			name: "BlankLogEntry",
+			in:   logging.Entry{},
+			want: logpb.LogEntry{},
+		}, {
+			name: "Already set Trace",
+			in:   logging.Entry{Trace: "t1"},
+			want: logpb.LogEntry{Trace: "t1"},
+		}, {
+			name: "No X-Trace-Context header",
+			in: logging.Entry{
+				HTTPRequest: &logging.HTTPRequest{
+					Request: &http.Request{URL: u, Header: http.Header{"foo": {"bar"}}},
+				},
+			},
+			want: logpb.LogEntry{},
+		}, {
+			name: "X-Trace-Context header with all fields",
+			in: logging.Entry{
+				TraceSampled: false,
+				HTTPRequest: &logging.HTTPRequest{
+					Request: &http.Request{
+						URL:    u,
+						Header: http.Header{"X-Cloud-Trace-Context": {"105445aa7843bc8bf206b120001000/000000000000004a;o=1"}},
+					},
+				},
+			},
+			want: logpb.LogEntry{
+				Trace:        "projects/P/traces/105445aa7843bc8bf206b120001000",
+				SpanId:       "000000000000004a",
+				TraceSampled: true,
+			},
+		}, {
+			name: "X-Trace-Context header with all fields; TraceSampled explicitly set",
+			in: logging.Entry{
+				TraceSampled: true,
+				HTTPRequest: &logging.HTTPRequest{
+					Request: &http.Request{
+						URL:    u,
+						Header: http.Header{"X-Cloud-Trace-Context": {"105445aa7843bc8bf206b120001000/000000000000004a;o=0"}},
+					},
+				},
+			},
+			want: logpb.LogEntry{
+				Trace:        "projects/P/traces/105445aa7843bc8bf206b120001000",
+				SpanId:       "000000000000004a",
+				TraceSampled: true,
+			},
+		}, {
+			name: "X-Trace-Context header with all fields; TraceSampled from Header",
+			in: logging.Entry{
+				HTTPRequest: &logging.HTTPRequest{
+					Request: &http.Request{
+						URL:    u,
+						Header: http.Header{"X-Cloud-Trace-Context": {"105445aa7843bc8bf206b120001000/000000000000004a;o=1"}},
+					},
+				},
+			},
+			want: logpb.LogEntry{
+				Trace:        "projects/P/traces/105445aa7843bc8bf206b120001000",
+				SpanId:       "000000000000004a",
+				TraceSampled: true,
+			},
+		}, {
+			name: "X-Trace-Context header with blank trace",
+			in: logging.Entry{
+				HTTPRequest: &logging.HTTPRequest{
+					Request: &http.Request{
+						URL:    u,
+						Header: http.Header{"X-Cloud-Trace-Context": {"/0;o=1"}},
+					},
+				},
+			},
+			want: logpb.LogEntry{
+				TraceSampled: true,
+			},
+		}, {
+			name: "X-Trace-Context header with blank span",
+			in: logging.Entry{
+				HTTPRequest: &logging.HTTPRequest{
+					Request: &http.Request{
+						URL:    u,
+						Header: http.Header{"X-Cloud-Trace-Context": {"105445aa7843bc8bf206b120001000/;o=0"}},
+					},
+				},
+			},
+			want: logpb.LogEntry{
+				Trace: "projects/P/traces/105445aa7843bc8bf206b120001000",
+			},
+		}, {
+			name: "X-Trace-Context header with missing traceSampled aka ?o=*",
+			in: logging.Entry{
+				HTTPRequest: &logging.HTTPRequest{
+					Request: &http.Request{
+						URL:    u,
+						Header: http.Header{"X-Cloud-Trace-Context": {"105445aa7843bc8bf206b120001000/0"}},
+					},
+				},
+			},
+			want: logpb.LogEntry{
+				Trace: "projects/P/traces/105445aa7843bc8bf206b120001000",
+			},
+		}, {
+			name: "X-Trace-Context header with all blank fields",
+			in: logging.Entry{
+				HTTPRequest: &logging.HTTPRequest{
+					Request: &http.Request{
+						URL:    u,
+						Header: http.Header{"X-Cloud-Trace-Context": {""}},
+					},
+				},
+			},
+			want: logpb.LogEntry{},
+		}, {
+			name: "Invalid X-Trace-Context header but already set TraceID",
+			in: logging.Entry{
+				HTTPRequest: &logging.HTTPRequest{
+					Request: &http.Request{
+						URL:    u,
+						Header: http.Header{"X-Cloud-Trace-Context": {"t3"}},
+					},
+				},
+				Trace: "t4",
+			},
+			want: logpb.LogEntry{
+				Trace: "t4",
+			},
+		}, {
+			name: "Already set TraceID and SpanID",
+			in:   logging.Entry{Trace: "t1", SpanID: "007"},
+			want: logpb.LogEntry{
+				Trace:  "t1",
+				SpanId: "007",
+			},
+		}, {
+			name: "Empty request produces an error",
+			in: logging.Entry{
+				HTTPRequest: &logging.HTTPRequest{
+					RequestSize: 128,
+				},
+			},
+			wantError: errors.New("logging: HTTPRequest must have a non-nil Request"),
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			e, err := logging.ToLogEntry(test.in, "projects/P")
+			if err != nil && test.wantError == nil {
+				t.Fatalf("Unexpected error: %+v: %v", test.in, err)
+			}
+			if err == nil && test.wantError != nil {
+				t.Fatalf("Error is expected: %+v: %v", test.in, test.wantError)
+			}
+			if test.wantError != nil {
+				return
+			}
+			if got := e.Trace; got != test.want.Trace {
+				t.Errorf("TraceId: %+v: got %q, want %q", test.in, got, test.want.Trace)
+			}
+			if got := e.SpanId; got != test.want.SpanId {
+				t.Errorf("SpanId: %+v: got %q, want %q", test.in, got, test.want.SpanId)
+			}
+			if got := e.TraceSampled; got != test.want.TraceSampled {
+				t.Errorf("TraceSampled: %+v: got %t, want %t", test.in, got, test.want.TraceSampled)
+			}
+		})
+	}
+}
+
 // compareEntries compares most fields list of Entries against expected. compareEntries does not compare:
 //   - HTTPRequest
 //   - Operation