search

reactiveui / rfcs

RFCs for changes to ReactiveUI
https://reactiveui.net/rfcs
5 stars 5 forks source link

Application Performance Monitoring Integration #23

Open dpvreony opened 5 years ago

dpvreony commented 5 years ago

Application Performance Monitoring Integration

Summary

To offer Application Performance Monitoring integration similar to how the Logging integration works.

Motivation

Scenarios As a developer I want to know how often a command\event subscription is used. The duration and success rate As a developer I want to know of failures As a developer I want to be able to group events by feature/subfeature (with subfeature recursion)

I appreciate the examples are focused at ReactiveCommand. But you could define a feature as being a view\viewmodel. Part of this belongs at the splat level as the APM toolset could be a direct target of the ILogger, depending on the features offered by the APM library.

Detailed design

When I played with the concept I used a disposable class so start event happened at construction. End event happened as dispose. I could call the error method as required (Ie exception). Alternatively I could just use generic helper methods and never touch the IDisposable wrapper.

The idea would be either to have a feature usage tracking aware subclass to reactive object or allow an IEnableFeatureUsageTracking extension interface. You could then hook up a reactive command that is aware of the parent feature and allows specifying the subfeature details.

Possible consumers Visual Studio App Center (with appreciation wpf\winforms etc still trailing on support) New relic Application insights Exceptionless

There was an interface for the target framework to have flexibility on how it works

    public interface IFeatureUsageTrackingSession<out TReferenceType> : IFeatureUsageTrackingSession
    {
        TReferenceType FeatureReference { get; }

        TReferenceType ParentReference { get; }

        string FeatureName { get; }
    }

There were interfaces so ReactiveUI and other consumers wouldn't get bogged down in the design details of the target framework

    public interface IFeatureUsageTrackingSession : IDisposable
    {
        IFeatureUsageTrackingSession SubFeature(string description);

        void OnException(Exception exception);
    }

    public interface IEnableFeatureUsageTracking
    {
    }

A sample implementation for App Insights

namespace Splat.ApplicationInsights
{
    using System;
    using Microsoft.ApplicationInsights;
    using Microsoft.ApplicationInsights.DataContracts;

    public class ApplicationInsightsFeatureUsageTracking : IFeatureUsageTrackingSession<Guid>
    {
        private readonly TelemetryClient _telemetry;

        public ApplicationInsightsFeatureUsageTracking(string featureName) : this(featureName, Guid.Empty)
        {
        }

        internal ApplicationInsightsFeatureUsageTracking(string featureName, Guid parentReference)
        {
            this.FeatureName = featureName;
            this.FeatureReference = Guid.NewGuid();
            this.ParentReference = parentReference;
            this._telemetry = Locator.Current.GetService<TelemetryClient>();

            TrackEvent("Feature Usage Start");
        }

        public Guid FeatureReference { get; }

        public Guid ParentReference { get; }

        public string FeatureName { get; }

        public void Dispose()
        {
            TrackEvent("Feature Usage End");
        }

        public IFeatureUsageTrackingSession SubFeature(string description)
        {
            return new ApplicationInsightsFeatureUsageTracking(description, this.ParentReference);
        }

        public void OnException(Exception exception)
        {
            var telemetry = new ExceptionTelemetry(exception);
            PrepareEventData(telemetry);

            this._telemetry.TrackException(telemetry);
        }

        private void TrackEvent(string eventName)
        {
            var eventTelemetry = new EventTelemetry(eventName);
            PrepareEventData(eventTelemetry);

            this._telemetry.TrackEvent(eventTelemetry);
        }

        private void PrepareEventData<TTelemetry>(TTelemetry eventTelemetry) where TTelemetry : ISupportProperties
        {
            eventTelemetry.Properties.Add("Name", FeatureName);
            eventTelemetry.Properties.Add("Reference", FeatureReference.ToString());

            if (ParentReference != Guid.Empty)
            {
                eventTelemetry.Properties.Add("ParentReference", ParentReference.ToString());
            }
        }
    }
}

How we teach this

In terms of Splat have some wrappers that show simple usage. Doesn't necessarily need the IDisposable pattern.

        public static async Task SubFeatureAsync(
            this IFeatureUsageTrackingSession featureUsageTracking,
            string featureName,
            Task action)
        {
            using (var subFeature = featureUsageTracking.SubFeature(featureName))
            {
                try
                {
                    await action;
                }
                catch (Exception ex)
                {
                    subFeature.OnException(ex);

                    //re-throw so parent logic can deal with
                    throw;
                }
            }
        }

ReactiveUI integration into the ReactiveCommand itself. Or show how to use the Splat helpers inside the executing subscription. Draw back to this is multiple try\catch.

Aside from Feature Usage Tracking. There is also the error\crash reporting piece, we could report UnhandledExceptions at the RxApp level. You then have a training piece available that as part of testing and development people can use an APM tool to see the list of Exceptions getting down to the core.

Feature: The top level ViewModel. SubFeature: ReactiveCommand, Event, Possibly even the IViewFor<>

It can be sold as an opt-in feature to new and existing users. Same way the logging works.

In terms of ReactiveCommands. do you want an optional string argument for the feature name that dictates if a command even uses this?

Drawbacks

source: https://docs.microsoft.com/en-us/appcenter/analytics/event-metrics source: https://docs.microsoft.com/en-us/appcenter/sdk/analytics/uwp

Might need something like the code below. But dictating to users how to Feature Usage Tracking in a pre-defined format?

Analytics.TrackEvent("FeatureUsage", new Dictionary<string, string> {
    { "Name", featureName },
    { Reference, FeatureReference},
    { ParentReference, ParentReference},
});

Alternatives

Offer interfaces but not the end integrations and patterns of implementations. Or have ways to overload.

Unresolved questions

Is it actually useful \ desirable? What level of integration for Splat\ReactiveUI? How to avoid overcomplicating ReactiveObject and making this opt in? or just treat the inheriting classname as the default featurename? if so, use weaving etc rather than reflection? People need to be aware of the costings of the APM tool they choose. This is around giving the facilities to make decisions based on what users are experiencing.

dpvreony commented 5 years ago

based on conversation just had on WPF\Winforms\UWP parity on Visual Studio App Center https://github.com/Microsoft/AppCenter-SDK-DotNet/issues/620

glennawatson commented 5 years ago

@paulcbetts comments in slack

This has potential but I would definitely consider performance implications In particular, TrackEvent should not await anything or return a Task Because it will encourage people to write naïve implementations that like, issue an HTTP request per trace And await the result (Since if they don't await the result all their tools flip out)

RLittlesII commented 5 years ago

Is it actually useful \ desirable?

I definitely think this is useful. I have a similar AppCenter implementation that I use that is based on an IDisposable. There are limitations and I read a while back (maybe a year), that people were using AppCenter as a bridge to Application Insights. This could essentially turn into feature metrics. Developers could then bring their own backing implementation, say Raygun, and just register it and use it.

What level of integration for Splat\ReactiveUI?

I would think some extension methods, but really should be a separate package that can stand on it's own. This has value outside just the boundaries of ReactiveUI and Splat. So if it is possible to publish a package that doesn't take dependencies on ReactiveUI or Splat that would be ideal to me.

People need to be aware of the costings of the APM tool they choose. This is around giving the facilities to make decisions based on what users are experiencing.

costings meaning the overhead on performance?

glennawatson commented 5 years ago

I would think some extension methods, but really should be a separate package that can stand on it's own. This has value outside just the boundaries of ReactiveUI and Splat. So if it is possible to publish a package that doesn't take dependencies on ReactiveUI or Splat that would be ideal to me.

Definitely separate nuget package is what we want, whether we put that under the Splat brand not sure. Most likely just so users don't have to hunt it down.

RLittlesII commented 5 years ago

Definitely separate nuget package is what we want, whether we put that under the Splat brand not sure. Most likely just so users don't have to hunt it down.

I think I would advocate for its own branding. That will be dependant on how decoupled it is from Splat though.

dpvreony commented 5 years ago

People need to be aware of the costings of the APM tool they choose. This is around giving the facilities to make decisions based on what users are experiencing.

costings meaning the overhead on performance?

financial and performance. application insights for example had legacy pricing model which is per node\device where now it is volume of data. but application insights is a valid target for https://github.com/worldbeater/ReactiveUI.RazorExample perf wise and blocking one example is where firewalls can block hockey app crash reporting on startup, thus the app sits there if you await. so putting it on a Task\ReactiveCommand that wasn't visible to the user and success could be tracked by state into an mini APM viewmodel.

I wouldn't expect to support hockeyapp as it's migrating to appcenter.

Probably worth building up a table of event types. (Global Error Handling, View Navigation, Feature Usage, etc) and agreeing which is the most appropriate.

for example https://docs.microsoft.com/en-us/azure/azure-monitor/app/api-custom-events-metrics TrackPageView for ReactiveUI View Navigation. Would you want it for ViewModel creation as well if people have short lived viewmodels?

In theory you could treat everything as a page view with a url format /namespace/class/property - not that i'm advocating that approach :)

glennawatson commented 5 years ago

My opinion Let's keep splat our generic utility library that is rx unaware. We should have this 100% as a separate nuget like our adapters but might be worth having a interface to the concept in the main splat nuget. It's fairly lightweight so not something I can see as standalone.

dpvreony commented 5 years ago

bit more reading for .net core and desktop to application insights https://oren.codes/2019/03/29/telemetry-in-desktop-apps/ https://github.com/onovotny/AppInsights.WindowsDesktop

Oren Novotny
Telemetry in Desktop Apps
Summary Learn how to collect telemetry in desktop applications for .NET Framework and .NET Core. Intro One of the key parts of product development is the ability to get telemetry out of your apps. …
GitHub
onovotny/AppInsights.WindowsDesktop
Application Insights for Desktop Applications . Contribute to onovotny/AppInsights.WindowsDesktop development by creating an account on GitHub.
dpvreony commented 5 years ago

AppCenter 2.2 preview now supports Winforms and WPF https://github.com/microsoft/appcenter-sdk-dotnet/releases/tag/2.2.1-preview

GitHub
microsoft/appcenter-sdk-dotnet
Development repository for the App Center SDK for .NET platforms, including Xamarin - microsoft/appcenter-sdk-dotnet