Skip to content

Small refactoring to remove duplicate code around SocketException detection #3123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andrewlock merged 1 commit into from
Aug 30, 2022
Merged

Small refactoring to remove duplicate code around SocketException detection #3123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 2 additions & 15 deletions tracer/src/Datadog.Trace/Agent/Api.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Datadog.Trace.DogStatsd;
using Datadog.Trace.Logging;
using Datadog.Trace.PlatformHelpers;
using Datadog.Trace.Util.Http;
using Datadog.Trace.Vendors.Newtonsoft.Json;
using Datadog.Trace.Vendors.StatsdClient;

Expand Down Expand Up @@ -157,21 +158,7 @@ private async Task<bool> SendWithRetry<T>(Uri endpoint, SendCallback<T> callback
}

// Before retry delay
bool isSocketException = false;
Exception innerException = exception;

while (innerException != null)
{
if (innerException is SocketException)
{
isSocketException = true;
break;
}

innerException = innerException.InnerException;
}

if (isSocketException)
if (exception.IsSocketException())
{
_log.Debug(exception, "Unable to communicate with the trace agent at {AgentEndpoint}", _apiRequestFactory.Info(endpoint));
}
Expand Down
17 changes: 2 additions & 15 deletions tracer/src/Datadog.Trace/Ci/Agent/CIWriterHttpSender.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Datadog.Trace.Ci.Agent.Payloads;
using Datadog.Trace.Configuration;
using Datadog.Trace.Logging;
using Datadog.Trace.Util.Http;

namespace Datadog.Trace.Ci.Agent
{
Expand Down Expand Up @@ -140,21 +141,7 @@ private async Task SendPayloadAsync<T>(Uri url, Func<IApiRequest, T, Task<IApiRe
}

// Before retry delay
bool isSocketException = false;
Exception innerException = exception;

while (innerException != null)
{
if (innerException is SocketException)
{
isSocketException = true;
break;
}

innerException = innerException.InnerException;
}

if (isSocketException)
if (exception.IsSocketException())
{
Log.Debug(exception, "Unable to communicate with {AgentEndpoint}", _apiRequestFactory.Info(url));
}
Expand Down
17 changes: 2 additions & 15 deletions tracer/src/Datadog.Trace/Ci/IntelligentTestRunnerClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using Datadog.Trace.Ci.Configuration;
using Datadog.Trace.Logging;
using Datadog.Trace.Util;
using Datadog.Trace.Util.Http;
using Datadog.Trace.Vendors.Newtonsoft.Json;

namespace Datadog.Trace.Ci;
Expand Down Expand Up @@ -314,21 +315,7 @@ private async Task<T> WithRetries<T, TState>(Func<TState, bool, Task<T>> sendDel
}

// Before retry delay
bool isSocketException = false;
Exception? innerException = exceptionDispatchInfo.SourceException;

while (innerException != null)
{
if (innerException is SocketException)
{
isSocketException = true;
break;
}

innerException = innerException.InnerException;
}

if (isSocketException)
if (exceptionDispatchInfo.SourceException.IsSocketException())
{
Log.Debug(exceptionDispatchInfo.SourceException, "Unable to communicate with the server");
}
Expand Down
18 changes: 2 additions & 16 deletions tracer/src/Datadog.Trace/Logging/DirectSubmission/Sink/LogsApi.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Net.Sockets;
using System.Threading.Tasks;
using Datadog.Trace.Agent;
using Datadog.Trace.Util.Http;

namespace Datadog.Trace.Logging.DirectSubmission.Sink
{
Expand Down Expand Up @@ -133,7 +134,7 @@ public async Task<bool> SendLogsAsync(ArraySegment<byte> logs, int numberOfLogs)
}

// Before retry delay
if (IsSocketException(exception))
if (exception.IsSocketException())
{
Log.Debug(exception, "Unable to communicate with the logs intake at {IntakeEndpoint}", _apiRequestFactory.Info(_logsIntakeEndpoint));
}
Expand All @@ -144,20 +145,5 @@ public async Task<bool> SendLogsAsync(ArraySegment<byte> logs, int numberOfLogs)
nextSleepDuration *= 2;
}
}

private static bool IsSocketException(Exception? exception)
{
while (exception is not null)
{
if (exception is SocketException)
{
return true;
}

exception = exception.InnerException;
}

return false;
}
}
}
10 changes: 3 additions & 7 deletions tracer/src/Datadog.Trace/Telemetry/Transports/JsonTelemetryTransport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.Threading.Tasks;
using Datadog.Trace.Agent;
using Datadog.Trace.Logging;
using Datadog.Trace.Util.Http;
using Datadog.Trace.Vendors.Newtonsoft.Json;
using Datadog.Trace.Vendors.Newtonsoft.Json.Serialization;

Expand Down Expand Up @@ -83,13 +84,8 @@ internal static string SerializeTelemetry(TelemetryData data)

private static bool IsFatalException(Exception ex)
{
return ex is SocketException
#if !NETFRAMEWORK
or WebException { InnerException: System.Net.Http.HttpRequestException { InnerException: SocketException } }
or System.Net.Http.HttpRequestException { InnerException: SocketException }
#endif
or WebException { Response: HttpWebResponse { StatusCode: HttpStatusCode.NotFound } }
or WebException { InnerException: SocketException };
return ex.IsSocketException()
|| ex is WebException { Response: HttpWebResponse { StatusCode: HttpStatusCode.NotFound } };
}
}
}
29 changes: 29 additions & 0 deletions tracer/src/Datadog.Trace/Util/Http/HttpExceptionExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// <copyright file="HttpExceptionExtensions.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

#nullable enable

using System;
using System.Net.Sockets;

namespace Datadog.Trace.Util.Http;

internal static class HttpExceptionExtensions

@lucaspimentel lucaspimentel Aug 26, 2022 •
edited
Loading

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.

Should this be called ExceptionExtensions since the only method is an extension on Exception, not HttpException? (level of care low)

@andrewlock andrewlock Aug 30, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, probably 🤔 I made it HttpExceptionExtensions because I wanted to put it in the Http sub namespace, to reduce the places it's in scope. I'll leave it there for now, but happy to shift it later

{
public static bool IsSocketException(this Exception? exception)
{
while (exception is not null)
{
if (exception is SocketException)
{
return true;
}

exception = exception.InnerException;
}

return false;
}
}
50 changes: 50 additions & 0 deletions tracer/test/Datadog.Trace.Tests/Util/Http/HttpExceptionExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// <copyright file="HttpExceptionExtensions.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

using System;
using System.IO;
using System.Net;
using System.Net.Sockets;
using System.Net.WebSockets;
using Datadog.Trace.Util.Http;
using FluentAssertions;
using Xunit;

namespace Datadog.Trace.Tests.Util.Http;

public class HttpExceptionExtensions
{
public static TheoryData<Exception> SocketExceptions { get; } = new()
{
new SocketException(),
new WebException("msg", new SocketException()),
#if !NETFRAMEWORK
new System.Net.Http.HttpRequestException("msg", new SocketException()),
new WebException("msg", new System.Net.Http.HttpRequestException("msg", new SocketException())),
#endif
};

public static TheoryData<Exception> NonSocketExceptions { get; } = new()
{
null,
new WebSocketException(),
new Exception(),
new IOException(),
};

[Theory]
[MemberData(nameof(SocketExceptions))]
public void IsSocketException_True(Exception exception)
{
exception.IsSocketException().Should().BeTrue();
}

[Theory]
[MemberData(nameof(NonSocketExceptions))]
public void IsSocketException_False(Exception exception)
{
exception.IsSocketException().Should().BeFalse();
}
}