Skip to content

Commit e525e44

Browse files
authored
Fix Windows build failure (open-telemetry#78)
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.
1 parent adc00a6 commit e525e44

File tree

2 files changed

+30
-9
lines changed

2 files changed

+30
-9
lines changed

cmd/otelcol/main.go

+25-9
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@ func useConfigFromEnvVar() {
222222
func checkMemorySettingsMiBFromEnvVar(envVar string, memTotalSizeMiB int) int {
223223
// Check if the memory limit is specified via the env var
224224
// Ensure memory limit is valid
225+
args := os.Args[1:]
225226
var envVarResult int = 0
226227
envVarVal := os.Getenv(envVar)
227228
switch {
@@ -237,6 +238,8 @@ func checkMemorySettingsMiBFromEnvVar(envVar string, memTotalSizeMiB int) int {
237238
envVarResult = val
238239
case memTotalSizeMiB > 0:
239240
break
241+
case contains(args, "--config"):
242+
break
240243
default:
241244
log.Printf("Usage: %s=12345 %s=us0 %s=684 %s=1024 %s=256 %s", tokenEnvVarName, realmEnvVarName, ballastEnvVarName, memLimitMiBEnvVarName, memSpikeMiBEnvVarName, os.Args[0])
242245
log.Fatalf("ERROR: Missing environment variable %s", envVar)
@@ -245,12 +248,16 @@ func checkMemorySettingsMiBFromEnvVar(envVar string, memTotalSizeMiB int) int {
245248
}
246249

247250
func useMemorySettingsMiBFromEnvVar(memTotalSizeMiB int) {
251+
args := os.Args[1:]
248252
// Check if memory limit is specified via environment variable
249253
memLimit := checkMemorySettingsMiBFromEnvVar(memLimitMiBEnvVarName, memTotalSizeMiB)
250-
// Use if set, otherwise memory total size must be specified
254+
// Use if set, otherwise check if memory total size is specified via environment variable
251255
if memLimit == 0 {
256+
// Use if set, otherwise config parameter must be passed
252257
if memTotalSizeMiB == 0 {
253-
panic("PANIC: Both memory limit MiB and memory total size are set to zero. This should never happen.")
258+
if !contains(args, "--config") {
259+
panic("PANIC: Both memory limit MiB and memory total size are set to zero. This should never happen.")
260+
}
254261
}
255262
// If not set, compute based on memory total size specified
256263
// and default memory limit percentage const
@@ -262,14 +269,19 @@ func useMemorySettingsMiBFromEnvVar(memTotalSizeMiB int) {
262269
} else {
263270
memLimit = (memTotalSizeMiB - defaultMemoryLimitMaxMiB)
264271
}
265-
log.Printf("Set memory limit to %d MiB", memLimit)
272+
if memLimit != 0 {
273+
log.Printf("Set memory limit to %d MiB", memLimit)
274+
}
266275
}
267276
// Check if memory spike is specified via environment variable
268277
memSpike := checkMemorySettingsMiBFromEnvVar(memSpikeMiBEnvVarName, memTotalSizeMiB)
269-
// Use if set, otherwise memory total size must be specified
278+
// Use if set, otherwise check if memory total size is specified via environment variable
270279
if memSpike == 0 {
280+
// Use if set, otherwise config parameter must be passed
271281
if memTotalSizeMiB == 0 {
272-
panic("PANIC: Both memory limit MiB and memory total size are set to zero. This should never happen.")
282+
if !contains(args, "--config") {
283+
panic("PANIC: Both memory limit MiB and memory total size are set to zero. This should never happen.")
284+
}
273285
}
274286
// If not set, compute based on memory total size specified
275287
// and default memory spike percentage const
@@ -281,7 +293,9 @@ func useMemorySettingsMiBFromEnvVar(memTotalSizeMiB int) {
281293
} else {
282294
memSpike = defaultMemorySpikeMaxMiB
283295
}
284-
log.Printf("Set memory spike limit to %d MiB", memSpike)
296+
if memSpike != 0 {
297+
log.Printf("Set memory spike limit to %d MiB", memSpike)
298+
}
285299
}
286300
setMemorySettingsToEnvVar(memLimit, memLimitMiBEnvVarName, memSpike, memSpikeMiBEnvVarName)
287301
}
@@ -315,13 +329,15 @@ func useMemorySettingsPercentageFromEnvVar() {
315329

316330
func setMemorySettingsToEnvVar(limit int, limitName string, spike int, spikeName string) {
317331
// Ensure spike and limit are valid
318-
if spike >= limit {
332+
if spike >= limit && spike != 0 {
319333
log.Fatalf("%s env variable must be less than %s env variable but got %d and %d respectively", spikeName, limitName, spike, limit)
320334
}
321335

322336
// Set memory environment variables
323-
os.Setenv(limitName, strconv.Itoa(limit))
324-
os.Setenv(spikeName, strconv.Itoa(spike))
337+
if spike != 0 {
338+
os.Setenv(limitName, strconv.Itoa(limit))
339+
os.Setenv(spikeName, strconv.Itoa(spike))
340+
}
325341
}
326342

327343
func runInteractive(params service.Parameters) error {

cmd/otelcol/main_test.go

+5
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@ func TestUseConfigFromEnvVar(t *testing.T) {
8383
if !(result) {
8484
t.Error("Expected true got false while looking for --config")
8585
}
86+
useMemorySettingsMiBFromEnvVar(0)
87+
_, present := os.LookupEnv(memLimitMiBEnvVarName)
88+
if present {
89+
t.Error("Expected false got true while looking for environment variable")
90+
}
8691
os.Unsetenv(configEnvVarName)
8792
}
8893

0 commit comments

Comments
 (0)