Azure-cosmos-dotnet-v3: Make RequestOptions.Properties public so that we can push info from application into CosmosRequestHandler

[william-liebenberg]

Is your feature request related to a problem? Please describe.
I really want to prevent polluting my code with unnecessary logging and instrumentation code.

I love that there is a CosmosRequestHandler that I can use to intercept the CosmosRequestMessage and execute some code before and after calling SendAsync()

Currently, in the CosmosRequestHandler:SendAsync() method there is no way for me to determine the origin of the request. Meaning I have no idea which part of my application made this request.

All I can get now from the CosmosRequestMessage is the requestUri but it really does not help pinpoint the origin of the request.

I found that all the CRUD methods CreateItemAsync(), ReadItemAsync(), CreateItemQuery<T>() etc. on the CosmosContainer (except GetItemsIterator()) class take a RequestOption (or QueryRequestOptions, ItemRequestOptions etc.) as input. The RequestOptions.Properties field is marked as internal so I can't populate it with my own values.

Describe the solution you'd like

I would be happy with any of the following possibilities:

1. Make the RequestOptions.Properties field public
2. Allow us to add custom properties through the RequestOptions (and others) constructors
3. Add an alternative property bag or RequestContext that I can populate from my application code, and consume/intercept in the CosmosRequestHandler

I cloned the latest SDK and made the changes to it myself to check if it will work... and it does exactly what I want :)

Adding custom properties from my application, I can intercept them through the CosmosRequestHandler and take care of all the cross-cutting concerns. This is so neat and tidy compared to the BAD alternative:

public async Task<Vehicle> GetVehicle(string registration)
{
    var options = new ItemRequestOptions
    {
        // only works if this is public
        Properties = new ConcurrentDictionary<string, object>() { ["origin"] = nameof(GetVehicle) }
    };

    return await _container.ReadItemAsync<Vehicle>(
        partitionKey: "Vehicles", 
        id: registration, 
        requestOptions: options);
}

then in my CosmosRequestHandler I can do this:

public class RequestUnitTracker : CosmosRequestHandler
{
    public double RequestUnitsConsumed { get; set; }
    private readonly ILogger<RequestUnitTracker> _logger;

    public RequestUnitTracker(ILogger<RequestUnitTracker> logger)
    {
        _logger = logger;
    }

    public override async Task<CosmosResponseMessage> SendAsync(CosmosRequestMessage request, CancellationToken cancellationToken)
    {   
        // measure latency
        var watch = Stopwatch.StartNew();
        watch.Start();

        // send the request
        CosmosResponseMessage response = await base.SendAsync(request, cancellationToken);
        watch.Stop();

        long elapsed = watch.ElapsedMilliseconds;
        response.Headers.Add("elapsed-time-ms", $"{elapsed}");

        // track the total RU's that were consumed
        RequestUnitsConsumed += response.Headers.RequestCharge;

        // track origin
        request.Properties.TryGetValue("origin", out object origin);

        // logs... don't forget the logs!
        _logger.LogInformation("{timeStamp} - {statusCode} - {method} - {requestUri} - {origin} - Charge: {requestCharge} Latency: {latency}",
            DateTimeOffset.UtcNow,
            response.StatusCode, 
            request.Method, 
            request.RequestUri, // <-- THIS IS NOT SPECIFIC ENOUGH
            origin, // <-- I NEED THIS
            response.Headers.RequestCharge, 
            $"{elapsed}ms");

        return response;
    }
}

Describe alternatives you've considered

To get around this issue in my application I have to handle each and every instance where I make a request with the Cosmos SDK and manually log the method name/class name where the request originates from. Pollution levels are high :)

public async Task<Vehicle> GetVehicle(string registration)
{
    var watch = var watch = Stopwatch.StartNew();
    ItemResponse<Vehicle> response = await _container.ReadItemAsync<Vehicle>("Vehicles", registration);

    // this has to be called for each and every response I get back from Cosmos. Easily forget to add it and miss tracking the call. Also, now its too late to do anything I want in the <see cref="CosmosRequestHandler"/>
    await LogResponse(nameof(GetVehicle), response, watch.ElapsedMilliseconds);

    return response;
}

I've tried a few different ways to deal with these cross-cutting concerns but they all end up with far too much code and it just looks messy after a while.

Additional context
If there is another way to achieve what I'm trying to do I would be happy to try that instead.

Thanks

[j82w]

Would a single string property like "UserClientRequestId" be a better solution?

• Property will not be set in the headers
• Avoid creating dictionary for each request
• Avoid the dictionary insert and lookup performance hit

Is there a scenario where multiple values need to be stored?

[william-liebenberg]

Yes! that is a great suggestion. It will definitely solve the issue.

In my case right now I don't need multiple values... but if I ever did need multiple values, I could re-use this UserClientRequestId value to form a correlation between data that I store myself in my app and the request that gets handled in the CosmosRequestHandler.

:)

[william-liebenberg]

Just to check, this UserClientRequestId would be a public property on the RequestOptions class?

[j82w]

Yes, the UserClientRequestId would be public.

[j82w]

Another way you can do this currently is extend the request option type, and add your custom field to it.

[william-liebenberg]

OK that could work. I just need to then cast the 'RequestOptions' to my custom type and use it...

This solution will be fine for now but I would prefer your original suggestion of the 'UserClientRequestId' field on the base 'RequestOptions' more. It means I won't need to extend multiple classes like 'ItemRequestOptions', 'QueryRequestOptions', etc. And cast/check types in the RequestHandler.

[william-liebenberg]

Hi j82w,

I had a quick attempt to get your recent suggestion working, but it's not possible at this stage either.

If I extend the ItemRequestOptions class (or others) to add my own UserClientRequestId field, I won't be able to retrieve it from the RequestMessage given to the handler because the RequestMessage.RequestOptions field also marked as internal.

So the question now is, can we have the RequestMessage.RequestOptions field as a public?

[stzhmsfttest]

I would like this feature as well.
We want to have the usage of custom handlers to be able to integrate with our web api flow as well which it will be convenient to pass custom parameters in Properties for handlers to deal with.

[william-liebenberg]

Hi all,

Is there a possibility to review and discuss this request?

I keep coming up against the issue of not being able to accurately trace requests and responses for my apps.

Are the proposed solutions above still valid in your opinion or is there another approach that could achieve the same outcome with the current SDK?

Thanks

[j82w]

@william-liebenberg it's still a possibility. Do you need the UserClientRequestId to be accessible or would it being in the Diagnostic string be enough? The diagnostic string isn't meant to be parsed.

[william-liebenberg]

Yes, just being accessible would be perfect. Ideally I wouldn't want to parse through the diagnostics for every request.