Skip to content

Commit

Permalink
feat: add logging support to generated clients (#1577)
Browse files Browse the repository at this point in the history 
This change instruments all generated clients with logging. In order to reduce the amount of codegen this would take I refactored some common logic about executing the request and parsing the response into a common helper. This change should reduce LoC for generated clients as a nice side-effect.
  • Loading branch information
@codyoss
codyoss authored Dec 6, 2024
1 parent 2b705af commit ae37381
Show file tree
Hide file tree
Showing 35 changed files with 298 additions and 187 deletions.
6 changes: 6 additions & 0 deletions internal/gengapic/client_init_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ func TestClientInit(t *testing.T) {
{Name: "iampb", Path: "cloud.google.com/go/iam/apiv1/iampb"}: true,
{Name: "locationpb", Path: "google.golang.org/genproto/googleapis/cloud/location"}: true,
{Name: "mypackagepb", Path: "github.com/googleapis/mypackage"}: true,
{Path: "log/slog"}: true,
},
wantNumSnps: 6,
},
Expand All @@ -436,6 +437,7 @@ func TestClientInit(t *testing.T) {
{Name: "iampb", Path: "cloud.google.com/go/iam/apiv1/iampb"}: true,
{Name: "locationpb", Path: "google.golang.org/genproto/googleapis/cloud/location"}: true,
{Name: "mypackagepb", Path: "github.com/googleapis/mypackage"}: true,
{Path: "log/slog"}: true,
},
wantNumSnps: 6,
},
Expand All @@ -453,6 +455,7 @@ func TestClientInit(t *testing.T) {
{Name: "gtransport", Path: "google.golang.org/api/transport/grpc"}: true,
{Name: "mypackagepb", Path: "github.com/googleapis/mypackage"}: true,
{Name: "httptransport", Path: "google.golang.org/api/transport/http"}: true,
{Path: "log/slog"}: true,
},
wantNumSnps: 1,
},
Expand All @@ -472,6 +475,7 @@ func TestClientInit(t *testing.T) {
{Path: "context"}: true,
{Path: "google.golang.org/api/option"}: true,
{Path: "google.golang.org/grpc"}: true,
{Path: "log/slog"}: true,
},
wantNumSnps: 6,
},
Expand All @@ -489,6 +493,7 @@ func TestClientInit(t *testing.T) {
{Path: "google.golang.org/api/option/internaloption"}: true,
{Path: "google.golang.org/grpc"}: true,
{Path: "net/http"}: true,
{Path: "log/slog"}: true,
},
wantNumSnps: 1,
},
Expand All @@ -506,6 +511,7 @@ func TestClientInit(t *testing.T) {
{Path: "net/http"}: true,
{Name: "httptransport", Path: "google.golang.org/api/transport/http"}: true,
{Name: "mypackagepb", Path: "github.com/googleapis/mypackage"}: true,
{Path: "log/slog"}: true,
},
wantNumSnps: 1,
setExt: func() (protoreflect.ExtensionType, interface{}) {
Expand Down
62 changes: 62 additions & 0 deletions internal/gengapic/gengapic.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,10 @@ func (g *generator) genAndCommitHelpers(scopes []string) error {
g.reset()
g.imports[pbinfo.ImportSpec{Path: "context"}] = true
g.imports[pbinfo.ImportSpec{Path: "google.golang.org/api/option"}] = true
g.imports[pbinfo.ImportSpec{Path: "github.com/googleapis/gax-go/v2/internallog"}] = true

p("const serviceName = %q", g.serviceConfig.GetName())
p("")

p("// For more information on implementing a client constructor hook, see")
p("// https://github.com/googleapis/google-cloud-go/wiki/Customizing-constructors.")
Expand All @@ -257,6 +261,64 @@ func (g *generator) genAndCommitHelpers(scopes []string) error {
}
p(" }")
p("}")
p("")
if containsTransport(g.opts.transports, rest) {
g.imports[pbinfo.ImportSpec{Path: "io"}] = true
g.imports[pbinfo.ImportSpec{Path: "log/slog"}] = true
g.imports[pbinfo.ImportSpec{Path: "net/http"}] = true
g.imports[pbinfo.ImportSpec{Path: "google.golang.org/api/googleapi"}] = true

p("func executeHTTPRequest(ctx context.Context, client *http.Client, req *http.Request, logger *slog.Logger, body []byte, rpc string) ([]byte, error) {")
p(` logger.DebugContext(ctx, "api request", "serviceName", serviceName, "rpcName", rpc, "request", internallog.HTTPRequest(req, body))`)
p(" resp, err := client.Do(req)")
p(" if err != nil{")
p(" return nil, err")
p(" }")
p(" defer resp.Body.Close()")
p(" buf, err := io.ReadAll(resp.Body)")
p(" if err != nil {")
p(" return nil, err")
p(" }")
p(` logger.DebugContext(ctx, "api response", "serviceName", serviceName, "rpcName", rpc, "response", internallog.HTTPResponse(resp, buf))`)
p(" if err = googleapi.CheckResponse(resp); err != nil {")
p(" return nil, err")
p(" }")
p(" return buf, nil")
p("}")
p("")

p("func executeStreamingHTTPRequest(ctx context.Context, client *http.Client, req *http.Request, logger *slog.Logger, body []byte, rpc string) (*http.Response, error) {")
p(` logger.DebugContext(ctx, "api request", "serviceName", serviceName, "rpcName", rpc, "request", internallog.HTTPRequest(req, body))`)
p(" resp, err := client.Do(req)")
p(" if err != nil{")
p(" return nil, err")
p(" }")
p(` logger.DebugContext(ctx, "api response", "serviceName", serviceName, "rpcName", rpc, "response", internallog.HTTPResponse(resp, nil))`)
p(" if err = googleapi.CheckResponse(resp); err != nil {")
p(" return nil, err")
p(" }")
p(" return resp, nil")
p("}")
p("")
}

if containsTransport(g.opts.transports, grpc) {
g.imports[pbinfo.ImportSpec{Path: "github.com/googleapis/gax-go/v2/internallog/grpclog"}] = true
g.imports[pbinfo.ImportSpec{Path: "google.golang.org/grpc"}] = true
g.imports[pbinfo.ImportSpec{Path: "google.golang.org/protobuf/proto"}] = true

p("func executeRPC[I proto.Message, O proto.Message](ctx context.Context, fn func(context.Context, I, ...grpc.CallOption) (O, error), req I, opts []grpc.CallOption, logger *slog.Logger, rpc string) (O, error) {")
p(" var zero O")
p(` logger.DebugContext(ctx, "api request", "serviceName", serviceName, "rpcName", rpc, "request", grpclog.ProtoMessageRequest(ctx, req))`)
p(" resp, err := fn(ctx, req, opts...)")
p(" if err != nil {")
p(" return zero, err")
p(" }")
p(` logger.DebugContext(ctx, "api response", "serviceName", serviceName, "rpcName", rpc, "response", grpclog.ProtoMessageResponse(resp))`)
p(" return resp, err")
p("}")
p("")
}

outFile := filepath.Join(g.opts.outDir, "helpers.go")
g.commit(outFile, g.opts.pkgName)
Expand Down
4 changes: 2 additions & 2 deletions internal/gengapic/gengapic_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1128,12 +1128,12 @@ func TestGRPCStubCall(t *testing.T) {
}{
{
name: "foo.FooService.GetFoo",
want: "c.client.GetFoo(ctx, req, settings.GRPC...)",
want: `executeRPC(ctx, c.client.GetFoo, req, settings.GRPC, c.logger, "GetFoo")`,
in: getFoo,
},
{
name: "foo.BarService.GetBar",
want: "c.barClient.GetBar(ctx, req, settings.GRPC...)",
want: `executeRPC(ctx, c.barClient.GetBar, req, settings.GRPC, c.logger, "GetBar")`,
in: getBar,
},
} {
Expand Down
6 changes: 5 additions & 1 deletion internal/gengapic/gengrpc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func (g *generator) emptyUnaryGRPCCall(servName string, m *descriptorpb.MethodDe
func (g *generator) grpcStubCall(method *descriptorpb.MethodDescriptorProto) string {
service := g.descInfo.ParentElement[method]
stub := pbinfo.ReduceServName(service.GetName(), g.opts.pkgName)
return fmt.Sprintf("c.%s.%s(ctx, req, settings.GRPC...)", grpcClientField(stub), method.GetName())
return fmt.Sprintf("executeRPC(ctx, c.%s.%s, req, settings.GRPC, c.logger, %q)", grpcClientField(stub), method.GetName(), method.GetName())
}

func (g *generator) grpcClientOptions(serv *descriptorpb.ServiceDescriptorProto, servName string) error {
Expand Down Expand Up @@ -295,6 +295,8 @@ func (g *generator) grpcClientInit(serv *descriptorpb.ServiceDescriptorProto, se

p("// The x-goog-* metadata to be sent with each request.")
p("xGoogHeaders []string")
p("")
p("logger *slog.Logger")

p("}")
p("")
Expand All @@ -303,6 +305,7 @@ func (g *generator) grpcClientInit(serv *descriptorpb.ServiceDescriptorProto, se
g.imports[imp] = true

g.grpcClientUtilities(serv, servName, imp, hasRPCForLRO)
g.imports[pbinfo.ImportSpec{Path: "log/slog"}] = true
}

func (g *generator) grpcClientUtilities(serv *descriptorpb.ServiceDescriptorProto, servName string, imp pbinfo.ImportSpec, hasRPCForLRO bool) {
Expand Down Expand Up @@ -337,6 +340,7 @@ func (g *generator) grpcClientUtilities(serv *descriptorpb.ServiceDescriptorProt
p(" connPool: connPool,")
p(" %s: %s.New%sClient(connPool),", grpcClientField(servName), imp.Name, serv.GetName())
p(" CallOptions: &client.CallOptions,")
p(" logger: internaloption.GetLogger(opts),")
g.mixinStubsInit()
p("")
p(" }")
Expand Down
Loading

0 comments on commit ae37381

Please sign in to comment.