feat: Add google.api.api_version annotation to echo.proto

[parthea]

This PR allows folks to run manual tests against AIP-4236 (See aip-dev/google.aip.dev#1331)

gapic-showcase run -v will log the request headers in verbose mode as per https://github.com/googleapis/gapic-showcase/blob/main/CHANGELOG.md#v090--2020-04-22

I created #1493 for echoing the x-goog-api-version header so that it can be used in automated integration tests.

[parthea]

Link to AIP once aip-dev/google.aip.dev#1331 is merged

[parthea]

Wait for #1485 which should resolve the error below which appears in https://github.com/googleapis/gapic-showcase/actions/runs/8402715704/job/23012403762?pr=1484

2024/03/23 15:19:14 google/showcase/v1beta1/echo.proto:47:10: Option "(google.api.api_version)" unknown. Ensure that your proto definition file imports the proto which defines the option.

Submodule path 'schema/googleapis': checked out '5c1f64e746af6e53713884df3eb7719f3956a66f'

PR #1485 updates the git submodule from SHA 5c1f64e746af6e53713884df3eb7719f3956a66f to SHA d81d0b9e6993d6ab425dff4d7c3d05fb2e59fa57. See the diff here which includes the google.api.api_version annotation.

[noahdietz]

Adding the annotation is fine & necessary, but how do you expect generator integration tests to test that the generated code sends the proper api version?

There are two likely routes:

• the showcase server "echos" back the metadata/headers it was sent as trailing metadata/response headers so that the generator integration tests can capture them and assert what it sent was expected
• the server reports an error/fails the request if the API version isn't present in (one of) the expected places

I'd prefer the first one as it is not a breaking change.

[noahdietz]

Here is where it currently echos things back through the response:

gapic-showcase/server/services/echo_service.go

Lines 281 to 307 in 0622c2f

	func echoHeaders(ctx context.Context) {
	md, ok := metadata.FromIncomingContext(ctx)
	if !ok {
	return
	}
	values := md.Get("x-goog-request-params")
	for _, value := range values {
	header := metadata.Pairs("x-goog-request-params", value)
	grpc.SetHeader(ctx, header)
	}
	}
	func echoStreamingHeaders(stream grpc.ServerStream) {
	md, ok := metadata.FromIncomingContext(stream.Context())
	if !ok {
	return
	}
	values := md.Get("x-goog-request-params")
	for _, value := range values {
	header := metadata.Pairs("x-goog-request-params", value)
	if stream.SetHeader(header) != nil {
	return
	}
	}
	}

[noahdietz]

I think this is the proper place to implement additional HTTP header/query string translation assuming API version will be a System Parameter:

gapic-showcase/util/genrest/resttools/systemparam.go

Lines 24 to 47 in 0622c2f

	// SystemParameters encapsulates the system parameters recognized by Showcase. These are a subset of
	// Google's accepted system parameters described in
	// https://cloud.google.com/apis/docs/system-parameters.
	type SystemParameters struct {
	EnumEncodingAsInt bool
	}
	// GetSystemParameters returns the SystemParameters encoded in request, and the query parameters in
	// the request's query string that are not themselves system parameters.
	func GetSystemParameters(request *http.Request) (systemParams *SystemParameters, queryParams map[string][]string, err error) {
	return processQueryString(request.URL.RawQuery)
	}
	// processQueryString returns the SystemParameters encoded in queryString, and the query parameters in
	// queryString that are not themselves system parameters.
	//
	// Since we want GAPICs to be strict in what they emit, and Showcase helps guarantee that, this
	// function is strict in what it accepts:
	// - no more than one instance of the $alt system parameter
	// - $alt may be appear as "$alt" or "alt", always lower case
	// - the only possible values for $alt are "json" or "json;enum-encoding=int"; again, lower case
	// - the semicolon in "json;enum-encoding=int" must be URL escaped as "%3B" or "%3b"
	// - the equal sign in "json;enum-encoding=int" may or may not be URL-escaped
	func processQueryString(queryString string) (systemParams *SystemParameters, queryParams map[string][]string, err error) {

[noahdietz]

There might also be some code that needs updating in the genrest generator to ensure that system params end up being sent to the grpc handler implementation as grpc metadata.

[noahdietz]

See comments.

[parthea]

Adding the annotation is fine & necessary, but how do you expect generator integration tests to test that the generated code sends the proper api version?

I only performed manual tests using this updated proto. gapic-showcase run -v will log the request headers in verbose mode as per https://github.com/googleapis/gapic-showcase/blob/main/CHANGELOG.md#v090--2020-04-22. I can see that the header exists in the logs. I created #1493 to follow up on adding support to echo back the x-goog-api-version header sent with the request to the showcase server.

2024/04/08 22:35:34 Received Unary Request for Method: /google.showcase.v1beta1.Echo/Echo
2024/04/08 22:35:34     Request headers:
2024/04/08 22:35:34       content-type: application/grpc
2024/04/08 22:35:34       user-agent: grpc-python/1.63.0.dev0 grpc-c/39.0.0 (linux; chttp2)
2024/04/08 22:35:34       x-goog-api-version: moose

[parthea]

@noahdietz, Please could you take another look?

[noahdietz]

Ok and just to make sure.....we don't want a "real" version here?

I understand from generator/client perspective this is an opaque string, but there is something to be said about having good test data...

[parthea]

Fixed in a377321

[noahdietz]

I created #1493 to follow up on adding support to echo back the x-goog-api-version header sent with the request to the showcase server.

Thanks SGTM