swiftlang / sourcekit-lsp

Language Server Protocol implementation for Swift and C-based languages
Apache License 2.0
3.27k stars 271 forks source link

Update log message to public level #675

Closed Kyle-Ye closed 10 months ago

Kyle-Ye commented 1 year ago

The current log will be <private> to end user unless the user is debugging on it.

And it makes the log message of SourceKit a bit of useless.

image

Inspired by digging the original issue https://github.com/apple/swift/issues/62274#issuecomment-1337940045

I think we could improve the logger method to add public modifier

os_log("%{public}@", log: xx, type: .info, xx) // If we use old API
Logger().info("\(xx, privacy: .public)") // If we can drop macOS 10.15- Support to use new API
ahoppen commented 1 year ago

rdar://102990224

ahoppen commented 1 year ago

I agree that the current situation is not ideal.

But I think the tricky bit is that the log message might contain potentially privacy-sensitive information that we don’t want to log, so marking the entire opaque string is not an option IMO. So each log call would need to decide which information it wants to mark as public and private.

ahoppen commented 10 months ago

I redesigned the logging infrastructure in https://github.com/apple/sourcekit-lsp/pull/859 and now log calls can specify which parts should be public and which ones should be private. All log calls should now log some public information.

Kyle-Ye commented 10 months ago

I redesigned the logging infrastructure in #859 and now log calls can specify which parts should be public and which ones should be private. All log calls should now log some public information.

I see it was merged into main. Is there a plan to cherry-pick it to release/5.10?

ahoppen commented 10 months ago

That refactoring is quite an invasive change to the entire codebase and I don’t plan to cherry-pick it to release/5.10 which doesn’t contain any of the commits that the PR relies on.