Skip to content

Refactor HTTP logging handler for efficiency#1649

Open
paulomorgado wants to merge 1 commit into
sipsorcery-org:masterfrom
paulomorgado:HttpLoggingHandler
Open

Refactor HTTP logging handler for efficiency#1649
paulomorgado wants to merge 1 commit into
sipsorcery-org:masterfrom
paulomorgado:HttpLoggingHandler

Conversation

@paulomorgado
Copy link
Copy Markdown
Contributor

Refactored SendAsync to skip logging when not enabled and moved logging logic to a dedicated method. Switched requestId generation to use a cryptographically secure hex string. Improved log level checks to reduce unnecessary work. Enhanced body logging to respect debug level and _logBody flag. Simplified JSON detection and body truncation logic, introducing a constant and StringBuilder for efficiency. Made minor code structure and formatting improvements.

Refactored SendAsync to skip logging when not enabled and moved logging logic to a dedicated method. Switched requestId generation to use a cryptographically secure hex string. Improved log level checks to reduce unnecessary work. Enhanced body logging to respect debug level and _logBody flag. Simplified JSON detection and body truncation logic, introducing a constant and StringBuilder for efficiency. Made minor code structure and formatting improvements.
{
// Log request with better formatting
var requestId = Guid.NewGuid().ToString("N").Substring(0, 8); // Short ID for correlation
if (!_logger.IsEnabled(LogLevel.Information))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this be LogLevel.Debug to match what's in SendAsyncWithLogging?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let me think.

If LogLevel.Debug is enabled, then LogLevel.Information is enabled. Right?

So, SendAsyncWithLogging is only invoked it LogLevel.Information is enabled.

If LogLevel.Debug is enabled, the debug logging is also executed.

Right?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, but if it is the Information level enabled, and not Debug, then it will go into SendAsyncWithLogging and then fail. It may as well short circuit before calling SendAsyncWithLogging.

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