Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
8 changes: 6 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,11 @@ 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 => _clientRequestId = value;
}
Comment thread
pakrym marked this conversation as resolved.

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

bool isDistributedTracingEnabled = options.Diagnostics.IsDistributedTracingEnabled;

policies.Add(ClientRequestIdPolicy.Shared);

policies.AddRange(perCallPolicies);

policies.AddRange(options.PerCallPolicies);

policies.Add(ClientRequestIdPolicy.Shared);

DiagnosticsOptions diagnostics = options.Diagnostics;
Comment thread
pakrym marked this conversation as resolved.
if (diagnostics.IsTelemetryEnabled)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,14 @@ protected ClientRequestIdPolicy()

public override void OnSendingRequest(HttpMessage message)
{
message.Request.Headers.Add(ClientRequestIdHeader, message.Request.ClientRequestId);
if (message.Request.Headers.TryGetValue(ClientRequestIdHeader, out string? value))
{
message.Request.ClientRequestId = value;
}
else
{
message.Request.Headers.Add(ClientRequestIdHeader, message.Request.ClientRequestId);
}
message.Request.Headers.Add(EchoClientRequestId, "true");
}
}
Expand Down
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[] { ClientRequestIdPolicy.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();
}
}
}
30 changes: 30 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,35 @@ 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.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 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();
}

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