gcp/observability: Add support for Environment Variable GRPC_CONFIG_OBSERVABILITY_JSON (#5525)

* gcp/observability: Add support for Environment Variable GRPC_CONFIG_OBSERVABILITY_JSON

gcp/observability/config.go

@@ -20,7 +20,9 @@ package observability
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"regexp"
 
@@ -31,9 +33,10 @@ import (
 )
 
 const (
-	envObservabilityConfig    = "GRPC_CONFIG_OBSERVABILITY"
-	envProjectID              = "GOOGLE_CLOUD_PROJECT"
-	logFilterPatternRegexpStr = `^([\w./]+)/((?:\w+)|[*])$`
+	envObservabilityConfig     = "GRPC_CONFIG_OBSERVABILITY"
+	envObservabilityConfigJSON = "GRPC_CONFIG_OBSERVABILITY_JSON"
+	envProjectID               = "GOOGLE_CLOUD_PROJECT"
+	logFilterPatternRegexpStr  = `^([\w./]+)/((?:\w+)|[*])$`
 )
 
 var logFilterPatternRegexp = regexp.MustCompile(logFilterPatternRegexpStr)
@@ -72,21 +75,33 @@ func validateFilters(config *configpb.ObservabilityConfig) error {
 	return nil
 }
 
+// unmarshalAndVerifyConfig unmarshals a json string representing an
+// observability config into its protobuf format, and also verifies the
+// configuration's fields for validity.
+func unmarshalAndVerifyConfig(rawJSON json.RawMessage) (*configpb.ObservabilityConfig, error) {
+	var config configpb.ObservabilityConfig
+	if err := protojson.Unmarshal(rawJSON, &config); err != nil {
+		return nil, fmt.Errorf("error parsing observability config: %v", err)
+	}
+	if err := validateFilters(&config); err != nil {
+		return nil, fmt.Errorf("error parsing observability config: %v", err)
+	}
+	if config.GlobalTraceSamplingRate > 1 || config.GlobalTraceSamplingRate < 0 {
+		return nil, fmt.Errorf("error parsing observability config: invalid global trace sampling rate %v", config.GlobalTraceSamplingRate)
+	}
+	logger.Infof("Parsed ObservabilityConfig: %+v", &config)
+	return &config, nil
+}
+
 func parseObservabilityConfig() (*configpb.ObservabilityConfig, error) {
-	// Parse the config from ENV var
-	if content := os.Getenv(envObservabilityConfig); content != "" {
-		var config configpb.ObservabilityConfig
-		if err := protojson.Unmarshal([]byte(content), &config); err != nil {
-			return nil, fmt.Errorf("error parsing observability config from env %v: %v", envObservabilityConfig, err)
+	if fileSystemPath := os.Getenv(envObservabilityConfigJSON); fileSystemPath != "" {
+		content, err := ioutil.ReadFile(fileSystemPath) // TODO: Switch to os.ReadFile once dropped support for go 1.15
+		if err != nil {
+			return nil, fmt.Errorf("error reading observability configuration file %q: %v", fileSystemPath, err)
 		}
-		if err := validateFilters(&config); err != nil {
-			return nil, fmt.Errorf("error parsing observability config: %v", err)
-		}
-		if config.GlobalTraceSamplingRate > 1 || config.GlobalTraceSamplingRate < 0 {
-			return nil, fmt.Errorf("error parsing observability config: invalid global trace sampling rate %v", config.GlobalTraceSamplingRate)
-		}
-		logger.Infof("Parsed ObservabilityConfig: %+v", &config)
-		return &config, nil
+		return unmarshalAndVerifyConfig(content)
+	} else if content := os.Getenv(envObservabilityConfig); content != "" {
+		return unmarshalAndVerifyConfig([]byte(content))
 	}
 	// If the ENV var doesn't exist, do nothing
 	return nil, nil

gcp/observability/observability_test.go

@@ -21,7 +21,9 @@ package observability
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"net"
 	"os"
 	"sync"
@@ -675,6 +677,7 @@ func (s) TestRefuseStartWithInvalidPatterns(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed to convert config to JSON: %v", err)
 	}
+	os.Setenv(envObservabilityConfigJSON, "")
 	os.Setenv(envObservabilityConfig, string(configJSON))
 	// If there is at least one invalid pattern, which should not be silently tolerated.
 	if err := Start(context.Background()); err == nil {
@@ -682,7 +685,94 @@ func (s) TestRefuseStartWithInvalidPatterns(t *testing.T) {
 	}
 }
 
+// createTmpConfigInFileSystem creates a random observability config at a random
+// place in the temporary portion of the file system dependent on system. It
+// also sets the environment variable GRPC_CONFIG_OBSERVABILITY_JSON to point to
+// this created config.
+func createTmpConfigInFileSystem(rawJSON string) (*os.File, error) {
+	configJSONFile, err := ioutil.TempFile(os.TempDir(), "configJSON-")
+	if err != nil {
+		return nil, fmt.Errorf("cannot create file %v: %v", configJSONFile.Name(), err)
+	}
+	_, err = configJSONFile.Write(json.RawMessage(rawJSON))
+	if err != nil {
+		return nil, fmt.Errorf("cannot write marshalled JSON: %v", err)
+	}
+	os.Setenv(envObservabilityConfigJSON, configJSONFile.Name())
+	return configJSONFile, nil
+}
+
+// TestJSONEnvVarSet tests a valid observability configuration specified by the
+// GRPC_CONFIG_OBSERVABILITY_JSON environment variable, whose value represents a
+// file path pointing to a JSON encoded config.
+func (s) TestJSONEnvVarSet(t *testing.T) {
+	configJSON := `{
+		"destinationProjectId": "fake",
+		"logFilters":[{"pattern":"*","headerBytes":1073741824,"messageBytes":1073741824}]
+	}`
+	configJSONFile, err := createTmpConfigInFileSystem(configJSON)
+	defer configJSONFile.Close()
+	if err != nil {
+		t.Fatalf("failed to create config in file system: %v", err)
+	}
+	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
+	defer cancel()
+	if err := Start(ctx); err != nil {
+		t.Fatalf("error starting observability with valid config through file system: %v", err)
+	}
+	defer End()
+}
+
+// TestBothConfigEnvVarsSet tests the scenario where both configuration
+// environment variables are set. The file system environment variable should
+// take precedence, and an error should return in the case of the file system
+// configuration being invalid, even if the direct configuration environment
+// variable is set and valid.
+func (s) TestBothConfigEnvVarsSet(t *testing.T) {
+	configJSON := `{
+		"destinationProjectId":"fake",
+		"logFilters":[{"pattern":":-)"}, {"pattern":"*"}]
+	}`
+	configJSONFile, err := createTmpConfigInFileSystem(configJSON)
+	defer configJSONFile.Close()
+	if err != nil {
+		t.Fatalf("failed to create config in file system: %v", err)
+	}
+	// This configuration should be ignored, as precedence 2.
+	validConfig := &configpb.ObservabilityConfig{
+		EnableCloudLogging:   true,
+		DestinationProjectId: "fake",
+		LogFilters: []*configpb.ObservabilityConfig_LogFilter{
+			{
+				Pattern:      "*",
+				HeaderBytes:  infinitySizeBytes,
+				MessageBytes: infinitySizeBytes,
+			},
+		},
+	}
+	validConfigJSON, err := protojson.Marshal(validConfig)
+	if err != nil {
+		t.Fatalf("failed to convert config to JSON: %v", err)
+	}
+	os.Setenv(envObservabilityConfig, string(validConfigJSON))
+	if err := Start(context.Background()); err == nil {
+		t.Fatalf("Invalid patterns not triggering error")
+	}
+}
+
+// TestErrInFileSystemEnvVar tests the scenario where an observability
+// configuration is specified with environment variable that specifies a
+// location in the file system for configuration, and this location doesn't have
+// a file (or valid configuration).
+func (s) TestErrInFileSystemEnvVar(t *testing.T) {
+	os.Setenv(envObservabilityConfigJSON, "/this-file/does-not-exist")
+	if err := Start(context.Background()); err == nil {
+		t.Fatalf("Invalid file system path not triggering error")
+	}
+}
+
 func (s) TestNoEnvSet(t *testing.T) {
+	os.Setenv(envObservabilityConfigJSON, "")
 	os.Setenv(envObservabilityConfig, "")
 	// If there is no observability config set at all, the Start should return an error.
 	if err := Start(context.Background()); err == nil {