You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by yu...@apache.org on 2023/12/04 20:35:39 UTC
(thrift) branch go-log-valuer updated (513fa096b -> a234ff825)
This is an automated email from the ASF dual-hosted git repository.
yuxuan pushed a change to branch go-log-valuer
in repository https://gitbox.apache.org/repos/asf/thrift.git
omit 513fa096b THRIFT-5745: Implement slog.LogValuer on go TStructs
omit f4c63e260 THRIFT-5744: Switch to slog for go library
add 4115e952b THRIFT-5746 Upgrade to net8 Client: netstd Patch: Jens Geyer
add ac2deed0f THRIFT-5746 Upgrade to net8 Client: netstd Patch: Jens Geyer
add 8a6bcc76f THRIFT-5747 warning: The macro `AC_HELP_STRING' is obsolete. Patch: Jens Geyer
add 87e7be754 Bump com.diffplug.spotless from 6.22.0 to 6.23.2 in /lib/kotlin
add a58e91533 Bump jvm from 1.9.20 to 1.9.21 in /lib/kotlin
add f4b6d9600 Bump actions/setup-java from 3 to 4
add 697c08bad Bump com.github.spotbugs from 5.2.1 to 6.0.1 in /lib/kotlin
add 022d0278e Github Actions: Remove --disable-tests
add 10caefc63 THRIFT-5744: Switch to slog for go library
new a234ff825 THRIFT-5745: Implement slog.LogValuer on go TStructs
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (513fa096b)
\
N -- N -- N refs/heads/go-log-valuer (a234ff825)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.github/workflows/build.yml | 5 +-
aclocal/ax_thrift_internal.m4 | 2 +-
build/docker/old/debian-stretch/Dockerfile | 8 +-
build/docker/old/ubuntu-artful/Dockerfile | 8 +-
build/docker/old/ubuntu-disco/Dockerfile | 8 +-
build/docker/old/ubuntu-xenial/Dockerfile | 8 +-
build/docker/ubuntu-bionic/Dockerfile | 8 +-
build/docker/ubuntu-focal/Dockerfile | 8 +-
build/docker/ubuntu-jammy/Dockerfile | 8 +-
.../cpp/src/thrift/generate/t_netstd_generator.cc | 225 ++++++++++++++-------
.../cpp/src/thrift/generate/t_netstd_generator.h | 6 +-
configure.ac | 7 +-
contrib/fb303/aclocal/ax_thrift_internal.m4 | 2 +-
lib/go/test/fuzz/Makefile.am | 9 +-
lib/go/test/fuzz/fuzz.go | 6 +-
lib/go/test/fuzz/go.mod | 12 +-
lib/go/test/fuzz/go.sum | 0
lib/java/build.gradle | 4 +-
lib/kotlin/settings.gradle.kts | 2 +-
.../Thrift.Benchmarks/Thrift.Benchmarks.csproj | 5 +-
.../Thrift.IntegrationTests.csproj | 9 +-
.../Thrift.PublicInterfaces.Compile.Tests.csproj | 21 +-
.../Thrift.Tests/Collections/TCollectionsTests.cs | 40 ++--
.../Tests/Thrift.Tests/DataModel/DeepCopy.cs | 79 +++-----
.../Tests/Thrift.Tests/DataModel/NullValuesSet.cs | 1 +
.../Protocols/TJsonProtocolHelperTests.cs | 54 ++---
.../Thrift.Tests/Protocols/TJsonProtocolTests.cs | 6 +-
lib/netstd/Tests/Thrift.Tests/Thrift.Tests.csproj | 9 +-
lib/netstd/Thrift/.editorconfig | 5 +
.../Thrift/Processor/TMultiplexedProcessor.cs | 8 +-
.../Thrift/Processor/TSingletonProcessorFactory.cs | 7 +-
lib/netstd/Thrift/Protocol/Entities/TField.cs | 5 +-
lib/netstd/Thrift/Protocol/Entities/TList.cs | 5 +-
lib/netstd/Thrift/Protocol/Entities/TMap.cs | 5 +-
lib/netstd/Thrift/Protocol/Entities/TMessage.cs | 5 +-
lib/netstd/Thrift/Protocol/Entities/TSet.cs | 5 +-
lib/netstd/Thrift/Protocol/Entities/TStruct.cs | 3 +
lib/netstd/Thrift/Protocol/TBinaryProtocol.cs | 17 +-
lib/netstd/Thrift/Protocol/TCompactProtocol.cs | 2 +
lib/netstd/Thrift/Protocol/TJSONProtocol.cs | 2 +
lib/netstd/Thrift/Protocol/TMultiplexedProtocol.cs | 3 +
lib/netstd/Thrift/Protocol/ToString.cs | 20 +-
.../Thrift/Protocol/Utilities/TBase64Utils.cs | 8 +-
.../Thrift/Protocol/Utilities/TGuidExtensions.cs | 4 +-
.../Protocol/Utilities/TJsonProtocolConstants.cs | 3 +
lib/netstd/Thrift/Server/TServer.cs | 3 +-
lib/netstd/Thrift/Server/TSimpleAsyncServer.cs | 2 +
lib/netstd/Thrift/Thrift.csproj | 16 +-
.../Thrift/Transport/Client/TTlsSocketTransport.cs | 3 +
.../Thrift/Transport/Layered/TFramedTransport.cs | 4 +
.../Thrift/Transport/Layered/TLayeredTransport.cs | 7 +-
.../Transport/Server/TNamedPipeServerTransport.cs | 10 +-
.../Transport/Server/TServerSocketTransport.cs | 3 +
.../Thrift/Transport/Server/TServerTransport.cs | 13 +-
lib/netstd/Thrift/Transport/TTransport.cs | 6 +-
test/netstd/Client/Client.csproj | 14 +-
test/netstd/Client/Performance/PerformanceTests.cs | 6 +-
test/netstd/Client/TestClient.cs | 24 +--
test/netstd/Server/Server.csproj | 14 +-
test/netstd/Server/TestServer.cs | 25 ++-
tutorial/netstd/Client/Client.csproj | 4 +-
tutorial/netstd/Interfaces/Interfaces.csproj | 4 +-
tutorial/netstd/Server/Server.csproj | 4 +-
63 files changed, 468 insertions(+), 361 deletions(-)
delete mode 100644 lib/go/test/fuzz/go.sum
(thrift) 01/01: THRIFT-5745: Implement slog.LogValuer on go TStructs
Posted by yu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yuxuan pushed a commit to branch go-log-valuer
in repository https://gitbox.apache.org/repos/asf/thrift.git
commit a234ff825cf32589813bc297ba89d37bef246004
Author: Yuxuan 'fishy' Wang <yu...@reddit.com>
AuthorDate: Tue Nov 21 16:42:00 2023 -0800
THRIFT-5745: Implement slog.LogValuer on go TStructs
Client: go
Implement slog.LogValuer for all TStruct and TException generated by the
compiler for go code. Also add SlogTStructWrapper in the library so we
don't have to repeat it in the compiler generated go code.
---
compiler/cpp/src/thrift/generate/t_go_generator.cc | 64 ++++++++--
lib/go/test/tests/slog_tstruct_wrapper_test.go | 132 +++++++++++++++++++++
lib/go/thrift/slog.go | 55 +++++++++
lib/go/thrift/slog_test.go | 37 ++++++
4 files changed, 275 insertions(+), 13 deletions(-)
diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc b/compiler/cpp/src/thrift/generate/t_go_generator.cc
index 54422c826..5bb71fcbd 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 \"<nil>\"" << 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 \"<nil>\"" << 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,30 @@ void t_go_generator::generate_go_struct_definition(ostream& out,
out << indent() << "var _ thrift.TException = (*" << tstruct_name << ")(nil)"
<< endl << endl;
}
+
+ if (!read_write_private_) {
+ // 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 000000000..b248cf2c5
--- /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=<nil>`,
+ },
+ {
+ label: "exception",
+ value: (*forwardtypetest.Exc)(nil),
+ want: `level=INFO msg=bar struct=<nil>`,
+ },
+ } {
+ 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 000000000..22545d843
--- /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 recursion
+ 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 000000000..f4155d4e0
--- /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 recursion when
+ // json encoding it. More comprehensive tests are under lib/go/test.
+ 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"))
+}