Print logger name on logs tool calls - #670
Conversation
WalkthroughThis change introduces a new logger_name method to multiple log toolset classes, returning a string identifier for each logger. The get_parameterized_one_liner method is updated to dynamically prefix descriptions with the logger name where applicable. Corresponding test assertions are updated to reflect these prefixed descriptions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Tool
participant Toolset
User->>Tool: Invoke get_parameterized_one_liner()
Tool->>Toolset: Call logger_name()
Toolset-->>Tool: Return logger name (e.g., "DataDog")
Tool->>Tool: Prefix one-liner with logger name (if non-empty)
Tool-->>User: Return prefixed one-liner description
Estimated code review effort2 (~15 minutes) Possibly related PRs
Suggested reviewers
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
holmes/plugins/toolsets/coralogix/toolset_coralogix_logs.py(1 hunks)holmes/plugins/toolsets/datadog/toolset_datadog_logs.py(1 hunks)holmes/plugins/toolsets/datadog/toolset_datadog_traces.py(4 hunks)holmes/plugins/toolsets/grafana/toolset_grafana_loki.py(1 hunks)holmes/plugins/toolsets/logging_utils/logging_api.py(2 hunks)holmes/plugins/toolsets/opensearch/opensearch_logs.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holmes/plugins/toolsets/logging_utils/logging_api.py (2)
Learnt from: nherment
PR: #408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
Learnt from: nherment
PR: #408
File: holmes/plugins/toolsets/kubernetes_logs.py:100-102
Timestamp: 2025-05-15T05:14:06.519Z
Learning: The fetch_logs method in KubernetesLogsToolset is designed to apply the limit parameter after filtering and combining both current and previous logs, rather than using the API's tail_lines parameter, to ensure the limit applies to the final combined log set.
🧬 Code Graph Analysis (5)
holmes/plugins/toolsets/opensearch/opensearch_logs.py (4)
holmes/plugins/toolsets/grafana/toolset_grafana_loki.py (1)
logger_name(70-71)holmes/plugins/toolsets/coralogix/toolset_coralogix_logs.py (1)
logger_name(65-66)holmes/plugins/toolsets/datadog/toolset_datadog_logs.py (1)
logger_name(154-155)holmes/plugins/toolsets/logging_utils/logging_api.py (1)
logger_name(45-46)
holmes/plugins/toolsets/coralogix/toolset_coralogix_logs.py (4)
holmes/plugins/toolsets/grafana/toolset_grafana_loki.py (1)
logger_name(70-71)holmes/plugins/toolsets/datadog/toolset_datadog_logs.py (1)
logger_name(154-155)holmes/plugins/toolsets/opensearch/opensearch_logs.py (1)
logger_name(69-70)holmes/plugins/toolsets/logging_utils/logging_api.py (1)
logger_name(45-46)
holmes/plugins/toolsets/datadog/toolset_datadog_logs.py (4)
holmes/plugins/toolsets/grafana/toolset_grafana_loki.py (1)
logger_name(70-71)holmes/plugins/toolsets/coralogix/toolset_coralogix_logs.py (1)
logger_name(65-66)holmes/plugins/toolsets/opensearch/opensearch_logs.py (1)
logger_name(69-70)holmes/plugins/toolsets/logging_utils/logging_api.py (1)
logger_name(45-46)
holmes/plugins/toolsets/grafana/toolset_grafana_loki.py (4)
holmes/plugins/toolsets/coralogix/toolset_coralogix_logs.py (1)
logger_name(65-66)holmes/plugins/toolsets/datadog/toolset_datadog_logs.py (1)
logger_name(154-155)holmes/plugins/toolsets/opensearch/opensearch_logs.py (1)
logger_name(69-70)holmes/plugins/toolsets/logging_utils/logging_api.py (1)
logger_name(45-46)
holmes/plugins/toolsets/logging_utils/logging_api.py (4)
holmes/plugins/toolsets/grafana/toolset_grafana_loki.py (1)
logger_name(70-71)holmes/plugins/toolsets/coralogix/toolset_coralogix_logs.py (1)
logger_name(65-66)holmes/plugins/toolsets/datadog/toolset_datadog_logs.py (1)
logger_name(154-155)holmes/plugins/toolsets/opensearch/opensearch_logs.py (1)
logger_name(69-70)
⏰ 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: llm_evals
- GitHub Check: Pre-commit checks
- GitHub Check: build (3.10)
- GitHub Check: build (3.12)
- GitHub Check: build (3.11)
🔇 Additional comments (7)
holmes/plugins/toolsets/logging_utils/logging_api.py (2)
45-46: LGTM! Well-designed base implementation.The default empty string return value provides a sensible fallback and ensures backward compatibility for any toolsets that don't override this method.
129-132: LGTM! Clean conditional prefixing logic.The implementation correctly:
- Only adds the logger name prefix when it's non-empty
- Maintains backward compatibility with existing toolsets
- Uses clear and readable conditional formatting
The f-string approach is efficient and the logic is self-documenting.
holmes/plugins/toolsets/opensearch/opensearch_logs.py (1)
69-70: LGTM! Consistent implementation.The logger name "OpenSearch" appropriately identifies this toolset and follows the established pattern across all logging toolsets.
holmes/plugins/toolsets/grafana/toolset_grafana_loki.py (1)
70-71: LGTM! Appropriate logger identification.The logger name "Loki" correctly identifies this Grafana Loki toolset and maintains consistency with the unified logging API pattern.
holmes/plugins/toolsets/coralogix/toolset_coralogix_logs.py (1)
65-66: LGTM! Consistent toolset identification.The logger name "Coralogix" properly identifies this toolset and aligns with the standardized approach across all logging toolsets.
holmes/plugins/toolsets/datadog/toolset_datadog_logs.py (1)
154-155: LGTM! Consistent DataDog identification.The logger name "DataDog" appropriately identifies this toolset and maintains consistency with the unified logging API pattern and the related DataDog traces toolset.
holmes/plugins/toolsets/datadog/toolset_datadog_traces.py (1)
204-550: No dynamiclogger_namepatterns detected—current hardcoded prefixes are consistent across toolsets.A full-code search found no
logger_namemethod definitions or usages in other toolsets, nor any dynamic one-liner prefix logic. The implementation intoolset_datadog_traces.pyaligns with the rest of the codebase; no change is needed here.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/plugins/toolsets/datadog/traces/test_datadog_traces.py (2)
253-253: Fix the Yoda condition for better readability.The test logic is correct, but the assertion uses a Yoda condition which should be reversed for conventional style.
- assert "DataDog: fetch trace details for ID abc123" == one_liner + assert one_liner == "DataDog: fetch trace details for ID abc123"
342-342: Fix the Yoda condition for better readability.The test logic is correct, but the assertion uses a Yoda condition which should be reversed for conventional style.
- assert "DataDog: search spans with query: @http.status_code:500" == one_liner + assert one_liner == "DataDog: search spans with query: @http.status_code:500"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/plugins/toolsets/datadog/traces/test_datadog_traces.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
tests/plugins/toolsets/datadog/traces/test_datadog_traces.py
253-253: Yoda condition detected
(SIM300)
342-342: Yoda condition detected
(SIM300)
⏰ 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). (7)
- GitHub Check: build (3.12)
- GitHub Check: build (3.11)
- GitHub Check: build (3.10)
- GitHub Check: build (3.12)
- GitHub Check: build (3.10)
- GitHub Check: build (3.11)
- GitHub Check: llm_evals
🔇 Additional comments (1)
tests/plugins/toolsets/datadog/traces/test_datadog_traces.py (1)
253-253: Well-implemented test updates for logger name prefixing.The test assertions have been correctly updated to reflect the new "DataDog: " prefix functionality. The changes are minimal, focused, and align well with the described feature enhancement for better log source identification.
Also applies to: 342-342
No description provided.