diff --git a/gcp/observability/config.go b/gcp/observability/config.go index 428d527a30c..ecda5b23007 100644 --- a/gcp/observability/config.go +++ b/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 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 := 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) } - 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 diff --git a/gcp/observability/observability_test.go b/gcp/observability/observability_test.go index 62936ccec10..c5fa59c6648 100644 --- a/gcp/observability/observability_test.go +++ b/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 {