Added support for Enviornment Variable path specifier

grpc · Jul 20, 2022 · abc0189 · abc0189
1 parent a094a10
commit abc0189
Show file tree

Hide file tree

Showing 2 changed files with 136 additions and 16 deletions.
diff --git a/gcp/observability/config.go b/gcp/observability/config.go
@@ -20,6 +20,7 @@ package observability
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"os"
 	"regexp"
@@ -31,9 +32,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 +74,32 @@ func validateFilters(config *configpb.ObservabilityConfig) error {
 	return nil
 }
 
+// unmarshalConfig unmarshals a json string representing an observability config
+// into it's protobuf format.
+func unmarshalConfig(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 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)
+	if fileSystemPath := os.Getenv(envObservabilityConfigJSON); fileSystemPath != "" {
+		content, err := os.ReadFile(fileSystemPath)
+		if err != nil {
+			return nil, fmt.Errorf("error reading file from env var %v: %v", envObservabilityConfigJSON, err)
 		}
-		logger.Infof("Parsed ObservabilityConfig: %+v", &config)
-		return &config, nil
+		return unmarshalConfig(content)
+	} else if content := os.Getenv(envObservabilityConfig); content != "" {
+		return unmarshalConfig([]byte(content))
 	}
 	// If the ENV var doesn't exist, do nothing
 	return nil, nil

diff --git a/gcp/observability/observability_test.go b/gcp/observability/observability_test.go
@@ -675,14 +675,121 @@ 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 {
 		t.Fatalf("Invalid patterns not triggering error")
 	}
 }
 
+// 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) {
+	config := &configpb.ObservabilityConfig{
+		EnableCloudLogging:   true,
+		DestinationProjectId: "fake",
+		LogFilters: []*configpb.ObservabilityConfig_LogFilter{
+			{
+				Pattern:      "*",
+				HeaderBytes:  infinitySizeBytes,
+				MessageBytes: infinitySizeBytes,
+			},
+		},
+	}
+
+	configJSON, err := protojson.Marshal(config)
+	if err != nil {
+		t.Fatalf("failed to convert config to JSON: %v", err)
+	}
+
+	configJSONFile, err := os.Create("/tmp/configJSON")
+	if err != nil {
+		t.Fatalf("cannot create file /tmp/configJSON: %v", err)
+	}
+	defer configJSONFile.Close()
+	_, err = configJSONFile.Write(configJSON)
+	if err != nil {
+		t.Fatalf("cannot write marshalled JSON: %v", err)
+	}
+	os.Setenv(envObservabilityConfigJSON, "/tmp/configJSON")
+
+	if err := Start(context.Background()); 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 and error should return in the case of the file system
+// configuration being invalid, even if the direct configuration enviornment
+// variable is set and valid.
+func (s) TestBothConfigEnvVarsSet(t *testing.T) {
+	invalidConfig := &configpb.ObservabilityConfig{
+		EnableCloudLogging:   true,
+		DestinationProjectId: "fake",
+		LogFilters: []*configpb.ObservabilityConfig_LogFilter{
+			{
+				Pattern: ":-)",
+			},
+			{
+				Pattern: "*",
+			},
+		},
+	}
+	invalidConfigJSON, err := protojson.Marshal(invalidConfig)
+	if err != nil {
+		t.Fatalf("failed to convert config to JSON: %v", err)
+	}
+
+	invalidConfigJSONFile, err := os.Create("/tmp/InvalidConfigJSON")
+	if err != nil {
+		t.Fatalf("cannot create file /tmp/InvalidConfigJSON: %v", err)
+	}
+	defer invalidConfigJSONFile.Close()
+	_, err = invalidConfigJSONFile.Write(invalidConfigJSON)
+	if err != nil {
+		t.Fatalf("cannot write marshalled JSON: %v", err)
+	}
+	os.Setenv(envObservabilityConfigJSON, "/tmp/InvalidConfigJSON")
+
+	// 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 {