bigquery/storage/managedwriter: unclear how to send data formatted using nested protobuf messages

[GlenDC]

Client

bigquery/storage/managedwriter.Client

Environment

  System Version:   macOS 11.6 (20G165)
  Kernel Version:   Darwin 20.6.0

Go Environment

➜ go version
go version go1.17 darwin/amd64

➜ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/glendc/Library/Caches/go-build"
GOENV="/Users/glendc/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/glendc/go/pkg/mod"
GONOPROXY="none"
GONOSUMDB="*.otainsight.com"
GOOS="darwin"
GOPATH="/Users/glendc/go"
GOPRIVATE="*.otainsight.com"
GOPROXY="https://A0nDv-BXv8hwgMEClBkwFucZBbll3v678@go-proxy.fury.io/otainsight/"
GOROOT="/usr/local/Cellar/go/1.17/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.17/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/glendc/code/github.com/OTA-Insight/bqwriter/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/sw/7chll2gj4bb7lw75h8t2svf40000gn/T/go-build1869597339=/tmp/go-build -gno-record-gcc-switches -fno-common"

Code

syntax = "proto3";
package benchmark;
option go_package = "github.com/OTA-Insight/bqwriter/internal/test/benchmark";

import "google/protobuf/timestamp.proto";

message TemporaryDataParameterProto3 {
    string name = 1;
    bytes value = 2;
}

message TemporaryDataProto3 {
    string name = 1;
    int64 uuid = 2;
    google.protobuf.Timestamp timestamp = 3;
    bool truth = 4;
    repeated benchmark.TemporaryDataParameterProto3 parameters = 5;
}

// ...

writer, err := managedwriter.NewClient(/* ... */)
// ...

// ...

protoDescriptor := protodesc.ToDescriptorProto((&TemporaryDataProto3{}).ProtoReflect().Descriptor())
writerOpts := []managedwriter.WriterOption{
    // ...
    managedwriter.WithSchemaDescriptor(protoDescriptor),
    // ...
}

// ...

writer.NewManagedStream(ctx, writerOpts...)

// ...

// later I use `proto.Marshal` to marshal such a created value `TemporaryDataProto3`
// and use `AppendRows` to add it to the `_default` stream

Expected behavior

I would expect this to work and send the data to BQ

Actual behavior

Get errors:

ready append resulted in error: rpc error: code = InvalidArgument desc = Invalid proto schema: BqMessage.proto: TemporaryDataProto3.timestamp: ".google.protobuf.Timestamp" is not defined.
BqMessage.proto: TemporaryDataProto3.parameters: ".benchmark.TemporaryDataParameterProto3" is not defined. Entity: projects/oi-bigquery/datasets/benchmarks_bqwriter/tables/tmp/_default
ready append resulted in error: rpc error: code = InvalidArgument desc = Invalid proto schema: BqMessage.proto: TemporaryDataProto3.timestamp: ".google.protobuf.Timestamp" is not defined.
BqMessage.proto: TemporaryDataProto3.parameters: ".benchmark.TemporaryDataParameterProto3" is not defined. Entity: projects/oi-bigquery/datasets/benchmarks_bqwriter/tables/tmp/_default

I tried to fix it with a hack such as:

protoDescriptor := protodesc.ToDescriptorProto((&TemporaryDataProto3{}).ProtoReflect().Descriptor())
protoDescriptor.NestedType = append(
    protoDescriptor.NestedType,
    protodesc.ToDescriptorProto((&timestamppb.Timestamp{}).ProtoReflect().Descriptor()),
    protodesc.ToDescriptorProto((&TemporaryDataParameterProto3{}).ProtoReflect().Descriptor()),
)

That doesn't work either, please I cannot imagine that it is ever the goal for us to having to create such a type descriptor ourselves, so lengthy and cryptic.

I got only so far by the way by following examples such as https://github.com/googleapis/google-cloud-go/blob/master/bigquery/storage/managedwriter/integration_test.go#L280 as the documentation was for me like chinese when it came to knowing how to pass on my data... Might be just me with my lack of experience with this entire stack, but never the less I believe the documentation can use a lot of work here.

[shollyman]

I've got a TODO that's basically samples for doing all_the_things using the managedwriter. I expect to get to that soon.

There's two issues as I can see in your report:

1) You're using proto3, which unfortunately does not work as expected with the write API currently, which expects proto2 semantics for processing the differences in null vs default values. This isn't the source of the error, but it is a significant gotcha.

2) You're using an un-normalized descriptor for the schema. The schema for the stream must be a self-contained descriptor, but you've got two descriptors: TemporaryDataProto3 and TemporaryDataParameterProto3. Fortunately, there's a normalization function that will rewrite and consolidate the messages. It is present in the adapt subpackage: https://pkg.go.dev/cloud.google.com/go/bigquery/storage/managedwriter/adapt#NormalizeDescriptor

[GlenDC]

Perfect, these 2 points that you noticed seem to be the main issues I was facing indeed. Now it works a lot better. What I do not know however is what I should use for known types such as TIMESTAMP? In proto3 I was using import "google/protobuf/timestamp.proto";, however that seems to give me the following errors when using a proto2 based model:

ready append resulted in error: rpc error: code = InvalidArgument desc = The proto field mismatched with BigQuery field at benchmark_TemporaryDataProto2.timestamp, the proto field type message, BigQuery field type TIMESTAMP Entity: projects/oi-bigquery/datasets/benchmarks_bqwriter/tables/tmp/_default

Having this documented could be nice as well I imagine.

[shollyman]

The proto well-known types aren't yet properly supported, and Timestamp is among them. The public docs have a section on wire format conversions: https://cloud.google.com/bigquery/docs/write-api#data_type_conversions. Short answer: send an int64 with epoch-micros.

[GlenDC]

Alright perfect. Having that clearer in the documentation could be a nice bonus. Either way, this does answer my question and as such my last remaining question has been resolved.

I did find a lot of issues with type definitions in general. Another tricky one I found is how when using the batch client with auto-detected bigQuery schema that the casing somehow has to be correct, despite the field names being case insensitive. Yet you get duplicate field errors if you don't do so.

A lot of implicit knowledge is required. Perhaps that's unavoidable, but I do feel that a high level client such as managed writer, which already abstracts a lot of the implicit required knowledge away, could benefit from documenting quite extensively how types are expected and how not. Certainly not a deal breaker, but it would increase its value by a lot IMHO.

Either way, for what I'm concerned the issue can be closed as resolved. Thanks!

[GlenDC]

As a last note, please do know that I am willing to contribute to the BQ packages, so feel free to reach out to me, or mark issues with extensive details that others can pick up. Would be happy to help out as a token of gratitude.

[shollyman]

It's great to get feedback on sharp edges for this API and the managedwriter abstraction, so thanks for being so clear and detailed. I just wanted to note I'll have limited availability for the next few weeks due to the holidays, so responses may be delayed.

Might be worth taking a look at https://github.com/googleapis/google-cloud-go/pull/5102; it proposes some further evolution of the AppendRows() method.

[shollyman]

Closing this one for now. https://pkg.go.dev/cloud.google.com/go/bigquery/storage/managedwriter has an augmented godoc that covers sharp edges

[brunsgaard]

@shollyman Thanks for your answer on this thread. Do you know if there is a plan to support the Timestamp proto type in the future by the backend team?