Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions sdk/core/Azure.Core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## 1.3.0-preview.1 (Unreleased)

- Read client request ID value used for logging and tracing off the initial request object if available.

## 1.2.0 (2020-04-03)

Expand Down
12 changes: 10 additions & 2 deletions sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,11 @@ private sealed class PipelineRequest : Request
private readonly HttpRequestMessage _requestMessage;

private PipelineContentAdapter? _requestContent;
private string? _clientRequestId;

public PipelineRequest()
{
_requestMessage = new HttpRequestMessage();
ClientRequestId = Guid.NewGuid().ToString();
}

public override RequestMethod Method
Expand All @@ -185,7 +185,15 @@ public override RequestMethod Method

public override RequestContent? Content { get; set; }

public override string ClientRequestId { get; set; }
public override string ClientRequestId
{
get => _clientRequestId ??= Guid.NewGuid().ToString();
set
{
Argument.AssertNotNull(value, nameof(value));
_clientRequestId = value;
}
}

protected internal override void AddHeader(string name, string value)
{
Expand Down
2 changes: 2 additions & 0 deletions sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ public static HttpPipeline Build(ClientOptions options, HttpPipelinePolicy[] per

bool isDistributedTracingEnabled = options.Diagnostics.IsDistributedTracingEnabled;

policies.Add(ReadClientRequestIdPolicy.Shared);

policies.AddRange(perCallPolicies);

policies.AddRange(options.PerCallPolicies);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ namespace Azure.Core.Pipeline
{
internal class ClientRequestIdPolicy : HttpPipelineSynchronousPolicy
{
private const string ClientRequestIdHeader = "x-ms-client-request-id";
private const string EchoClientRequestId = "x-ms-return-client-request-id";
internal const string ClientRequestIdHeader = "x-ms-client-request-id";
internal const string EchoClientRequestId = "x-ms-return-client-request-id";

protected ClientRequestIdPolicy()
{
Expand All @@ -16,8 +16,8 @@ protected ClientRequestIdPolicy()

public override void OnSendingRequest(HttpMessage message)
{
message.Request.Headers.Add(ClientRequestIdHeader, message.Request.ClientRequestId);
message.Request.Headers.Add(EchoClientRequestId, "true");
message.Request.Headers.SetValue(ClientRequestIdHeader, message.Request.ClientRequestId);
message.Request.Headers.SetValue(EchoClientRequestId, "true");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

namespace Azure.Core.Pipeline
{
internal class ReadClientRequestIdPolicy : HttpPipelineSynchronousPolicy
{
protected ReadClientRequestIdPolicy()
{
}

public static ReadClientRequestIdPolicy Shared { get; } = new ReadClientRequestIdPolicy();

public override void OnSendingRequest(HttpMessage message)
{
if (message.Request.Headers.TryGetValue(ClientRequestIdPolicy.ClientRequestIdHeader, out string? value))
{
message.Request.ClientRequestId = value;
}
}
}
}
30 changes: 30 additions & 0 deletions sdk/core/Azure.Core/tests/ClientRequestIdPolicyTests.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Threading;
using System.Threading.Tasks;
using Azure.Core.Pipeline;
using Azure.Core.Testing;
using Moq;
using NUnit.Framework;

namespace Azure.Core.Tests
Expand All @@ -23,5 +26,32 @@ public async Task SetsHeaders()
Assert.AreEqual(request.ClientRequestId, requestId);
Assert.AreEqual("true", returnRequestId);
}

[Test]
public async Task ReadsRequestIdValueOfRequest()
{
var policy = new Mock<HttpPipelineSynchronousPolicy>();
policy.CallBase = true;
policy.Setup(p => p.OnReceivedResponse(It.IsAny<HttpMessage>()))
.Callback<HttpMessage>(message =>
{
Assert.AreEqual("ExternalClientId",message.Request.ClientRequestId);
Assert.True(message.Request.TryGetHeader("x-ms-client-request-id", out string requestId));
Assert.AreEqual("ExternalClientId", requestId);
}).Verifiable();

var transport = new MockTransport(new MockResponse(200));
var pipeline = new HttpPipeline(transport, new[] { ReadClientRequestIdPolicy.Shared, policy.Object });

using (Request request = pipeline.CreateRequest())
{
request.Method = RequestMethod.Get;
request.Uri.Reset(new Uri("http://example.com"));
request.Headers.Add("x-ms-client-request-id", "ExternalClientId");
await pipeline.SendRequestAsync(request, CancellationToken.None);
}

policy.Verify();
}
}
}
8 changes: 8 additions & 0 deletions sdk/core/Azure.Core/tests/HttpClientTransportTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,14 @@ public async Task RequestContentIsNotDisposedOnSend()
Assert.True(disposeTrackingContent.IsDisposed);
}

[Test]
public void ClientRequestIdSetterThrowsOnNull()
{
var transport = new HttpClientTransport();
var request = transport.CreateRequest();
Assert.Throws<ArgumentNullException>(() => request.ClientRequestId = null);
}

public class DisposeTrackingContent : RequestContent
{
public override Task WriteToAsync(Stream stream, CancellationToken cancellation)
Expand Down
61 changes: 61 additions & 0 deletions sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Threading.Tasks;
using Azure.Core.Pipeline;
using Azure.Core.Testing;
using Moq;
using NUnit.Framework;

namespace Azure.Core.Tests
Expand Down Expand Up @@ -77,6 +78,66 @@ public async Task VersionDoesntHaveCommitHash()
Regex.Escape($" ({RuntimeInformation.FrameworkDescription}; {RuntimeInformation.OSDescription})"), userAgent);
}

[Test]
public async Task CustomClientRequestIdAvailableInPerCallPolicies()
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.

Tests policy ordering.

{
var policy = new Mock<HttpPipelineSynchronousPolicy>();
policy.CallBase = true;
policy.Setup(p => p.OnSendingRequest(It.IsAny<HttpMessage>()))
.Callback<HttpMessage>(message =>
{
Assert.AreEqual("ExternalClientId",message.Request.ClientRequestId);
Assert.True(message.Request.TryGetHeader("x-ms-client-request-id", out string requestId));
Assert.AreEqual("ExternalClientId", requestId);
}).Verifiable();

var options = new TestOptions();
options.Transport = new MockTransport(new MockResponse(200));
options.AddPolicy(policy.Object, HttpPipelinePosition.PerCall);

var pipeline = HttpPipelineBuilder.Build(options);
using (Request request = pipeline.CreateRequest())
{
request.Method = RequestMethod.Get;
request.Uri.Reset(new Uri("http://example.com"));
request.Headers.Add("x-ms-client-request-id", "ExternalClientId");
await pipeline.SendRequestAsync(request, CancellationToken.None);
}

policy.Verify();
}

[Test]
public async Task CustomClientRequestIdSetInPerCallPolicyAppliedAsAHeader()
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.

Sorry for causing this problem - but thank you for making the scenario work.

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.

It's a good test I with I had before.

{
var policy = new Mock<HttpPipelineSynchronousPolicy>();
policy.CallBase = true;
policy.Setup(p => p.OnSendingRequest(It.IsAny<HttpMessage>()))
.Callback<HttpMessage>(message =>
{
message.Request.ClientRequestId = "MyPolicyClientId";
}).Verifiable();

var options = new TestOptions();
var transport = new MockTransport(new MockResponse(200));
options.Transport = transport;
options.AddPolicy(policy.Object, HttpPipelinePosition.PerCall);

var pipeline = HttpPipelineBuilder.Build(options);
using (Request request = pipeline.CreateRequest())
{
request.Method = RequestMethod.Get;
request.Uri.Reset(new Uri("http://example.com"));
request.Headers.Add("x-ms-client-request-id", "ExternalClientId");
await pipeline.SendRequestAsync(request, CancellationToken.None);
}

policy.Verify();

Assert.True(transport.SingleRequest.Headers.TryGetValue("x-ms-client-request-id", out var value));
Assert.AreEqual("MyPolicyClientId", value);
}

private class TestOptions : ClientOptions
{
public TestOptions()
Expand Down