Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ public string GetContent()
}
}

public string GetHeaders() => HttpResponseHeaderFormatter.Format(_httpResponseMessageWrapper.Headers);

public bool IsSuccessStatusCode => _httpResponseMessageWrapper.IsSuccessStatusCode;
public HttpStatusCode StatusCode => _httpResponseMessageWrapper.StatusCode;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

using System.Collections.Generic;
using System.Linq;
#if NETFRAMEWORK
using System.Net;
#endif

namespace NewRelic.Agent.Core.DataTransport.Client;

/// <summary>
/// Formats HTTP response headers into a single-line string for debug logging.
/// </summary>
public static class HttpResponseHeaderFormatter
{
private const string NoHeaders = "(none)";

public static string Format(IEnumerable<KeyValuePair<string, IEnumerable<string>>> headers)
{
if (headers == null)
{
return NoHeaders;
}

var formatted = headers
.Select(header => $"{header.Key}=[{string.Join(", ", header.Value)}]")
.ToList();

return formatted.Count == 0 ? NoHeaders : string.Join("; ", formatted);
}

#if NETFRAMEWORK
public static string Format(WebHeaderCollection headers)
{
if (headers == null)
{
return NoHeaders;
}

var keyValuePairs = headers.AllKeys
.Select(key => new KeyValuePair<string, IEnumerable<string>>(key, headers.GetValues(key) ?? new string[0]));

return Format(keyValuePairs);
}
#endif
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0
#if !NETFRAMEWORK
using System.Collections.Generic;
using System.Net;
using System.Net.Http;
using NewRelic.Agent.Core.DataTransport.Client.Interfaces;
Expand All @@ -22,6 +23,7 @@ public HttpResponseMessageWrapper(HttpResponseMessage responseMessage)
public IHttpContentWrapper Content => _responseMessage.Content == null ? null : new HttpContentWrapper(_responseMessage.Content);
public bool IsSuccessStatusCode => _responseMessage.IsSuccessStatusCode;
public HttpStatusCode StatusCode => _responseMessage.StatusCode;
public IEnumerable<KeyValuePair<string, IEnumerable<string>>> Headers => _responseMessage.Headers;

public void Dispose()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ public interface IHttpResponse : IDisposable
bool IsSuccessStatusCode { get; }
HttpStatusCode StatusCode { get; }
string GetContent();
string GetHeaders();
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

using System;
using System.Collections.Generic;
using System.Net;

namespace NewRelic.Agent.Core.DataTransport.Client.Interfaces;
Expand All @@ -14,4 +15,5 @@ public interface IHttpResponseMessageWrapper : IDisposable
IHttpContentWrapper Content { get; }
bool IsSuccessStatusCode { get; }
HttpStatusCode StatusCode { get; }
IEnumerable<KeyValuePair<string, IEnumerable<string>>> Headers { get; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ public string GetContent()
}
}

public string GetHeaders() => HttpResponseHeaderFormatter.Format(_response.Headers);

public bool IsSuccessStatusCode => (200 <= (int)_response.StatusCode) && ((int)_response.StatusCode <= 299);
public HttpStatusCode StatusCode => _response.StatusCode;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ public string SendData(string method, ConnectionInfo connectionInfo, string seri

if (!response.IsSuccessStatusCode)
{
if (Log.IsDebugEnabled)
Log.Debug("Request({0}): Invocation of \"{1}\" returned response headers : {2}", requestGuid, method, response.GetHeaders());

ThrowExceptionFromHttpResponseMessage(serializedData, response.StatusCode, responseContent, requestGuid);
}

Expand All @@ -80,6 +83,8 @@ public string SendData(string method, ConnectionInfo connectionInfo, string seri
// Possibly combine these logs? makes parsing harder in tests...
Log.Debug("Request({0}): Invoked \"{1}\" with : {2}", requestGuid, method, serializedData);
Log.Debug("Request({0}): Invocation of \"{1}\" yielded response : {2}", requestGuid, method, responseContent);
if (Log.IsDebugEnabled)
Comment thread
jaffinito marked this conversation as resolved.
Log.Debug("Request({0}): Invocation of \"{1}\" returned response headers : {2}", requestGuid, method, response.GetHeaders());

DataTransportAuditLogger.Log(DataTransportAuditLogger.AuditLogDirection.Received, DataTransportAuditLogger.AuditLogSource.Collector, responseContent);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

using System.Collections.Generic;
using NUnit.Framework;

namespace NewRelic.Agent.Core.DataTransport.Client;

[TestFixture]
public class HttpResponseHeaderFormatterTests
{
[Test]
public void Format_ReturnsNoneSentinel_WhenHeadersAreNull()
{
var result = HttpResponseHeaderFormatter.Format((IEnumerable<KeyValuePair<string, IEnumerable<string>>>)null);

Assert.That(result, Is.EqualTo("(none)"));
}

[Test]
public void Format_ReturnsNoneSentinel_WhenHeadersAreEmpty()
{
var result = HttpResponseHeaderFormatter.Format(new List<KeyValuePair<string, IEnumerable<string>>>());

Assert.That(result, Is.EqualTo("(none)"));
}

[Test]
public void Format_FormatsSingleHeaderWithSingleValue()
{
var headers = new List<KeyValuePair<string, IEnumerable<string>>>
{
new("cf-ray", new[] { "abc123-DFW" })
};

var result = HttpResponseHeaderFormatter.Format(headers);

Assert.That(result, Is.EqualTo("cf-ray=[abc123-DFW]"));
}

[Test]
public void Format_JoinsMultipleValuesForOneHeader()
{
var headers = new List<KeyValuePair<string, IEnumerable<string>>>
{
new("x-request-id", new[] { "id-1", "id-2" })
};

var result = HttpResponseHeaderFormatter.Format(headers);

Assert.That(result, Is.EqualTo("x-request-id=[id-1, id-2]"));
}

[Test]
public void Format_JoinsMultipleHeadersWithSemicolon()
{
var headers = new List<KeyValuePair<string, IEnumerable<string>>>
{
new("cf-ray", new[] { "abc123-DFW" }),
new("x-request-id", new[] { "id-1" })
};

var result = HttpResponseHeaderFormatter.Format(headers);

Assert.That(result, Is.EqualTo("cf-ray=[abc123-DFW]; x-request-id=[id-1]"));
}

#if NETFRAMEWORK
[Test]
public void Format_WebHeaderCollection_FormatsHeaders()
{
var headers = new System.Net.WebHeaderCollection
{
{ "cf-ray", "abc123-DFW" },
{ "x-request-id", "id-1" }
};

var result = HttpResponseHeaderFormatter.Format(headers);

Assert.That(result, Does.Contain("cf-ray=[abc123-DFW]"));
Assert.That(result, Does.Contain("x-request-id=[id-1]"));
}

[Test]
public void Format_WebHeaderCollection_ReturnsNoneSentinel_WhenEmpty()
{
var result = HttpResponseHeaderFormatter.Format(new System.Net.WebHeaderCollection());

Assert.That(result, Is.EqualTo("(none)"));
}
#endif
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,5 +102,20 @@ public void StatusCode_ReturnsCorrectStatusCode()

Assert.That(result, Is.EqualTo(HttpStatusCode.OK));
}

[Test]
public void GetHeaders_ReturnsFormattedHeaders()
{
var headers = new List<KeyValuePair<string, IEnumerable<string>>>
{
new("cf-ray", new[] { "abc123-DFW" }),
new("x-request-id", new[] { "id-1" })
};
_mockHttpResponseMessage.Arrange(message => message.Headers).Returns(headers);

var result = _httpResponse.GetHeaders();

Assert.That(result, Is.EqualTo("cf-ray=[abc123-DFW]; x-request-id=[id-1]"));
}
}
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,26 @@ public void StatusCode_ShouldReturnCorrectStatusCode()
Assert.That(webRequestClientResponse.StatusCode, Is.EqualTo(HttpStatusCode.NotFound));
}

[Test]
public void GetHeaders_ShouldReturnFormattedHeaders()
{
// Arrange
var headers = new WebHeaderCollection
{
{ "cf-ray", "abc123-DFW" },
{ "x-request-id", "id-1" }
};
Mock.Arrange(() => _response.Headers).Returns(headers);
var webRequestClientResponse = new WebRequestClientResponse(_requestGuid, _response);

// Act
var result = webRequestClientResponse.GetHeaders();

// Assert
Assert.That(result, Does.Contain("cf-ray=[abc123-DFW]"));
Assert.That(result, Does.Contain("x-request-id=[id-1]"));
}

private byte[] CompressString(string text)
{
var bytes = Encoding.UTF8.GetBytes(text);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public class HttpCollectorWireTests
private IAgentHealthReporter _agentHealthReporter;
private IHttpClientFactory _httpClientFactory;
private ILogger _mockILogger;
private global::NewRelic.Agent.Extensions.Logging.ILogger _nrLogger;

[SetUp]
public void SetUp()
Expand All @@ -32,6 +33,16 @@ public void SetUp()

_mockILogger = Mock.Create<ILogger>();
Log.Logger = _mockILogger;

// The agent's Log facade is independent of Serilog's static logger; initialize it so IsDebugEnabled is controllable.
_nrLogger = Mock.Create<global::NewRelic.Agent.Extensions.Logging.ILogger>();
global::NewRelic.Agent.Extensions.Logging.Log.Initialize(_nrLogger);
}

[TearDown]
public void TearDown()
{
global::NewRelic.Agent.Extensions.Logging.Log.Initialize(new global::NewRelic.Agent.Extensions.Logging.NoOpLogger());
}

private HttpCollectorWire CreateHttpCollectorWire(Dictionary<string, string> requestHeadersMap = null)
Expand Down Expand Up @@ -207,4 +218,89 @@ public void SendData_ShouldNotCallAuditLog_UnlessAuditLogIsEnabled(bool isEnable
// Assert
Mock.Assert(() => mockForContextLogger.Fatal(Arg.AnyString), isEnabled ? Occurs.Exactly(3) : Occurs.Never());
}

[Test]
public void SendData_LogsResponseHeaders_OnSuccess_WhenDebugEnabled()
{
// Arrange
Mock.Arrange(() => _configuration.AgentLicenseKey).Returns("license_key");
Mock.Arrange(() => _configuration.CollectorMaxPayloadSizeInBytes).Returns(1024 * 1024);
Mock.Arrange(() => _nrLogger.IsDebugEnabled).Returns(true);

var connectionInfo = new ConnectionInfo(_configuration);

var mockHttpResponse = Mock.Create<IHttpResponse>();
Mock.Arrange(() => mockHttpResponse.StatusCode).Returns(HttpStatusCode.OK);
Mock.Arrange(() => mockHttpResponse.IsSuccessStatusCode).Returns(true);
Mock.Arrange(() => mockHttpResponse.GetContent()).Returns("{}");
Mock.Arrange(() => mockHttpResponse.GetHeaders()).Returns("cf-ray=[abc123-DFW]");

var mockHttpClient = Mock.Create<IHttpClient>();
Mock.Arrange(() => mockHttpClient.Send(Arg.IsAny<IHttpRequest>())).Returns(mockHttpResponse);
Mock.Arrange(() => _httpClientFactory.GetOrCreateClient(Arg.IsAny<IWebProxy>(), Arg.IsAny<IConfiguration>())).Returns(mockHttpClient);

var collectorWire = CreateHttpCollectorWire();

// Act
_ = collectorWire.SendData("test_method", connectionInfo, "{ \"key\": \"value\" }", Guid.NewGuid());

// Assert
Mock.Assert(() => mockHttpResponse.GetHeaders(), Occurs.Once());
}

[Test]
public void SendData_LogsResponseHeaders_OnError_WhenDebugEnabled()
{
// Arrange
Mock.Arrange(() => _configuration.AgentLicenseKey).Returns("license_key");
Mock.Arrange(() => _configuration.CollectorMaxPayloadSizeInBytes).Returns(1024 * 1024);
Mock.Arrange(() => _nrLogger.IsDebugEnabled).Returns(true);

var connectionInfo = new ConnectionInfo(_configuration);

var mockHttpResponse = Mock.Create<IHttpResponse>();
Mock.Arrange(() => mockHttpResponse.StatusCode).Returns(HttpStatusCode.InternalServerError);
Mock.Arrange(() => mockHttpResponse.IsSuccessStatusCode).Returns(false);
Mock.Arrange(() => mockHttpResponse.GetContent()).Returns("{}");
Mock.Arrange(() => mockHttpResponse.GetHeaders()).Returns("cf-ray=[err-DFW]");

var mockHttpClient = Mock.Create<IHttpClient>();
Mock.Arrange(() => mockHttpClient.Send(Arg.IsAny<IHttpRequest>())).Returns(mockHttpResponse);
Mock.Arrange(() => _httpClientFactory.GetOrCreateClient(Arg.IsAny<IWebProxy>(), Arg.IsAny<IConfiguration>())).Returns(mockHttpClient);

var collectorWire = CreateHttpCollectorWire();

// Act and Assert
Assert.Throws<HttpException>(() => collectorWire.SendData("test_method", connectionInfo, "invalidjson", Guid.NewGuid()));
Mock.Assert(() => mockHttpResponse.GetHeaders(), Occurs.Once());
}

[Test]
public void SendData_DoesNotReadResponseHeaders_WhenDebugDisabled()
{
// Arrange
Mock.Arrange(() => _configuration.AgentLicenseKey).Returns("license_key");
Mock.Arrange(() => _configuration.CollectorMaxPayloadSizeInBytes).Returns(1024 * 1024);
Mock.Arrange(() => _nrLogger.IsDebugEnabled).Returns(false);

var connectionInfo = new ConnectionInfo(_configuration);

var mockHttpResponse = Mock.Create<IHttpResponse>();
Mock.Arrange(() => mockHttpResponse.StatusCode).Returns(HttpStatusCode.OK);
Mock.Arrange(() => mockHttpResponse.IsSuccessStatusCode).Returns(true);
Mock.Arrange(() => mockHttpResponse.GetContent()).Returns("{}");
Mock.Arrange(() => mockHttpResponse.GetHeaders()).Returns("cf-ray=[abc123-DFW]");

var mockHttpClient = Mock.Create<IHttpClient>();
Mock.Arrange(() => mockHttpClient.Send(Arg.IsAny<IHttpRequest>())).Returns(mockHttpResponse);
Mock.Arrange(() => _httpClientFactory.GetOrCreateClient(Arg.IsAny<IWebProxy>(), Arg.IsAny<IConfiguration>())).Returns(mockHttpClient);

var collectorWire = CreateHttpCollectorWire();

// Act
_ = collectorWire.SendData("test_method", connectionInfo, "{ \"key\": \"value\" }", Guid.NewGuid());

// Assert
Mock.Assert(() => mockHttpResponse.GetHeaders(), Occurs.Never());
}
}
Loading