ApplicationInsights-dotnet icon indicating copy to clipboard operation
ApplicationInsights-dotnet copied to clipboard

TrackException ignores Exception.Data

Open ishepherd opened this issue 4 years ago • 4 comments

Is your feature request related to a problem?

I decided to use App Insights in parallel with Rollbar.NET. I found that the standard .NET Exception Data dictionary (picked up by Rollbar automatically) is not picked up by App Insights.

It's a standard way to add extra context to exceptions. I feel it should "just work".

Describe the solution you'd like.

  • TelemetryClient.TrackException to discover the Data and seed the ExceptionTelemetry with it.
  • Other properties passed to TrackException could be applied 'over the top'.

Describe alternatives you've considered.

Write custom code for this conversion.

Additional context.

Other examples of this feature request:

  • https://feedback.azure.com/forums/357324-azure-monitor-application-insights/suggestions/34265551-support-exception-data-add
  • Possible same thing for ILogger: https://github.com/microsoft/ApplicationInsights-dotnet/issues/1479

ishepherd avatar Jun 23 '20 12:06 ishepherd

Here is my hacky solution if it interests anyone.

I can only use this hack when I am calling TrackException from my own code. Often frameworks (like Azure Functions) are catching exceptions automatically and logging them, and I get no chance to inject my hack there. So a SDK enhancement would be very helpful.

public static void TrackExceptionsWithData(this TelemetryClient telemetry,
    Exception ex, Dictionary<string, object> properties = null)
{
    var combined = new Dictionary<string, string>(ex.Data.Count + (properties?.Count ?? 0));

    // We want to send Exception Data to App Insights.
    foreach (string key in ex.Data.Keys)
    {
        var value = ex.Data[key];
        var keyStr = "Data." + key;
        if (value is IEnumerable enumerable)
        {
            int i = 0;
            foreach (var item in enumerable)
            {
                combined.Add($"{keyStr}.{i}", item?.ToString());
                i++;
            }
        }
        else
        {
            combined.Add(keyStr, value?.ToString());
        }
    }

    // If there are any extra custom properties to send, add those too.
    if (properties != null)
    {
        foreach (var kv in properties)
        {
            combined[kv.Key] = kv.Value?.ToString();
        }
    }

    telemetry.TrackException(ex, combined);
}

ishepherd avatar Jun 26 '20 02:06 ishepherd

This issue is stale because it has been open 300 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Jan 04 '22 00:01 github-actions[bot]

Could someone from @msftdata look into this issue please? It can't be that difficult to implement and it's really frustrating losing the extra information that can be really helpful to diagnose issues.

simonvane avatar Jan 04 '22 00:01 simonvane

I would love to see Exception.Data as a searchable property in app insights. Seems like common sense to be honest.

coreConvention avatar May 02 '22 23:05 coreConvention

For those wanting a workaround, I created a gist to show how this data can be included for any/all exception telemetry using a telemetry initializer. You would need to customize the behavior of serializing the dictionary value to suit your needs. https://gist.github.com/RyanThomas73/534027e37679a96ffe5688340c62c56a

Adding it to the custom dimensions via this telemetry initializer has a number of drawbacks that would ideally be improved upon by an SDK enhancement.

  1. The telemetry data should be associated with the corresponding exception that it came from. Ideal State: The data would be recorded+displayed as part of the individual ExceptionDetail for the exception it corresponds to. Middleground: The data could be recorded as a custom dimension but a specific dictionary set of data would include the exception detail id that it corresponds to.

  2. Ideally improved performance over the telemetry initialize could be achieved by eliminating the need for an additional traversal of the exception tree. Letting the SDK handle this when it traverses the exception tree here perhaps: https://github.com/microsoft/ApplicationInsights-dotnet/blob/a99b7d646b903df17cb6c8f6814a8bef5bc934d0/BASE/src/Microsoft.ApplicationInsights/DataContracts/ExceptionTelemetry.cs#L386

  3. Purely as an ideal state enhancement, allowing library consumers fine grained control over this behavior: e.g. opt in/opt out support configurable support for the serialization behavior of the dictionary data

RyanThomas73 avatar Dec 15 '22 17:12 RyanThomas73

A telemetry initializer is the right approach here.

pharring avatar Dec 15 '22 19:12 pharring

A telemetry initializer is the right approach here.

Yeah it has the limitations/drawbacks I mentioned above though such as no clear way to link/associate the data with the corresponding exception detail.

RyanThomas73 avatar Dec 15 '22 21:12 RyanThomas73

This issue is stale because it has been open 300 days with no activity. Remove stale label or this will be closed in 7 days. Commenting will instruct the bot to automatically remove the label.

github-actions[bot] avatar Oct 12 '23 00:10 github-actions[bot]

There is still a need for this. The issue is only stale because Microsoft haven't responded. Please could you remove the stale tag and consider the enhancement.

simonvane avatar Oct 12 '23 04:10 simonvane

Use a telemetry initializer. An example was already posted in https://github.com/microsoft/ApplicationInsights-dotnet/issues/1932#issuecomment-1353438915

pharring avatar Oct 12 '23 15:10 pharring

Thanks @pharring. I know how to work around it but that's something we then have to do in every one of thousands of services when it's something that should just work.

simonvane avatar Oct 12 '23 19:10 simonvane