-
Notifications
You must be signed in to change notification settings - Fork 0
[FEAT] 안드로이드 로그 디버그모드 관리 #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @stopstone, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
이 PR은 안드로이드 애플리케이션의 로깅 시스템을 개선하기 위한 것입니다. 기존의 기본 Log 사용 시 발생할 수 있는 릴리즈 모드에서의 로그 노출 및 관리 문제를 해결하고, 디버그 환경에서만 로그가 출력되도록 하여 보다 체계적이고 일관된 로깅 경험을 제공합니다. 이를 통해 개발자는 디버깅 효율성을 높이고, 프로덕션 빌드에서는 불필요한 로그 노출을 방지할 수 있습니다.
Highlights
- 로깅 시스템 개선: 안드로이드 기본 Log 사용 시 릴리즈 모드에서 로그 관리가 어려운 문제를 해결하고, 로그 확장을 통해 전역적으로 일관된 로깅 시스템을 구축합니다.
- 디버그/릴리즈 환경별 로그 관리: BuildConfig.DEBUG를 활용하여 디버그 모드에서만 로그가 출력되도록 하여, 환경에 따라 로그 레벨을 다르게 관리할 수 있습니다.
- 로그 포맷팅 및 태그 관리: 호출자 정보(파일 이름, 메서드, 라인 번호)를 포함한 로그 메시지를 자동으로 생성하고, 기본 태그("Near") 또는 커스텀 태그를 사용하여 로그를 관리할 수 있습니다.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
이 PR은 디버그 빌드에서만 로그를 출력하고, 로그 호출 위치(클래스, 메소드, 라인 번호)를 자동으로 포함시켜주는 커스텀 로깅 유틸리티를 추가했네요. 로깅 시스템을 중앙에서 관리하고 일관성을 높이려는 좋은 시도입니다. 다만, 로그 호출 위치를 찾는 buildLogMessage 함수의 구현에 몇 가지 개선할 점이 보입니다. 스택 트레이스를 분석하는 로직이 특정 조건에서 의도와 다르게 동작할 수 있어 수정이 필요해 보입니다. 아래에 자세한 리뷰를 남겼습니다.
| return try { | ||
| val stackTrace = Thread.currentThread().stackTrace | ||
|
|
||
| // 실제 호출자를 찾기 위해 스택을 순회 | ||
| for (i in 4 until stackTrace.size) { | ||
| val element = stackTrace[i] | ||
| val fileName = element.fileName ?: continue | ||
| val methodName = element.methodName ?: continue | ||
|
|
||
| // 로그 관련 메서드들을 건너뛰고 실제 호출자 찾기 | ||
| if (!methodName.startsWith("log") && | ||
| !methodName.contains("\$default") && | ||
| !fileName.contains("Log") | ||
| ) { | ||
| val cleanFileName = | ||
| fileName | ||
| .replace(".java", "") | ||
| .replace(".kt", "") | ||
|
|
||
| return "[$cleanFileName::$methodName (${element.fileName}:${element.lineNumber})] $message" | ||
| } | ||
| } | ||
|
|
||
| // 찾지 못하면 기본 인덱스 사용 | ||
| if (stackTrace.size > STACK_TRACE_INDEX) { | ||
| val element = stackTrace[STACK_TRACE_INDEX] | ||
| val fileName = | ||
| element.fileName | ||
| ?.replace(".java", "") | ||
| ?.replace(".kt", "") | ||
| ?: "Unknown" | ||
|
|
||
| "[$fileName::${element.methodName} (${element.fileName}:${element.lineNumber})] $message" | ||
| } else { | ||
| message | ||
| } | ||
| } catch (exception: Exception) { | ||
| // 스택 트레이스 추출 실패 시 원본 메시지 반환 | ||
| message | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildLogMessage 함수에서 호출자 정보를 찾기 위해 스택 트레이스를 사용하는 로직이 불안정하고 잠재적인 버그를 포함하고 있어 개선이 필요합니다.
- 하드코딩된 인덱스:
for (i in 4 until stackTrace.size)와STACK_TRACE_INDEX = 5처럼 스택의 특정 인덱스를 사용하면, 코드의 변화(예: 인라인 함수, 람다 사용)에 따라 스택 구조가 바뀌었을 때 호출자를 정확히 찾지 못하게 됩니다. 현재 로직으로는 실제 호출자인 3번 인덱스를 건너뛰게 되어 잘못된 정보가 로깅될 수 있습니다. - 취약한 파일 이름 필터링:
!fileName.contains("Log")조건은 파일 이름에 "Log"가 포함된 다른 클래스(예:LoginViewModel)의 로그까지 건너뛸 위험이 있습니다. 로거 파일 자체(NearLog.kt)만 명시적으로 제외하는 것이 더 안전합니다. - 파일 확장자 제거 방식:
.replace(".java", "").replace(".kt", "")방식은.java나.kt가 파일명 중간에 들어가는 경우 오작동할 수 있으며, 다른 확장자를 가진 파일에는 대응하기 어렵습니다.substringBeforeLast('.')를 사용하는 것이 더 안정적입니다.
이러한 문제들을 해결하기 위해, 스택 트레이스를 순회하며 현재 로깅 파일(NearLog.kt)이 아닌 첫 번째 요소를 찾아 정보를 가공하는 방식으로 수정하는 것을 제안합니다. 이 방식이 훨씬 더 안정적이고 유지보수하기 좋습니다.
추가적으로, Thread.currentThread().stackTrace는 호출될 때마다 스택 전체를 순회하므로 약간의 성능 저하를 유발할 수 있습니다. 하지만 디버그 빌드에서만 사용되므로 허용 가능한 트레이드오프일 수 있습니다. 이 점을 인지하고 계시면 좋을 것 같습니다.
return try {
val stackTrace = Thread.currentThread().stackTrace
// 스택 트레이스에서 현재 로거 파일을 제외한 첫 번째 호출 지점을 찾습니다.
// stackTrace[0] = Thread.getStackTrace
// stackTrace[1] = buildLogMessage
// stackTrace[2] = logd, logi, ...
// 따라서 3번 인덱스부터 검색을 시작하는 것이 합리적입니다.
val callerElement = stackTrace.drop(3).firstOrNull {
it.fileName != "NearLog.kt"
}
if (callerElement != null) {
val fileName = callerElement.fileName
val cleanFileName = fileName?.substringBeforeLast('.') ?: "Unknown"
"[$cleanFileName::${callerElement.methodName} (${fileName}:${callerElement.lineNumber})] $message"
} else {
message
}
} catch (exception: Exception) {
// 스택 트레이스 추출 실패 시 원본 메시지 반환
message
}
rhkrwngud445
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다! 작업 감사해요👍
예외를 처리하는 관점에서 코멘트 남겼습니다! 예외를 회피할지, 핸들링할지의 지석님의 의견이 궁금합니다!
| * 스택 트레이스 추출에 실패하면 원본 메시지를 반환합니다 | ||
| */ | ||
| private fun buildLogMessage(message: String): String { | ||
| return try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
꼼꼼하게 처리주셨네요👍
예외가 발생하면 어떤 상황에서 발생할까요? 만약 예외가 발생하고 catch되어 message로 나오는 상황이면 어떤 예외인지,어쩌면 예외가 발생했는지도 모르게 다른 부분을 디버깅 하는 상황이 생길 수 있을 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코멘트 감사합니다 :D
우선 예외가 발생하는 경우는 로그에 해당 파일까지 남기는 형태로 작성하였는데, 파일을 찾는 방식이
앱의 스택 트레이스를 순회하면서 탐색하고 있습니다! 그래서 이 순회과정에서 문제가 생길 수 있어 try~catch로 예외처리를 두었어요
예외처리를 두긴 하였지만 웬만하면 에러가 발생 안하지 않을까.. 싶습니다
- 기존: 고정된 스택 인덱스를 사용하여 호출자 정보를 찾음 - 변경: 스택 트레이스를 순회하여 NearLog.kt 파일이 아닌 첫 번째 호출자를 찾도록 변경 - 추가: 스택 트레이스 접근 권한이 없거나 기타 예외 발생 시, 로그 메시지에 관련 정보를 포함하여 디버깅 용이성 향상
- `NearLog`를 object로 변경하여 싱글턴으로 사용하도록 수정 - 로그 메시지 생성 로직을 `runCatching`으로 예외처리 - 중복되는 로그 출력 로직을 `writeLog` 함수로 통합 - `loge` 함수 오버로딩을 단일 함수로 통합하고, `name` 파라미터를 nullable로 변경 - 함수명을 `logv` -> `v`, `logd` -> `d` 등으로 축약하여 간결하게 변경
작업 내용
확인 방법
utils/NearLog.kt에 로그 확삼 함수 구현하였습니다.
참고 사항
구현부
디버그모드에서만 로그가 출력되도록 구현하였습니다
일반 Log.d()와는 다르게 tag를 message 파라미터 뒤에 구현하였습니다.
tag의 생략을 고려하여 위와 같이 구현하였습니다.
사용법
결과
관련 이슈