diff --git a/pkg/events/events.go b/pkg/events/events.go index 92252a56..56d03c63 100644 --- a/pkg/events/events.go +++ b/pkg/events/events.go @@ -84,7 +84,7 @@ type HTTPRequest struct { Endpoint string `log:"http.url_details.endpoint"` // Route is the URL path without interpolating the path variables. - Route string `log:"http.route"` + Route string `log:"http.route,omitempty"` } // FillFieldsFromRequest fills in the standard request fields diff --git a/tools/logger/generating.md b/tools/logger/generating.md index acf5e482..2dfe0125 100644 --- a/tools/logger/generating.md +++ b/tools/logger/generating.md @@ -7,9 +7,9 @@ Consider a `struct` like so: ```golang -type OrgInfo struct { - Org string - Guid string +type Org struct { + Shortname string + GUID string } ``` @@ -20,14 +20,14 @@ this: ```golang -type OrgInfo struct - Org string `log:"or.org.shortname"` - Guid string `log:"or.org.guid"` +type Org struct + Shortname string `log:"or.org.shortname"` + GUID string `log:"or.org.guid"` } ``` -With the addition of the annotation (which matches the [standard +With the addition of the `log` tags for fields (which matches the [standard attributes](https://app.datadoghq.com/logs/pipelines/standard-attributes)), we can use [logger](https://github.com/getoutreach/gobox/tree/master/tools/logger) @@ -35,22 +35,57 @@ to generate the `MarshalLog` method: ```bash -# run from directory where the OrgInfo struct is stored +# run from directory where the Org struct is stored $> go run github.com/getoutreach/gobox/tools/logger -output marshalers.go $> cat marshalers.go // Code generated by "logger -output marshalers.go"; DO NOT EDIT. package log -func (s *OrgInfo) MarshalLog(addField func(key string, value interface{})) { +func (s *Org) MarshalLog(addField func(key string, value interface{})) { if s == nil { return } - addField(or.org.shortname, Org) - addField(or.org.guid, Guid) + addField(or.org.shortname, Shortname) + addField(or.org.guid, GUID) } ``` +### Annotations +Supported annotations: + +- `omitempty`: makes the field optional. + + It is available for basic types, types aliased to a basic type, + and pointers of any type. The annotation parser does **not** + validate if the annotation is applied to supported types. + + Example: + + ```go + type Org struct + Shortname string `log:"or.org.shortname,omitempty"` + GUID *Custom `log:"or.org.guid,omitempty"` + } + ``` + + Generated code (post-formatted with `gofmt`): + + ```go + func (s *Org) MarshalLog(addField func(key string, value interface{})) { + if s == nil { + return + } + if s.Shortname != "" { + addField("or.org.shortname", s.Shortname) + } + if s.GUID != nil { + addField("or.org.guid", s.GUID) + } + } + ``` + + ## Using go generate Go `generate` is the standard way to generate pre-build artifacts diff --git a/tools/logger/logger.go b/tools/logger/logger.go index d865cd25..eff2a5c1 100644 --- a/tools/logger/logger.go +++ b/tools/logger/logger.go @@ -46,6 +46,10 @@ func (s *{{ .name }}) MarshalLog(addField func(key string, value interface{})) { addField("{{.key}}", s.{{.name}}.UTC().Format(time.RFC3339Nano))` simpleFieldFormat = ` addField("{{.key}}", s.{{.name}})` + optionalFieldFormat = ` +if s.{{.name}} != %s { + addField("{{.key}}", s.{{.name}}) +}` nestedMarshalerFormat = ` s.{{.name}}.MarshalLog(addField)` nestedNilableMarshalerFormat = ` @@ -54,6 +58,10 @@ if s.{{.name}} != nil { }` ) +const ( + annotationOmitEmpty = "omitempty" +) + func main() { flag.Usage = usage @@ -131,18 +139,29 @@ func filterStructs(pkg *packages.Package) ([]string, []*types.Struct) { func processStruct(w io.Writer, s *types.Struct, name string) { write(w, functionHeaderFormat, map[string]string{"name": name}) for kk := 0; kk < s.NumFields(); kk++ { - if field, ok := reflect.StructTag(s.Tag(kk)).Lookup("log"); ok { - args := map[string]string{"key": field, "name": s.Field(kk).Name()} - switch { - case s.Field(kk).Type().String() == "time.Time": - write(w, timeFieldFormat, args) - case field == "." && isNilable(s.Field(kk).Type()): - write(w, nestedNilableMarshalerFormat, args) - case field == ".": - write(w, nestedMarshalerFormat, args) - default: - write(w, simpleFieldFormat, args) - } + field, ok := reflect.StructTag(s.Tag(kk)).Lookup("log") + if !ok { + continue + } + + var annotations []string + fieldParts := strings.SplitN(field, ",", 2) + field = fieldParts[0] + if len(fieldParts) > 1 { + annotations = fieldParts[1:] + } + args := map[string]string{"key": field, "name": s.Field(kk).Name()} + switch { + case s.Field(kk).Type().String() == "time.Time": + write(w, timeFieldFormat, args) + case field == "." && isNilable(s.Field(kk).Type()): + write(w, nestedNilableMarshalerFormat, args) + case field == ".": + write(w, nestedMarshalerFormat, args) + case contains(annotations, annotationOmitEmpty): + write(w, getOptionalFieldFormat(s.Field(kk).Type()), args) + default: + write(w, simpleFieldFormat, args) } } fmt.Fprintf(w, "\n}\n") @@ -168,3 +187,31 @@ func usage() { fmt.Fprintf(os.Stderr, "Flags:\n") flag.PrintDefaults() } + +func getOptionalFieldFormat(p types.Type) string { + var defaultValue string + switch p.Underlying().String() { + case "string": + defaultValue = `""` + case "int", "int8", "int16", "int32", "int64", + "uint", "uint8", "uint16", "uint32", "uint64": + defaultValue = "0" + case "float32", "float64": + defaultValue = "0.0" + case "bool": + defaultValue = "false" + default: + defaultValue = "nil" + } + + return fmt.Sprintf(optionalFieldFormat, defaultValue) +} + +func contains[T comparable](slice []T, item T) bool { + for _, s := range slice { + if s == item { + return true + } + } + return false +} diff --git a/tools/logger/logger_test.go b/tools/logger/logger_test.go new file mode 100644 index 00000000..4cadf53c --- /dev/null +++ b/tools/logger/logger_test.go @@ -0,0 +1,262 @@ +package main + +import ( + "bytes" + "go/types" + "testing" + + "gotest.tools/v3/assert" +) + +func TestGetOptionalFieldFormat(t *testing.T) { + newNamedType := func(name string, underlying types.Type) types.Type { + return types.NewNamed( + types.NewTypeName(0, nil, name, nil), + underlying, + nil, + ) + } + + tests := []struct { + name string + typ types.Type + expected string + }{ + { + name: "string type", + typ: types.Typ[types.String], + expected: "\nif s.{{.name}} != \"\" {\n\taddField(\"{{.key}}\", s.{{.name}})\n}", + }, + { + name: "int type", + typ: types.Typ[types.Int], + expected: "\nif s.{{.name}} != 0 {\n\taddField(\"{{.key}}\", s.{{.name}})\n}", + }, + { + name: "float64 type", + typ: types.Typ[types.Float64], + expected: "\nif s.{{.name}} != 0.0 {\n\taddField(\"{{.key}}\", s.{{.name}})\n}", + }, + { + name: "bool type", + typ: types.Typ[types.Bool], + expected: "\nif s.{{.name}} != false {\n\taddField(\"{{.key}}\", s.{{.name}})\n}", + }, + { + name: "custom string type (type Token string)", + typ: newNamedType("Token", types.Typ[types.String]), + expected: "\nif s.{{.name}} != \"\" {\n\taddField(\"{{.key}}\", s.{{.name}})\n}", + }, + { + name: "custom int type (type Digits int)", + typ: newNamedType("Digits", types.Typ[types.Int]), + expected: "\nif s.{{.name}} != 0 {\n\taddField(\"{{.key}}\", s.{{.name}})\n}", + }, + { + name: "custom float type (type Decimal float64)", + typ: newNamedType("Decimal", types.Typ[types.Float64]), + expected: "\nif s.{{.name}} != 0.0 {\n\taddField(\"{{.key}}\", s.{{.name}})\n}", + }, + { + name: "custom bool type (type Truther bool)", + typ: newNamedType("Truther", types.Typ[types.Bool]), + expected: "\nif s.{{.name}} != false {\n\taddField(\"{{.key}}\", s.{{.name}})\n}", + }, + { + name: "pointer type", + typ: types.NewPointer(types.Typ[types.Int]), + expected: "\nif s.{{.name}} != nil {\n\taddField(\"{{.key}}\", s.{{.name}})\n}", + }, + { + name: "pointer to custom type", + typ: types.NewPointer(newNamedType("Nullable", types.Typ[types.String])), + expected: "\nif s.{{.name}} != nil {\n\taddField(\"{{.key}}\", s.{{.name}})\n}", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := getOptionalFieldFormat(tt.typ) + if got != tt.expected { + t.Errorf("getOptionalFieldFormat() =\n%v\nwant:\n%v", got, tt.expected) + } + }) + } +} + +func TestProcessStructWithTimeField(t *testing.T) { + var buf bytes.Buffer + timeType := types.NewNamed( + types.NewTypeName(0, types.NewPackage("time", "time"), "Time", nil), + types.NewStruct(nil, nil), + nil, + ) + + s := types.NewStruct([]*types.Var{ + types.NewVar(0, nil, "CreatedAt", timeType), + }, []string{ + `log:"created_at"`, + }) + + processStruct(&buf, s, "TimeStruct") + + expected := ` +func (s *TimeStruct) MarshalLog(addField func(key string, value interface{})) { + if s == nil { + return + } + +addField("created_at", s.CreatedAt.UTC().Format(time.RFC3339Nano)) +} +` + assert.Equal(t, expected, buf.String()) +} + +func TestProcessStructWithNestedMarshaler(t *testing.T) { + var buf bytes.Buffer + nestedType := types.NewNamed( + types.NewTypeName(0, nil, "NestedStruct", nil), + types.NewStruct(nil, nil), + nil, + ) + s := types.NewStruct([]*types.Var{ + types.NewVar(0, nil, "Child", types.NewPointer(nestedType)), + }, []string{ + `log:"."`, + }) + + processStruct(&buf, s, "ParentStruct") + + expected := ` +func (s *ParentStruct) MarshalLog(addField func(key string, value interface{})) { + if s == nil { + return + } + +if s.Child != nil { + s.Child.MarshalLog(addField) +} +} +` + assert.Equal(t, expected, buf.String()) +} + +func TestProcessStructWithOmitemptyField(t *testing.T) { + var buf bytes.Buffer + s := types.NewStruct([]*types.Var{ + types.NewVar(0, nil, "Status", types.Typ[types.String]), + }, []string{ + `log:"status,omitempty"`, + }) + + processStruct(&buf, s, "OptionalStruct") + + expected := ` +func (s *OptionalStruct) MarshalLog(addField func(key string, value interface{})) { + if s == nil { + return + } + +if s.Status != "" { + addField("status", s.Status) +} +} +` + assert.Equal(t, expected, buf.String()) +} + +func TestContains(t *testing.T) { + var uninitializedStrings []string + var uninitializedIntegers []int + var uninitializedBooleans []bool + tests := []struct { + name string + slice any + item any + underlyingType string + expected bool + }{ + { + name: "string slice with matching item", + slice: []string{"a", "b", "c"}, + item: "b", + underlyingType: "string", + expected: true, + }, + { + name: "string slice without matching item", + slice: []string{"a", "b", "c"}, + item: "d", + underlyingType: "string", + expected: false, + }, + { + name: "empty string slice", + slice: []string{}, + item: "a", + underlyingType: "string", + expected: false, + }, + { + name: "uninitialized string slice", + slice: uninitializedStrings, + item: "a", + underlyingType: "string", + expected: false, + }, + { + name: "int slice with matching item", + slice: []int{1, 2, 3}, + item: 2, + underlyingType: "int", + expected: true, + }, + { + name: "uninitialized int slice", + slice: uninitializedIntegers, + item: 4, + underlyingType: "int", + expected: false, + }, + { + name: "bool slice with matching item", + slice: []bool{true, false}, + item: true, + underlyingType: "bool", + expected: true, + }, + { + name: "bool slice without matching item", + slice: []bool{false}, + item: true, + underlyingType: "bool", + expected: false, + }, + { + name: "uninitialized bool slice", + slice: uninitializedBooleans, + item: false, + underlyingType: "bool", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + switch tt.underlyingType { + case "string": + if got := contains(tt.slice.([]string), tt.item.(string)); got != tt.expected { + t.Errorf("contains() = %v, want %v", got, tt.expected) + } + case "int": + if got := contains(tt.slice.([]int), tt.item.(int)); got != tt.expected { + t.Errorf("contains() = %v, want %v", got, tt.expected) + } + case "bool": + if got := contains(tt.slice.([]bool), tt.item.(bool)); got != tt.expected { + t.Errorf("contains() = %v, want %v", got, tt.expected) + } + } + }) + } +}