diff --git a/sdk/core/Azure.Core/CHANGELOG.md b/sdk/core/Azure.Core/CHANGELOG.md index 22c9782366b2..f1c7d152f716 100644 --- a/sdk/core/Azure.Core/CHANGELOG.md +++ b/sdk/core/Azure.Core/CHANGELOG.md @@ -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) diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs b/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs index 48a5d587e35e..215040c717fd 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs @@ -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 @@ -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) { diff --git a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs index aa8c2b3df3d5..7828efd17799 100644 --- a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs +++ b/sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs @@ -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); diff --git a/sdk/core/Azure.Core/src/Pipeline/Internal/ClientRequestIdPolicy.cs b/sdk/core/Azure.Core/src/Pipeline/Internal/ClientRequestIdPolicy.cs index e44e3e14fcb3..763418c20240 100644 --- a/sdk/core/Azure.Core/src/Pipeline/Internal/ClientRequestIdPolicy.cs +++ b/sdk/core/Azure.Core/src/Pipeline/Internal/ClientRequestIdPolicy.cs @@ -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() { @@ -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"); } } } diff --git a/sdk/core/Azure.Core/src/Pipeline/Internal/ReadClientRequestIdPolicy.cs b/sdk/core/Azure.Core/src/Pipeline/Internal/ReadClientRequestIdPolicy.cs new file mode 100644 index 000000000000..d0092ad8cfb8 --- /dev/null +++ b/sdk/core/Azure.Core/src/Pipeline/Internal/ReadClientRequestIdPolicy.cs @@ -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; + } + } + } +} \ No newline at end of file diff --git a/sdk/core/Azure.Core/tests/ClientRequestIdPolicyTests.cs b/sdk/core/Azure.Core/tests/ClientRequestIdPolicyTests.cs index d82ca49d1b04..1f360c05af9b 100644 --- a/sdk/core/Azure.Core/tests/ClientRequestIdPolicyTests.cs +++ b/sdk/core/Azure.Core/tests/ClientRequestIdPolicyTests.cs @@ -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 @@ -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(); + policy.CallBase = true; + policy.Setup(p => p.OnReceivedResponse(It.IsAny())) + .Callback(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(); + } } } diff --git a/sdk/core/Azure.Core/tests/HttpClientTransportTests.cs b/sdk/core/Azure.Core/tests/HttpClientTransportTests.cs index 412c0b1b32e5..58fd352d7a0b 100644 --- a/sdk/core/Azure.Core/tests/HttpClientTransportTests.cs +++ b/sdk/core/Azure.Core/tests/HttpClientTransportTests.cs @@ -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(() => request.ClientRequestId = null); + } + public class DisposeTrackingContent : RequestContent { public override Task WriteToAsync(Stream stream, CancellationToken cancellation) diff --git a/sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs b/sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs index 13d50bab3b39..7adcde29522d 100644 --- a/sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs +++ b/sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs @@ -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 @@ -77,6 +78,66 @@ public async Task VersionDoesntHaveCommitHash() Regex.Escape($" ({RuntimeInformation.FrameworkDescription}; {RuntimeInformation.OSDescription})"), userAgent); } + [Test] + public async Task CustomClientRequestIdAvailableInPerCallPolicies() + { + var policy = new Mock(); + policy.CallBase = true; + policy.Setup(p => p.OnSendingRequest(It.IsAny())) + .Callback(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() + { + var policy = new Mock(); + policy.CallBase = true; + policy.Setup(p => p.OnSendingRequest(It.IsAny())) + .Callback(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()