Simple tracking online users in ASP.NET

Question

I wrote simple online users tracking for my ASP.NET MVC project.

1. In Global.asax I added:

   protected void Session_Start(Object sender, EventArgs e)
   {
    // get current context
    HttpContext currentContext = HttpContext.Current;
    if (currentContext != null)
    {
    if (!currentContext.Request.Browser.Crawler)
    {
    WebsiteVisitor currentVisitor = new WebsiteVisitor(currentContext);
    OnlineVisitorsContainer.Visitors[currentVisitor.SessionId] = currentVisitor;
    }
    }
   }
   protected void Session_End(Object sender, EventArgs e)
   {
    // Code that runs when a session ends.
    // Note: The Session_End event is raised only when the sessionstate mode
    // is set to InProc in the Web.config file. If session mode is set to StateServer
    // or SQLServer, the event is not raised.
    if (this.Session != null)
    {
    WebsiteVisitor visitor;
    OnlineVisitorsContainer.Visitors.TryRemove(this.Session.SessionID, out visitor);
    }
   }
   protected void Application_PreRequestHandlerExecute(object sender, EventArgs eventArgs)
   {
    var session = HttpContext.Current.Session;
    if (session != null && HttpContext.Current.User != null && HttpContext.Current.User.Identity.IsAuthenticated)
    {
    if (OnlineVisitorsContainer.Visitors.ContainsKey(session.SessionID))
    OnlineVisitorsContainer.Visitors[session.SessionID].AuthUser = HttpContext.Current.User.Identity.Name;
    }
   }
   

2. Here is my WebsiteVisitor class:

   using System;
   using System.Collections.Concurrent;
   using System.Collections.Generic;
   using System.Web;
   namespace WebApplication.Areas.Admin.Models
   {
    public class WebsiteVisitor
    {
    public string SessionId { get; set; }
    public string IpAddress { get; set; }
    public string AuthUser { get; set; }
    public string UrlReferrer { get; set; }
    public string EnterUrl { get; set; }
    public string UserAgent { get; set; }
    public DateTime SessionStarted { get; set; }
    public WebsiteVisitor(HttpContext context)
    {
    if (context != null && context.Request != null && context.Session != null)
    {
    this.SessionId = context.Session.SessionID;
    this.SessionStarted = DateTime.UtcNow;
    //this.UserAgent = String.IsNullOrEmpty(context.Request.UserAgent) ? "" : context.Request.UserAgent;
    this.UserAgent = context.Request.UserAgent ?? String.Empty;
    this.IpAddress = context.Request.UserHostAddress;
    //-------------------------------------------------------------
    if (context.Request.IsAuthenticated)
    {
    this.AuthUser = context.User.Identity.Name;
    if (!String.IsNullOrEmpty(context.Request.ServerVariables["REMOTE_USER"]))
    this.AuthUser = context.Request.ServerVariables["REMOTE_USER"];
    else if (!String.IsNullOrEmpty(context.Request.ServerVariables["AUTH_USER"]))
    this.AuthUser = context.Request.ServerVariables["AUTH_USER"];
    }
    //-------------------------------------------------------------
    if (context.Request.UrlReferrer != null)
    {
    this.UrlReferrer = String.IsNullOrWhiteSpace(context.Request.UrlReferrer.OriginalString) ? "" : context.Request.UrlReferrer.OriginalString;
    }
    this.EnterUrl = String.IsNullOrWhiteSpace(context.Request.Url.OriginalString) ? "" : context.Request.Url.OriginalString;
    }
    }
    }
    /// <summary>
    /// Online visitors list
    /// </summary>
    public static class OnlineVisitorsContainer
    {
    public static readonly ConcurrentDictionary<string, WebsiteVisitor> Visitors = new ConcurrentDictionary<string, WebsiteVisitor>();
    }
   }
   

3. And the last step is to write Action method in controller and display this data in view:

   public ActionResult WhoIsOnline()
   {
    if (OnlineVisitorsContainer.Visitors != null)
    { 
    return View(OnlineVisitorsContainer.Visitors.Values.OrderByDescending(x => x.SessionStarted));
    }
    return HttpNotFound();
   }
   

Do you see any improvement / issue?

• Kiquenet: WhoIsOnline is working fine? //The Session_OnEnd event is only supported when the session-state HttpSessionState.Mode property value is InProc, which is the default. // If the session-state Mode is set to StateServer or SQLServer, then the Session_OnEnd event in the Global.asax file is ignored. // If the session state Mode property value is Custom, then support for the Session_OnEnd event is determined by the custom session-state store provider.

Answer 1

MY EYES!!!

Seriously, use indentation. I didn't even realize these were ifs on a cursory scan, which would be very dangerous for anyone editing this code (and the others like it):

if (!String.IsNullOrEmpty(context.Request.ServerVariables["REMOTE_USER"]))
this.AuthUser = context.Request.ServerVariables["REMOTE_USER"];
else if (!String.IsNullOrEmpty(context.Request.ServerVariables["AUTH_USER"]))
this.AuthUser = context.Request.ServerVariables["AUTH_USER"];

Also, use braces. VS 2017 automatically offers to put them in, so you can fix everything in the project at once and just use them in the future.

---

Not sure which version of C# you are using, but if you are using C# 6 (and you really have no good reason not to), you can simplify this statement:

if (context != null && context.Request != null && context.Session != null)

to

if (context?.Request != null && context.Session != null)

Also, I'd make that (and most of your other ifs) a guard clause and remove the extra level of indentation:

if (context?.Request == null || context.Session == null)
{
 return;
}

Additional indentation levels add complexity, which increases the work you must do to understand the code.

Answer 2

In most of the cases it is ok. But in a multi-webserver environment (where you have 1 database and multiple web servers with same dns name, for load balancing purpose, as it could be using Azure) each web server will keep its own list of visitors and will never show all the visitors. To avoid this you should store the visitor list in the database.