Bugfix/slack bot debugging (onyx-dot-app#4782)

* adding some logging

* better var name

---------

Co-authored-by: Richard Kuo (Onyx) <rkuo@onyx.app>

Author: 2 people

backend/onyx/onyxbot/slack/listener.py

@@ -16,6 +16,7 @@
 from prometheus_client import Gauge
 from prometheus_client import start_http_server
 from redis.lock import Lock
+from redis.lock import Lock as RedisLock
 from slack_sdk import WebClient
 from slack_sdk.errors import SlackApiError
 from slack_sdk.http_retry import ConnectionErrorRetryHandler
@@ -105,7 +106,6 @@
 from shared_configs.contextvars import CURRENT_TENANT_ID_CONTEXTVAR
 from shared_configs.contextvars import get_current_tenant_id
 
-
 logger = setup_logger()
 
 # Prometheus metric for HPA
@@ -264,23 +264,23 @@ def acquire_tenants(self) -> None:
         - If a tenant in self.tenant_ids no longer has Slack bots, remove it (and release the lock in this scope).
         """
 
+        token: Token[str | None]
+
         # tenants that are disabled (e.g. their trial is over and haven't subscribed)
         # for non-cloud, this will return an empty set
         gated_tenants = fetch_ee_implementation_or_noop(
             "onyx.server.tenants.product_gating",
             "get_gated_tenants",
             set(),
         )()
-        all_tenants = [
+        all_active_tenants = [
             tenant_id
             for tenant_id in get_all_tenant_ids()
             if tenant_id not in gated_tenants
         ]
 
-        token: Token[str | None]
-
         # 1) Try to acquire locks for new tenants
-        for tenant_id in all_tenants:
+        for tenant_id in all_active_tenants:
             if (
                 DISALLOWED_SLACK_BOT_TENANT_LIST is not None
                 and tenant_id in DISALLOWED_SLACK_BOT_TENANT_LIST
@@ -295,13 +295,13 @@ def acquire_tenants(self) -> None:
             # Respect max tenant limit per pod
             if len(self.tenant_ids) >= MAX_TENANTS_PER_POD:
                 logger.info(
-                    f"Max tenants per pod reached ({MAX_TENANTS_PER_POD}); not acquiring more."
+                    f"Max tenants per pod reached, not acquiring more: {MAX_TENANTS_PER_POD=}"
                 )
                 break
 
             redis_client = get_redis_client(tenant_id=tenant_id)
             # Acquire a Redis lock (non-blocking)
-            rlock = redis_client.lock(
+            rlock: RedisLock = redis_client.lock(
                 OnyxRedisLocks.SLACK_BOT_LOCK, timeout=TENANT_LOCK_EXPIRATION
             )
             lock_acquired = rlock.acquire(blocking=False)
@@ -450,6 +450,7 @@ def start_socket_client(
                     bot_name = (
                         user_info["user"]["real_name"] or user_info["user"]["name"]
                     )
+                    socket_client.bot_name = bot_name
                     logger.info(
                         f"Started socket client for Slackbot with name '{bot_name}' (tenant: {tenant_id}, app: {slack_bot_id})"
                     )
@@ -692,7 +693,7 @@ def prefilter_requests(req: SocketModeRequest, client: TenantSocketModeClient) -
     if not check_message_limit():
         return False
 
-    logger.debug(f"Handling Slack request with Payload: '{req.payload}'")
+    logger.debug(f"Handling Slack request: {client.bot_name=} '{req.payload=}'")
     return True
 
 

backend/onyx/onyxbot/slack/utils.py

@@ -117,30 +117,33 @@ def update_emote_react(
     remove: bool,
     client: WebClient,
 ) -> None:
-    try:
-        if not message_ts:
-            logger.error(
-                f"Tried to remove a react in {channel} but no message specified"
-            )
-            return
+    if not message_ts:
+        action = "remove" if remove else "add"
+        logger.error(f"update_emote_react - no message specified: {channel=} {action=}")
+        return
 
-        if remove:
+    if remove:
+        try:
             client.reactions_remove(
                 name=emoji,
                 channel=channel,
                 timestamp=message_ts,
             )
-        else:
-            client.reactions_add(
-                name=emoji,
-                channel=channel,
-                timestamp=message_ts,
-            )
-    except SlackApiError as e:
-        if remove:
+        except SlackApiError as e:
             logger.error(f"Failed to remove Reaction due to: {e}")
-        else:
-            logger.error(f"Was not able to react to user message due to: {e}")
+
+        return
+
+    try:
+        client.reactions_add(
+            name=emoji,
+            channel=channel,
+            timestamp=message_ts,
+        )
+    except SlackApiError as e:
+        logger.error(f"Was not able to react to user message due to: {e}")
+
+    return
 
 
 def remove_onyx_bot_tag(message_str: str, client: WebClient) -> str:
@@ -676,6 +679,7 @@ def __init__(self, tenant_id: str, slack_bot_id: int, *args: Any, **kwargs: Any)
         super().__init__(*args, **kwargs)
         self._tenant_id = tenant_id
         self.slack_bot_id = slack_bot_id
+        self.bot_name: str = "Unnamed"
 
     @contextmanager
     def _set_tenant_context(self) -> Generator[None, None, None]:

backend/onyx/secondary_llm_flows/chunk_usefulness.py

@@ -51,7 +51,10 @@ def _extract_usefulness(model_output: str) -> bool:
     messages = _get_usefulness_messages()
     filled_llm_prompt = dict_based_prompt_to_langchain_prompt(messages)
     model_output = message_to_string(llm.invoke(filled_llm_prompt))
-    logger.debug(model_output)
+
+    # NOTE(rkuo): all this does is print "Yes useful" or "Not useful"
+    # disabling becuase it's spammy, restore and give more context if this is needed
+    # logger.debug(model_output)
 
     return _extract_usefulness(model_output)
 
@@ -64,6 +67,8 @@ def llm_batch_eval_sections(
     metadata_list: list[dict[str, str | list[str]]],
     use_threads: bool = True,
 ) -> list[bool]:
+    answer: list[bool]
+
     if DISABLE_LLM_DOC_RELEVANCE:
         raise RuntimeError(
             "LLM Doc Relevance is globally disabled, "
@@ -86,12 +91,13 @@ def llm_batch_eval_sections(
         )
 
         # In case of failure/timeout, don't throw out the section
-        return [True if item is None else item for item in parallel_results]
+        answer = [True if item is None else item for item in parallel_results]
+        return answer
 
-    else:
-        return [
-            llm_eval_section(query, section_content, llm, title, metadata)
-            for section_content, title, metadata in zip(
-                section_contents, titles, metadata_list
-            )
-        ]
+    answer = [
+        llm_eval_section(query, section_content, llm, title, metadata)
+        for section_content, title, metadata in zip(
+            section_contents, titles, metadata_list
+        )
+    ]
+    return answer