Skip to content

Commit

Permalink
Fix Windows build failure (open-telemetry#78)
Browse files Browse the repository at this point in the history
Addresses open-telemetry#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.
  • Loading branch information
flands authored Feb 3, 2021
1 parent adc00a6 commit e525e44
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 9 deletions.
34 changes: 25 additions & 9 deletions cmd/otelcol/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions cmd/otelcol/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down

0 comments on commit e525e44

Please sign in to comment.