profiler: replace target with service and service version

"Target" is a term alien to GCP. Rename target terminology to service
and service versioni. Also, make service a required field, instead of
using a default value.

Change-Id: I6b78d46cc189a6324a0cbf95ad6bdb46f18b1d51
Reviewed-on: https://code-review.googlesource.com/16250
Reviewed-by: kokoro <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
Reviewed-by: Alexey Alexandrov <[email protected]>

profiler/profiler.go

@@ -16,19 +16,21 @@
 //
 // This package is still experimental and subject to change.
 //
+// Usage example:
+//
+//   import "cloud.google.com/go/profiler"
+//   ...
+//   err := profiler.Start(profiler.Config{Service: "my-service"})
+//   if err != nil {
+//       // TODO: Handle error.
+//   }
+//
 // Calling Start will start a goroutine to collect profiles and
 // upload to Cloud Profiler server, at the rhythm specified by
 // the server.
 //
-// The caller should provide the target string in the config so Cloud
-// Profiler knows how to group the profile data. Otherwise the target
-// string is set to "unknown".
-//
-// Optionally DebugLogging can be set in the config to enable detailed
-// logging from profiler.
-//
-// Start should only be called once. The first call will start
-// the profiling goroutine. Any additional calls will be ignored.
+// The caller must provide the service string in the config, and
+// may provide other information as well. See Config for details.
 package profiler
 
 import (
@@ -73,6 +75,7 @@ const (
 	apiAddress       = "cloudprofiler.googleapis.com:443"
 	xGoogAPIMetadata = "x-goog-api-client"
 	zoneNameLabel    = "zone"
+	versionLabel     = "version"
 	instanceLabel    = "instance"
 	scope            = "https://www.googleapis.com/auth/monitoring.write"
 
@@ -85,10 +88,25 @@ const (
 
 // Config is the profiler configuration.
 type Config struct {
-	// Target groups related deployments together, defaults to "unknown".
-	Target string
-
-	// DebugLogging enables detailed debug logging from profiler.
+	// Service (or deprecated Target) must be provided to start the profiler.
+	// It specifies the name of the service under which the profiled data
+	// will be recorded and exposed at the Cloud Profiler UI for the project.
+	// You can specify an arbitrary string, but see Deployment.target at
+	// https://github.com/googleapis/googleapis/blob/master/google/devtools/cloudprofiler/v2/profiler.proto
+	// for restrictions.
+	// NOTE: The string should be the same across different replicas of
+	// your service so that the globally constant profiling rate is
+	// maintained. Do not put things like PID or unique pod ID in the name.
+	Service string
+
+	// ServiceVersion is an optional field specifying the version of the
+	// service. It can be an arbitrary string. Cloud Profiler profiles
+	// once per minute for each version of each service in each zone.
+	// ServiceVersion defaults to an empty string.
+	ServiceVersion string
+
+	// DebugLogging enables detailed debug logging from profiler. It
+	// defaults to false.
 	DebugLogging bool
 
 	// ProjectID is the Cloud Console project ID to use instead of
@@ -116,14 +134,19 @@ type Config struct {
 	// agent API. Defaults to the production environment, overridable
 	// for testing.
 	APIAddr string
+
+	// Target is deprecated, use Service instead.
+	Target string
 }
 
 // startError represents the error occured during the
 // initializating and starting of the agent.
 var startError error
 
-// Start starts a goroutine to collect and upload profiles.
-// See package level documentation for details.
+// Start starts a goroutine to collect and upload profiles. The
+// caller must provide the service string in the config. See
+// Config for details. Start should only be called once. Any
+// additional calls will be ignored.
 func Start(cfg Config, options ...option.ClientOption) error {
 	startOnce.Do(func() {
 		startError = start(cfg, options...)
@@ -132,7 +155,10 @@ func Start(cfg Config, options ...option.ClientOption) error {
 }
 
 func start(cfg Config, options ...option.ClientOption) error {
-	initializeConfig(cfg)
+	if err := initializeConfig(cfg); err != nil {
+		debugLog("failed to initialize config: %v", err)
+		return err
+	}
 
 	ctx := context.Background()
 
@@ -336,12 +362,17 @@ func initializeDeployment() (*pb.Deployment, error) {
 		}
 	}
 
+	labels := map[string]string{
+		zoneNameLabel: zone,
+	}
+	if config.ServiceVersion != "" {
+		labels[versionLabel] = config.ServiceVersion
+	}
+
 	return &pb.Deployment{
 		ProjectId: projectID,
 		Target:    config.Target,
-		Labels: map[string]string{
-			zoneNameLabel: zone,
-		},
+		Labels:    labels,
 	}, nil
 }
 
@@ -372,15 +403,21 @@ func initializeResources(ctx context.Context, conn *grpc.ClientConn, d *pb.Deplo
 	}, ctx
 }
 
-func initializeConfig(cfg Config) {
+func initializeConfig(cfg Config) error {
 	config = cfg
 
+	if config.Service != "" {
+		config.Target = config.Service
+	}
+
 	if config.Target == "" {
-		config.Target = "unknown"
+		return errors.New("service name must be specified in the configuration")
 	}
+
 	if config.APIAddr == "" {
 		config.APIAddr = apiAddress
 	}
+	return nil
 }
 
 // pollProfilerService starts an endless loop to poll Cloud Profiler

profiler/profiler_example_test.go

@@ -19,13 +19,7 @@ import (
 )
 
 func ExampleStart() {
-	// The caller should provide the target string in the config so Cloud
-	// Profiler knows how to group the profile data. Otherwise the target
-	// string is set to "unknown".
-	//
-	// Optionally DebugLogging can be set in the config to enable detailed
-	// logging from profiler.
-	err := profiler.Start(profiler.Config{Target: "my-target"})
+	err := profiler.Start(profiler.Config{Service: "my-service", ServiceVersion: "v1"})
 	if err != nil {
 		//TODO: Handle error.
 	}

profiler/profiler_test.go

@@ -38,18 +38,22 @@ import (
 )
 
 const (
-	testProjectID    = "test-project-ID"
-	testInstanceName = "test-instance-name"
-	testZoneName     = "test-zone-name"
-	testTarget       = "test-target"
+	testProjectID      = "test-project-ID"
+	testInstanceName   = "test-instance-name"
+	testZoneName       = "test-zone-name"
+	testTarget         = "test-target"
+	testService        = "test-service"
+	testServiceVersion = "test-service-version"
 )
 
 func createTestDeployment() *pb.Deployment {
-	labels := make(map[string]string)
-	labels[zoneNameLabel] = testZoneName
+	labels := map[string]string{
+		zoneNameLabel: testZoneName,
+		versionLabel:  testServiceVersion,
+	}
 	return &pb.Deployment{
 		ProjectId: testProjectID,
-		Target:    testTarget,
+		Target:    testService,
 		Labels:    labels,
 	}
 }
@@ -348,7 +352,8 @@ func TestInitializeDeployment(t *testing.T) {
 		return testZoneName, nil
 	}
 
-	config = Config{Target: testTarget}
+	cfg := Config{Service: testService, ServiceVersion: testServiceVersion}
+	initializeConfig(cfg)
 	d, err := initializeDeployment()
 	if err != nil {
 		t.Errorf("initializeDeployment() got error: %v, want no error", err)
@@ -359,6 +364,48 @@ func TestInitializeDeployment(t *testing.T) {
 	}
 }
 
+func TestInitializeConfig(t *testing.T) {
+	oldConfig := config
+	defer func() {
+		config = oldConfig
+	}()
+
+	for _, tt := range []struct {
+		config          Config
+		wantTarget      string
+		wantErrorString string
+	}{
+		{
+			Config{Service: testService},
+			testService,
+			"",
+		},
+		{
+			Config{Target: testTarget},
+			testTarget,
+			"",
+		},
+		{
+			Config{},
+			"",
+			"service name must be specified in the configuration",
+		},
+	} {
+		errorString := ""
+		if err := initializeConfig(tt.config); err != nil {
+			errorString = err.Error()
+		}
+
+		if errorString != tt.wantErrorString {
+			t.Errorf("initializeConfig(%v) got error: %v, want %v", tt.config, errorString, tt.wantErrorString)
+		}
+
+		if config.Target != tt.wantTarget {
+			t.Errorf("initializeConfig(%v) got target: %v, want %v", tt.config, config.Target, tt.wantTarget)
+		}
+	}
+}
+
 func TestInitializeProfileLabels(t *testing.T) {
 	defer func() {
 		getInstanceName = gcemd.InstanceName