[Draft] Minimize noisy Storage exceptions via DPG methods

[tg-msft]

This shows how we can reduce logging noise for expected failures by using DPG methods from HLCs. #25626 is tracking the meta issue, but this is a proof of concept for both internal only (like BlobClient.Exists for #18592) and externally visible (like BlobLeaseClient.Acquire for #21511) cases.

This is currently blocked on #27614 before we can ship it.

There are also a few tests in DataLake like FileClientTest.ExistsAsync_CPK complaining about not matching the recordings, but it's difficult to debug at the moment because of a known issue in the test proxy. I'm ignoring those for the moment.

[annelo-msft]

This will suppress an exception from being thrown from within GetProperties, but error logging and tracing will still occur in the pipeline. In order to suppress those as well, you'll need to add classifiers using context.AddClassifier(...)

[tg-msft]

Sigh... That's what I get for rushing and only checking the Storage tests. I updated and verified that distributed tracing is clean too.

Current:

Fixed:

#define Fixed

using Azure;
using Azure.Core;
using Azure.Monitor.OpenTelemetry.Exporter;
using Azure.Storage.Blobs;
using Azure.Storage.Blobs.Models;
using Azure.Storage.Blobs.Specialized;
using OpenTelemetry.Trace;
using System.Diagnostics;

// Settings
const string StorageConnStr = "...";
const string AppInsightsConnStr = "...";
const string Name =
#if !Fixed
    "Current";
#else
    "Fixed";
#endif

// Collect telemetry in Azure Monitor
AppContext.SetSwitch("Azure.Experimental.EnableActivitySource", true);
using TracerProvider telemetry =
    OpenTelemetry.Sdk
    .CreateTracerProviderBuilder()
    .AddSource(Name, "Azure.*")
    .AddAzureMonitorTraceExporter(options => options.ConnectionString = AppInsightsConnStr)
    .Build();
using ActivitySource source = new(Name);
using Activity? group = source.StartActivity(Name)!;


// Check if a fake blob exists
BlobClient fake = new BlobClient(StorageConnStr, "fakecontainer", "fakeblob");
Console.WriteLine($"Exists: {fake.Exists().Value}");


// Try acquiring a lease from a fake blob
#if !Fixed

try
{
    BlobLease lease = fake.GetBlobLeaseClient().Acquire(TimeSpan.FromSeconds(15));
    Console.WriteLine($"Lease: {lease.LeaseId}");
}
catch (RequestFailedException ex)
{
    Console.WriteLine($"Failed to obtain lease!\n{ex}");
}

#else

RequestContext context =
    new RequestContext() { ErrorOptions = ErrorOptions.NoThrow }
    .AddClassifier(404, BlobErrorCode.BlobNotFound.ToString(), false)
    .AddClassifier(404, BlobErrorCode.ContainerNotFound.ToString(), false)
    .AddClassifier(409, BlobErrorCode.LeaseAlreadyPresent.ToString(), false);
Response<BlobLease> response =
    fake.GetBlobLeaseClient().Acquire(
        TimeSpan.FromSeconds(15),
        conditions: null,
        context);
Response raw = response.GetRawResponse();
if (raw.Status == 201)
{
    Console.WriteLine($"Lease: {response.Value.LeaseId}");
}
else
{
    Console.WriteLine($"Failed to obtain lease!\n{raw.Content.ToString()}");
}

#endif

[kasobol-msft]

How do we determine that we want to add RequestContext to HLC methods vs using it internally.
I.e. why this instead of AcquireIfNotYetAcuired API that uses RequestContext internally?

[tg-msft]

Because it's not really doing AcquireIfNotYetAcquired. It's doing AcquireIfBlobNotFoundOrContainerNotFoundOrLeaseAlreadyAcquired and we don't want to add an overload for that specific, non-general scenario.

[annelo-msft]

If BlockBlobClient is using ClientDiagnostics internally to create a RequestFailedException, adding these classifiers should be enough to suppress any unwanted errors while preserving correct exception creation.

[ghost]

Hi @tg-msft. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

[tg-msft]

Be gone from here, bot.

[ghost]

Hi @tg-msft. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

[ghost]

Hi @tg-msft. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing "/reopen" if you'd like to continue working on these changes. Please be sure to use the command to reopen or remove the "no-recent-activity" label; otherwise, this is likely to be closed again with the next cleanup pass.

[azure-sdk]

API change check

APIView has identified API level changes in this PR and created following API reviews.

Azure.Storage.Blobs

[ghost]

Hi @tg-msft. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

[ghost]

Hi @tg-msft. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing "/reopen" if you'd like to continue working on these changes. Please be sure to use the command to reopen or remove the "no-recent-activity" label; otherwise, this is likely to be closed again with the next cleanup pass.