Skip to content

Refactor logging to use structured message templates for DTLS#1669

Open
paulomorgado wants to merge 2 commits into
sipsorcery-org:masterfrom
paulomorgado:strings-11
Open

Refactor logging to use structured message templates for DTLS#1669
paulomorgado wants to merge 2 commits into
sipsorcery-org:masterfrom
paulomorgado:strings-11

Conversation

@paulomorgado
Copy link
Copy Markdown
Contributor

Refactored logging in DtlsClient and DtlsServer to use structured message templates with argument placeholders, replacing string concatenation. Added a new Log.Debug overload and a helper for formatting templates in Log.cs. Ensured all debug logs are guarded by Log.DebugEnabled, aligning with user preferences for structured logging.

Split of #1639

Refactored logging in DtlsClient and DtlsServer to use structured message templates with argument placeholders, replacing string concatenation. Added a new Log.Debug overload and a helper for formatting templates in Log.cs. Ensured all debug logs are guarded by Log.DebugEnabled, aligning with user preferences for structured logging.
@sipsorcery
Copy link
Copy Markdown
Member

I don't think this PR achieves its goals. There doesn't seem to be a lot of point standardising the log message templates to align with the rest of the library when the actual logging mechanism is completely different. The proper fix would seem to be making the DTLS logging use the same ILogger mechanism as the rest of the library.

Replaced the custom Log class and all related usages with Microsoft.Extensions.Logging. Updated DtlsClient and DtlsServer to use structured logging via ILogger, including logger.IsEnabled checks for value processing. Removed Log.cs and introduced static logger instances using LogFactory.CreateLogger<T>(). All log messages now use structured templates and appropriate log levels.
@paulomorgado
Copy link
Copy Markdown
Contributor Author

Good point!

I've converted to Microsoft.Extensions.Logging like the rest of the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants