OSLogWriter sends os logs with same type for both warn and error levels

[MarcoFilosi]

It seems OSLogWriter sends os logs with type error for both warn and error levels.
I think it would be better to have different types for these 2 levels assigning fault type to error level.

In LogWriter.swift at line 186 (on master branch) we have:

        case LogLevel.warn:  return .error
        case LogLevel.error: return .error

we could just modify it in this way:

        case LogLevel.warn:  return .error
        case LogLevel.error: return .fault

[cnoon]

Hi @MarcoFilosi,

Thanks for putting this ticket together. After more investigation, I tend to agree with you that that change is warranted. I'm assuming that we went with .error mapping across the Willow .warn and .error log levels because of the description of the os_log .fault level.

I honestly can't remember making a conscious decision to go with .error for both log levels, but I would guess that I did.

However, reading through it again, it doesn't really seem to me that .fault should be avoided, and certainly falls more inline with Willow's concept of .error.

os_log

Docs can be found here.

Error

Error-level messages are always saved in the data store. They remain there until a storage quota is exceeded, at which point, the oldest messages are purged. Error-level messages are intended for reporting process-level errors. If an activity object exists, logging at this level captures information for the entire process chain.

Fault

Fault-level messages are always saved in the data store. They remain there until a storage quota is exceeded, at which point, the oldest messages are purged. Fault-level messages are intended for capturing system-level or multi-process errors only. If an activity object exists, logging at this level captures information for the entire process chain.

---

I'm really curious to get your thoughts as well @ejensen.

If @ejensen agrees @MarcoFilosi, would you mind putting together a PR with the fix?

[ejensen]

If I remember correctly the reason .fault was avoided was because of this description from OSLogType.fault:

Use this level to capture system-level or multi-process information to report system errors.

The "system-level or multi-process information" is not the typical case of an error log.

I don't feel strongly about avoiding .fault, but another option for using a different levels for error and warn is to switch warn to the OSLogType.default:

Use this level to capture information about things that might result a failure.

[cnoon]

Both good points @ejensen.

I also believe that's why we steered clear of OSLogType.fault originally. I also agree that Willow's LogLevel..warn log level does fall more inline with OSLogType.default. Would it make the most sense to switch LogLevel.warn = OSLogType.default and require a custom log level to explicitly use the OSLogType.fault level?

I wish we could have the following, but I don't think they map correctly given the Apple docs:

LogLevel.event = OSLogType.default
LogLevel.warn = OSLogType.error
LogLevel.error = OSLogType.fault

I think instead it probably makes the most sense to do the following (as you pointed out):

LogLevel.event = OSLogType.default
LogLevel.warn = OSLogType.default
LogLevel.error = OSLogType.error

You could always still use .fault in a couple different ways.

1. You could re-create the OSLogWriter functionality with different functionality. It's a really simple class.
2. We could make the logType(forLogLevel:) API open so you could subclass and override only that API.

Thoughts @ejensen and @MarcoFilosi?

[ejensen]

Firstly, I like the flexibility of the open func logType(forLogLevel:) idea, regardless of what we decide to set as the default log level.

I’m leaning towards LogLevel.warn = OSLogType.default

[cnoon]

@MarcoFilosi would you be willing to put this PR together?

[cnoon]

Ping here @MarcoFilosi...would you be willing to put this PR together?

[AnthonyMillerSF]

I agree that default is the right default logging level for LogLevel.warn. Any chance this PR is coming soon?

[ejensen]

I recently posted PR #53 to both make the logType(forLogLevel:) open, and map LogLevel.warn to OSLogType.default

[cnoon]

Closing this issue out since it was resolved in #53. Thanks again @ejensen!