Add triple chain-of-causation with kafka test - #850
Conversation
…g consumer group alive
WalkthroughAdds a multi-stage Kubernetes test fixture under tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency: namespace app-156, Zookeeper, a Python in-memory OpenSearch-like service, Kafka broker, producer (order-service), consumer (analytics-service), orchestration YAML, verification script, and increases a test-run timeout constant. Changes
Sequence Diagram(s)sequenceDiagram
participant Producer as order-service
participant Kafka as Kafka Broker
participant Consumer as analytics-service
participant OpenSearch as OpenSearch (Python)
Producer->>Kafka: Produce JSON message (topic: messages)
Kafka-->>Producer: ACK
Consumer->>Kafka: Poll (group: analytics-group)
Kafka-->>Consumer: Deliver messages
Consumer->>OpenSearch: Index document (refresh=true)
OpenSearch-->>Consumer: Index response (may delay / heavy CPU when MERGE_POLICY=tiered)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage1-base.yaml (6)
36-51: Harden Zookeeper container with securityContext (allowPrivilegeEscalation/root).Static analysis flags missing securityContext. Add least-privilege defaults.
spec: containers: - name: zookeeper image: bitnami/zookeeper:3.8 + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + drop: ["ALL"] ... + securityContext: + runAsNonRoot: true + seccompProfile: + type: RuntimeDefaultConfirm the bitnami image is compatible with readOnlyRootFilesystem; if not, relax only that setting but keep allowPrivilegeEscalation: false and drop caps.
81-99: Harden OpenSearch container with securityContext (allowPrivilegeEscalation/root).Apply least-privilege defaults to the Python container.
spec: containers: - name: opensearch image: python:3.9-slim + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + runAsUser: 1000 + runAsGroup: 1000 + capabilities: + drop: ["ALL"] ... + securityContext: + runAsNonRoot: true + seccompProfile: + type: RuntimeDefault
99-106: Add liveness/readiness probes (not just startupProbe).StartupProbe is good for boot, but adding liveness/readiness will improve stability for test orchestration.
startupProbe: httpGet: path: /_cluster/health port: 9200 initialDelaySeconds: 10 periodSeconds: 10 failureThreshold: 30 + readinessProbe: + httpGet: + path: /_cluster/health + port: 9200 + initialDelaySeconds: 5 + periodSeconds: 5 + failureThreshold: 6 + livenessProbe: + httpGet: + path: /_cluster/health + port: 9200 + initialDelaySeconds: 20 + periodSeconds: 10 + failureThreshold: 3
121-130: Use ThreadingHTTPServer to avoid single-threaded request handling.The current HTTPServer handles requests serially; with heavy background CPU, single-threading can amplify latency. ThreadingHTTPServer offers more realistic concurrent behavior for the eval.
- from http.server import HTTPServer, BaseHTTPRequestHandler + from http.server import ThreadingHTTPServer, BaseHTTPRequestHandler- httpd = HTTPServer(server_address, OpenSearchHandler) + httpd = ThreadingHTTPServer(server_address, OpenSearchHandler)Also applies to: 308-314
233-247: Guard against invalid JSON bodies in indexing endpoint.A bad JSON body will throw and crash the handler. Wrap json.loads with try/except and return 400.
- if index_name in documents: - doc = json.loads(body) if body else {} + if index_name in documents: + try: + doc = json.loads(body) if body else {} + except json.JSONDecodeError: + self.send_json_response(400, {'error': 'invalid_json'}) + return
240-249: Minor: REFRESH_INTERVAL is unused beyond logging.If intentional, comment it as a no-op for realism; otherwise wire it into timing logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage1-base.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
tests/llm/**/*.yaml
📄 CodeRabbit Inference Engine (CLAUDE.md)
tests/llm/**/*.yaml: In evals, ALWAYS use Kubernetes Secrets for scripts; do not embed scripts in inline manifests or ConfigMaps
Each eval test must use a dedicated Kubernetes namespace named app-
All pod names in evals must be unique and should not hint at the problem (use neutral names)
Files:
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage1-base.yaml
🪛 Checkov (3.2.334)
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage1-base.yaml
[MEDIUM] 21-51: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[MEDIUM] 21-51: Minimize the admission of root containers
(CKV_K8S_23)
[MEDIUM] 66-113: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[MEDIUM] 66-113: Minimize the admission of root containers
(CKV_K8S_23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Pre-commit checks
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
🔇 Additional comments (1)
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage1-base.yaml (1)
2-6: Namespace complies with eval guideline (dedicated app-156).Namespace name matches the required pattern app-.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/llm/utils/commands.py (3)
9-13: Harden env var parsing to avoid import-time ValueError when EVAL_SETUP_TIMEOUT is invalid.If someone sets EVAL_SETUP_TIMEOUT to a non-integer, importing this module will crash. Fall back to a sane default and log a warning instead.
Apply this diff:
-EVAL_SETUP_TIMEOUT = int(os.environ.get("EVAL_SETUP_TIMEOUT", "210")) +DEFAULT_EVAL_SETUP_TIMEOUT = 210 +_raw_timeout = os.environ.get("EVAL_SETUP_TIMEOUT", str(DEFAULT_EVAL_SETUP_TIMEOUT)) +try: + EVAL_SETUP_TIMEOUT = int(_raw_timeout) +except ValueError: + logging.warning( + "Invalid EVAL_SETUP_TIMEOUT=%r; defaulting to %s", + _raw_timeout, + DEFAULT_EVAL_SETUP_TIMEOUT, + ) + EVAL_SETUP_TIMEOUT = DEFAULT_EVAL_SETUP_TIMEOUT
30-39: Use Optional[...] where defaults are None to align with type hints and future mypy enforcement.This is a small cleanup that will help if we ever remove the file-level type ignore.
Apply this diff:
- def __init__( + def __init__( self, command: str, test_case_id: str, success: bool, - exit_code: int = None, - elapsed_time: float = 0, - error_type: str = None, - error_details: str = None, + exit_code: Optional[int] = None, + elapsed_time: float = 0.0, + error_type: Optional[str] = None, + error_details: Optional[str] = None, ):
56-69: Force Bash in subprocess invocation for verify scriptsThe
verify_setup.shfixture uses Bash-only syntax (#!/bin/bash,[[ … ]]), butsubprocess.run(shell=True)defaults to/bin/sh. This can break on shells without Bash extensions. To ensure compatibility, explicitly set the executable to Bash.Locations to update:
- tests/llm/utils/commands.py at the
subprocess.runcall inside_invoke_commandProposed diff:
result = subprocess.run( command, shell=True, + executable="/bin/bash", capture_output=True, text=True, check=True, stdin=subprocess.DEVNULL, cwd=cwd, timeout=EVAL_SETUP_TIMEOUT, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/llm/utils/commands.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.py: ALWAYS place Python imports at the top of the file, not inside functions or methods
Type hints are required (project is type-checked with mypy)
Use Ruff for formatting and linting (configured in pyproject.toml)
Files:
tests/llm/utils/commands.py
{tests/**/*.py,pyproject.toml}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Only use pytest markers that are defined in pyproject.toml; never introduce undefined markers in tests
Files:
tests/llm/utils/commands.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Pre-commit checks
- GitHub Check: Pre-commit checks
🔇 Additional comments (1)
tests/llm/utils/commands.py (1)
11-11: LGTM: Timeout bump to 210s is reasonable for the new multi-service setup.This should reduce flakiness when bringing up Kafka/OpenSearch in CI, and it remains easily overrideable via EVAL_SETUP_TIMEOUT.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (7)
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage1-base.yaml (2)
11-18: Rename technology-specific resources to neutral names (svc-a/svc-b) per eval guidelines.Pods/Services must be neutral and not hint at the scenario. Please rename zookeeper → svc-a and opensearch → svc-b across metadata names, labels/selectors, and container names.
Apply:
@@ -metadata: - name: zookeeper +metadata: + name: svc-a @@ - selector: - app: zookeeper + selector: + app: svc-a @@ -metadata: - name: zookeeper +metadata: + name: svc-a @@ - matchLabels: - app: zookeeper + matchLabels: + app: svc-a @@ - labels: - app: zookeeper + labels: + app: svc-a @@ - - name: zookeeper + - name: svc-a @@ -metadata: - name: opensearch +metadata: + name: svc-b @@ - selector: - app: opensearch + selector: + app: svc-b @@ -metadata: - name: opensearch +metadata: + name: svc-b @@ - matchLabels: - app: opensearch + matchLabels: + app: svc-b @@ - labels: - app: opensearch + labels: + app: svc-b @@ - - name: opensearch + - name: svc-bRun this to locate all remaining tech-specific names that must be neutralized across the fixture set:
#!/bin/bash # Scan the repo for tech-specific names in test YAMLs that violate neutrality rg -n -C2 -g 'tests/llm/**/*.yaml' -e '\b(zookeeper|opensearch|kafka|order-service|analytics-service)\b'Also applies to: 24-38, 55-62, 68-81
105-112: Use Secrets for scripts; replace ConfigMap and volume with a Secret.Per tests/llm guidelines, embed scripts in Secrets, not ConfigMaps. Also align the script/volume name with the neutral svc-b naming.
@@ - - name: opensearch-script + - name: svc-b-script mountPath: /app @@ - - name: opensearch-script - configMap: - name: opensearch-script + - name: svc-b-script + secret: + secretName: svc-b-script --- -# ConfigMap for OpenSearch -apiVersion: v1 -kind: ConfigMap +# Secret for OpenSearch script +apiVersion: v1 +kind: Secret metadata: - name: opensearch-script + name: svc-b-script namespace: app-156 -data: +type: Opaque +stringData: opensearch_server.py: | import json import time import os import threading from http.server import HTTPServer, BaseHTTPRequestHandlerAlso applies to: 114-121
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage2-kafka.yaml (1)
5-12: Neutralize Kafka resource names and update env to match svc-a/svc-c.Rename kafka → svc-c and ensure ZK reference points to svc-a. Also update advertised listeners.
@@ -metadata: - name: kafka +metadata: + name: svc-c @@ - selector: - app: kafka + selector: + app: svc-c @@ -metadata: - name: kafka +metadata: + name: svc-c @@ - matchLabels: - app: kafka + matchLabels: + app: svc-c @@ - labels: - app: kafka + labels: + app: svc-c @@ - - name: kafka + - name: svc-c image: bitnami/kafka:3.5 @@ - - name: KAFKA_CFG_ZOOKEEPER_CONNECT - value: "zookeeper:2181" + - name: KAFKA_CFG_ZOOKEEPER_CONNECT + value: "svc-a:2181" @@ - - name: KAFKA_CFG_ADVERTISED_LISTENERS - value: "PLAINTEXT://kafka:9092" + - name: KAFKA_CFG_ADVERTISED_LISTENERS + value: "PLAINTEXT://svc-c:9092"Also applies to: 18-29, 31-45
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage4-producer.yaml (2)
1-9: Store producer script in a Secret and mount via secret volume.ConfigMaps must not embed scripts in evals. Convert to Secret and adjust the volume reference.
-# Order Service ConfigMap +# Producer script Secret apiVersion: v1 -kind: ConfigMap +kind: Secret metadata: - name: order-service-script + name: svc-e-script namespace: app-156 -data: +type: Opaque +stringData: order_service.py: | @@ - - name: order-service-script - configMap: - name: order-service-script + - name: svc-e-script + secret: + secretName: svc-e-scriptAlso applies to: 88-90
53-67: Neutralize producer resource names and update bootstrap servers to svc-c.Rename order-service → svc-e across metadata/container/labels/volumes and set KAFKA_BOOTSTRAP_SERVERS to svc-c.
@@ -metadata: - name: order-service +metadata: + name: svc-e @@ - matchLabels: - app: order-service + matchLabels: + app: svc-e @@ - labels: - app: order-service + labels: + app: svc-e @@ - - name: order-service + - name: svc-e image: python:3.9-slim @@ - - name: KAFKA_BOOTSTRAP_SERVERS - value: "kafka:9092" + - name: KAFKA_BOOTSTRAP_SERVERS + value: "svc-c:9092" @@ - - name: order-service-script + - name: svc-e-script mountPath: /app @@ - - name: order-service-script + - name: svc-e-scriptAlso applies to: 59-64, 74-80, 82-87, 88-90
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage3-consumer.yaml (2)
1-8: Move analytics script to a Secret and mount via secret volume.Replace ConfigMap with a Secret and update the volume to use secretName.
-# Analytics Service ConfigMap +# Analytics script Secret apiVersion: v1 -kind: ConfigMap +kind: Secret metadata: - name: analytics-service-script + name: svc-d-script namespace: app-156 -data: +type: Opaque +stringData: analytics_service.py: | @@ - - name: analytics-service-script - configMap: - name: analytics-service-script + - name: svc-d-script + secret: + secretName: svc-d-scriptAlso applies to: 133-136
94-108: Neutralize consumer resource names and align service env with svc-b/svc-c.Rename analytics-service → svc-d across metadata/container/labels/volumes. Update OPENSEARCH_HOST to svc-b and KAFKA_BOOTSTRAP_SERVERS to svc-c.
@@ -metadata: - name: analytics-service +metadata: + name: svc-d @@ - matchLabels: - app: analytics-service + matchLabels: + app: svc-d @@ - labels: - app: analytics-service + labels: + app: svc-d @@ - - name: analytics-service + - name: svc-d image: python:3.9-slim @@ - - name: KAFKA_BOOTSTRAP_SERVERS - value: "kafka:9092" + - name: KAFKA_BOOTSTRAP_SERVERS + value: "svc-c:9092" @@ - - name: OPENSEARCH_HOST - value: "opensearch" + - name: OPENSEARCH_HOST + value: "svc-b" @@ - - name: analytics-service-script + - name: svc-d-script mountPath: /app @@ - - name: analytics-service-script + - name: svc-d-scriptAlso applies to: 100-105, 114-121, 123-136
🧹 Nitpick comments (4)
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage1-base.yaml (1)
36-50: Harden containers with a minimal securityContext (optional).Reduce Checkov findings by setting non-root and disabling privilege escalation. Safe for these evals.
@@ - name: svc-a image: bitnami/zookeeper:3.8 + securityContext: + runAsNonRoot: true + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true @@ - name: svc-b image: python:3.9-slim + securityContext: + runAsNonRoot: true + allowPrivilegeEscalation: false + readOnlyRootFilesystem: trueAlso applies to: 80-98
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage2-kafka.yaml (1)
30-52: Add basic securityContext to reduce container risk (optional).@@ - name: svc-c image: bitnami/kafka:3.5 + securityContext: + runAsNonRoot: true + allowPrivilegeEscalation: false + readOnlyRootFilesystem: truetests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage4-producer.yaml (1)
65-73: Optional: add securityContext and minor startup hardening.Add a basic securityContext. Consider using pip with no cache to reduce layer size and speed.
@@ - - name: svc-e + - name: svc-e image: python:3.9-slim command: ["sh", "-c"] args: - | - pip install kafka-python + pip install --no-cache-dir kafka-python python /app/order_service.py + securityContext: + runAsNonRoot: true + allowPrivilegeEscalation: false + readOnlyRootFilesystem: trueAlso applies to: 81-87
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage3-consumer.yaml (1)
106-114: Add a minimal securityContext for the consumer container (optional).@@ - name: svc-d image: python:3.9-slim command: ["sh", "-c"] args: - | pip install kafka-python opensearch-py python /app/analytics_service.py + securityContext: + runAsNonRoot: true + allowPrivilegeEscalation: false + readOnlyRootFilesystem: trueAlso applies to: 126-132
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage1-base.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage2-kafka.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage3-consumer.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage4-producer.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
tests/llm/**/*.yaml
📄 CodeRabbit Inference Engine (CLAUDE.md)
tests/llm/**/*.yaml: In evals, ALWAYS use Kubernetes Secrets for scripts; do not embed scripts in inline manifests or ConfigMaps
Each eval test must use a dedicated Kubernetes namespace named app-
All pod names in evals must be unique and should not hint at the problem (use neutral names)
Files:
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage1-base.yamltests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage2-kafka.yamltests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage3-consumer.yamltests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage4-producer.yaml
🧠 Learnings (2)
📚 Learning: 2025-08-17T08:42:48.763Z
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T08:42:48.763Z
Learning: Applies to tests/llm/**/*.yaml : All pod names in evals must be unique and should not hint at the problem (use neutral names)
Applied to files:
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage1-base.yaml
📚 Learning: 2025-08-17T08:42:48.763Z
Learnt from: CR
PR: robusta-dev/holmesgpt#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T08:42:48.763Z
Learning: Applies to tests/llm/**/*.yaml : In evals, ALWAYS use Kubernetes Secrets for scripts; do not embed scripts in inline manifests or ConfigMaps
Applied to files:
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage1-base.yaml
🪛 Checkov (3.2.334)
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage1-base.yaml
[MEDIUM] 21-50: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[MEDIUM] 21-50: Minimize the admission of root containers
(CKV_K8S_23)
[MEDIUM] 65-112: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[MEDIUM] 65-112: Minimize the admission of root containers
(CKV_K8S_23)
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage2-kafka.yaml
[MEDIUM] 15-61: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[MEDIUM] 15-61: Minimize the admission of root containers
(CKV_K8S_23)
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage3-consumer.yaml
[MEDIUM] 91-135: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[MEDIUM] 91-135: Minimize the admission of root containers
(CKV_K8S_23)
tests/llm/fixtures/test_ask_holmes/156_kafka_opensearch_latency/app/stage4-producer.yaml
[MEDIUM] 50-90: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[MEDIUM] 50-90: Minimize the admission of root containers
(CKV_K8S_23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build (3.12)
- GitHub Check: build (3.10)
- GitHub Check: build (3.11)
- GitHub Check: build (3.10)
- GitHub Check: build (3.11)
- GitHub Check: build (3.12)
2 participants
Test case demonstrating a chain of causation for Kafka latency: