Skip to content

Commit

Permalink
feat(logging): make toLogEntry function public (#3863)
Browse files Browse the repository at this point in the history 
This will allows clients of the library to easily migrate from using
Logger abstraction to calling WriteLogEntries directly.
  • Loading branch information
@loburm
loburm committed Apr 9, 2021
1 parent 5391a31 commit 71828c2
Show file tree
Hide file tree
Showing 4 changed files with 241 additions and 158 deletions.
23 changes: 23 additions & 0 deletions logging/examples_test.go
View file
Expand Up @@ -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() {
Expand Down Expand Up @@ -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")
Expand Down
44 changes: 36 additions & 8 deletions logging/logging.go
View file
Expand Up @@ -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...)
Expand Down Expand Up @@ -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".
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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")
}
Expand All @@ -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
Expand All @@ -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,
Expand Down
181 changes: 181 additions & 0 deletions logging/logging_test.go
View file
Expand Up @@ -19,10 +19,13 @@ package logging_test
import (
"context"
"encoding/json"
"errors"
"flag"
"fmt"
"log"
"math/rand"
"net/http"
"net/url"
"os"
"strings"
"sync"
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 71828c2

Please sign in to comment.