getsentry / sentry-unity

Development of Sentry SDK for Unity
https://docs.sentry.io/platforms/unity/
MIT License
206 stars 51 forks source link

Spans for HTTP requests #737

Open bruno-garcia opened 2 years ago

bruno-garcia commented 2 years ago

When a user makes an HTTP request to their backend, we want to measure how long that took. And propagate the trace id so we can have end to end tracing.

This can be split in:

HttpMessageHandler

The .NET approach. Used in Sentry Defenses: https://github.com/getsentry/sentry-defenses/blob/2989949f857f1cccb46814ce0a05d50f603f9222/game/Assets/Scripts/Manager/BugSpawner.cs#L37

We need to find a way add the handler automatically. At least on the one assembly created by Unity when compiling the scripts.

Steps:

UnityWebRequest (Unity Client)

Example HTTP client usage: https://github.com/getsentry/sentry-unity/blob/7774137c35ce4a3653b934e15ed15f0078a2ee36/src/Sentry.Unity/UnityWebRequestTransport.cs#L75-L81 We need a way to plug Sentry in there. First 'manually' with some C# code. Then automatically, possibly with IL weaving.

SimonCropp commented 2 years ago

i took a look into implementing the custom uploadHandler and downloadHandler and hit a wall.

The IL replacement plan is to detect usage of existing handlers and wrap them in our own. but that wrapping does not seem possible. The reason is that the the Dispose in the base classes is not virtual. So if we wrap an exiting instance, we override Dispose on our implementation so as to then call Dispose on the inner handler.

I considered re-implementing each of the DownloadHandler implementations that ship with Unity. Unfortunately they all use internal methods in the base class. This might be possible with some reflection. Note it would only work for the known subset of handlers that ship with unity

we cant inherit and override Unitys default handlers since they are all sealed

bruno-garcia commented 2 years ago

Our goal is to wrap that HTTP request into a span/crumb creation logic, like we do here: https://github.com/getsentry/sentry-dotnet/blob/36ccacf43b8aeedc493585ea57bc05880b285773/src/Sentry/SentryHttpMessageHandler.cs#L52-L96

Like if the HTTP request code would be here: https://github.com/getsentry/sentry-dotnet/blob/36ccacf43b8aeedc493585ea57bc05880b285773/src/Sentry/SentryHttpMessageHandler.cs#L76

So during IL weaving we would find:

var www = new UnityWebRequest 
{ 
    url = message.RequestUri.ToString(), 
    method = message.Method.Method.ToUpperInvariant(), 
    uploadHandler = new UploadHandlerRaw(contentMemoryStream.ToArray()), 
    downloadHandler = new DownloadHandlerBuffer() 
}; 

// REST OF CODE

Then move the two parameters up to an assignment, so we can easily copy its values, add span:

var myUrlValue = message.RequestUri.ToString(), 
var myMethodValue = message.Method.Method.ToUpperInvariant(), 

var span = _hub.GetSpan()?.StartChild(
                "http.client",
                // e.g. "GET https://example.com/"
                $"{requestMethod} {url}");
try
{
    var www = new UnityWebRequest 
    { 
        url = myUrlValue.
        method = myMethodValue,
        uploadHandler = new UploadHandlerRaw(contentMemoryStream.ToArray()), 
        downloadHandler = new DownloadHandlerBuffer() 
    }; 

    // REST OF CODE

    var breadcrumbData = new Dictionary<string, string>
    {
        { "url", myUrlValue },
        { "method", myMethodValue },
        { "status_code", ((int)response.StatusCode).ToString() }
    };
    _hub.AddBreadcrumb(string.Empty, "http", "http", breadcrumbData);

    // This will handle unsuccessful status codes as well
    span?.Finish(SpanStatusConverter.FromHttpStatusCode(response.StatusCode));

    return response;
}
catch (Exception ex)
{
    span?.Finish(ex);
    throw;
}