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/api/Azure.Core.net10.0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ protected ClientOptions(Azure.Core.DiagnosticsOptions? diagnostics) { }
protected ClientOptions(Microsoft.Extensions.Configuration.IConfigurationSection section, Azure.Core.DiagnosticsOptions? diagnostics) { }
public static Azure.Core.ClientOptions Default { get { throw null; } }
public Azure.Core.DiagnosticsOptions Diagnostics { get { throw null; } }
protected int MaxApplicationIdLength { get { throw null; } set { } }
public Azure.Core.RetryOptions Retry { get { throw null; } }
public Azure.Core.Pipeline.HttpPipelinePolicy? RetryPolicy { get { throw null; } set { } }
public Azure.Core.Pipeline.HttpPipelineTransport Transport { get { throw null; } set { } }
Expand Down
1 change: 1 addition & 0 deletions sdk/core/Azure.Core/api/Azure.Core.net462.cs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ protected ClientOptions(Azure.Core.DiagnosticsOptions? diagnostics) { }
protected ClientOptions(Microsoft.Extensions.Configuration.IConfigurationSection section, Azure.Core.DiagnosticsOptions? diagnostics) { }
public static Azure.Core.ClientOptions Default { get { throw null; } }
public Azure.Core.DiagnosticsOptions Diagnostics { get { throw null; } }
protected int MaxApplicationIdLength { get { throw null; } set { } }
public Azure.Core.RetryOptions Retry { get { throw null; } }
public Azure.Core.Pipeline.HttpPipelinePolicy? RetryPolicy { get { throw null; } set { } }
public Azure.Core.Pipeline.HttpPipelineTransport Transport { get { throw null; } set { } }
Expand Down
1 change: 1 addition & 0 deletions sdk/core/Azure.Core/api/Azure.Core.net472.cs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ protected ClientOptions(Azure.Core.DiagnosticsOptions? diagnostics) { }
protected ClientOptions(Microsoft.Extensions.Configuration.IConfigurationSection section, Azure.Core.DiagnosticsOptions? diagnostics) { }
public static Azure.Core.ClientOptions Default { get { throw null; } }
public Azure.Core.DiagnosticsOptions Diagnostics { get { throw null; } }
protected int MaxApplicationIdLength { get { throw null; } set { } }
public Azure.Core.RetryOptions Retry { get { throw null; } }
public Azure.Core.Pipeline.HttpPipelinePolicy? RetryPolicy { get { throw null; } set { } }
public Azure.Core.Pipeline.HttpPipelineTransport Transport { get { throw null; } set { } }
Expand Down
1 change: 1 addition & 0 deletions sdk/core/Azure.Core/api/Azure.Core.net8.0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ protected ClientOptions(Azure.Core.DiagnosticsOptions? diagnostics) { }
protected ClientOptions(Microsoft.Extensions.Configuration.IConfigurationSection section, Azure.Core.DiagnosticsOptions? diagnostics) { }
public static Azure.Core.ClientOptions Default { get { throw null; } }
public Azure.Core.DiagnosticsOptions Diagnostics { get { throw null; } }
protected int MaxApplicationIdLength { get { throw null; } set { } }
public Azure.Core.RetryOptions Retry { get { throw null; } }
public Azure.Core.Pipeline.HttpPipelinePolicy? RetryPolicy { get { throw null; } set { } }
public Azure.Core.Pipeline.HttpPipelineTransport Transport { get { throw null; } set { } }
Expand Down
1 change: 1 addition & 0 deletions sdk/core/Azure.Core/api/Azure.Core.netstandard2.0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ protected ClientOptions(Azure.Core.DiagnosticsOptions? diagnostics) { }
protected ClientOptions(Microsoft.Extensions.Configuration.IConfigurationSection section, Azure.Core.DiagnosticsOptions? diagnostics) { }
public static Azure.Core.ClientOptions Default { get { throw null; } }
public Azure.Core.DiagnosticsOptions Diagnostics { get { throw null; } }
protected int MaxApplicationIdLength { get { throw null; } set { } }
public Azure.Core.RetryOptions Retry { get { throw null; } }
public Azure.Core.Pipeline.HttpPipelinePolicy? RetryPolicy { get { throw null; } set { } }
public Azure.Core.Pipeline.HttpPipelineTransport Transport { get { throw null; } set { } }
Expand Down
14 changes: 14 additions & 0 deletions sdk/core/Azure.Core/src/ClientOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,20 @@ public HttpPipelineTransport Transport
/// </summary>
public DiagnosticsOptions Diagnostics { get; }

/// <summary>
/// Gets or sets the maximum allowed length for <see cref="DiagnosticsOptions.ApplicationId"/>.
/// </summary>
/// <remarks>
/// The default value is 24 characters. SDK authors can increase this
/// in their derived <see cref="ClientOptions"/> constructor to accommodate longer application identifiers.
/// </remarks>
/// <exception cref="ArgumentOutOfRangeException">Thrown when the value is less than 24.</exception>
protected int MaxApplicationIdLength
{
get => Diagnostics.MaxApplicationIdLength;
set => Diagnostics.MaxApplicationIdLength = value;
}

/// <summary>
/// Gets the client retry options.
/// </summary>
Expand Down
34 changes: 26 additions & 8 deletions sdk/core/Azure.Core/src/DiagnosticsOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ namespace Azure.Core
/// </summary>
public class DiagnosticsOptions
{
private const int MaxApplicationIdLength = 24;

private string? _applicationId;
private int _maxApplicationIdLength = DefaultMaxApplicationIdLength;
private const int DefaultMaxApplicationIdLength = 24;

/// <summary>
/// Creates a new instance of <see cref="DiagnosticsOptions"/> with default values.
Expand Down Expand Up @@ -102,6 +102,7 @@ internal DiagnosticsOptions(DiagnosticsOptions? diagnosticsOptions)
{
if (diagnosticsOptions != null)
{
_maxApplicationIdLength = diagnosticsOptions.MaxApplicationIdLength;
ApplicationId = diagnosticsOptions.ApplicationId;
IsLoggingEnabled = diagnosticsOptions.IsLoggingEnabled;
IsTelemetryEnabled = diagnosticsOptions.IsTelemetryEnabled;
Expand Down Expand Up @@ -198,21 +199,38 @@ private static IList<string> GetDefaultLoggedHeaders()
public IList<string> LoggedQueryParameters { get; internal set; }

/// <summary>
/// Gets or sets the value sent as the first part of "User-Agent" headers for all requests issues by this client. Defaults to <see cref="DefaultApplicationId"/>.
/// Gets or sets the maximum allowed length for <see cref="ApplicationId"/>.
/// </summary>
public string? ApplicationId
/// <remarks>
/// The default value is 24 characters. This can be increased to accommodate longer
/// application identifiers. Values less than 24 are not permitted.
/// </remarks>
/// <exception cref="ArgumentOutOfRangeException">Thrown when the value is less than 24.</exception>
internal int MaxApplicationIdLength
{
get => _applicationId;
get => _maxApplicationIdLength;
set
{
if (value != null && value.Length > MaxApplicationIdLength)
if (value < DefaultMaxApplicationIdLength)
{
throw new ArgumentOutOfRangeException(nameof(value), $"{nameof(ApplicationId)} must be shorter than {MaxApplicationIdLength + 1} characters");
throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(MaxApplicationIdLength)} must be at least {DefaultMaxApplicationIdLength}.");
}
_applicationId = value;
_maxApplicationIdLength = value;
}
}

/// <summary>
/// Gets or sets the value sent as the first part of "User-Agent" headers for all requests issues by this client. Defaults to <see cref="DefaultApplicationId"/>.
/// </summary>
/// <remarks>
/// The length of <see cref="ApplicationId"/> is validated against <see cref="MaxApplicationIdLength"/> when the HTTP pipeline is built.
/// </remarks>
public string? ApplicationId
{
get => _applicationId;
set => _applicationId = value;
}

/// <summary>
/// Gets or sets the default application id. Default application id would be set on all instances.
/// </summary>
Expand Down
6 changes: 5 additions & 1 deletion sdk/core/Azure.Core/src/Pipeline/HttpPipelineBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,11 @@ void AddNonNullPolicies(HttpPipelinePolicy[] policiesToAdd)
internal static TelemetryPolicy CreateTelemetryPolicy(ClientOptions options)
{
var type = options.GetType();
var userAgentValue = new TelemetryDetails(type.Assembly, options.Diagnostics.ApplicationId);
var userAgentValue = new TelemetryDetails(
type.Assembly,
options.Diagnostics.ApplicationId,
runtimeInformation: null,
maxApplicationIdLength: options.Diagnostics.MaxApplicationIdLength);
return new TelemetryPolicy(userAgentValue);
}
}
Expand Down
10 changes: 5 additions & 5 deletions sdk/core/Azure.Core/src/TelemetryDetails.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace Azure.Core
/// </summary>
public class TelemetryDetails
{
private const int MaxApplicationIdLength = 24;
private const int DefaultMaxApplicationIdLength = 24;
private readonly string _userAgent;

/// <summary>
Expand All @@ -35,15 +35,15 @@ public class TelemetryDetails
/// <param name="applicationId">An optional value to be prepended to the <see cref="TelemetryDetails"/>.
/// This value overrides the behavior of the <see cref="DiagnosticsOptions.ApplicationId"/> property for the <see cref="HttpMessage"/> it is applied to.</param>
public TelemetryDetails(Assembly assembly, string? applicationId = null)
: this(assembly, applicationId, new RuntimeInformationWrapper())
: this(assembly, applicationId, new RuntimeInformationWrapper(), maxApplicationIdLength: DefaultMaxApplicationIdLength)
{ }

internal TelemetryDetails(Assembly assembly, string? applicationId = null, RuntimeInformationWrapper? runtimeInformation = default)
internal TelemetryDetails(Assembly assembly, string? applicationId = null, RuntimeInformationWrapper? runtimeInformation = default, int maxApplicationIdLength = DefaultMaxApplicationIdLength)
{
Argument.AssertNotNull(assembly, nameof(assembly));
if (applicationId?.Length > MaxApplicationIdLength)
if (applicationId?.Length > maxApplicationIdLength)
{
throw new ArgumentOutOfRangeException(nameof(applicationId), $"{nameof(applicationId)} must be shorter than {MaxApplicationIdLength + 1} characters");
throw new ArgumentOutOfRangeException(nameof(applicationId), $"{nameof(applicationId)} must be shorter than {maxApplicationIdLength + 1} characters. To allow longer values, the SDK library author can increase MaxApplicationIdLength in their ClientOptions-derived constructor.");
}

Assembly = assembly;
Expand Down
47 changes: 47 additions & 0 deletions sdk/core/Azure.Core/tests/ClientOptionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,57 @@ public void AcceptsCustomDiagnosticsOptions([Values(true, false)] bool useCustom
}
}

[Test]
public void DefaultMaxApplicationIdLengthIs24()
{
var options = new TestClientOptionsWithCustomMaxApplicationIdLength();
Assert.AreEqual(24, options.GetMaxApplicationIdLength());
}
Comment thread
m-redding marked this conversation as resolved.

[Test]
public void ApplicationIdExceedingDefaultMaxLengthIsAcceptedByOptions()
{
var options = new TestClientOptions();
var longApplicationId = new string('a', 25);

options.Diagnostics.ApplicationId = longApplicationId;

Assert.AreEqual(longApplicationId, options.Diagnostics.ApplicationId);
}

[TestCase(23)]
[TestCase(0)]
[TestCase(-1)]
public void SettingMaxApplicationIdLengthBelowDefaultThrows(int maxLength)
{
var options = new TestClientOptions();

var ex = Assert.Throws<ArgumentOutOfRangeException>(() => options.Diagnostics.MaxApplicationIdLength = maxLength);
Assert.That(ex.Message, Does.Contain("MaxApplicationIdLength must be at least 24"));
}

[Test]
public void SettingMaxApplicationIdLengthToLargeValueSucceeds()
{
var options = new TestClientOptionsWithCustomMaxApplicationIdLength(1000);

Assert.AreEqual(1000, options.GetMaxApplicationIdLength());
}

private class TestClientOptions : ClientOptions
{
}

private class TestClientOptionsWithCustomMaxApplicationIdLength : ClientOptions
{
public TestClientOptionsWithCustomMaxApplicationIdLength(int maxApplicationIdLength = 24)
{
MaxApplicationIdLength = maxApplicationIdLength;
}

public int GetMaxApplicationIdLength() => MaxApplicationIdLength;
}

private class TestDiagnosticsOptions : DiagnosticsOptions
{
public int MeExtraProperty { get; set; }
Expand Down
47 changes: 47 additions & 0 deletions sdk/core/Azure.Core/tests/HttpPipelineBuilderTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,44 @@ public async Task VersionDoesntHaveCommitHash()
Regex.Escape($" ({RuntimeInformation.FrameworkDescription}; {RuntimeInformation.OSDescription})"), userAgent);
}

[Test]
public async Task CreateTelemetryPolicyHonorsCustomMaxApplicationIdLength()
{
var longApplicationId = new string('a', 50);
var options = new TestOptionsWithCustomMaxApplicationIdLength(50);
options.Diagnostics.ApplicationId = longApplicationId;

var transport = new MockTransport(new MockResponse(200));
var telemetryPolicy = HttpPipelineBuilder.CreateTelemetryPolicy(options);

await SendGetRequest(transport, telemetryPolicy);

Assert.True(transport.SingleRequest.TryGetHeader("User-Agent", out var userAgent));
StringAssert.StartsWith(longApplicationId + " ", userAgent);
}

[Test]
public void ApplicationIdExceedingDefaultMaxLengthThrowsAtPipelineBuild()
{
var options = new TestOptions();
options.Diagnostics.ApplicationId = new string('a', 25);

var ex = Assert.Throws<ArgumentOutOfRangeException>(() => HttpPipelineBuilder.Build(options));
Assert.That(ex.Message, Does.Contain("applicationId must be shorter than 25 characters"));
Assert.That(ex.Message, Does.Contain("MaxApplicationIdLength"));
}

[Test]
public void ApplicationIdExceedingCustomMaxLengthThrowsAtPipelineBuild()
{
var options = new TestOptionsWithCustomMaxApplicationIdLength(50);
options.Diagnostics.ApplicationId = new string('a', 51);

var ex = Assert.Throws<ArgumentOutOfRangeException>(() => HttpPipelineBuilder.Build(options));
Assert.That(ex.Message, Does.Contain("applicationId must be shorter than 51 characters"));
Assert.That(ex.Message, Does.Contain("MaxApplicationIdLength"));
}

[Test]
public async Task CustomClientRequestIdAvailableInPerCallPolicies()
{
Expand Down Expand Up @@ -338,6 +376,15 @@ public TestOptions()
}
}

private class TestOptionsWithCustomMaxApplicationIdLength : ClientOptions
{
public TestOptionsWithCustomMaxApplicationIdLength(int maxApplicationIdLength)
{
MaxApplicationIdLength = maxApplicationIdLength;
Retry.Delay = TimeSpan.Zero;
}
}

private class CounterPolicy : HttpPipelineSynchronousPolicy
{
public override void OnSendingRequest(HttpMessage message)
Expand Down
38 changes: 38 additions & 0 deletions sdk/core/Azure.Core/tests/TelemetryDetailsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,44 @@ public void CtorPopulatesProperties([Values(null, appId)] string applicationId)
Assert.AreEqual(applicationId, target.ApplicationId);
}

[Test]
public void ApplicationIdExceedingDefaultMaxLengthThrows()
{
var longApplicationId = new string('a', 25);

var ex = Assert.Throws<ArgumentOutOfRangeException>(() => new TelemetryDetails(typeof(TelemetryDetailsTests).Assembly, longApplicationId));
Assert.That(ex.Message, Does.Contain("applicationId must be shorter than 25 characters"));
}

[Test]
public void ApplicationIdAtDefaultMaxLengthSucceeds()
{
var applicationId = new string('a', 24);

var target = new TelemetryDetails(typeof(TelemetryDetailsTests).Assembly, applicationId);

Assert.AreEqual(applicationId, target.ApplicationId);
}

[Test]
public void CustomMaxApplicationIdLengthAllowsLongerApplicationId()
{
var longApplicationId = new string('a', 50);

var target = new TelemetryDetails(typeof(TelemetryDetailsTests).Assembly, longApplicationId, maxApplicationIdLength: 50);

Assert.AreEqual(longApplicationId, target.ApplicationId);
}

[Test]
public void CustomMaxApplicationIdLengthStillEnforcesLimit()
{
var tooLongApplicationId = new string('a', 51);

var ex = Assert.Throws<ArgumentOutOfRangeException>(() => new TelemetryDetails(typeof(TelemetryDetailsTests).Assembly, tooLongApplicationId, maxApplicationIdLength: 50));
Assert.That(ex.Message, Does.Contain("applicationId must be shorter than 51 characters"));
}

[Test]
public void AppliesToMessage()
{
Expand Down
7 changes: 0 additions & 7 deletions sdk/core/Azure.Core/tests/TelemetryPolicyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,6 @@ public async Task ApplicationIdIsIncluded()
StringAssert.StartsWith("application-id ", userAgent);
}

[Test]
public void ApplicationIdLimitedTo24Chars()
{
var options = new DiagnosticsOptions();
Assert.Throws<ArgumentOutOfRangeException>(() => options.ApplicationId = "0123456789012345678912345");
}

private class TestOptions : ClientOptions
{ }
}
Expand Down
Loading