feat: add chatbot response logging to server database - #90
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 25 minutes and 42 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough챗봇 엔드포인트에 요청 레이턴시 측정 및 데이터베이스 로깅 기능을 추가했습니다. 새로운 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server as /chatbot<br/>Endpoint
participant ThreadPool as Daemon<br/>Thread
participant Cache as Cache<br/>Layer
participant DB as PostgreSQL<br/>Database
participant SSH as SSH<br/>Tunnel
Client->>Server: 챗봇 쿼리 요청
Note over Server: time.monotonic() 시작
alt 캐시 히트
Server->>Cache: 캐시 조회
Cache-->>Server: 응답 반환
else 캐시 미스
Server->>Server: API 호출 및 처리
Server-->>Server: 응답 생성
end
Note over Server: time.monotonic() 종료<br/>레이턴시 계산
Server-->>Client: 응답 반환
Server->>ThreadPool: _log_chatbot() 호출<br/>(데몬 스레드)
ThreadPool->>SSH: SSH 터널 확인
alt SSH_HOST 설정됨
ThreadPool->>SSH: 터널 연결
SSH->>DB: 로그 기록
else SSH_HOST 미설정
ThreadPool->>DB: 직접 연결으로<br/>로그 기록
end
Note over DB: chatbot_logs 테이블<br/>삽입
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Contributor
Author
|
@coderabbitai 해당 pr에 대한 리뷰 부탁합니다. |
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
LLM/OSS/Open_AI_OSS.py (1)
902-921: 반복되는 비동기 로깅 호출을 헬퍼로 추출하세요.동일한
threading.Thread(target=_log_chatbot, args=(...), daemon=True).start()블록이 이 핸들러 안에서 6번(902-908, 917-921, 994, 1004, 1015, 1030) 반복됩니다. 파라미터 순서가 미묘히 달라 실수로 인자를 뒤바꿔 넣을 여지가 큽니다. 시작 시각(_start)과 기본값들을 감싼 클로저 헬퍼로 묶는 편이 안전합니다.♻️ 제안 리팩터
async def chat(req: ChatReq, username: str = Depends(verify_jwt_token)): _start = time.monotonic() + def _fire_log(mode_: str, text_: str, url_: Optional[str], cache_hit_: bool) -> None: + latency_ = int((time.monotonic() - _start) * 1000) + threading.Thread( + target=_log_chatbot, + args=(user_text, mode_, text_, url_, cache_hit_, latency_), + daemon=True, + ).start()이후 모든 호출부를
_fire_log(mode, resp.get("text",""), resp.get("url"), False)같은 형태로 치환하면 인자 순서 실수와 중복이 사라집니다. 단, 앞서 제안드린 큐+워커 구조로 바꾸면 이 헬퍼 자체가_LOG_Q.put_nowait(...)한 줄로 더 단순해집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@LLM/OSS/Open_AI_OSS.py` around lines 902 - 921, Extract the repeated threading.Thread(... target=_log_chatbot ...) calls into a small helper/closure (e.g. _fire_log) that captures _start and any default values so all call sites (including where _cached is used and inside _cache_and_return) call _fire_log(mode, text, url, cached_flag) instead of repeating the threading.Thread invocation; update every occurrence around _log_chatbot, _start, _cached, and _cache_and_return to call this helper to avoid argument-order bugs and duplicated code, optionally making the helper push to a queue if you later swap to a queue+worker model.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@LLM/OSS/Open_AI_OSS.py`:
- Around line 72-78: Replace the print-based error handling in the broad except
block with structured logging: add logger = logging.getLogger(__name__) at the
top of Open_AI_OSS.py, then in the except Exception as e: block call
logger.exception("Failed during chat operation", exc_info=True) (or
logger.error(..., exc_info=True)) so the full stacktrace is recorded; keep the
broad except to avoid crashing the flow but ensure the logging call itself
cannot raise (optionally wrap the logger call in a tiny try/except that swallows
logging failures) and leave the finally cleanup (conn.close(), tunnel.stop())
as-is.
- Around line 40-62: The code hardcodes SSH port 22 and remote DB port 5433 and
omits a configurable DB port for direct connections; update the
SSHTunnelForwarder and psycopg2.connect calls to read SSH port, remote DB port,
and DB port from environment variables instead of literals: use
os.getenv("SSH_PORT", "22") for the SSH port when constructing
SSHTunnelForwarder (instead of (ssh_host, 22)), use os.getenv("REMOTE_DB_PORT",
"5433") for the remote_bind_address port, and ensure the non-tunnel
psycopg2.connect includes port=int(os.getenv("DB_PORT", "5432")). Keep types
consistent (cast to int where needed) and preserve existing uses of
tunnel.local_bind_port for the tunneled psycopg2.connect.
- Around line 34-78: The _log_chatbot function opens an SSH tunnel and a new
psycopg2 connection per request which causes high latency, resource exhaustion,
and lost logs; refactor by initializing a single long-lived SSH tunnel and a
ThreadedConnectionPool at application startup (e.g., in a new
_init_logging_infra called on app startup), replace per-call
SSHTunnelForwarder.start()/stop() and psycopg2.connect() with
borrowing/returning connections from the pool, move asynchronous logging work
off the request path into a bounded ThreadPoolExecutor or queue+worker (avoid
daemon threads), and ensure you set a statement_timeout on connections before
executing the INSERT to avoid long-running statements; update _log_chatbot to
acquire a pooled connection, run the INSERT, and release the connection, and add
proper shutdown logic to close the tunnel, pool, and executor on app
termination.
---
Nitpick comments:
In `@LLM/OSS/Open_AI_OSS.py`:
- Around line 902-921: Extract the repeated threading.Thread(...
target=_log_chatbot ...) calls into a small helper/closure (e.g. _fire_log) that
captures _start and any default values so all call sites (including where
_cached is used and inside _cache_and_return) call _fire_log(mode, text, url,
cached_flag) instead of repeating the threading.Thread invocation; update every
occurrence around _log_chatbot, _start, _cached, and _cache_and_return to call
this helper to avoid argument-order bugs and duplicated code, optionally making
the helper push to a queue if you later swap to a queue+worker model.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4fa148e0-fd82-4207-99fc-dc98c12b8341
📒 Files selected for processing (1)
LLM/OSS/Open_AI_OSS.py
Comment on lines
+40
to
+62
| tunnel = SSHTunnelForwarder( | ||
| (ssh_host, 22), | ||
| ssh_username=os.getenv("SSH_USER"), | ||
| ssh_pkey=os.getenv("SSH_KEY_PATH"), | ||
| remote_bind_address=("localhost", 5433), | ||
| ) | ||
| tunnel.start() | ||
| conn = psycopg2.connect( | ||
| host="localhost", | ||
| port=tunnel.local_bind_port, | ||
| dbname=os.getenv("DB_NAME"), | ||
| user=os.getenv("DB_USER"), | ||
| password=os.getenv("DB_PASSWORD"), | ||
| connect_timeout=3, | ||
| ) | ||
| else: | ||
| conn = psycopg2.connect( | ||
| host="localhost", | ||
| dbname=os.getenv("DB_NAME"), | ||
| user=os.getenv("DB_USER"), | ||
| password=os.getenv("DB_PASSWORD"), | ||
| connect_timeout=3, | ||
| ) |
There was a problem hiding this comment.
하드코딩된 SSH 포트(22)·원격 DB 포트(5433)와 누락된 DB_PORT를 환경변수화하세요.
(ssh_host, 22)의 22,remote_bind_address=("localhost", 5433)의 5433이 코드에 박혀 있어 SSH를 비표준 포트로 운용하거나 원격 PostgreSQL 포트가 5432인 환경에서는 동작하지 않습니다.else브랜치(비터널)는port를 지정하지 않아 항상 5432로 접속합니다. 터널 경로와 포트가 달라 동일 환경변수 세트로 로컬/운영 양쪽에서 쓰기 어렵습니다.
🔧 제안 수정
- if ssh_host:
+ if ssh_host:
tunnel = SSHTunnelForwarder(
- (ssh_host, 22),
+ (ssh_host, int(os.getenv("SSH_PORT", "22"))),
ssh_username=os.getenv("SSH_USER"),
ssh_pkey=os.getenv("SSH_KEY_PATH"),
- remote_bind_address=("localhost", 5433),
+ remote_bind_address=(
+ os.getenv("REMOTE_DB_HOST", "localhost"),
+ int(os.getenv("REMOTE_DB_PORT", "5432")),
+ ),
)
tunnel.start()
conn = psycopg2.connect(
host="localhost",
port=tunnel.local_bind_port,
dbname=os.getenv("DB_NAME"),
user=os.getenv("DB_USER"),
password=os.getenv("DB_PASSWORD"),
connect_timeout=3,
)
else:
conn = psycopg2.connect(
- host="localhost",
+ host=os.getenv("DB_HOST", "localhost"),
+ port=int(os.getenv("DB_PORT", "5432")),
dbname=os.getenv("DB_NAME"),
user=os.getenv("DB_USER"),
password=os.getenv("DB_PASSWORD"),
connect_timeout=3,
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@LLM/OSS/Open_AI_OSS.py` around lines 40 - 62, The code hardcodes SSH port 22
and remote DB port 5433 and omits a configurable DB port for direct connections;
update the SSHTunnelForwarder and psycopg2.connect calls to read SSH port,
remote DB port, and DB port from environment variables instead of literals: use
os.getenv("SSH_PORT", "22") for the SSH port when constructing
SSHTunnelForwarder (instead of (ssh_host, 22)), use os.getenv("REMOTE_DB_PORT",
"5433") for the remote_bind_address port, and ensure the non-tunnel
psycopg2.connect includes port=int(os.getenv("DB_PORT", "5432")). Keep types
consistent (cast to int where needed) and preserve existing uses of
tunnel.local_bind_port for the tunneled psycopg2.connect.
Comment on lines
+72
to
+78
| except Exception as e: | ||
| print(f"[chatbot_log ERROR] {e}") | ||
| finally: | ||
| if conn: | ||
| conn.close() | ||
| if tunnel: | ||
| tunnel.stop() |
There was a problem hiding this comment.
print 대신 logging을 사용하고 스택트레이스를 남겨주세요.
- 운영 환경에서
print는 구조화 로깅/레벨 필터링이 안 되고 stdout 버퍼링 이슈도 있습니다.logging.getLogger(__name__).exception(...)로 예외 전문을 남기는 편이 디버깅에 훨씬 유리합니다. - Ruff BLE001(블라인드
except Exception)도 동시에 개선됩니다. 로깅 실패가 요청 흐름을 막으면 안 되므로 포괄 catch 자체는 유지하되, 로그에는 반드시exc_info를 포함시키세요. - 참고로
daemon=True스레드는 프로세스 종료 시 INSERT 중간에 잘려 로그가 유실될 수 있으니, 위의 워커/큐 전환 시에는 graceful shutdown(queue.join()또는 sentinel)도 함께 고려하세요.
🔧 제안 수정
- except Exception as e:
- print(f"[chatbot_log ERROR] {e}")
+ except Exception:
+ logger.exception("chatbot_log failed")파일 상단에 logger = logging.getLogger(__name__) 선언 필요.
🧰 Tools
🪛 Ruff (0.15.10)
[warning] 72-72: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@LLM/OSS/Open_AI_OSS.py` around lines 72 - 78, Replace the print-based error
handling in the broad except block with structured logging: add logger =
logging.getLogger(__name__) at the top of Open_AI_OSS.py, then in the except
Exception as e: block call logger.exception("Failed during chat operation",
exc_info=True) (or logger.error(..., exc_info=True)) so the full stacktrace is
recorded; keep the broad except to avoid crashing the flow but ensure the
logging call itself cannot raise (optionally wrap the logger call in a tiny
try/except that swallows logging failures) and leave the finally cleanup
(conn.close(), tunnel.stop()) as-is.
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
관련 이슈
Closes #89
🎯 배경
🔍 주요 내용
chatbot_logsthreading비동기 처리변경 요약
챗봇의 모든 응답을 PostgreSQL 데이터베이스에 자동으로 기록하여 사용자 질의, 응답 모드, 응답 내용, 캐시 여부, 응답 시간 등을 추적할 수 있도록 개선했습니다.
주요 변경점
_log_chatbot()헬퍼 함수 추가: SSH 터널을 통한 원격 DB 접속 지원 및 chatbot_logs 테이블에 INSERT/chatbot엔드포인트에time.monotonic()을 이용한 응답 레이턴시 측정 로직 추가_cache_and_return()헬퍼 함수 내에서 로깅 처리psycopg2,sshtunnel임포트주의/리스크
DB_NAME,DB_USER,DB_PASSWORD,SSH_HOST등)가 올바르게 설정되지 않으면 로깅이 작동하지 않음다음 액션
chatbot_logs테이블 스키마 생성 및 마이그레이션 실행 필요