Skip to content

Commit 71828c2

Browse files
loburmloburm
authored
feat(logging): make toLogEntry function public (#3863)
This will allows clients of the library to easily migrate from using Logger abstraction to calling WriteLogEntries directly.
1 parent 5391a31 commit 71828c2

4 files changed

Lines changed: 241 additions & 158 deletions

File tree

‎logging/examples_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ import (
2222
"os"
2323

2424
"cloud.google.com/go/logging"
25+
vkit "cloud.google.com/go/logging/apiv2"
2526
"go.opencensus.io/trace"
27+
logpb "google.golang.org/genproto/googleapis/logging/v2"
2628
)
2729

2830
func ExampleNewClient() {
@@ -106,6 +108,27 @@ func ExampleHTTPRequest() {
106108
lg.Log(httpEntry)
107109
}
108110

111+
func ExampleToLogEntry() {
112+
e := logging.Entry{
113+
Payload: "Message",
114+
}
115+
le, err := logging.ToLogEntry(e, "my-project")
116+
if err != nil {
117+
// TODO: Handle error.
118+
}
119+
client, err := vkit.NewClient(context.Background())
120+
if err != nil {
121+
// TODO: Handle error.
122+
}
123+
_, err = client.WriteLogEntries(context.Background(), &logpb.WriteLogEntriesRequest{
124+
Entries: []*logpb.LogEntry{le},
125+
LogName: "stdout",
126+
})
127+
if err != nil {
128+
// TODO: Handle error.
129+
}
130+
}
131+
109132
func ExampleLogger_LogSync() {
110133
ctx := context.Background()
111134
client, err := logging.NewClient(ctx, "my-project")

‎logging/logging.go‎

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,7 @@ type Client struct {
137137
// By default NewClient uses WriteScope. To use a different scope, call
138138
// NewClient using a WithScopes option (see https://godoc.org/google.golang.org/api/option#WithScopes).
139139
func NewClient(ctx context.Context, parent string, opts ...option.ClientOption) (*Client, error) {
140-
if !strings.ContainsRune(parent, '/') {
141-
parent = "projects/" + parent
142-
}
140+
parent = makeParent(parent)
143141
opts = append([]option.ClientOption{
144142
option.WithScopes(WriteScope),
145143
}, opts...)
@@ -172,6 +170,13 @@ func NewClient(ctx context.Context, parent string, opts ...option.ClientOption)
172170
return client, nil
173171
}
174172

173+
func makeParent(parent string) string {
174+
if !strings.ContainsRune(parent, '/') {
175+
return "projects/" + parent
176+
}
177+
return parent
178+
}
179+
175180
// Ping reports whether the client's connection to the logging service and the
176181
// authentication configuration are valid. To accomplish this, Ping writes a
177182
// log entry "ping" to a log named "ping".
@@ -803,7 +808,7 @@ func jsonValueToStructValue(v interface{}) *structpb.Value {
803808
// and will block, it is intended primarily for debugging or critical errors.
804809
// Prefer Log for most uses.
805810
func (l *Logger) LogSync(ctx context.Context, e Entry) error {
806-
ent, err := l.toLogEntry(e)
811+
ent, err := toLogEntryInternal(e, l.client, l.client.parent)
807812
if err != nil {
808813
return err
809814
}
@@ -818,7 +823,7 @@ func (l *Logger) LogSync(ctx context.Context, e Entry) error {
818823

819824
// Log buffers the Entry for output to the logging service. It never blocks.
820825
func (l *Logger) Log(e Entry) {
821-
ent, err := l.toLogEntry(e)
826+
ent, err := toLogEntryInternal(e, l.client, l.client.parent)
822827
if err != nil {
823828
l.client.error(err)
824829
return
@@ -894,7 +899,26 @@ func deconstructXCloudTraceContext(s string) (traceID, spanID string, traceSampl
894899
return
895900
}
896901

897-
func (l *Logger) toLogEntry(e Entry) (*logpb.LogEntry, error) {
902+
// ToLogEntry takes an Entry structure and converts it to the LogEntry proto.
903+
// A parent can take any of the following forms:
904+
// projects/PROJECT_ID
905+
// folders/FOLDER_ID
906+
// billingAccounts/ACCOUNT_ID
907+
// organizations/ORG_ID
908+
// for backwards compatibility, a string with no '/' is also allowed and is interpreted
909+
// as a project ID.
910+
//
911+
// ToLogEntry is implied when users invoke Logger.Log or Logger.LogSync,
912+
// but its exported as a pub function here to give users additional flexibility
913+
// when using the library. Don't call this method manually if Logger.Log or
914+
// Logger.LogSync are used, it is intended to be used together with direct call
915+
// to WriteLogEntries method.
916+
func ToLogEntry(e Entry, parent string) (*logpb.LogEntry, error) {
917+
// We have this method to support logging agents that need a bigger flexibility.
918+
return toLogEntryInternal(e, nil, makeParent(parent))
919+
}
920+
921+
func toLogEntryInternal(e Entry, client *Client, parent string) (*logpb.LogEntry, error) {
898922
if e.LogName != "" {
899923
return nil, errors.New("logging: Entry.LogName should be not be set when writing")
900924
}
@@ -913,7 +937,7 @@ func (l *Logger) toLogEntry(e Entry) (*logpb.LogEntry, error) {
913937
// https://cloud.google.com/appengine/docs/flexible/go/writing-application-logs.
914938
traceID, spanID, traceSampled := deconstructXCloudTraceContext(traceHeader)
915939
if traceID != "" {
916-
e.Trace = fmt.Sprintf("%s/traces/%s", l.client.parent, traceID)
940+
e.Trace = fmt.Sprintf("%s/traces/%s", parent, traceID)
917941
}
918942
if e.SpanID == "" {
919943
e.SpanID = spanID
@@ -927,7 +951,11 @@ func (l *Logger) toLogEntry(e Entry) (*logpb.LogEntry, error) {
927951
}
928952
req, err := fromHTTPRequest(e.HTTPRequest)
929953
if err != nil {
930-
l.client.error(err)
954+
if client != nil {
955+
client.error(err)
956+
} else {
957+
return nil, err
958+
}
931959
}
932960
ent := &logpb.LogEntry{
933961
Timestamp: ts,

‎logging/logging_test.go‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,13 @@ package logging_test
1919
import (
2020
"context"
2121
"encoding/json"
22+
"errors"
2223
"flag"
2324
"fmt"
2425
"log"
2526
"math/rand"
27+
"net/http"
28+
"net/url"
2629
"os"
2730
"strings"
2831
"sync"
@@ -41,6 +44,7 @@ import (
4144
"google.golang.org/api/iterator"
4245
"google.golang.org/api/option"
4346
mrpb "google.golang.org/genproto/googleapis/api/monitoredres"
47+
logpb "google.golang.org/genproto/googleapis/logging/v2"
4448
"google.golang.org/grpc"
4549
"google.golang.org/grpc/codes"
4650
"google.golang.org/grpc/status"
@@ -247,6 +251,183 @@ func TestContextFunc(t *testing.T) {
247251
}
248252
}
249253

254+
func TestToLogEntry(t *testing.T) {
255+
u := &url.URL{Scheme: "http"}
256+
tests := []struct {
257+
name string
258+
in logging.Entry
259+
want logpb.LogEntry
260+
wantError error
261+
}{
262+
{
263+
name: "BlankLogEntry",
264+
in: logging.Entry{},
265+
want: logpb.LogEntry{},
266+
}, {
267+
name: "Already set Trace",
268+
in: logging.Entry{Trace: "t1"},
269+
want: logpb.LogEntry{Trace: "t1"},
270+
}, {
271+
name: "No X-Trace-Context header",
272+
in: logging.Entry{
273+
HTTPRequest: &logging.HTTPRequest{
274+
Request: &http.Request{URL: u, Header: http.Header{"foo": {"bar"}}},
275+
},
276+
},
277+
want: logpb.LogEntry{},
278+
}, {
279+
name: "X-Trace-Context header with all fields",
280+
in: logging.Entry{
281+
TraceSampled: false,
282+
HTTPRequest: &logging.HTTPRequest{
283+
Request: &http.Request{
284+
URL: u,
285+
Header: http.Header{"X-Cloud-Trace-Context": {"105445aa7843bc8bf206b120001000/000000000000004a;o=1"}},
286+
},
287+
},
288+
},
289+
want: logpb.LogEntry{
290+
Trace: "projects/P/traces/105445aa7843bc8bf206b120001000",
291+
SpanId: "000000000000004a",
292+
TraceSampled: true,
293+
},
294+
}, {
295+
name: "X-Trace-Context header with all fields; TraceSampled explicitly set",
296+
in: logging.Entry{
297+
TraceSampled: true,
298+
HTTPRequest: &logging.HTTPRequest{
299+
Request: &http.Request{
300+
URL: u,
301+
Header: http.Header{"X-Cloud-Trace-Context": {"105445aa7843bc8bf206b120001000/000000000000004a;o=0"}},
302+
},
303+
},
304+
},
305+
want: logpb.LogEntry{
306+
Trace: "projects/P/traces/105445aa7843bc8bf206b120001000",
307+
SpanId: "000000000000004a",
308+
TraceSampled: true,
309+
},
310+
}, {
311+
name: "X-Trace-Context header with all fields; TraceSampled from Header",
312+
in: logging.Entry{
313+
HTTPRequest: &logging.HTTPRequest{
314+
Request: &http.Request{
315+
URL: u,
316+
Header: http.Header{"X-Cloud-Trace-Context": {"105445aa7843bc8bf206b120001000/000000000000004a;o=1"}},
317+
},
318+
},
319+
},
320+
want: logpb.LogEntry{
321+
Trace: "projects/P/traces/105445aa7843bc8bf206b120001000",
322+
SpanId: "000000000000004a",
323+
TraceSampled: true,
324+
},
325+
}, {
326+
name: "X-Trace-Context header with blank trace",
327+
in: logging.Entry{
328+
HTTPRequest: &logging.HTTPRequest{
329+
Request: &http.Request{
330+
URL: u,
331+
Header: http.Header{"X-Cloud-Trace-Context": {"/0;o=1"}},
332+
},
333+
},
334+
},
335+
want: logpb.LogEntry{
336+
TraceSampled: true,
337+
},
338+
}, {
339+
name: "X-Trace-Context header with blank span",
340+
in: logging.Entry{
341+
HTTPRequest: &logging.HTTPRequest{
342+
Request: &http.Request{
343+
URL: u,
344+
Header: http.Header{"X-Cloud-Trace-Context": {"105445aa7843bc8bf206b120001000/;o=0"}},
345+
},
346+
},
347+
},
348+
want: logpb.LogEntry{
349+
Trace: "projects/P/traces/105445aa7843bc8bf206b120001000",
350+
},
351+
}, {
352+
name: "X-Trace-Context header with missing traceSampled aka ?o=*",
353+
in: logging.Entry{
354+
HTTPRequest: &logging.HTTPRequest{
355+
Request: &http.Request{
356+
URL: u,
357+
Header: http.Header{"X-Cloud-Trace-Context": {"105445aa7843bc8bf206b120001000/0"}},
358+
},
359+
},
360+
},
361+
want: logpb.LogEntry{
362+
Trace: "projects/P/traces/105445aa7843bc8bf206b120001000",
363+
},
364+
}, {
365+
name: "X-Trace-Context header with all blank fields",
366+
in: logging.Entry{
367+
HTTPRequest: &logging.HTTPRequest{
368+
Request: &http.Request{
369+
URL: u,
370+
Header: http.Header{"X-Cloud-Trace-Context": {""}},
371+
},
372+
},
373+
},
374+
want: logpb.LogEntry{},
375+
}, {
376+
name: "Invalid X-Trace-Context header but already set TraceID",
377+
in: logging.Entry{
378+
HTTPRequest: &logging.HTTPRequest{
379+
Request: &http.Request{
380+
URL: u,
381+
Header: http.Header{"X-Cloud-Trace-Context": {"t3"}},
382+
},
383+
},
384+
Trace: "t4",
385+
},
386+
want: logpb.LogEntry{
387+
Trace: "t4",
388+
},
389+
}, {
390+
name: "Already set TraceID and SpanID",
391+
in: logging.Entry{Trace: "t1", SpanID: "007"},
392+
want: logpb.LogEntry{
393+
Trace: "t1",
394+
SpanId: "007",
395+
},
396+
}, {
397+
name: "Empty request produces an error",
398+
in: logging.Entry{
399+
HTTPRequest: &logging.HTTPRequest{
400+
RequestSize: 128,
401+
},
402+
},
403+
wantError: errors.New("logging: HTTPRequest must have a non-nil Request"),
404+
},
405+
}
406+
for _, test := range tests {
407+
t.Run(test.name, func(t *testing.T) {
408+
e, err := logging.ToLogEntry(test.in, "projects/P")
409+
if err != nil && test.wantError == nil {
410+
t.Fatalf("Unexpected error: %+v: %v", test.in, err)
411+
}
412+
if err == nil && test.wantError != nil {
413+
t.Fatalf("Error is expected: %+v: %v", test.in, test.wantError)
414+
}
415+
if test.wantError != nil {
416+
return
417+
}
418+
if got := e.Trace; got != test.want.Trace {
419+
t.Errorf("TraceId: %+v: got %q, want %q", test.in, got, test.want.Trace)
420+
}
421+
if got := e.SpanId; got != test.want.SpanId {
422+
t.Errorf("SpanId: %+v: got %q, want %q", test.in, got, test.want.SpanId)
423+
}
424+
if got := e.TraceSampled; got != test.want.TraceSampled {
425+
t.Errorf("TraceSampled: %+v: got %t, want %t", test.in, got, test.want.TraceSampled)
426+
}
427+
})
428+
}
429+
}
430+
250431
// compareEntries compares most fields list of Entries against expected. compareEntries does not compare:
251432
// - HTTPRequest
252433
// - Operation

0 commit comments

Comments
 (0)