Skip to content

fixing failures due to non standard pod name in eval 101. test 100b is for testing nonstandard labels. - #1237

Merged
Sheeproid merged 2 commits into
masterfrom
fix-test-loki-label
Dec 24, 2025
Merged

Sheeproid merged 2 commits into
masterfrom
fix-test-loki-label

Conversation

@Sheeproid

@Sheeproid Sheeproid commented Dec 24, 2025 •

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Tests
    • Updated test fixtures for log metadata extraction and processing scenarios
    • Refined test configuration for Grafana Loki integration

✏️ Tip: You can customize this high-level summary in your review settings.

…s for testing nonstandard labels.

Signed-off-by: Tomer Keshet <tomer@robusta.dev>
@coderabbitai

coderabbitai Bot commented Dec 24, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Test fixture configurations updated for a Loki historical logs scenario: renaming JSON field extraction from pod_name to pod in Promtail configuration and removing the redundant labels block from the Loki toolset configuration.

Changes

Cohort / File(s) Summary
Loki test fixture updates
tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/promtail-config.yaml, tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/toolsets.yaml
Renamed JSON expression field from pod_name to pod in Promtail log extraction; removed labels block (pod, namespace) from Loki toolset while retaining empty api_key

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested reviewers

  • aantn
  • moshemorad

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main purpose: fixing test 101 failures caused by non-standard pod naming conventions and clarifying that test 100b handles non-standard labels.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9202490 and 4508944.

📒 Files selected for processing (2)
  • tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/promtail-config.yaml
  • tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/toolsets.yaml
🧰 Additional context used
📓 Path-based instructions (4)
tests/**

📄 CodeRabbit inference engine (CLAUDE.md)

Tests: Match source structure under tests/

Files:

  • tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/toolsets.yaml
  • tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/promtail-config.yaml
tests/llm/**/*.yaml

📄 CodeRabbit inference engine (CLAUDE.md)

tests/llm/**/*.yaml: ALWAYS use Secrets for scripts in eval tests, not inline manifests or ConfigMaps, to prevent code visibility with kubectl describe
All pod names must be unique across LLM tests - never reuse pod names between tests
Never use resource names that hint at the problem or expected behavior in evals - use neutral names that don't give away what the LLM should discover

Files:

  • tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/toolsets.yaml
  • tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/promtail-config.yaml
tests/llm/**

📄 CodeRabbit inference engine (CLAUDE.md)

tests/llm/**: Each LLM test must use a dedicated namespace app- to prevent conflicts when tests run simultaneously
No fake/obvious logs in eval scenarios - avoid logs like 'Memory usage stabilized at 800MB'
No hints in eval filenames - use realistic names like 'training_pipeline.py' instead of 'disk_consumer.py'
Implement full architecture even if complex in evals (e.g., use Loki for log aggregation properly, not simplified alternatives)

Files:

  • tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/toolsets.yaml
  • tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/promtail-config.yaml
**/*toolsets.yaml

📄 CodeRabbit inference engine (CLAUDE.md)

All toolset-specific configuration must go under a 'config' field in toolsets.yaml, not at the top level

Files:

  • tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/toolsets.yaml
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/**/*.yaml : All pod names must be unique across LLM tests - never reuse pod names between tests
📚 Learning: 2025-12-21T13:17:48.366Z
Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to holmes/plugins/toolsets/** : Toolsets: holmes/plugins/toolsets/{name}.yaml or {name}/ directory structure

Applied to files:

  • tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/toolsets.yaml
📚 Learning: 2025-12-21T13:17:48.366Z
Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to **/*toolsets.yaml : All toolset-specific configuration must go under a 'config' field in toolsets.yaml, not at the top level

Applied to files:

  • tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/toolsets.yaml
📚 Learning: 2025-12-21T13:17:48.366Z
Learnt from: CR
Repo: HolmesGPT/holmesgpt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-21T13:17:48.366Z
Learning: Applies to tests/llm/**/*.yaml : All pod names must be unique across LLM tests - never reuse pod names between tests

Applied to files:

  • tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/promtail-config.yaml
⏰ 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). (5)
  • GitHub Check: build (3.11)
  • GitHub Check: build (3.12)
  • GitHub Check: build (3.10)
  • GitHub Check: llm_evals
  • GitHub Check: build
🔇 Additional comments (2)
tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/promtail-config.yaml (1)

39-39: LGTM! Field renaming is consistent across extraction and labeling.

The change from pod_name to pod is applied consistently in both the JSON extraction stage (Line 39) and the labels stage (Line 46), ensuring proper log metadata extraction and label propagation for the standardized pod field name.

Also applies to: 46-46

tests/llm/fixtures/test_ask_holmes/101_loki_historical_logs_pod_deleted/toolsets.yaml (1)

6-10: LGTM! Configuration structure follows guidelines.

The Loki toolset configuration properly places all settings under the config field as required by coding guidelines. The empty api_key is appropriate for test fixtures.


Comment @coderabbitai help to get the list of available commands and usage tips.

@Sheeproid
Sheeproid requested a review from RoiGlinik December 24, 2025 10:12
@Sheeproid
Sheeproid enabled auto-merge (squash) December 24, 2025 10:12
@github-actions

github-actions Bot commented Dec 24, 2025 •

Copy link
Copy Markdown
Contributor

Dev Docker images are ready for this commit:

Use this tag to pull the image for testing.

⚠️ Temporary images are deleted after 30 days. Copy to a permanent registry before using them:

gcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:66d913c
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:66d913c me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:66d913c
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:66d913c

Patch Helm values in one line (choose the chart you use):

  • HolmesGPT chart:
helm upgrade --install holmesgpt ./helm/holmes \
  --set registry=me-west1-docker.pkg.dev/robusta-development/development \
  --set image=holmes-dev:66d913c
  • Robusta wrapper chart:
helm upgrade --install robusta robusta/robusta \
  --reuse-values \
  --set holmes.registry=me-west1-docker.pkg.dev/robusta-development/development \
  --set holmes.image=holmes-dev:66d913c

@github-actions

Copy link
Copy Markdown
Contributor

Results of HolmesGPT evals

  • ask_holmes: 32/37 test cases were successful, 0 regressions, 2 setup failures, 3 mock failures
Test suite Test case Status
ask 01_how_many_pods ✅
ask 02_what_is_wrong_with_pod ✅
ask 04_related_k8s_events ✅
ask 05_image_version ✅
ask 09_crashpod ✅
ask 10_image_pull_backoff ✅
ask 110_k8s_events_image_pull ✅
ask 11_init_containers ✅
ask 13a_pending_node_selector_basic ✅
ask 14_pending_resources ✅
ask 15_failed_readiness_probe ✅
ask 163_compaction_follow_up ✅
ask 17_oom_kill 🚧
ask 18_oom_kill_from_issues_history ✅
ask 19_detect_missing_app_details ✅
ask 20_long_log_file_search ✅
ask 24_misconfigured_pvc ✅
ask 24a_misconfigured_pvc_basic ✅
ask 28_permissions_error 🚧
ask 39_failed_toolset ✅
ask 41_setup_argo ✅
ask 42_dns_issues_steps_new_tools ✅
ask 43_current_datetime_from_prompt ✅
ask 45_fetch_deployment_logs_simple ✅
ask 51_logs_summarize_errors ✅
ask 53_logs_find_term ✅
ask 54_not_truncated_when_getting_pods ✅
ask 59_label_based_counting ✅
ask 60_count_less_than ✅
ask 61_exact_match_counting ✅
ask 63_fetch_error_logs_no_errors ✅
ask 79_configmap_mount_issue ✅
ask 83_secret_not_found ✅
ask 86_configmap_like_but_secret ✅
ask 93_calling_datadog[0] 🔧
ask 93_calling_datadog[1] 🔧
ask 93_calling_datadog[2] 🔧

Legend

  • ✅ the test was successful
  • :minus: the test was skipped
  • ⚠️ the test failed but is known to be flaky or known to fail
  • 🚧 the test had a setup failure (not a code regression)
  • 🔧 the test failed due to mock data issues (not a code regression)
  • 🚫 the test was throttled by API rate limits/overload
  • ❌ the test failed and should be fixed before merging the PR

@Sheeproid
Sheeproid merged commit daaad07 into master Dec 24, 2025
10 of 11 checks passed
@Sheeproid
Sheeproid deleted the fix-test-loki-label branch December 24, 2025 11:14
moshemorad pushed a commit that referenced this pull request Dec 25, 2025
…s for testing nonstandard labels. (#1237)

Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Mohse Morad <moshemorad12340@gmail.com>
FilipGrebowski pushed a commit to FilipGrebowski/holmesgpt that referenced this pull request Dec 27, 2025
…s for testing nonstandard labels. (HolmesGPT#1237)

Signed-off-by: Tomer Keshet <tomer@robusta.dev>
Signed-off-by: Filip Grebowski <grebowskifilip@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants