From e525e44b00acbaae3d71088fa49849177f0ef512 Mon Sep 17 00:00:00 2001
From: Steve Flanders <sflanders@splunk.com>
Date: Wed, 3 Feb 2021 09:58:31 -0500
Subject: [PATCH] Fix Windows build failure (#78)

Addresses #77

A behavior change was introduced such that either total memory or
limit/spike environment variables had to be set even if the config
parameter was passed. This fails for Windows as you cannot pass env vars
to the service.

To address, total or limit/spike is no longer required if config
parameter is addressed. Added a test to ensure this issue is not
reintroduced in the future.
---
 cmd/otelcol/main.go      | 34 +++++++++++++++++++++++++---------
 cmd/otelcol/main_test.go |  5 +++++
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/cmd/otelcol/main.go b/cmd/otelcol/main.go
index fa30504b2e1..b4a41c6a99e 100644
--- a/cmd/otelcol/main.go
+++ b/cmd/otelcol/main.go
@@ -222,6 +222,7 @@ func useConfigFromEnvVar() {
 func checkMemorySettingsMiBFromEnvVar(envVar string, memTotalSizeMiB int) int {
 	// Check if the memory limit is specified via the env var
 	// Ensure memory limit is valid
+	args := os.Args[1:]
 	var envVarResult int = 0
 	envVarVal := os.Getenv(envVar)
 	switch {
@@ -237,6 +238,8 @@ func checkMemorySettingsMiBFromEnvVar(envVar string, memTotalSizeMiB int) int {
 		envVarResult = val
 	case memTotalSizeMiB > 0:
 		break
+	case contains(args, "--config"):
+		break
 	default:
 		log.Printf("Usage: %s=12345 %s=us0 %s=684 %s=1024 %s=256 %s", tokenEnvVarName, realmEnvVarName, ballastEnvVarName, memLimitMiBEnvVarName, memSpikeMiBEnvVarName, os.Args[0])
 		log.Fatalf("ERROR: Missing environment variable %s", envVar)
@@ -245,12 +248,16 @@ func checkMemorySettingsMiBFromEnvVar(envVar string, memTotalSizeMiB int) int {
 }
 
 func useMemorySettingsMiBFromEnvVar(memTotalSizeMiB int) {
+	args := os.Args[1:]
 	// Check if memory limit is specified via environment variable
 	memLimit := checkMemorySettingsMiBFromEnvVar(memLimitMiBEnvVarName, memTotalSizeMiB)
-	// Use if set, otherwise memory total size must be specified
+	// Use if set, otherwise check if memory total size is specified via environment variable
 	if memLimit == 0 {
+		// Use if set, otherwise config parameter must be passed
 		if memTotalSizeMiB == 0 {
-			panic("PANIC: Both memory limit MiB and memory total size are set to zero. This should never happen.")
+			if !contains(args, "--config") {
+				panic("PANIC: Both memory limit MiB and memory total size are set to zero. This should never happen.")
+			}
 		}
 		// If not set, compute based on memory total size specified
 		// and default memory limit percentage const
@@ -262,14 +269,19 @@ func useMemorySettingsMiBFromEnvVar(memTotalSizeMiB int) {
 		} else {
 			memLimit = (memTotalSizeMiB - defaultMemoryLimitMaxMiB)
 		}
-		log.Printf("Set memory limit to %d MiB", memLimit)
+		if memLimit != 0 {
+			log.Printf("Set memory limit to %d MiB", memLimit)
+		}
 	}
 	// Check if memory spike is specified via environment variable
 	memSpike := checkMemorySettingsMiBFromEnvVar(memSpikeMiBEnvVarName, memTotalSizeMiB)
-	// Use if set, otherwise memory total size must be specified
+	// Use if set, otherwise check if memory total size is specified via environment variable
 	if memSpike == 0 {
+		// Use if set, otherwise config parameter must be passed
 		if memTotalSizeMiB == 0 {
-			panic("PANIC: Both memory limit MiB and memory total size are set to zero. This should never happen.")
+			if !contains(args, "--config") {
+				panic("PANIC: Both memory limit MiB and memory total size are set to zero. This should never happen.")
+			}
 		}
 		// If not set, compute based on memory total size specified
 		// and default memory spike percentage const
@@ -281,7 +293,9 @@ func useMemorySettingsMiBFromEnvVar(memTotalSizeMiB int) {
 		} else {
 			memSpike = defaultMemorySpikeMaxMiB
 		}
-		log.Printf("Set memory spike limit to %d MiB", memSpike)
+		if memSpike != 0 {
+			log.Printf("Set memory spike limit to %d MiB", memSpike)
+		}
 	}
 	setMemorySettingsToEnvVar(memLimit, memLimitMiBEnvVarName, memSpike, memSpikeMiBEnvVarName)
 }
@@ -315,13 +329,15 @@ func useMemorySettingsPercentageFromEnvVar() {
 
 func setMemorySettingsToEnvVar(limit int, limitName string, spike int, spikeName string) {
 	// Ensure spike and limit are valid
-	if spike >= limit {
+	if spike >= limit && spike != 0 {
 		log.Fatalf("%s env variable must be less than %s env variable but got %d and %d respectively", spikeName, limitName, spike, limit)
 	}
 
 	// Set memory environment variables
-	os.Setenv(limitName, strconv.Itoa(limit))
-	os.Setenv(spikeName, strconv.Itoa(spike))
+	if spike != 0 {
+		os.Setenv(limitName, strconv.Itoa(limit))
+		os.Setenv(spikeName, strconv.Itoa(spike))
+	}
 }
 
 func runInteractive(params service.Parameters) error {
diff --git a/cmd/otelcol/main_test.go b/cmd/otelcol/main_test.go
index 32fcee14de0..21b0bea8369 100644
--- a/cmd/otelcol/main_test.go
+++ b/cmd/otelcol/main_test.go
@@ -83,6 +83,11 @@ func TestUseConfigFromEnvVar(t *testing.T) {
 	if !(result) {
 		t.Error("Expected true got false while looking for --config")
 	}
+	useMemorySettingsMiBFromEnvVar(0)
+	_, present := os.LookupEnv(memLimitMiBEnvVarName)
+	if present {
+		t.Error("Expected false got true while looking for environment variable")
+	}
 	os.Unsetenv(configEnvVarName)
 }