gcp/observability: Add support for Environment Variable GRPC_CONFIG_OBSERVABILITY_JSON

gcp/observability/config.go

@@ -22,6 +22,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"regexp"
 
@@ -74,9 +75,10 @@ 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) {
+// unmarshalAndVerifyConfig unmarshals a json string representing an
+// observability config into it's 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)
@@ -93,13 +95,13 @@ func unmarshalConfig(rawJSON json.RawMessage) (*configpb.ObservabilityConfig, er
 
 func parseObservabilityConfig() (*configpb.ObservabilityConfig, error) {
 	if fileSystemPath := os.Getenv(envObservabilityConfigJSON); fileSystemPath != "" {
-		content, err := os.ReadFile(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 file from env var %v: %v", envObservabilityConfigJSON, err)
+			return nil, fmt.Errorf("error reading observability configuration file %q: %v", fileSystemPath, err)
 		}
-		return unmarshalConfig(content)
+		return unmarshalAndVerifyConfig(content)
 	} else if content := os.Getenv(envObservabilityConfig); content != "" {
-		return unmarshalConfig([]byte(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"
@@ -683,78 +685,58 @@ func (s) TestRefuseStartWithInvalidPatterns(t *testing.T) {
 	}
 }
 
-// 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")
+// 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 json.RawMessage) (*os.File, error) {
+	configJSONFile, err := ioutil.TempFile(os.TempDir(), "configJSON-")
 	if err != nil {
-		t.Fatalf("cannot create file /tmp/configJSON: %v", err)
+		return nil, fmt.Errorf("cannot create file %v: %v", configJSONFile.Name(), err)
 	}
 	defer configJSONFile.Close()
-	_, err = configJSONFile.Write(configJSON)
+	_, err = configJSONFile.Write(rawJSON)
 	if err != nil {
-		t.Fatalf("cannot write marshalled JSON: %v", err)
+		return nil, fmt.Errorf("cannot write marshalled JSON: %v", err)
 	}
-	os.Setenv(envObservabilityConfigJSON, "/tmp/configJSON")
+	os.Setenv(envObservabilityConfigJSON, configJSONFile.Name())
+	return configJSONFile, nil /// do you even need to return this file if you're already closing it here?
+}
 
-	if err := Start(context.Background()); err != 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 := json.RawMessage(`{
+		"destinationProjectId": "fake",
+		"logFilters":[{"pattern":"*","headerBytes":1073741824,"messageBytes":1073741824}]
+	}`)
+	_, err := createTmpConfigInFileSystem(configJSON)
+	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 and error should return in the case of the file system
+// 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) {
-	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")
+	configJSON := json.RawMessage(`{
+		"destinationProjectId":"fake",
+		"logFilters":[{"pattern":":-)"}, {"pattern":"*"}]
+	}`)
+	_, err := createTmpConfigInFileSystem(configJSON)
 	if err != nil {
-		t.Fatalf("cannot create file /tmp/InvalidConfigJSON: %v", err)
+		t.Fatalf("failed to create config in file system: %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,