fix(internal/gapicgen): use correct region tags for gensnippets (#4022)

A side effect of this is that only methods that correspond to RPCs get
region tags.

I changed the error handling so any snippet issues are only warnings and
don't `log.Fatal` (cc @noahdietz).

I deleted and regenerated all of the snippets, which led to non-Method
snippets (e.g. `NewClient`-type functions) being deleted.

The first commit has the actual generator changes. The second commit is
the regeneration.

cc @amanda-tarafa.

internal/gapicgen/generator/gapics.go

@@ -111,7 +111,7 @@ func (g *GapicGenerator) RegenSnippets(ctx context.Context) error {
 		return err
 	}
 	if err := gensnippets.Generate(g.googleCloudDir, snippetDir, apiShortnames); err != nil {
-		return fmt.Errorf("error generating snippets: %v", err)
+		log.Printf("warning: got the following non-fatal errors generating snippets: %v", err)
 	}
 	if err := replaceAllForSnippets(g.googleCloudDir, snippetDir); err != nil {
 		return err

internal/gapicgen/gensnippets/gensnippets.go

@@ -31,6 +31,8 @@ import (
 	"cloud.google.com/go/internal/godocfx/pkgload"
 	"cloud.google.com/go/third_party/go/doc"
 	"golang.org/x/sys/execabs"
+	"google.golang.org/genproto/googleapis/gapic/metadata"
+	"google.golang.org/protobuf/encoding/protojson"
 )
 
 // Generate reads all modules in rootDir and outputs their examples in outDir.
@@ -68,13 +70,13 @@ func Generate(rootDir, outDir string, apiShortnames map[string]string) error {
 			return fmt.Errorf("failed to load packages: %v", err)
 		}
 		for _, pi := range pis {
-			if err := processExamples(pi.Doc, pi.Fset, trimPrefix, outDir, apiShortnames); err != nil {
-				errs = append(errs, fmt.Errorf("failed to process examples: %v", err))
+			if eErrs := processExamples(pi.Doc, pi.Fset, trimPrefix, rootDir, outDir, apiShortnames); len(eErrs) > 0 {
+				errs = append(errs, fmt.Errorf("%v", eErrs))
 			}
 		}
 	}
 	if len(errs) > 0 {
-		log.Fatal(errs)
+		return fmt.Errorf("example errors: %v", errs)
 	}
 
 	if len(dirs) > 0 {
@@ -103,69 +105,90 @@ var skip = map[string]bool{
 	"cloud.google.com/go/translate":                true, // Has newer version.
 }
 
-func processExamples(pkg *doc.Package, fset *token.FileSet, trimPrefix, outDir string, apiShortnames map[string]string) error {
+func processExamples(pkg *doc.Package, fset *token.FileSet, trimPrefix, rootDir, outDir string, apiShortnames map[string]string) []error {
 	if skip[pkg.ImportPath] {
 		return nil
 	}
 	trimmed := strings.TrimPrefix(pkg.ImportPath, trimPrefix)
+	regionTags, err := computeRegionTags(rootDir, trimmed, apiShortnames)
+	if err != nil {
+		return []error{err}
+	}
+	if len(regionTags) == 0 {
+		// Nothing to do.
+		return nil
+	}
 	outDir = filepath.Join(outDir, trimmed)
 
-	shortname, ok := apiShortnames[pkg.ImportPath]
-	if !ok {
-		// Do our best to find a shortname. For example,
-		// cloud.google.com/go/bigtable/bttest should lead to
-		// cloud.google.com/go/bigtable.
-		bestMatch := ""
-		for path := range apiShortnames {
-			if strings.HasPrefix(pkg.ImportPath, path) {
-				if len(path) > len(bestMatch) {
-					bestMatch = path
-				}
+	// Note: only process methods because they correspond to RPCs.
+
+	var errs []error
+	for _, t := range pkg.Types {
+		for _, m := range t.Methods {
+			if len(m.Examples) == 0 {
+				// Nothing to do for this method.
+				continue
+			}
+			dir := filepath.Join(outDir, t.Name, m.Name)
+			regionTag, ok := regionTags[t.Name][m.Name]
+			if !ok {
+				errs = append(errs, fmt.Errorf("could not find region tag for %s %s.%s", pkg.ImportPath, t.Name, m.Name))
+				continue
+			}
+			if err := writeExamples(dir, m.Examples, fset, regionTag); err != nil {
+				errs = append(errs, err)
 			}
 		}
-		if bestMatch == "" {
-			return fmt.Errorf("could not find API shortname for %v", pkg.ImportPath)
-		}
-		log.Printf("The best match for %q is %q", pkg.ImportPath, bestMatch)
-		shortname = apiShortnames[bestMatch]
 	}
-	regionTag := shortname + "_generated" + strings.ReplaceAll(trimmed, "/", "_")
-
-	// Note: variables and constants don't have examples.
+	return errs
+}
 
-	for _, f := range pkg.Funcs {
-		dir := filepath.Join(outDir, f.Name)
-		if err := writeExamples(dir, f.Examples, fset, regionTag); err != nil {
-			return err
-		}
+// computeRegionTags gets the region tags for the given path, keyed by client name and method name.
+func computeRegionTags(rootDir, path string, apiShortnames map[string]string) (regionTags map[string]map[string]string, err error) {
+	metadataPath := filepath.Join(rootDir, path, "gapic_metadata.json")
+	f, err := os.ReadFile(metadataPath)
+	if err != nil {
+		// If there is no gapic_metadata.json file, don't generate snippets.
+		// This isn't an error, though, because some packages aren't GAPICs and
+		// shouldn't get snippets in the first place.
+		return nil, nil
+	}
+	m := metadata.GapicMetadata{}
+	if err := protojson.Unmarshal(f, &m); err != nil {
+		return nil, err
+	}
+	shortname, ok := apiShortnames[m.GetLibraryPackage()]
+	if !ok {
+		return nil, fmt.Errorf("could not find shortname for %q", m.GetLibraryPackage())
 	}
+	protoParts := strings.Split(m.GetProtoPackage(), ".")
+	apiVersion := protoParts[len(protoParts)-1]
 
-	for _, t := range pkg.Types {
-		dir := filepath.Join(outDir, t.Name)
-		if err := writeExamples(dir, t.Examples, fset, regionTag); err != nil {
-			return err
-		}
-		for _, f := range t.Funcs {
-			fDir := filepath.Join(dir, f.Name)
-			if err := writeExamples(fDir, f.Examples, fset, regionTag); err != nil {
-				return err
-			}
-		}
-		for _, m := range t.Methods {
-			mDir := filepath.Join(dir, m.Name)
-			if err := writeExamples(mDir, m.Examples, fset, regionTag); err != nil {
-				return err
+	regionTags = map[string]map[string]string{}
+	for sName, s := range m.GetServices() {
+		for _, c := range s.GetClients() {
+			for rpc, methods := range c.GetRpcs() {
+				if len(methods.GetMethods()) != 1 {
+					return nil, fmt.Errorf("%s %s %s found %d methods", m.GetLibraryPackage(), sName, c.GetLibraryClient(), len(methods.GetMethods()))
+				}
+				if methods.GetMethods()[0] != rpc {
+					return nil, fmt.Errorf("%s %s %s %q does not match %q", m.GetLibraryPackage(), sName, c.GetLibraryClient(), methods.GetMethods()[0], rpc)
+				}
+
+				// Every Go method is synchronous.
+				regionTag := fmt.Sprintf("%s_%s_generated_%s_%s_sync", shortname, apiVersion, sName, rpc)
+
+				if regionTags[c.GetLibraryClient()] == nil {
+					regionTags[c.GetLibraryClient()] = map[string]string{}
+				}
+				regionTags[c.GetLibraryClient()][rpc] = regionTag
 			}
 		}
 	}
-	return nil
+	return regionTags, nil
 }
 
 func writeExamples(outDir string, exs []*doc.Example, fset *token.FileSet, regionTag string) error {
-	if len(exs) == 0 {
-		// Nothing to do.
-		return nil
-	}
 	for _, ex := range exs {
 		dir := outDir
 		if len(exs) > 1 {
@@ -207,7 +230,12 @@ func writeExamples(outDir string, exs []*doc.Example, fset *token.FileSet, regio
 		if _, err := f.WriteString(header()); err != nil {
 			return err
 		}
-		tag := regionTag + "_" + ex.Name
+
+		tag := regionTag
+		if len(ex.Suffix) > 0 {
+			tag += "_" + ex.Suffix
+		}
+
 		// Include an extra newline to keep separate from the package declaration.
 		if _, err := fmt.Fprintf(f, "// [START %v]\n\n", tag); err != nil {
 			return err

internal/gapicgen/go.mod

@@ -14,6 +14,8 @@ require (
 	golang.org/x/oauth2 v0.0.0-20210413134643-5e61552d6c78
 	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
 	golang.org/x/sys v0.0.0-20210415045647-66c3f260301c
+	google.golang.org/genproto v0.0.0-20210402141018-6c239bbf2bb1
+	google.golang.org/protobuf v1.26.0
 	gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
 	gopkg.in/src-d/go-git.v4 v4.13.1
 	gopkg.in/yaml.v2 v2.4.0

internal/gapicgen/go.sum

@@ -477,6 +477,7 @@ google.golang.org/genproto v0.0.0-20210222152913-aa3ee6e6a81c/go.mod h1:FWY/as6D
 google.golang.org/genproto v0.0.0-20210303154014-9728d6b83eeb/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
 google.golang.org/genproto v0.0.0-20210310155132-4ce2db91004e/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
 google.golang.org/genproto v0.0.0-20210319143718-93e7006c17a6/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
+google.golang.org/genproto v0.0.0-20210402141018-6c239bbf2bb1 h1:E7wSQBXkH3T3diucK+9Z1kjn4+/9tNG7lZLr75oOhh8=
 google.golang.org/genproto v0.0.0-20210402141018-6c239bbf2bb1/go.mod h1:9lPAdzaEmUacj36I+k7YKbEc5CXzPIeORRgDAUOu28A=
 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
 google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38=

internal/generated/snippets/accessapproval/apiv1/Client/ApproveApprovalRequest/main.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// [START accessapproval_generated_accessapproval_apiv1_Client_ApproveApprovalRequest]
+// [START accessapproval_v1_generated_AccessApproval_ApproveApprovalRequest_sync]
 
 package main
 
@@ -43,4 +43,4 @@ func main() {
 	_ = resp
 }
 
-// [END accessapproval_generated_accessapproval_apiv1_Client_ApproveApprovalRequest]
+// [END accessapproval_v1_generated_AccessApproval_ApproveApprovalRequest_sync]

internal/generated/snippets/accessapproval/apiv1/Client/DeleteAccessApprovalSettings/main.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// [START accessapproval_generated_accessapproval_apiv1_Client_DeleteAccessApprovalSettings]
+// [START accessapproval_v1_generated_AccessApproval_DeleteAccessApprovalSettings_sync]
 
 package main
 
@@ -39,4 +39,4 @@ func main() {
 	}
 }
 
-// [END accessapproval_generated_accessapproval_apiv1_Client_DeleteAccessApprovalSettings]
+// [END accessapproval_v1_generated_AccessApproval_DeleteAccessApprovalSettings_sync]

internal/generated/snippets/accessapproval/apiv1/Client/DismissApprovalRequest/main.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// [START accessapproval_generated_accessapproval_apiv1_Client_DismissApprovalRequest]
+// [START accessapproval_v1_generated_AccessApproval_DismissApprovalRequest_sync]
 
 package main
 
@@ -43,4 +43,4 @@ func main() {
 	_ = resp
 }
 
-// [END accessapproval_generated_accessapproval_apiv1_Client_DismissApprovalRequest]
+// [END accessapproval_v1_generated_AccessApproval_DismissApprovalRequest_sync]

internal/generated/snippets/accessapproval/apiv1/Client/GetAccessApprovalSettings/main.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// [START accessapproval_generated_accessapproval_apiv1_Client_GetAccessApprovalSettings]
+// [START accessapproval_v1_generated_AccessApproval_GetAccessApprovalSettings_sync]
 
 package main
 
@@ -43,4 +43,4 @@ func main() {
 	_ = resp
 }
 
-// [END accessapproval_generated_accessapproval_apiv1_Client_GetAccessApprovalSettings]
+// [END accessapproval_v1_generated_AccessApproval_GetAccessApprovalSettings_sync]

internal/generated/snippets/accessapproval/apiv1/Client/GetApprovalRequest/main.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// [START accessapproval_generated_accessapproval_apiv1_Client_GetApprovalRequest]
+// [START accessapproval_v1_generated_AccessApproval_GetApprovalRequest_sync]
 
 package main
 
@@ -43,4 +43,4 @@ func main() {
 	_ = resp
 }
 
-// [END accessapproval_generated_accessapproval_apiv1_Client_GetApprovalRequest]
+// [END accessapproval_v1_generated_AccessApproval_GetApprovalRequest_sync]

internal/generated/snippets/accessapproval/apiv1/Client/ListApprovalRequests/main.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// [START accessapproval_generated_accessapproval_apiv1_Client_ListApprovalRequests]
+// [START accessapproval_v1_generated_AccessApproval_ListApprovalRequests_sync]
 
 package main
 
@@ -51,4 +51,4 @@ func main() {
 	}
 }
 
-// [END accessapproval_generated_accessapproval_apiv1_Client_ListApprovalRequests]
+// [END accessapproval_v1_generated_AccessApproval_ListApprovalRequests_sync]

internal/generated/snippets/accessapproval/apiv1/Client/UpdateAccessApprovalSettings/main.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// [START accessapproval_generated_accessapproval_apiv1_Client_UpdateAccessApprovalSettings]
+// [START accessapproval_v1_generated_AccessApproval_UpdateAccessApprovalSettings_sync]
 
 package main
 
@@ -43,4 +43,4 @@ func main() {
 	_ = resp
 }
 
-// [END accessapproval_generated_accessapproval_apiv1_Client_UpdateAccessApprovalSettings]
+// [END accessapproval_v1_generated_AccessApproval_UpdateAccessApprovalSettings_sync]

internal/generated/snippets/analytics/admin/apiv1alpha/AnalyticsAdminClient/AuditUserLinks/main.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// [START analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_AuditUserLinks]
+// [START analyticsadmin_v1alpha_generated_AnalyticsAdminService_AuditUserLinks_sync]
 
 package main
 
@@ -51,4 +51,4 @@ func main() {
 	}
 }
 
-// [END analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_AuditUserLinks]
+// [END analyticsadmin_v1alpha_generated_AnalyticsAdminService_AuditUserLinks_sync]

internal/generated/snippets/analytics/admin/apiv1alpha/AnalyticsAdminClient/BatchCreateUserLinks/main.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// [START analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_BatchCreateUserLinks]
+// [START analyticsadmin_v1alpha_generated_AnalyticsAdminService_BatchCreateUserLinks_sync]
 
 package main
 
@@ -43,4 +43,4 @@ func main() {
 	_ = resp
 }
 
-// [END analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_BatchCreateUserLinks]
+// [END analyticsadmin_v1alpha_generated_AnalyticsAdminService_BatchCreateUserLinks_sync]

internal/generated/snippets/analytics/admin/apiv1alpha/AnalyticsAdminClient/BatchDeleteUserLinks/main.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// [START analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_BatchDeleteUserLinks]
+// [START analyticsadmin_v1alpha_generated_AnalyticsAdminService_BatchDeleteUserLinks_sync]
 
 package main
 
@@ -39,4 +39,4 @@ func main() {
 	}
 }
 
-// [END analyticsadmin_generated_analytics_admin_apiv1alpha_AnalyticsAdminClient_BatchDeleteUserLinks]
+// [END analyticsadmin_v1alpha_generated_AnalyticsAdminService_BatchDeleteUserLinks_sync]