Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Creates a log viewer #89

Closed
wants to merge 11 commits into from
Closed

Creates a log viewer #89

wants to merge 11 commits into from

Conversation

vishnuravi
Copy link
Member

@vishnuravi vishnuravi commented Nov 10, 2024

Creates a log viewer

♻️ Current situation & Problem

Currently there is no mechanism within the application itself to view or share application logs for debugging purposes. This makes it challenging to address issues occurring on remote users' devices that cannot be readily attached to a debugger.

⚙️ Release Notes

  • Creates a log viewer to show application logs
  • Allows filtering by date and log level (e.g. info, debug, error, notice)
  • Allow sharing log data using the share sheet
  • Log viewer can be accessed from the account view

Screenshot

📚 Documentation

Inline documentation added.

✅ Testing

UI tests added.

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

Copy link

codecov bot commented Nov 10, 2024

Codecov Report

Attention: Patch coverage is 54.30712% with 122 lines in your changes missing coverage. Please review.

Project coverage is 77.81%. Comparing base (fed2d72) to head (bf23aae).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
TemplateApplication/Logging/LogLevel.swift 25.50% 38 Missing ⚠️
TemplateApplication/Logging/LogViewer.swift 77.78% 28 Missing ⚠️
TemplateApplication/Logging/LogsListView.swift 18.19% 27 Missing ⚠️
TemplateApplication/Logging/LogManager.swift 59.46% 15 Missing ⚠️
TemplateApplication/Logging/LogManagerError.swift 0.00% 8 Missing ⚠️
...ion/Logging/OSLogEntryLog+FormattedLogOutput.swift 0.00% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #89      +/-   ##
==========================================
- Coverage   86.92%   77.81%   -9.11%     
==========================================
  Files          21       27       +6     
  Lines         688      955     +267     
==========================================
+ Hits          598      743     +145     
- Misses         90      212     +122     
Files with missing lines Coverage Δ
TemplateApplication/Account/AccountSheet.swift 74.42% <100.00%> (+4.15%) ⬆️
...ion/Logging/OSLogEntryLog+FormattedLogOutput.swift 0.00% <0.00%> (ø)
TemplateApplication/Logging/LogManagerError.swift 0.00% <0.00%> (ø)
TemplateApplication/Logging/LogManager.swift 59.46% <59.46%> (ø)
TemplateApplication/Logging/LogsListView.swift 18.19% <18.19%> (ø)
TemplateApplication/Logging/LogViewer.swift 77.78% <77.78%> (ø)
TemplateApplication/Logging/LogLevel.swift 25.50% <25.50%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea08d6d...bf23aae. Read the comment docs.

Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

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

Wondering how we should proceed with this PR. As iOS apparently doesn't provide you any way to access stored logs (only ones from the current app launch) this might be less relevant with its query mechanism.

What do you think about the next steps here @vishnuravi?

Comment on lines +21 to +24
@State private var isLoading = false
@State private var queryTask: Task<Void, Never>?
@State private var showingAlert = false
@State private var errorMessage = ""
Copy link
Member

Choose a reason for hiding this comment

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

We can simplify this by merging isLoading, showingAlert, and errorMessage in using ViewState.

As a result you can use viewstatealert(state:) and processingoverlay(isprocessing:overlay:) in the view.

}
}

var color: Color {
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest to move this extension to OSLogEntryLog.Level.

import SwiftUI


enum LogLevel: String, CaseIterable, Identifiable {
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest to explore using OptionSet to model this here. It would be easily used to configure the selection instead of duplicating all the OSLogEntryLog.Levels. Alternatively it could be morphed into a LogLevelSelection (which describes this a bit better); and use an enum with an associated type with an OSLogEntryLog.Level.

import OSLog
import Spezi
import SwiftUI

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

Comment on lines +33 to +34
let logs = try store.getEntries(at: position, matching: predicate)
.reversed()
Copy link
Member

Choose a reason for hiding this comment

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

Instead of reversing the list we can pass this as an argument to getEntries as an Option: https://developer.apple.com/documentation/oslog/oslogenumerator/options

Comment on lines +40 to +42
if let logLevel, logEntry.level != logLevel {
return false
}
Copy link
Member

Choose a reason for hiding this comment

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

You might even be able to optimize this query by moving this into the predicate; given the configuration options documented in man log you should be able to create predicates that cover this:

PREDICATE-BASED FILTERING
     Using predicate-based filters via the --predicate option allows users to focus on messages based on the provided filter criteria.  For detailed information on the use of predicate based filtering,
     please refer to the Predicate Programming Guide: https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/Predicates/Articles/pSyntax.html

     The filter argument defines one or more pattern clauses following NSPredicate rules.  See log help predicates for the full list of supported keys.  Supported keys include:

     eventType          The type of event: activityCreateEvent, activityTransitionEvent, logEvent, signpostEvent, stateEvent, timesyncEvent, traceEvent and userActionEvent.

     eventMessage       The pattern within the message text, or activity name of a log/trace entry.

     messageType        For logEvent and traceEvent, the type of the message itself: default, info, debug, error or fault.

     process            The name of the process the originated the event.

     processImagePath   The full path of the process that originated the event.

     sender             The name of the library, framework, kernel extension, or mach-o image, that originated the event.

     senderImagePath    The full path of the library, framework, kernel extension, or mach-o image, that originated the event.

     subsystem          The subsystem used to log an event.  Only works with log messages generated with os_log(3) APIs.

     category           The category used to log an event.  Only works with log messages generated with os_log(3) APIs.  When category is used, the subsystem filter should also be provided.

PREDICATE-BASED FILTERING EXAMPLES
     Filter for specific subsystem:
      log show --predicate 'subsystem == "com.example.my_subsystem"'

     Filter for specific subsystem and category:
      log show --predicate '(subsystem == "com.example.my_subsystem") && (category == "desired_category")'

     Filter for specific subsystem and categories:
      log show --predicate '(subsystem == "com.example.my_subsystem") && (category IN { "category1", "category2" })'

     Filter for a specific subsystem and sender(s):
      log show --predicate '(subsystem == "com.example.my_subsystem") && ((senderImagePath ENDSWITH "mybinary") || (senderImagePath ENDSWITH "myframework"))'

PREDICATE-BASED FILTERING EXAMPLES WITH LOG LINE

     log show system_logs.logarchive --predicate 'subsystem == "com.example.subsystem" and category contains "CHECK"'

     Timestamp                       Thread     Type        Activity     PID
     2016-06-13 11:46:37.248693-0700 0x7c393    Default     0x0          10371  timestamp: [com.example.subsystem.CHECKTIME] Time is 06/13/2016 11:46:37

     log show --predicate 'processImagePath endswith "hidd" and senderImagePath contains[cd] "IOKit"' --info

     Timestamp                       Thread     Type        Activity     PID
     2016-06-10 13:54:34.593220-0700 0x250      Info        0x0          113    hidd: (IOKit) [com.apple.iohid.default] Loaded 6 HID plugins

I could imagine that the predicate String creation of the messageType part could be part of the LocLevelSelection OptionSet?
Unfortunately there doesn't seem to be a way to filter based on the date of the log message; the end date filtering would be on us but that should be doable.

import OSLog


extension Array where Element == OSLogEntryLog {
Copy link
Member

Choose a reason for hiding this comment

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

It might even make sense to move the DataRepresentation or FileRepresentation into this extension and directly pass the array of OLLogEntryLogs in the ShareLink?

extension Array where Element == OSLogEntryLog {
func formattedLogOutput() -> String {
self.map { entry in
"[\(entry.date.formatted())] [\(entry.category)] [\(entry.level.rawValue)]: \(entry.composedMessage)"
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering it there are any logging formats out there that we could use. E.g., how is a macOS .log formatted? We might take some inspiration from there and can then easily open it up in the Console app? If we don't address this here we should think about doing it once we move this into its own module + work on some of the infrastructure building here.

@State private var errorMessage = ""

var body: some View {
VStack {
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest to package the selection elements into a Form. It would be nicer to format all the elections and will make it more consistent with the other views we have in the other setting views, e.g., license views as well as the main setting views.

startDate: Date,
endDate: Date? = nil,
logLevel: OSLogEntryLog.Level? = nil
) throws -> [OSLogEntryLog] {
Copy link
Member

Choose a reason for hiding this comment

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

I tested the views in the application and I could not see any log messages emitted by, e.g., a default logger in the view:

Logger().log(level: .error, "Test Log Message ...")

Not sure if this is just for me but it would be good to test the basic behavior in a UI/unit test within the template application to ensure that the basic query functionality is working as expected.

@PSchmiedmayer PSchmiedmayer added the enhancement New feature or request label Jan 19, 2025
@vishnuravi
Copy link
Member Author

Hi @PSchmiedmayer thanks for the detailed review! I'll definitely incorporate your recommendations. I have also been thinking about whether this feature is really useful enough to be included with the template application, especially considering that iOS only lets us access logs from the current app launch and does not let us access any stored logs. I think it would be better to move this feature into a separate repository and continue to work on it there. I'll do that and then close this PR, if it makes sense to you.

@PSchmiedmayer
Copy link
Member

Sounds fair; might be a good approach and could be a simple and reusable view that could be pulled into any apps if we need to. We might want to prototype them in Lifespace and incorporate it as a dependency there to have a first testbed for a reusable component 👍

@vishnuravi
Copy link
Member Author

vishnuravi commented Jan 20, 2025

Sounds fair; might be a good approach and could be a simple and reusable view that could be pulled into any apps if we need to. We might want to prototype them in Lifespace and incorporate it as a dependency there to have a first testbed for a reusable component 👍

That's a good plan. We have actually used this view successfully in LifeSpace to debug some issues with HealthKit data upload, and I imagine it could be useful for some other projects as well.

@vishnuravi
Copy link
Member Author

Development of this feature will continue in the LifeSpace project: https://github.com/StanfordBDHG/LifeSpace/tree/main/LifeSpace/Logging and will be broken out into a separate repository when complete. This PR will be closed.

@vishnuravi vishnuravi closed this Jan 21, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants