diff --git a/docs/DIAGNOSE.md b/docs/DIAGNOSE.md index 7b437d5..14d0b08 100644 --- a/docs/DIAGNOSE.md +++ b/docs/DIAGNOSE.md @@ -52,7 +52,13 @@ secret strength, private directory permissions, or actual traffic coverage. Checks follow the `genaisketch` connector conventions from collector v0.3.0. OTLP receivers with explicit gRPC/HTTP protocol maps, batch and basic memory-limiter processors, Prometheus exporters, and basic OTLP/HTTP or OTLP -exporter settings are recognized. Trace and metric pipeline references, signal +exporter settings are recognized. Memory-limiter percentage bounds and the +percentage spike-versus-limit rule follow the +[OpenTelemetry Collector v0.161.0 validation](https://github.com/open-telemetry/opentelemetry-collector/blob/v0.161.0/internal/memorylimiter/config.go). +A nonzero `limit_mib` takes precedence over percentage settings; configured +percentage fields are still validated. An omitted spike limit defaults to 20% +of the effective limit. These checks are static and do not probe host memory or +certify that a chosen limit is sufficient. Trace and metric pipeline references, signal direction, connector inputs/outputs, duplicate references, and unused components are checked. Multiple connector instances or connector input/output pipelines need an independent disjointness review and yield indeterminate. diff --git a/internal/diagnose/locations.go b/internal/diagnose/locations.go index bd8a49c..5e6640e 100644 --- a/internal/diagnose/locations.go +++ b/internal/diagnose/locations.go @@ -20,7 +20,7 @@ mcp slices fields weights dedup summary_export hllpp frequent_items bloom algo s llm_operations enabled tool_errors request_id_from name keys from_resource_attributes from_attributes canonicalization domain field weight directory producer_id scope_id key_id interval fallback_when_missing timeout send_batch_size send_batch_max_size check_interval -limit_mib spike_limit_mib verbosity telemetry logs level`) +limit_mib spike_limit_mib limit_percentage spike_limit_percentage verbosity telemetry logs level`) func schemaPaths(root *yaml.Node) map[*yaml.Node]string { paths := make(map[*yaml.Node]string) diff --git a/internal/diagnose/pipeline.go b/internal/diagnose/pipeline.go index 877be55..765b284 100644 --- a/internal/diagnose/pipeline.go +++ b/internal/diagnose/pipeline.go @@ -52,19 +52,35 @@ func (c *checker) configuration(n *yaml.Node) { } } case group == "processors" && kind == "memory_limiter": - p := c.object(config, "check_interval", "limit_mib", "spike_limit_mib") + p := c.object(config, "check_interval", "limit_mib", "spike_limit_mib", "limit_percentage", "spike_limit_percentage") c.duration(p["check_interval"], time.Millisecond, time.Hour) c.integer(p["limit_mib"], 1, 1<<30) c.integer(p["spike_limit_mib"], 0, 1<<30) - if p["limit_mib"] == nil { - c.add("unsupported_mapping", "unsupported", config) + c.integer(p["limit_percentage"], 0, 100) + c.integer(p["spike_limit_percentage"], 0, 100) + integerValue := func(n *yaml.Node) int64 { + if n == nil { + return 0 + } + v, _ := strconv.ParseInt(n.Value, 10, 64) + return v } - if a, b := p["limit_mib"], p["spike_limit_mib"]; a != nil && b != nil { - av, _ := strconv.ParseInt(a.Value, 10, 64) - bv, _ := strconv.ParseInt(b.Value, 10, 64) - if bv >= av { - c.add("unsupported_mapping", "unsupported", config) + limitMiB := integerValue(p["limit_mib"]) + limitPercentage := integerValue(p["limit_percentage"]) + if limitMiB == 0 && limitPercentage == 0 { + limit := config + if p["limit_percentage"] != nil { + limit = p["limit_percentage"] + } else if p["limit_mib"] != nil { + limit = p["limit_mib"] } + c.add("unsupported_mapping", "unsupported", limit) + } + if spikeMiB := p["spike_limit_mib"]; spikeMiB != nil && limitMiB > 0 && integerValue(spikeMiB) >= limitMiB { + c.add("unsupported_mapping", "unsupported", spikeMiB) + } + if spikePercentage := p["spike_limit_percentage"]; spikePercentage != nil && limitPercentage > 0 && integerValue(spikePercentage) >= limitPercentage { + c.add("unsupported_mapping", "unsupported", spikePercentage) } case group == "exporters" && kind == "prometheus": p := c.object(config, "endpoint", "translation_strategy") diff --git a/internal/diagnose/pipeline_memory_limiter_test.go b/internal/diagnose/pipeline_memory_limiter_test.go new file mode 100644 index 0000000..5498671 --- /dev/null +++ b/internal/diagnose/pipeline_memory_limiter_test.go @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: Apache-2.0 +// Code authors: Vijay and Codex + +package diagnose + +import ( + "encoding/json" + "strings" + "testing" +) + +func memoryLimiterInput(t *testing.T, settings string) string { + t.Helper() + input := strings.ReplaceAll(fixture(t, "safe"), "\r\n", "\n") + const processors = "processors:\n batch: {timeout: 1s}" + if !strings.Contains(input, processors) { + t.Fatal("safe fixture no longer has expected processors") + } + input = strings.Replace(input, processors, processors+"\n memory_limiter:\n check_interval: 1s\n"+settings, 1) + const pipeline = "processors: [batch]" + if !strings.Contains(input, pipeline) { + t.Fatal("safe fixture no longer has expected trace processors") + } + return strings.Replace(input, pipeline, "processors: [batch, memory_limiter]", 1) +} + +func TestMemoryLimiterPercentageSettings(t *testing.T) { + for _, tc := range []struct { + name string + settings string + wantStatus string + wantFindingID string + wantPath string + lineField string + redact string + }{ + {name: "percentage only", settings: " limit_percentage: 80\n spike_limit_percentage: 20\n", wantStatus: "supported_safe"}, + {name: "default spike percentage", settings: " limit_percentage: 80\n", wantStatus: "supported_safe"}, + {name: "100 percent limit", settings: " limit_percentage: 100\n spike_limit_percentage: 99\n", wantStatus: "supported_safe"}, + {name: "zero percentage requires a fixed MiB limit", settings: " limit_percentage: 0\n", wantStatus: "indeterminate", wantFindingID: "unsupported_mapping", wantPath: "processors.memory_limiter.limit_percentage", lineField: "limit_percentage", redact: ""}, + {name: "one limit is required", settings: "", wantStatus: "indeterminate", wantFindingID: "unsupported_mapping", wantPath: "processors.memory_limiter", lineField: "check_interval"}, + {name: "percentage limit above range", settings: " limit_percentage: 101\n spike_limit_percentage: 20\n", wantStatus: "indeterminate", wantFindingID: "unsupported_mapping", wantPath: "processors.memory_limiter.limit_percentage", lineField: "limit_percentage", redact: "101"}, + {name: "percentage limit below range", settings: " limit_percentage: -1\n", wantStatus: "indeterminate", wantFindingID: "unsupported_mapping", wantPath: "processors.memory_limiter.limit_percentage", lineField: "limit_percentage", redact: "-1"}, + {name: "spike percentage above range", settings: " limit_percentage: 80\n spike_limit_percentage: 101\n", wantStatus: "indeterminate", wantFindingID: "unsupported_mapping", wantPath: "processors.memory_limiter.spike_limit_percentage", lineField: "spike_limit_percentage", redact: "101"}, + {name: "spike at limit", settings: " limit_percentage: 80\n spike_limit_percentage: 80\n", wantStatus: "indeterminate", wantFindingID: "unsupported_mapping", wantPath: "processors.memory_limiter.spike_limit_percentage", lineField: "spike_limit_percentage", redact: "80"}, + {name: "spike above limit", settings: " limit_percentage: 80\n spike_limit_percentage: 90\n", wantStatus: "indeterminate", wantFindingID: "unsupported_mapping", wantPath: "processors.memory_limiter.spike_limit_percentage", lineField: "spike_limit_percentage", redact: "90"}, + {name: "fixed MiB pair takes precedence over percentages", settings: " limit_mib: 1024\n spike_limit_mib: 128\n limit_percentage: 80\n spike_limit_percentage: 20\n", wantStatus: "supported_safe"}, + {name: "percentage validation still applies with fixed MiB", settings: " limit_mib: 1024\n spike_limit_mib: 128\n limit_percentage: 101\n", wantStatus: "indeterminate", wantFindingID: "unsupported_mapping", wantPath: "processors.memory_limiter.limit_percentage", lineField: "limit_percentage", redact: "101"}, + {name: "unknown setting remains unsupported", settings: " limit_percentage: 80\n mystery_setting: PRIVATE_SENTINEL_8c4a\n", wantStatus: "indeterminate", wantFindingID: "unsupported_field", wantPath: "processors.memory_limiter.[key-3]", lineField: "mystery_setting", redact: "PRIVATE_SENTINEL_8c4a"}, + {name: "existing MiB-only form", settings: " limit_mib: 1024\n spike_limit_mib: 128\n", wantStatus: "supported_safe"}, + } { + t.Run(tc.name, func(t *testing.T) { + input := memoryLimiterInput(t, tc.settings) + report := check(t, input) + if report.Status != tc.wantStatus { + t.Fatalf("status = %q, want %q; findings: %+v", report.Status, tc.wantStatus, report.Findings) + } + if tc.wantFindingID != "" { + wantLine := strings.Count(input[:strings.Index(input, tc.lineField)], "\n") + 1 + found := false + for _, finding := range report.Findings { + if finding.ID == tc.wantFindingID && finding.Path == tc.wantPath { + found = true + if finding.Line != wantLine { + t.Errorf("finding line = %d, want %d", finding.Line, wantLine) + } + } + } + if !found { + t.Fatalf("missing %s at %s; findings: %+v", tc.wantFindingID, tc.wantPath, report.Findings) + } + encoded, err := json.Marshal(report) + if err != nil { + t.Fatal(err) + } + if tc.redact != "" && strings.Contains(string(encoded), tc.redact) { + t.Fatalf("report leaked the rejected value %q", tc.redact) + } + } + }) + } +}