Skip to content

Commit eb3afda

Browse files
tippmar-nrtippmar-nr
authored
summary: SocketsHttpHandler plus Deadlock Refactor
feat: Use `SocketsHttpHandler` to configure `HttpClient` in .NET 6+. (#2931) fix: Refactoring to reduce the likelihood of a deadlock in `HttpClient.SendAsync()`. (#2931) chore: Minor refactoring and logging updates. (#2931)
1 parent a69ba47 commit eb3afda

32 files changed

+499
-178
lines changed

‎src/Agent/NewRelic/Agent/Core/AgentShim.cs

+8-3
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,15 @@ static void Initialize()
2121
}
2222

2323
private static bool _initialized = false;
24-
private static object _initLock = new object();
24+
private static SemaphoreSlim _lockSemaphore = new SemaphoreSlim(1, 1);
2525

2626
static bool TryInitialize(string method)
2727
{
28-
if (Monitor.IsEntered(_initLock)) return false;
28+
if (_lockSemaphore.CurrentCount == 0) return false;
2929
if (DeferInitialization(method)) return false;
3030

31-
lock (_initLock)
31+
_lockSemaphore.Wait();
32+
try
3233
{
3334
if (!_initialized)
3435
{
@@ -38,6 +39,10 @@ static bool TryInitialize(string method)
3839

3940
return true;
4041
}
42+
finally
43+
{
44+
_lockSemaphore.Release();
45+
}
4146
}
4247

4348
private static HashSet<string> _deferInitializationOnTheseMethods = new HashSet<string>

‎src/Agent/NewRelic/Agent/Core/Aggregators/CustomEventAggregator.cs

+7-5
Original file line numberDiff line numberDiff line change
@@ -89,14 +89,16 @@ protected void InternalHarvest(string transactionId = null)
8989
var customEvents = originalCustomEvents.Where(node => node != null).Select(node => node.Data).ToList();
9090

9191
// if we don't have any events to publish then don't
92-
if (customEvents.Count <= 0)
93-
return;
92+
var eventCount = customEvents.Count;
93+
if (eventCount > 0)
94+
{
95+
var responseStatus = DataTransportService.Send(customEvents, transactionId);
9496

95-
var responseStatus = DataTransportService.Send(customEvents, transactionId);
97+
HandleResponse(responseStatus, customEvents);
98+
}
9699

97-
HandleResponse(responseStatus, customEvents);
100+
Log.Finest($"Custom Event harvest finished. {eventCount} event(s) sent.");
98101

99-
Log.Finest("Custom Event harvest finished.");
100102
}
101103

102104
protected override void OnConfigurationUpdated(ConfigurationUpdateSource configurationUpdateSource)

‎src/Agent/NewRelic/Agent/Core/Aggregators/ErrorEventAggregator.cs

+7-6
Original file line numberDiff line numberDiff line change
@@ -99,14 +99,15 @@ protected void InternalHarvest(string transactionId = null)
9999
var eventHarvestData = new EventHarvestData(originalErrorEvents.Size, originalErrorEvents.GetAddAttemptsCount());
100100

101101
// if we don't have any events to publish then don't
102-
if (aggregatedEvents.Count <= 0)
103-
return;
104-
105-
var responseStatus = DataTransportService.Send(eventHarvestData, aggregatedEvents, transactionId);
102+
var eventCount = aggregatedEvents.Count;
103+
if (eventCount > 0)
104+
{
105+
var responseStatus = DataTransportService.Send(eventHarvestData, aggregatedEvents, transactionId);
106106

107-
HandleResponse(responseStatus, aggregatedEvents);
107+
HandleResponse(responseStatus, aggregatedEvents);
108+
}
108109

109-
Log.Finest("Error Event harvest finished.");
110+
Log.Finest($"Error Event harvest finished. {eventCount} event(s) sent.");
110111
}
111112

112113
protected override void OnConfigurationUpdated(ConfigurationUpdateSource configurationUpdateSource)

‎src/Agent/NewRelic/Agent/Core/Aggregators/ErrorTraceAggregator.cs

+8-6
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,16 @@ protected void InternalHarvest(string transactionId = null)
7979
_readerWriterLock.ExitWriteLock();
8080
}
8181

82-
if (errorTraceWireModels.Count <= 0)
83-
return;
84-
85-
var responseStatus = DataTransportService.Send(errorTraceWireModels, transactionId);
82+
// if we don't have any events to publish then don't
83+
var traceCount = errorTraceWireModels.Count;
84+
if (traceCount > 0)
85+
{
86+
var responseStatus = DataTransportService.Send(errorTraceWireModels, transactionId);
8687

87-
HandleResponse(responseStatus, errorTraceWireModels);
88+
HandleResponse(responseStatus, errorTraceWireModels);
89+
}
8890

89-
Log.Finest("Error Trace harvest finished.");
91+
Log.Finest($"Error Trace harvest finished. {traceCount} trace(s) sent.");
9092
}
9193

9294
protected override void OnConfigurationUpdated(ConfigurationUpdateSource configurationUpdateSource)

‎src/Agent/NewRelic/Agent/Core/Aggregators/LogEventAggregator.cs

+16-17
Original file line numberDiff line numberDiff line change
@@ -88,28 +88,27 @@ protected void InternalHarvest(string transactionId = null)
8888
Interlocked.Add(ref _logsDroppedCount, originalLogEvents.GetAndResetDroppedItemCount());
8989

9090
// if we don't have any events to publish then don't
91-
if (aggregatedEvents.Count <= 0)
91+
var eventCount = aggregatedEvents.Count;
92+
if (eventCount > 0)
9293
{
93-
return;
94-
}
94+
// matches metadata so that utilization and this match
95+
var hostname = !string.IsNullOrEmpty(_configuration.UtilizationFullHostName)
96+
? _configuration.UtilizationFullHostName
97+
: _configuration.UtilizationHostName;
9598

96-
// matches metadata so that utilization and this match
97-
var hostname = !string.IsNullOrEmpty(_configuration.UtilizationFullHostName)
98-
? _configuration.UtilizationFullHostName
99-
: _configuration.UtilizationHostName;
99+
var modelsCollection = new LogEventWireModelCollection(
100+
_configuration.ApplicationNames.ElementAt(0),
101+
_configuration.EntityGuid,
102+
hostname,
103+
_configuration.LabelsEnabled ? _labelsService.GetFilteredLabels(_configuration.LabelsExclude) : [],
104+
aggregatedEvents);
100105

101-
var modelsCollection = new LogEventWireModelCollection(
102-
_configuration.ApplicationNames.ElementAt(0),
103-
_configuration.EntityGuid,
104-
hostname,
105-
_configuration.LabelsEnabled ? _labelsService.GetFilteredLabels(_configuration.LabelsExclude) : [],
106-
aggregatedEvents);
106+
var responseStatus = DataTransportService.Send(modelsCollection, transactionId);
107107

108-
var responseStatus = DataTransportService.Send(modelsCollection, transactionId);
109-
110-
HandleResponse(responseStatus, aggregatedEvents);
108+
HandleResponse(responseStatus, aggregatedEvents);
109+
}
111110

112-
Log.Finest("Log Event harvest finished.");
111+
Log.Finest($"Log Event harvest finished. {eventCount} event(s) sent.");
113112
}
114113

115114
protected override void OnConfigurationUpdated(ConfigurationUpdateSource configurationUpdateSource)

‎src/Agent/NewRelic/Agent/Core/Aggregators/MetricAggregator.cs

+9-4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Linq;
67
using NewRelic.Agent.Core.DataTransport;
78
using NewRelic.Agent.Core.Events;
89
using NewRelic.Agent.Core.Metrics;
@@ -75,12 +76,16 @@ protected void InternalHarvest(string transactionId = null)
7576
var oldMetrics = GetStatsCollectionForHarvest();
7677

7778
oldMetrics.MergeUnscopedStats(MetricNames.SupportabilityMetricHarvestTransmit, MetricDataWireModel.BuildCountData());
78-
var metricsToSend = oldMetrics.ConvertToJsonForSending(_metricNameService);
79+
var metricsToSend = oldMetrics.ConvertToJsonForSending(_metricNameService).ToList();
7980

80-
var responseStatus = DataTransportService.Send(metricsToSend, transactionId);
81-
HandleResponse(responseStatus, metricsToSend);
81+
var metricCount = metricsToSend.Count;
82+
if (metricCount > 0)
83+
{
84+
var responseStatus = DataTransportService.Send(metricsToSend, transactionId);
85+
HandleResponse(responseStatus, metricsToSend);
86+
}
8287

83-
Log.Debug("Metric harvest finished.");
88+
Log.Finest($"Metric harvest finished. {metricCount} metric(s) sent.");
8489
}
8590

8691
protected override void OnConfigurationUpdated(ConfigurationUpdateSource configurationUpdateSource)

‎src/Agent/NewRelic/Agent/Core/Aggregators/SpanEventAggregator.cs

+8-8
Original file line numberDiff line numberDiff line change
@@ -123,16 +123,16 @@ protected void InternalHarvest(string transactionId = null)
123123

124124
var eventHarvestData = new EventHarvestData(spanEventsPriorityQueue.Size, spanEventsPriorityQueue.GetAddAttemptsCount());
125125
var wireModels = spanEventsPriorityQueue.Where(node => null != node).Select(node => node.Data).ToList();
126-
127-
// if we don't have any events to publish then don't
128-
if (wireModels.Count <= 0)
129-
return;
130-
131-
var responseStatus = DataTransportService.Send(eventHarvestData, wireModels, transactionId);
132126

133-
HandleResponse(responseStatus, wireModels);
127+
// if we don't have any events to publish then don't
128+
var eventCount = wireModels.Count;
129+
if (eventCount > 0)
130+
{
131+
var responseStatus = DataTransportService.Send(eventHarvestData, wireModels, transactionId);
132+
HandleResponse(responseStatus, wireModels);
133+
}
134134

135-
Log.Finest("Span Event harvest finished.");
135+
Log.Finest($"Span Event harvest finished. {eventCount} event(s) sent.");
136136
}
137137

138138
private void ReduceReservoirSize(int newSize)

‎src/Agent/NewRelic/Agent/Core/Aggregators/SqlTraceAggregator.cs

+8-7
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,15 @@ protected void InternalHarvest(string transactionId = null)
6666
.Take(_configuration.SqlTracesPerPeriod)
6767
.ToList();
6868

69-
if (!slowestTraces.Any())
70-
return;
71-
72-
var responseStatus = DataTransportService.Send(slowestTraces, transactionId);
73-
74-
HandleResponse(responseStatus, slowestTraces);
69+
// if we don't have any traces to publish then don't
70+
var traceCount = slowestTraces.Count;
71+
if (traceCount > 0)
72+
{
73+
var responseStatus = DataTransportService.Send(slowestTraces, transactionId);
74+
HandleResponse(responseStatus, slowestTraces);
75+
}
7576

76-
Log.Finest("SQL Trace harvest finished.");
77+
Log.Finest($"SQL Trace harvest finished. {traceCount} trace(s) sent.");
7778
}
7879

7980
private void HandleResponse(DataTransportResponseStatus responseStatus, ICollection<SqlTraceWireModel> traces)

‎src/Agent/NewRelic/Agent/Core/Aggregators/TransactionEventAggregator.cs

+7-5
Original file line numberDiff line numberDiff line change
@@ -96,14 +96,16 @@ protected void InternalHarvest(string transactionId = null)
9696
var eventHarvestData = new EventHarvestData(originalTransactionEvents.Size, originalTransactionEvents.GetAddAttemptsCount());
9797

9898
// if we don't have any events to publish then don't
99-
if (aggregatedEvents.Count <= 0)
100-
return;
99+
var eventCount = aggregatedEvents.Count;
100+
if (eventCount > 0)
101+
{
102+
var responseStatus = DataTransportService.Send(eventHarvestData, aggregatedEvents, transactionId);
101103

102-
var responseStatus = DataTransportService.Send(eventHarvestData, aggregatedEvents, transactionId);
104+
HandleResponse(responseStatus, aggregatedEvents);
105+
}
103106

104-
HandleResponse(responseStatus, aggregatedEvents);
107+
Log.Finest($"Transaction Event harvest finished. {eventCount} event(s) sent.");
105108

106-
Log.Finest("Transaction Event harvest finished.");
107109
}
108110

109111
protected override void OnConfigurationUpdated(ConfigurationUpdateSource configurationUpdateSource)

‎src/Agent/NewRelic/Agent/Core/Aggregators/TransactionTraceAggregator.cs

+11-5
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ public override void Collect(TransactionTraceWireModelComponents transactionTrac
4949

5050
protected void InternalHarvest(string transactionId = null)
5151
{
52+
Log.Finest("Transaction Trace harvest starting.");
53+
5254
var traceSamples = _transactionCollectors
5355
.Where(t => t != null)
5456
.SelectMany(t => t.GetCollectedSamples())
@@ -59,13 +61,17 @@ protected void InternalHarvest(string transactionId = null)
5961
.Select(t => t.CreateWireModel())
6062
.ToList();
6163

62-
if (!traceWireModels.Any())
63-
return;
64+
// if we don't have any traces to publish then don't
65+
var traceCount = traceWireModels.Count;
66+
if (traceCount > 0)
67+
{
68+
LogUnencodedTraceData(traceWireModels);
6469

65-
LogUnencodedTraceData(traceWireModels);
70+
var responseStatus = DataTransportService.Send(traceWireModels, transactionId);
71+
HandleResponse(responseStatus, traceSamples);
72+
}
6673

67-
var responseStatus = DataTransportService.Send(traceWireModels, transactionId);
68-
HandleResponse(responseStatus, traceSamples);
74+
Log.Finest($"Transaction Trace harvest finished. {traceCount} trace(s) sent.");
6975
}
7076

7177
private void HandleResponse(DataTransportResponseStatus responseStatus, ICollection<TransactionTraceWireModelComponents> traceSamples)

‎src/Agent/NewRelic/Agent/Core/DataTransport/Client/HttpClientBase.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ protected void TestConnection()
6666
wc.DownloadString(testAddress);
6767
}
6868
#else
69-
_lazyHttpClient.Value.GetAsync(testAddress).GetAwaiter().GetResult();
69+
_lazyHttpClient.Value.GetAsync(testAddress).ConfigureAwait(false).GetAwaiter().GetResult();
7070
#endif
7171
Log.Info("Connection test to \"{0}\" succeeded", testAddress);
7272
}

‎src/Agent/NewRelic/Agent/Core/DataTransport/Client/HttpClientWrapper.cs

+13-13
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
1-
// Copyright 2020 New Relic, Inc. All rights reserved.
1+
// Copyright 2020 New Relic, Inc. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

44
#if !NETFRAMEWORK
55
using System;
66
using System.Net.Http;
77
using System.Threading;
88
using System.Threading.Tasks;
9-
using NewRelic.Agent.Configuration;
10-
using NewRelic.Agent.Core.Config;
119
using NewRelic.Agent.Core.DataTransport.Client.Interfaces;
10+
using NewRelic.Agent.Extensions.Logging;
1211

1312
namespace NewRelic.Agent.Core.DataTransport.Client
1413
{
@@ -31,19 +30,20 @@ public void Dispose()
3130

3231
public async Task<IHttpResponseMessageWrapper> SendAsync(HttpRequestMessage message)
3332
{
34-
var cts = new CancellationTokenSource(_timeoutMilliseconds);
35-
return new HttpResponseMessageWrapper(await _httpClient.SendAsync(message, cts.Token));
36-
}
37-
38-
public TimeSpan Timeout
39-
{
40-
get
33+
using var cts = new CancellationTokenSource(_timeoutMilliseconds);
34+
try
4135
{
42-
return _httpClient.Timeout;
36+
// .ConfigureAwait(false) is used to avoid deadlocks.
37+
var httpResponseMessage = await _httpClient.SendAsync(message, cts.Token).ConfigureAwait(false);
38+
return new HttpResponseMessageWrapper(httpResponseMessage);
4339
}
44-
set
40+
catch (Exception e)
4541
{
46-
_httpClient.Timeout = value;
42+
Log.Debug(cts.IsCancellationRequested
43+
? $"HttpClient.SendAsync() timed out after {_timeoutMilliseconds}ms."
44+
: $"HttpClient.SendAsync() threw an unexpected exception: {e}");
45+
46+
throw;
4747
}
4848
}
4949
}

‎src/Agent/NewRelic/Agent/Core/DataTransport/Client/HttpContentWrapper.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public HttpContentWrapper(HttpContent httpContent)
2222

2323
public Stream ReadAsStream()
2424
{
25-
return _httpContent.ReadAsStreamAsync().GetAwaiter().GetResult();
25+
return _httpContent.ReadAsStreamAsync().ConfigureAwait(false).GetAwaiter().GetResult();
2626
}
2727

2828
public IHttpContentHeadersWrapper Headers => new HttpContentHeadersWrapper(_httpContent.Headers);

‎src/Agent/NewRelic/Agent/Core/DataTransport/Client/HttpResponse.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public string GetContent()
4848
using (responseStream)
4949
using (var reader = new StreamReader(responseStream, Encoding.UTF8))
5050
{
51-
var responseBody = reader.ReadLineAsync().GetAwaiter().GetResult();
51+
var responseBody = reader.ReadLineAsync().ConfigureAwait(false).GetAwaiter().GetResult();
5252

5353
if (responseBody != null)
5454
{

‎src/Agent/NewRelic/Agent/Core/DataTransport/Client/Interfaces/IHttpClientFactory.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2020 New Relic, Inc. All rights reserved.
1+
// Copyright 2020 New Relic, Inc. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

44
using System.Net;
@@ -8,6 +8,6 @@ namespace NewRelic.Agent.Core.DataTransport.Client.Interfaces
88
{
99
public interface IHttpClientFactory
1010
{
11-
public IHttpClient CreateClient(IWebProxy proxy, IConfiguration configuration);
11+
public IHttpClient GetOrCreateClient(IWebProxy proxy, IConfiguration configuration);
1212
}
1313
}

‎src/Agent/NewRelic/Agent/Core/DataTransport/Client/Interfaces/IHttpClientWrapper.cs

-2
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ namespace NewRelic.Agent.Core.DataTransport.Client.Interfaces
1111
public interface IHttpClientWrapper : IDisposable
1212
{
1313
Task<IHttpResponseMessageWrapper> SendAsync(HttpRequestMessage message);
14-
15-
TimeSpan Timeout { get; set; }
1614
}
1715
}
1816
#endif

0 commit comments

Comments
 (0)