Are our log messages logged at the right severity?

[josephdecock]

We sometimes hear from users who would prefer some of our log messages to be logged at a different level. We'd like to make our logs as useful as possible, but we also know that many users have metrics and alerts that are sensitive to log levels. We don't want to make useful alerts stop working, but we also don't want false positives for on-call support types. That's why we treat a log message's severity as an API contract and keep them stable.

We're considering a few options for how to approach this problem in the future:

• If there is community consensus that particular log messages should change to a higher or lower severity, we could make the breaking change in a future major version. This would be simple for us to implement, and all users would get it automatically. However, it would be a breaking change.
• We could build some kind of abstraction that would allow configuration to control the severity of log messages. This would allow for a very high degree of control and would avoid breaking changes, but would require some very detailed knowledge on the part of the user of what log messages we can emit, and very detailed configuration to control that.
• Many logging libraries allow for sophisticated filtering or transformation of log events. For example, Serilog has filters and custom sinks that could probably accomplish this. Relying on these tools and making no changes in our tools is a 3rd option. This has many of the same pros and cons as adding a new abstraction to IdentityServer, but it would be relying on existing tooling rather than introducing something new.

We'd love to hear from you!

• Are there log messages that you'd like to see changed? If so, which ones?
• If we built a log level configuration system to allow you to change log levels, would you use it?
• Is there a need for such a system in IdentityServer itself, or do existing log libraries already give you enough extensibility?

[aaronclawrence]

I would say "mostly yes" but sometimes things are logged as errors that may not be actual errors. From another issue I logged, ""Client secret validation failed for client" doesn't need to be an error in my understanding - it can just mean someone has configured a client wrong.

This message is logged as an error, but it's not an error in the software, it's a kind of expected condition for client's that aren't setup right, or whatever. A bit like a wrong password, it's not a software error.

We think it should be downgraded to a warning or even just an info...

[aaronclawrence]

Other errors that we dont think are actual problems, seen on our production server:
"Request validation failed"
"Invalid authorization code {@values}, details: {@details}"
"Invalid redirect_uri: {detail} {@requestDetails}"
"code_challenge is missing {@requestDetails}"

[aaronclawrence]

This one is in our case at least, not an error either - it's known problems such as a user that has been deleted. It's also an unhelpful message in general, it doesn't do anything to identify the user/subject. I had to look at our own custom logging to figure it out.
"Custom token request validator {@values}, details: {@details}"

[wcabus]

From https://github.com/orgs/DuendeSoftware/discussions/286: the error message "One or more errors occured during deserialization of persisted grants, returning successfull items." logged by DefaultPersistedGrantService.GetAllGrantsAsync could potentially also include the subject ID.

Subject ID is being logged elsewhere as well, so shouldn't be an issue to include it in this error message.

[DoxxableSetanta]

+1 on the client secret being an unnecessary error. I think it makes far more sense to be a warning using MS's guidelines. Being able to control it

We expose IdentityServer to our customers who have client credentials authentication into our system, and there's always some miscommunication of secrets in the initial phase of integration which leads to a deluge of error logs on our service and some antsy support folks who ping our team chat.

We use NLog in our company, and I'm not sure if what you're suggesting is possible...our custom sink pushes it to Azure EventHub, and I'm not sure if we can register some form of decorator/interceptor that's specific to that log event. I'd have to look into it.