Skip to content

Conversation

@zzangzzangguy
Copy link
Contributor

📌 이슈

✅ 작업 사항

OSLOg 도입

👀 ETC (추후 개발해야 할 것, 참고자료 등) ->

zzangzzangguy/TIL#50

@zzangzzangguy zzangzzangguy changed the title Feat/#127 create os log FEAT: #127 OSLog도입 Apr 28, 2025
@zzangzzangguy zzangzzangguy changed the title FEAT: #127 OSLog도입 [FEAT] #127 OSLog도입 Apr 28, 2025
@zzangzzangguy zzangzzangguy added the 🚀 feat 새로운 기능을 추가 label Apr 28, 2025
Copy link
Member

@0Hooni 0Hooni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다!

기존의 로그를 사용하는 파일에 영향을 주지 않기 위해 설계한 노력이 보여서 좋았어요👍🏻

다만 몇몇 코드에 있어서 의문이 좀 남는게 있었고, 변경이 필요하다 느낀 부분이 있어서 코멘트 확인 후 기현님의 의견 남겨주시면 감사하겠습니다 ☺️

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 이미지 파일이 이번 로거 작업이랑은 연관이 없어 보이는데 제거좀 해주실 수 있을까요?

그리고 이미지 추가하실때 svg로 추출하고 single scale로 적용해주시는게 이미지의 크기가 변경되는 상황에서 유리합니다!

지금 다른 작업의 내용에서 방금 말씀드린 svg 변경 적용해서 작업하시면 좋을것 같아요 ☺️

Comment on lines 85 to 86
fileName: String = #file,
line: Int = #line
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이걸 별도로 주입해주는 케이스가 있는지 잘 생각이 안나서 불필요한 파라미터라 생각이 들어요!!

혹시 해당 파라미터가 필요한 예시를 알 수 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이전에 질문주셨던 로거 위치처럼 편의상 등록해뒀기때문에 필요하지않는다고 다들 생각하신다면 디폴트로 처리해둬도 괜찮습니다!

Comment on lines 69 to 80
private static var loggers: [String: os.Logger] = [:]
private static func getLogger(for category: Level) -> os.Logger {
let categoryName = category.categoryName

if let cachedLogger = loggers[categoryName] {
return cachedLogger
}

let logger = os.Logger(subsystem: subsystem, category: categoryName)
loggers[categoryName] = logger
return logger
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로거를 생성할 때 의미하는 카테고리와 로그를 남길 때 파라미터로 받는 카테고리에 대해 혼용이 되고있는것 같아요!

제가 간단하게 공부한바로는 Logger를 생성할 때 사용되는 카테고리는 "debug", "release"와 같은 지금 사용되는 로거가 어느 상황(앱을 개발할때 또는 배포된 앱에서의 기록을 위해)에서 남길 기록들을 보관할것인지 결정하는 용도이고, Logger.log() 메서드에서 활용되는 카테고리는 해당 로그가 어떤 용도인지 "debug", "network", "event"와 같은 것을 표현하는 용도로 구분되는 것으로 확인했어요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네 확인했습니다
보다 사용할때 수월하도록
카테고리와 레벨정도로 분리해서 수정하겠습니다

Copy link
Contributor Author

@zzangzzangguy zzangzzangguy Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스크린샷 2025-04-30 오후 8 03 53

사진과 같이 level로 분리하여 사용할수있도록 수정되었습니다

Comment on lines 47 to 67

static var isShowFileName: Bool = false
static var isShowLine: Bool = false
static var isShowFileName: Bool = false // 파일 이름 포함여부
static var isShowLine: Bool = true // 라인 번호 포함 여부
static var isShowLog: Bool = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 사용되는 변수가 내부에서만 사용되는것 같은데 private로 감싸주거나 이 변수를 필요로하는 메서드 내부에서 let으로 선언해줘도 좋아보입니다 ☺️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private으로 따뜻하게 감싸줬습니다..

static var isShowLine: Bool = true // 라인 번호 포함 여부
static var isShowLog: Bool = true

private static var loggers: [String: os.Logger] = [:]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로거의 Key를 String으로 하기보단 enum 타입으로 하면 휴먼 에러도 방지하고 사용단계에서도 편할것 같습니다!! 👍🏻

Comment on lines 104 to 111
// 디버깅 시 Xcode 콘솔에서도 바로 확인할 수 있도록 print도 함께 사용 불필요시 제거
print("\(category.categoryIcon) [\(category.categoryName)]: \(message)")
if isShowFileName {
guard let fileNameOnly = fileName.components(separatedBy: "/").last else { return }
print(" \(category.categoryIcon) [FileName]: \(fileNameOnly)")
}
if isShowLine {
print(" \(category.categoryIcon) [Line]: \(line)")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기존의 디버그창 print를 개선하기 위해 도입한 로거라서 프린트 해주는 부분이 없는것이 좋아보여요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

콘솔창에서만 확인할수있도록 수정하겠습니다!

@0Hooni 0Hooni linked an issue Apr 30, 2025 that may be closed by this pull request
@zzangzzangguy zzangzzangguy requested a review from 0Hooni April 30, 2025 11:02
zzangzzangguy and others added 3 commits April 30, 2025 20:44
… into Feat/#127-create-OSLog

# Conflicts:
#	Poppool/CoreLayer/Infrastructure/Infrastructure/Service/KeyChainService.swift
#	Poppool/DataLayer/Data/Data/Network/Common/Requestable.swift
#	Poppool/DataLayer/Data/Data/Network/Provider/ProviderImpl.swift
#	Poppool/DataLayer/Data/Data/Network/Service/AppleLoginService.swift
#	Poppool/DataLayer/Data/Data/Network/Service/KakaoLoginService.swift
#	Poppool/PresentationLayer/Presentation/Presentation/Scene/Map/FindMap/MapGuideView/MapGuideReactor.swift
#	Poppool/PresentationLayer/Presentation/Presentation/Scene/Map/MapView/MapViewController.swift
#	Poppool/PresentationLayer/Presentation/Presentation/Utills/Controllers/BaseTabmanController.swift
#	Poppool/PresentationLayer/Presentation/Presentation/Utills/Controllers/BaseViewController.swift
#	Poppool/PresentationLayer/Presentation/Presentation/Utills/Interfaces/Sectionable/SectionSupplementaryItem.swift
#	Poppool/PresentationLayer/Presentation/Presentation/Utills/Interfaces/Sectionable/Sectionable.swift
Copy link
Member

@dongglehada dongglehada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이번 작업과 관련이 없는 UI 수정 및 이미지 수정에 관한 부분만 확인해 주시면 될 것 같습니다!! 나머지 부분은 코멘트 한 부분 외에는 확인 하였습니다 :>


/// : 아래 옵션 주석 해제시 파일명/라인 번호를 로그 메시지에 포함
// private static var isShowFileName: Bool = false // 파일 이름 포함 여부
// private static var isShowLine: Bool = true // 라인 번호 포함 여부
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 프로퍼티를 사용하지 않는 것으로 보이는데 어디에서 사용되고 있는지 알 수 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이전회의때 영훈님께서 로거의 위치를 알수가없다 하셔서 위와같은 기능이 있기에 추가해뒀다가 현재는 전문 제거되었습니다.

Copy link
Member

@0Hooni 0Hooni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이미지에 대한 변경이 여기서 이뤄지는 이유는 모르겠지만 다음부터는 작업의 분리가 잘 되어있으면 좋겠습니다!!

아직 로거가 여러개인 이유에 대한 설명이 부족한것 같아 추가로 코멘트 남겼습니다 ☺️

Comment on lines 93 to 100
if isShowFileName {
guard let fileNameOnly = fileName.components(separatedBy: "/").last else { return }
fullMessage += " | 📁 \(fileNameOnly)"
}

if isShowLine {
fullMessage += " | 📍 \(line)"
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 제가 리뷰했던거를 뭔가 그대로 받아들이신 부분인것 같아요 🥲

제가 말하려했던건 파일이름이랑 라인명이 필요 없다는게 아니라 해당 코드에서 fileName 대신에 #file처럼 바로 사용 가능한 변수가 있는데 그걸 쓰는건 어떠냐고 했던겁니다!!

저도 디버깅할때 파일명 라인명은 있는게 좋다 생각해서☺️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 ! 그렇군요 바로 사용가능한 변수명으로 지정해두는것이 저도 좋을것같습니다 ☺️

Comment on lines 69 to 74
private static var loggers: [String: os.Logger] = [:]
private static var loggers: [Level: os.Logger] = [:]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

결국 여러개의 로거를 쓰는것 같은데 혹시 로거가 여러개일때의 이점이 뭔지 알 수 있을까요?

지난번에도 달았던 코멘트였던거같은데 답변이 뭔가 제가 궁금해했던 부분을 정확히 긁어주진 못했던거 같아요 ㅠㅠ

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

질문의도를 제가 잘못파악했었군요...!!
카테고리로 예를들어 UI Network ...등등 어떤 계층에서 일어나는지 분류짓고
레벨로 위험도 라고 해야될까요? Debug 나 가령 error인지 지정해둘수있어 확인에 이점이있다 생각해서 추가해뒀습니다!

Copy link
Member

@dongglehada dongglehada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zzangzzangguy zzangzzangguy requested a review from 0Hooni May 2, 2025 04:48
Copy link
Member

@0Hooni 0Hooni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

늦게까지 작업하느라 수고하셨습니다☺️

@zzangzzangguy zzangzzangguy merged commit a983c7c into develop May 2, 2025
2 checks passed
@0Hooni 0Hooni deleted the Feat/#127-create-OSLog branch May 3, 2025 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🚀 feat 새로운 기능을 추가

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OSLog 도입

4 participants