diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc b/compiler/cpp/src/thrift/generate/t_go_generator.cc index 54422c82670..7718a0f878c 100644 --- a/compiler/cpp/src/thrift/generate/t_go_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_go_generator.cc @@ -706,6 +706,7 @@ string t_go_generator::go_imports_begin(bool consts) { } system_packages.push_back("errors"); system_packages.push_back("fmt"); + system_packages.push_back("log/slog"); system_packages.push_back("time"); // For the thrift import, always do rename import to make sure it's called thrift. system_packages.push_back("thrift \"" + gen_thrift_import_ + "\""); @@ -726,12 +727,13 @@ string t_go_generator::go_imports_end() { string import_end = string( ")\n\n" "// (needed to ensure safety because of naive import list construction.)\n" - "var _ = thrift.ZERO\n" - "var _ = fmt.Printf\n" - "var _ = errors.New\n" + "var _ = bytes.Equal\n" "var _ = context.Background\n" + "var _ = errors.New\n" + "var _ = fmt.Printf\n" + "var _ = slog.Log\n" "var _ = time.Now\n" - "var _ = bytes.Equal\n" + "var _ = thrift.ZERO\n" "// (needed by validator.)\n" "var _ = strings.Contains\n" "var _ = regexp.MatchString\n\n"); @@ -1308,7 +1310,9 @@ void t_go_generator::generate_go_struct_definition(ostream& out, indent_down(); out << indent() << "}" << endl << endl; out << indent() << "func New" << tstruct_name << "() *" << tstruct_name << " {" << endl; - out << indent() << " return &"; + indent_up(); + out << indent() << "return &"; + indent_down(); generate_go_struct_initializer(out, tstruct, is_result || is_args); out << indent() << "}" << endl << endl; // Default values for optional fields @@ -1339,16 +1343,22 @@ void t_go_generator::generate_go_struct_definition(ostream& out, string maybepointer = goOptType != goType ? "*" : ""; out << indent() << "func (p *" << tstruct_name << ") Get" << publicized_name << "() " << goType << " {" << endl; - out << indent() << " if !p.IsSet" << publicized_name << "() {" << endl; - out << indent() << " return " << def_var_name << endl; - out << indent() << " }" << endl; + indent_up(); + out << indent() << "if !p.IsSet" << publicized_name << "() {" << endl; + indent_up(); + out << indent() << "return " << def_var_name << endl; + indent_down(); + out << indent() << "}" << endl; out << indent() << "return " << maybepointer << "p." << publicized_name << endl; + indent_down(); out << indent() << "}" << endl; } else { out << endl; out << indent() << "func (p *" << tstruct_name << ") Get" << publicized_name << "() " << goType << " {" << endl; - out << indent() << " return p." << publicized_name << endl; + indent_up(); + out << indent() << "return p." << publicized_name << endl; + indent_down(); out << indent() << "}" << endl; } } @@ -1365,11 +1375,15 @@ void t_go_generator::generate_go_struct_definition(ostream& out, } out << indent() << "func (p *" << tstruct_name << ") String() string {" << endl; - out << indent() << " if p == nil {" << endl; - out << indent() << " return \"\"" << endl; - out << indent() << " }" << endl; - out << indent() << " return fmt.Sprintf(\"" << escape_string(tstruct_name) << "(%+v)\", *p)" + indent_up(); + out << indent() << "if p == nil {" << endl; + indent_up(); + out << indent() << "return \"\"" << endl; + indent_down(); + out << indent() << "}" << endl; + out << indent() << "return fmt.Sprintf(\"" << escape_string(tstruct_name) << "(%+v)\", *p)" << endl; + indent_down(); out << indent() << "}" << endl << endl; if (is_exception) { @@ -1388,6 +1402,28 @@ void t_go_generator::generate_go_struct_definition(ostream& out, out << indent() << "var _ thrift.TException = (*" << tstruct_name << ")(nil)" << endl << endl; } + + // Generate the implementation of slog.LogValuer, + // see: https://issues.apache.org/jira/browse/THRIFT-5745 + out << indent() << "func (p *" << tstruct_name << ") LogValue() slog.Value {" << endl; + indent_up(); + out << indent() << "if p == nil {" << endl; + indent_up(); + out << indent() << "return slog.AnyValue(nil)" << endl; + indent_down(); + out << indent() << "}" << endl; + out << indent() << "v := thrift.SlogTStructWrapper{" << endl; + indent_up(); + out << indent() << "Type: \"*" << package_name_ << "." << tstruct_name << "\"," << endl; + out << indent() << "Value: p," << endl; + indent_down(); + out << indent() << "}" << endl; + out << indent() << "return slog.AnyValue(v)" << endl; + indent_down(); + out << indent() << "}" << endl << endl; + + out << indent() << "var _ slog.LogValuer = (*" << tstruct_name << ")(nil)" + << endl << endl; } /** diff --git a/lib/go/test/tests/slog_tstruct_wrapper_test.go b/lib/go/test/tests/slog_tstruct_wrapper_test.go new file mode 100644 index 00000000000..b248cf2c5f2 --- /dev/null +++ b/lib/go/test/tests/slog_tstruct_wrapper_test.go @@ -0,0 +1,132 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package tests + +import ( + "log/slog" + "strings" + "testing" + + "github.com/apache/thrift/lib/go/test/gopath/src/forwardtypetest" + "github.com/apache/thrift/lib/go/thrift" +) + +func dropTime(groups []string, a slog.Attr) slog.Attr { + if len(groups) == 0 && a.Key == slog.TimeKey { + return slog.Attr{} + } + return a +} + +func TestSlogTStructWrapperJSON(t *testing.T) { + for _, c := range []struct { + label string + value thrift.TStruct + want string + }{ + { + label: "struct", + value: &forwardtypetest.Struct{ + Foo: &forwardtypetest.Exc{ + Code: thrift.Int32Ptr(10), + }, + }, + want: `{"level":"INFO","msg":"bar","struct":{"type":"*forwardtypetest.Struct","value":{"foo":{"code":10}}}}`, + }, + { + label: "exception", + value: &forwardtypetest.Exc{ + Code: thrift.Int32Ptr(10), + }, + want: `{"level":"INFO","msg":"bar","struct":{"type":"*forwardtypetest.Exc","value":{"code":10}}}`, + }, + { + label: "nil-struct", + value: (*forwardtypetest.Struct)(nil), + want: `{"level":"INFO","msg":"bar","struct":null}`, + }, + { + label: "exception", + value: (*forwardtypetest.Exc)(nil), + want: `{"level":"INFO","msg":"bar","struct":null}`, + }, + } { + t.Run(c.label, func(t *testing.T) { + var sb strings.Builder + logger := slog.New(slog.NewJSONHandler(&sb, &slog.HandlerOptions{ + AddSource: false, + ReplaceAttr: dropTime, + })) + + logger.Info("bar", "struct", c.value) + if got := strings.TrimSuffix(sb.String(), "\n"); got != c.want { + t.Errorf("got %q want %q", got, c.want) + } + }) + } +} + +func TestSlogTStructWrapperText(t *testing.T) { + for _, c := range []struct { + label string + value thrift.TStruct + want string + }{ + { + label: "struct", + value: &forwardtypetest.Struct{ + Foo: &forwardtypetest.Exc{ + Code: thrift.Int32Ptr(10), + }, + }, + want: `level=INFO msg=bar struct="*forwardtypetest.Struct{\"foo\":{\"code\":10}}"`, + }, + { + label: "exception", + value: &forwardtypetest.Exc{ + Code: thrift.Int32Ptr(10), + }, + want: `level=INFO msg=bar struct="*forwardtypetest.Exc{\"code\":10}"`, + }, + { + label: "nil-struct", + value: (*forwardtypetest.Struct)(nil), + want: `level=INFO msg=bar struct=`, + }, + { + label: "exception", + value: (*forwardtypetest.Exc)(nil), + want: `level=INFO msg=bar struct=`, + }, + } { + t.Run(c.label, func(t *testing.T) { + var sb strings.Builder + logger := slog.New(slog.NewTextHandler(&sb, &slog.HandlerOptions{ + AddSource: false, + ReplaceAttr: dropTime, + })) + + logger.Info("bar", "struct", c.value) + if got := strings.TrimSuffix(sb.String(), "\n"); got != c.want { + t.Errorf("got %q want %q", got, c.want) + } + }) + } +} diff --git a/lib/go/thrift/slog.go b/lib/go/thrift/slog.go new file mode 100644 index 00000000000..d9b79d2f5db --- /dev/null +++ b/lib/go/thrift/slog.go @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package thrift + +import ( + "encoding/json" + "fmt" + "strings" +) + +// SlogTStructWrapper is a wrapper used by the compiler to wrap TStruct and +// TException to be better logged by slog. +type SlogTStructWrapper struct { + Type string `json:"type"` + Value TStruct `json:"value"` +} + +var ( + _ fmt.Stringer = SlogTStructWrapper{} + _ json.Marshaler = SlogTStructWrapper{} +) + +func (w SlogTStructWrapper) MarshalJSON() ([]byte, error) { + // Use an alias to avoid infinite loop + type alias SlogTStructWrapper + return json.Marshal(alias(w)) +} + +func (w SlogTStructWrapper) String() string { + var sb strings.Builder + sb.WriteString(w.Type) + if err := json.NewEncoder(&sb).Encode(w.Value); err != nil { + // Should not happen, but just in case + return fmt.Sprintf("%s: %v", w.Type, w.Value) + } + // json encoder will write an additional \n at the end, get rid of it + return strings.TrimSuffix(sb.String(), "\n") +} diff --git a/lib/go/thrift/slog_test.go b/lib/go/thrift/slog_test.go new file mode 100644 index 00000000000..d40c98b4398 --- /dev/null +++ b/lib/go/thrift/slog_test.go @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package thrift + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestSlogTStructWrapperJSON(t *testing.T) { + // This test just ensures that we don't have infinite loop when json + // encoding it + v := SlogTStructWrapper{Type: "foo"} + var sb strings.Builder + if err := json.NewEncoder(&sb).Encode(v); err != nil { + t.Fatal(err) + } + t.Log(strings.TrimSuffix(sb.String(), "\n")) +}