log feedback to splunk

Author: anthony-nhs

packages/slackBotFunction/app/services/slack.py

@@ -0,0 +1,23 @@
+import traceback
+
+from slack_sdk import WebClient
+from app.core.config import get_logger
+
+
+logger = get_logger()
+
+
+def get_friendly_channel_name(channel_id: str, client: WebClient) -> str:
+    friendly_channel_name = channel_id
+    try:
+        conversations_info_response = client.conversations_info(channel=channel_id)
+        if conversations_info_response["ok"]:
+            friendly_channel_name = conversations_info_response["channel"]["name"]
+        else:
+            logger(
+                "There was a problem getting the friendly channel name",
+                extra={"conversations_info_response": conversations_info_response},
+            )
+    except Exception:
+        logger.error("There was an error getting the friendly channel name", extra={"error": traceback.format_exc()})
+    return friendly_channel_name

packages/slackBotFunction/app/slack/slack_events.py

@@ -22,6 +22,7 @@
     update_state_information,
 )
 from app.services.query_reformulator import reformulate_query
+from app.services.slack import get_friendly_channel_name
 
 logger = get_logger()
 
@@ -261,18 +262,7 @@ def log_query_stats(user_query, event, channel, client, thread_ts):
     end_time = time.time()
     duration = end_time - start_time
     is_direct_message = event.get("channel_type") == constants.CHANNEL_TYPE_IM
-    friendly_channel_name = channel
-    try:
-        conversations_info_response = client.conversations_info(channel=channel)
-        if conversations_info_response["ok"]:
-            friendly_channel_name = conversations_info_response["channel"]["name"]
-        else:
-            logger(
-                "There was a problem getting the friendly channel name",
-                extra={"conversations_info_response": conversations_info_response},
-            )
-    except Exception:
-        logger.error("There was an error getting the friendly channel name", extra={"error", traceback.format_exc()})
+    friendly_channel_name = get_friendly_channel_name(channel_id=channel, client=client)
     reporting_info = {
         "query_length": query_length,
         "start_time": start_time,
@@ -295,6 +285,7 @@ def store_feedback(
     feedback_type,
     user_id,
     channel_id,
+    client: WebClient,
     thread_ts=None,
     message_ts=None,
     feedback_text=None,
@@ -303,6 +294,14 @@ def store_feedback(
     Store user feedback with reference to Q&A record
     """
     try:
+        friendly_channel_name = get_friendly_channel_name(channel_id=channel_id, client=client)
+        reporting_info = {
+            "feedback_type": feedback_type,
+            "feedback_text": feedback_text,
+            "thread_ts": thread_ts,
+            "channel": friendly_channel_name,
+        }
+        logger.info("REPORTING: feedback_stats", extra={"reporting_info": reporting_info})
         now = int(time.time())
         ttl = now + constants.TTL_FEEDBACK
 

packages/slackBotFunction/app/slack/slack_handlers.py

@@ -105,13 +105,14 @@ def mention_handler(event, ack, body, client):
         feedback_text = cleaned.split(":", 1)[1].strip() if ":" in cleaned else ""
         try:
             store_feedback(
-                conversation_key,
-                "additional",
-                user_id,
-                channel_id,
-                thread_root,
-                None,
-                feedback_text,
+                conversation_key=conversation_key,
+                feedback_type="additional",
+                user_id=user_id,
+                channel_id=channel_id,
+                thread_ts=thread_root,
+                message_ts=None,
+                feedback_text=feedback_text,
+                client=client,
             )
         except Exception as e:
             logger.error(f"Failed to store channel feedback via mention: {e}", extra={"error": traceback.format_exc()})
@@ -149,13 +150,14 @@ def dm_message_handler(event, event_id, client):
         feedback_text = text.split(":", 1)[1].strip() if ":" in text else ""
         try:
             store_feedback(
-                conversation_key,
-                "additional",
-                user_id,
-                channel_id,
-                thread_root,
-                None,
-                feedback_text,
+                conversation_key=conversation_key,
+                feedback_type="additional",
+                user_id=user_id,
+                channel_id=channel_id,
+                thread_ts=thread_root,
+                message_ts=None,
+                feedback_text=feedback_text,
+                client=client,
             )
         except Exception as e:
             logger.error(f"Failed to store DM additional feedback: {e}", extra={"error": traceback.format_exc()})
@@ -208,13 +210,14 @@ def thread_message_handler(event, event_id, client):
         user_id = event.get("user", "unknown")
         try:
             store_feedback(
-                conversation_key,
-                "additional",
-                user_id,
-                channel_id,
-                thread_root,
-                None,
-                feedback_text,
+                conversation_key=conversation_key,
+                feedback_type="additional",
+                user_id=user_id,
+                channel_id=channel_id,
+                thread_ts=thread_root,
+                message_ts=None,
+                feedback_text=feedback_text,
+                client=client,
             )
         except Exception as e:
             logger.error(f"Failed to store channel additional feedback: {e}", extra={"error": traceback.format_exc()})
@@ -285,12 +288,13 @@ def feedback_handler(ack, body, client):
 
         try:
             store_feedback(
-                feedback_data["ck"],
-                feedback_type,
-                body["user"]["id"],
-                feedback_data["ch"],
-                feedback_data.get("tt"),
-                feedback_data.get("mt"),
+                conversation_key=feedback_data["ck"],
+                feedback_type=feedback_type,
+                user_id=body["user"]["id"],
+                channel_id=feedback_data["ch"],
+                thread_ts=feedback_data.get("tt"),
+                feedback_text=feedback_data.get("mt"),
+                client=client,
             )
             # Only post message if storage succeeded
             post_params = {"channel": feedback_data["ch"], "text": response_message}

packages/slackBotFunction/tests/test_feedback.py

@@ -1,22 +1,26 @@
 import sys
 import pytest
-from unittest.mock import patch
+from unittest.mock import Mock, patch
 from botocore.exceptions import ClientError
 
 
 @patch("app.services.dynamo.store_state_information")
 @patch("app.services.dynamo.get_state_information")
-def test_store_feedback(mock_get_state_information, mock_store_state_information, mock_env):
+@patch("app.services.slack.get_friendly_channel_name")
+def test_store_feedback(
+    mock_get_friendly_channel_name, mock_get_state_information, mock_store_state_information, mock_env
+):
     """Test feedback storage functionality"""
     # set up mocks
+    mock_client = Mock()
 
     # delete and import module to test
     if "app.slack.slack_events" in sys.modules:
         del sys.modules["app.slack.slack_events"]
     from app.slack.slack_events import store_feedback
 
     # perform operation
-    store_feedback("test-conversation", "positive", "U123", "C123")
+    store_feedback("test-conversation", "positive", "U123", "C123", mock_client)
 
     # assertions
     mock_store_state_information.assert_called_once()
@@ -27,17 +31,23 @@ def test_store_feedback(mock_get_state_information, mock_store_state_information
 
 @patch("app.services.dynamo.store_state_information")
 @patch("app.services.dynamo.get_state_information")
-def test_feedback_storage_with_additional_text(mock_get_state_information, mock_store_state_information, mock_env):
+@patch("app.services.slack.get_friendly_channel_name")
+def test_feedback_storage_with_additional_text(
+    mock_get_friendly_channel_name, mock_get_state_information, mock_store_state_information, mock_env
+):
     """Test feedback storage with additional feedback text"""
     # set up mocks
+    mock_client = Mock()
 
     # delete and import module to test
     if "app.slack.slack_events" in sys.modules:
         del sys.modules["app.slack.slack_events"]
     from app.slack.slack_events import store_feedback
 
     # perform operation
-    store_feedback("test-conversation", "additional", "U123", "C123", feedback_text="This is additional feedback")
+    store_feedback(
+        "test-conversation", "additional", "U123", "C123", mock_client, feedback_text="This is additional feedback"
+    )
 
     # assertions
     mock_store_state_information.assert_called_once()
@@ -239,18 +249,22 @@ def test_cleanup_previous_unfeedback_qa_does_not_throw_error(delete_state_inform
 
 @patch("app.services.dynamo.store_state_information")
 @patch("app.slack.slack_events.get_latest_message_ts")
-def test_store_feedback_no_message_ts_fallback(mock_get_latest_message_ts, mock_store_state_information, mock_env):
+@patch("app.services.slack.get_friendly_channel_name")
+def test_store_feedback_no_message_ts_fallback(
+    mock_get_friendly_channel_name, mock_get_latest_message_ts, mock_store_state_information, mock_env
+):
     """Test store_feedback fallback path when no message_ts"""
     # set up mocks
     mock_get_latest_message_ts.return_value = None
+    mock_client = Mock()
 
     # delete and import module to test
     if "app.slack.slack_events" in sys.modules:
         del sys.modules["app.slack.slack_events"]
     from app.slack.slack_events import store_feedback
 
     # perform operation
-    store_feedback("conv-key", "positive", "user-id", "channel-id")
+    store_feedback("conv-key", "positive", "user-id", "channel-id", mock_client)
 
     # assertions
     mock_store_state_information.assert_called_once()
@@ -361,12 +375,16 @@ def test_get_latest_message_ts_no_item(mock_get_state_information, mock_env):
 
 @patch("app.services.dynamo.store_state_information")
 @patch("app.slack.slack_events.get_latest_message_ts")
-def test_store_feedback_client_error_reraise(mock_get_latest_message_ts, mock_store_state_information, mock_env):
+@patch("app.services.slack.get_friendly_channel_name")
+def test_store_feedback_client_error_reraise(
+    mock_get_friendly_channel_name, mock_get_latest_message_ts, mock_store_state_information, mock_env
+):
     """Test store_feedback re-raises ClientError"""
     # set up mocks
     error = ClientError({"Error": {"Code": "ValidationException"}}, "PutItem")
     mock_store_state_information.side_effect = error
     mock_get_latest_message_ts.return_value = "123"
+    mock_client = Mock()
 
     # delete and import module to test
     if "app.slack.slack_events" in sys.modules:
@@ -375,7 +393,7 @@ def test_store_feedback_client_error_reraise(mock_get_latest_message_ts, mock_st
 
     # perform operation
     with pytest.raises(ClientError):
-        store_feedback("conv-key", "positive", "user-id", "channel-id")
+        store_feedback("conv-key", "positive", "user-id", "channel-id", mock_client)
 
 
 @patch("app.services.dynamo.store_state_information")

packages/slackBotFunction/tests/test_slack_handlers.py

@@ -1,5 +1,5 @@
 import sys
-from unittest.mock import Mock, patch
+from unittest.mock import ANY, Mock, patch
 from botocore.exceptions import ClientError
 
 
@@ -161,7 +161,15 @@ def decorator(func):
         yes_handler(mock_ack, mock_body, mock_client)
 
         mock_ack.assert_called_once()
-        mock_store_feedback.assert_called_once_with("conv-key", "positive", "U123", "C123", "123", "456")
+        mock_store_feedback.assert_called_once_with(
+            conversation_key="conv-key",
+            feedback_type="positive",
+            user_id="U123",
+            channel_id="C123",
+            thread_ts="123",
+            feedback_text="456",
+            client=ANY,
+        )
         mock_client.chat_postMessage.assert_called_once()
 
 
@@ -217,7 +225,15 @@ def decorator(func):
         no_handler(mock_ack, mock_body, mock_client)
 
         mock_ack.assert_called_once()
-        mock_store_feedback.assert_called_once_with("conv-key", "negative", "U123", "C123", "123", "456")
+        mock_store_feedback.assert_called_once_with(
+            conversation_key="conv-key",
+            feedback_type="negative",
+            user_id="U123",
+            channel_id="C123",
+            thread_ts="123",
+            feedback_text="456",
+            client=ANY,
+        )
         mock_client.chat_postMessage.assert_called_once()