Merge branch 'master' into ROB-2161_improved_truncation_methodology

Author: nherment

holmes/core/supabase_dal.py

@@ -342,6 +342,7 @@ def extract_relevant_issues(self, evidence):
             self.unzip_evidence_file(enrich)
             for enrich in evidence.data
             if enrich.get("enrichment_type") == "text_file"
+            or enrich.get("enrichment_type") == "alert_raw_data"
         ]
 
         data.extend(unzipped_files)

holmes/plugins/toolsets/newrelic/newrelic.jinja2

@@ -31,3 +31,12 @@ Note: Use `SHOW EVENT TYPES` to see all event types in the account, in addition
 
 ### 3. Metrics/graphs
 - You always need to embed the partial result in your response. Include the "tool_name" and "random_key". For example: << {"type": "newrelicql", "tool_name": "execute_prometheus_range_query", "random_key": "92jf2hf"} >>
+
+### Instructions for Handling Query Results
+- If you query DistributedTraceSummary:
+    - ALWAYS use the filter `WHERE root.span.eventType = 'Span'`
+    - When querying without aggregations (e.g. without count(*), average(attribute), sum(attribute), min(attribute), etc.):
+        - ALWAYS include an embed, and don't include the raw results as plain text, for example << {"type": "traces-summary", "tool_name": "newrelic_execute_nrql_query", "random_key": "928js0l"} >>. "random_key" should always be replaced with the "random_key" from the tool's output.
+        - ALWAYS start by querying all fields using `SELECT * FROM DistributedTraceSummary`. We need as much fields as possible to visualize the traces to the user. However, the trade-off is that we might exceed the context size. In that case, if you need to narrow down your search, follow this strategy: First, select only the essential fields: trace.id, spanCount, root.entity.accountId, root.entity.guid, root.entity.name, root.span.name, timestamp, duration.ms. These are the absolute minimum fields required for the visualization. If that's still failing, add the `LIMIT` keyword to the query. `LIMIT` should always be the second option, we prefer to show the user as much traces as we can.
+    - When querying with aggregations, NEVER use embeds.
+- If you query any other even type (like Span or Transaction), don't use any embeds - return the results as is.

holmes/plugins/toolsets/newrelic/newrelic.py

@@ -15,6 +15,7 @@
 from holmes.plugins.toolsets.newrelic.new_relic_api import NewRelicAPI
 import yaml
 import json
+from holmes.utils.keygen_utils import generate_random_key
 
 
 class ExecuteNRQLQuery(Tool):
@@ -109,7 +110,21 @@ def _invoke(
         result: List[Dict[str, Any]] = api.execute_nrql_query(query)
 
         qtype = params.get("query_type", "").lower()
-        if qtype == "logs":
+
+        if qtype == "traces":
+            response_data = {
+                "random_key": generate_random_key(),
+                "tool_name": self.name,
+                "query": query,
+                "data": result,
+                "is_eu": self._toolset.is_eu_datacenter,
+            }
+            return StructuredToolResult(
+                status=StructuredToolResultStatus.SUCCESS,
+                data=json.dumps(response_data, indent=2),
+                params=params,
+            )
+        elif qtype == "logs":
             formatted = self.format_logs(result)
             # For logs we keep your existing YAML output (unchanged)
             return StructuredToolResult(

holmes/version.py

@@ -57,11 +57,41 @@ def get_version() -> str:
         return __version__
 
     # we are running from an unreleased dev version
+    archival_file_path = os.path.join(this_path, ".git_archival.json")
+    if os.path.exists(archival_file_path):
+        try:
+            with open(archival_file_path, "r") as f:
+                archival_data = json.load(f)
+                refs = archival_data.get("refs", "")
+                hash_short = archival_data.get("hash-short", "")
+
+                # Check if Git substitution didn't happen (placeholders are still present)
+                if "$Format:" in refs or "$Format:" in hash_short:
+                    # Placeholders not substituted, skip to next method
+                    pass
+                else:
+                    # Valid archival data found
+                    return f"dev-{refs}-{hash_short}"
+        except Exception:
+            pass
+
+    # Now try git commands for development environments
     try:
+        env = os.environ.copy()
+        # Set ceiling to prevent walking up beyond the project root
+        # We want to allow access to holmes/.git but not beyond holmes
+        project_root = os.path.dirname(this_path)  # holmes
+        env["GIT_CEILING_DIRECTORIES"] = os.path.dirname(
+            project_root
+        )  # holmes's parent
+
         # Get the latest git tag
         tag = (
             subprocess.check_output(
-                ["git", "describe", "--tags"], stderr=subprocess.STDOUT, cwd=this_path
+                ["git", "describe", "--tags"],
+                stderr=subprocess.STDOUT,
+                cwd=this_path,
+                env=env,
             )
             .decode()
             .strip()
@@ -73,6 +103,7 @@ def get_version() -> str:
                 ["git", "rev-parse", "--abbrev-ref", "HEAD"],
                 stderr=subprocess.STDOUT,
                 cwd=this_path,
+                env=env,
             )
             .decode()
             .strip()
@@ -84,6 +115,7 @@ def get_version() -> str:
                 ["git", "status", "--porcelain"],
                 stderr=subprocess.STDOUT,
                 cwd=this_path,
+                env=env,
             )
             .decode()
             .strip()
@@ -95,19 +127,7 @@ def get_version() -> str:
     except Exception:
         pass
 
-    # we are running without git history, but we still might have git archival data (e.g. if we were pip installed)
-    archival_file_path = os.path.join(this_path, ".git_archival.json")
-    if os.path.exists(archival_file_path):
-        try:
-            with open(archival_file_path, "r") as f:
-                archival_data = json.load(f)
-                return f"dev-{archival_data['refs']}-{archival_data['hash-short']}"
-        except Exception:
-            pass
-
-        return "dev-version"
-
-    return "unknown-version"
+    return "dev-unknown"
 
 
 @cache

tests/llm/fixtures/test_ask_holmes/117_new_relic_tracing/instrumentation.yaml

@@ -1,4 +1,6 @@
-apiVersion: newrelic.com/v1alpha2
+# https://docs.newrelic.com/docs/kubernetes-pixie/kubernetes-integration/installation/k8s-agent-operator/
+# https://github.com/newrelic/k8s-agents-operator/blob/main/local/instrumentation.yaml
+apiVersion: newrelic.com/v1beta2
 kind: Instrumentation
 metadata:
   name: nr-node-app-117
@@ -13,7 +15,7 @@ spec:
         operator: "In"
         values: ["app-117"]
 ---
-apiVersion: newrelic.com/v1alpha2
+apiVersion: newrelic.com/v1beta2
 kind: Instrumentation
 metadata:
   name: nr-python-app-117

tests/llm/fixtures/test_ask_holmes/117_new_relic_tracing/setup.sh

@@ -1,6 +1,6 @@
-kubectl create namespace app-117
+kubectl apply -f instrumentation.yaml -n newrelic
 
-kubectl apply -f instrumentation.yaml
+kubectl create namespace app-117
 
 kubectl -n app-117 create secret generic checkout-src \
   --from-file=server.js=checkout/server.js \

tests/llm/fixtures/test_ask_holmes/117b_new_relic_block_embed/test_case.yaml

@@ -0,0 +1,11 @@
+user_prompt:
+- Show me traces for do-fra1-tomer and do-fra1-shahal clusters
+
+expected_output:
+  - 'The answer should include something similar to << { "type": "traces-summary", "tool_name": "newrelic_execute_nrql_query", "random_key": "<alphanumeric_characters>" } >>. Both tool_name and random_key MUST absolutely be present and the same as mentioned here.'
+
+tags:
+  - chain-of-causation
+  - traces
+  - newrelic
+  - network

tests/llm/fixtures/test_ask_holmes/117b_new_relic_block_embed/toolsets.yaml

@@ -0,0 +1,7 @@
+toolsets:
+  newrelic:
+    enabled: true
+    config:
+      nr_account_id: "{{env.NEW_RELIC_ACCOUNT_ID}}"
+      nr_api_key: "{{env.NEW_RELIC_API_KEY}}"
+      is_eu_datacenter: false # set "{{env.NEW_RELIC_IS_EU}}" with true to use eu