src/NLog.Targets.Network/NetworkSenders/INetworkSenderFactory.cs (1)

56-56: Consider enhancing the parameter documentation.

While the documentation includes the parameter, it only mentions "TCP_NODELAY" without explaining its purpose. Consider adding more context about what this socket option does (disables Nagle's algorithm) and its effect (reduces latency by sending packets immediately).

-/// <param name="noDelay">TCP_NODELAY</param>
+/// <param name="noDelay">When true, disables the Nagle algorithm by setting TCP_NODELAY, which reduces latency by sending packets immediately rather than waiting to build a larger packet.</param>

src/NLog.Targets.Network/NetworkSenders/NetworkSenderFactory.cs (1)

49-49: ⚠️ Potential issue

Updated Create method signature with noDelay parameter

The method signature has been updated to include the noDelay parameter, matching the interface. However, the implementation doesn't set the NoDelay property for the TcpNetworkSender instances it creates.

The code should be updated to pass the noDelay value to the TcpNetworkSenders created in the method. Here's the fix for the first case (other cases need similar changes):

return new TcpNetworkSender(url, AddressFamily.Unspecified)
{
    MaxQueueSize = maxQueueSize,
    OnQueueOverflow = onQueueOverflow,
    SslProtocols = sslProtocols,
    KeepAliveTime = keepAliveTime,
    SendTimeout = sendTimeout,
    SslCertificateOverride = sslCertificateOverride,
+   NoDelay = noDelay,
};

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

public QueuedNetworkSender Create(string url, int maxQueueSize, 
                                  NetworkTargetQueueOverflowAction onQueueOverflow, 
                                  int maxQueueSize, 
                                  int maxMessageSize, 
                                  System.Security.Authentication.SslProtocols sslProtocols, 
                                  X509Certificate2Collection sslCertificateOverride, 
                                  TimeSpan keepAliveTime, 
                                  TimeSpan sendTimeout, 
                                  bool noDelay)
{
    return new TcpNetworkSender(url, AddressFamily.Unspecified)
    {
        MaxQueueSize = maxQueueSize,
        OnQueueOverflow = onQueueOverflow,
        SslProtocols = sslProtocols,
        KeepAliveTime = keepAliveTime,
        SendTimeout = sendTimeout,
        SslCertificateOverride = sslCertificateOverride,
        NoDelay = noDelay,
    };
}

tests/NLog.Targets.Network.Tests/NetworkSenders/HttpNetworkSenderTests.cs (2)

138-138: 🛠️ Refactor suggestion

Updated second test assertion with noDelay parameter

Similar to the previous comment, the noDelay value should be true for consistency with the default in NetworkTarget.

-networkSenderFactoryMock.Received(1).Create("http://test.with.mock", 1234, NetworkTargetQueueOverflowAction.Block, 0, SslProtocols.None, null, TimeSpan.Zero, TimeSpan.Zero, false); // Only created one HttpNetworkSender
+networkSenderFactoryMock.Received(1).Create("http://test.with.mock", 1234, NetworkTargetQueueOverflowAction.Block, 0, SslProtocols.None, null, TimeSpan.Zero, TimeSpan.Zero, true); // Only created one HttpNetworkSender

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

            networkSenderFactoryMock.Received(1).Create("http://test.with.mock", 1234, NetworkTargetQueueOverflowAction.Block, 0, SslProtocols.None, null, TimeSpan.Zero, TimeSpan.Zero, true); // Only created one HttpNetworkSender

---

92-92: 🛠️ Refactor suggestion

Updated test assertion with the new noDelay parameter

The test assertion has been updated to include the noDelay parameter, but it's using false instead of the default value true set in the NetworkTarget class. This should be updated for consistency unless there's a specific reason to test with NoDelay disabled.

-networkSenderFactoryMock.Received(1).Create("http://test.with.mock", 1234, NetworkTargetQueueOverflowAction.Block, 0, SslProtocols.None, null, TimeSpan.Zero, TimeSpan.Zero, false);
+networkSenderFactoryMock.Received(1).Create("http://test.with.mock", 1234, NetworkTargetQueueOverflowAction.Block, 0, SslProtocols.None, null, TimeSpan.Zero, TimeSpan.Zero, true);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

            networkSenderFactoryMock.Received(1).Create("http://test.with.mock", 1234, NetworkTargetQueueOverflowAction.Block, 0, SslProtocols.None, null, TimeSpan.Zero, TimeSpan.Zero, true);