commit 0a69b5bd018679bef796d49fb2e1003ee6057c27
author Alexander Rakoczy <alex@golang.org> Fri Mar 19 17:24:02 2021 -0400
committer Alexander Rakoczy <alex@golang.org> Mon Mar 22 17:27:01 2021 +0000
tree ea9a73b3f8cb4d43b4f111b720cc9157af8dadd5
parent 3398ffb279687234af00bf093eefb74792a364b4

internal/metrics: fix detection of GCE environment

appengine.IsAppEngine mistakenly returns false, as it relies on the app
being built with the appenginevm build constraint. This is not set,
as we're supplying our own Docker image.

The playground is always running either locally or on appengine. We can
rely on metadata.OnGCE being correct.

Other calls that are made to the appengine library either use the
metadata server or environment variables, and do not rely on a
constraint being set. They also panic if the metadata server is
unavailable.

For golang/go#44822

Change-Id: I1de5fda4bfd1e909f1fd54cc44770525827a9c1a
Reviewed-on: https://go-review.googlesource.com/c/playground/+/303469
Trust: Alexander Rakoczy <alex@golang.org>
Run-TryBot: Alexander Rakoczy <alex@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>

internal/metrics/service.go

diff --git a/internal/metrics/service.go b/internal/metrics/service.go
index 00573a5..f6126dd 100644
--- a/internal/metrics/service.go
+++ b/internal/metrics/service.go
@@ -145,7 +145,10 @@
 //
 // The resource will be in StackDrvier's gae_instance type.
 func GAEResource(ctx context.Context) (*MonitoredResource, error) {
-	if !appengine.IsAppEngine() {
+	// appengine.IsAppEngine is confusingly false as we're using a custom
+	// container and building without the appenginevm build constraint.
+	// Check metadata.OnGCE instead.
+	if !metadata.OnGCE() {
 		return nil, fmt.Errorf("not running on appengine")
 	}
 	projID, err := metadata.ProjectID()

main.go

diff --git a/main.go b/main.go
index af9774e..ed59695 100644
--- a/main.go
+++ b/main.go
@@ -13,6 +13,7 @@
 
 	"cloud.google.com/go/compute/metadata"
 	"cloud.google.com/go/datastore"
+	"golang.org/x/playground/internal/metrics"
 )
 
 var log = newStdLogger()
@@ -44,7 +45,7 @@
 		}
 		s.log = log
 		return nil
-	})
+	}, enableMetrics)
 	if err != nil {
 		log.Fatalf("Error creating server: %v", err)
 	}
@@ -71,6 +72,21 @@
 	log.Fatalf("Error listening on :%v: %v", port, http.ListenAndServe(":"+port, s))
 }
 
+func enableMetrics(s *server) error {
+	gr, err := metrics.GAEResource(context.Background())
+	if err != nil {
+		s.log.Printf("metrics.GAEResource() = _, %q", err)
+	}
+	ms, err := metrics.NewService(gr, views)
+	if err != nil {
+		s.log.Printf("Failed to initialize metrics: metrics.NewService() = _, %q. (not on GCP?)", err)
+	}
+	if ms != nil && !metadata.OnGCE() {
+		s.mux.Handle("/metrics", ms)
+	}
+	return nil
+}
+
 func projectID() string {
 	id, err := metadata.ProjectID()
 	if err != nil && os.Getenv("GAE_INSTANCE") != "" {

server.go

diff --git a/server.go b/server.go
index 8b82d23..8352c51 100644
--- a/server.go
+++ b/server.go
@@ -5,15 +5,12 @@
 package main
 
 import (
-	"context"
 	"fmt"
 	"net/http"
 	"os"
 	"strings"
 	"time"
 
-	"cloud.google.com/go/compute/metadata"
-	"golang.org/x/playground/internal/metrics"
 	"golang.org/x/tools/godoc/static"
 )
 
@@ -46,17 +43,6 @@
 			s.modtime = fi.ModTime()
 		}
 	}
-	gr, err := metrics.GAEResource(context.Background())
-	if err != nil {
-		s.log.Printf("metrics.GaeService() = _, %q", err)
-	}
-	ms, err := metrics.NewService(gr, views)
-	if err != nil {
-		s.log.Printf("Failed to initialize metrics: metrics.NewService() = _, %q. (not on GCP?)", err)
-	}
-	if ms != nil && !metadata.OnGCE() {
-		s.mux.Handle("/statusz", ms)
-	}
 	s.init()
 	return s, nil
 }