serilog-web / classic

[Discontinued] Serilog web request logging and enrichment for classic ASP.NET applications
Apache License 2.0
79 stars 32 forks source link

HttpRequestClaimValueEnricher #37

Closed FSou1 closed 7 years ago

FSou1 commented 7 years ago

Hi.

What do you think about adding new enricher named HttpRequestClaimValueEnricher:

    public class HttpRequestClaimValueEnricher : ILogEventEnricher
    {
        /// <summary>
        /// The claim property name searched for value to enrich log events.
        /// </summary>
        private readonly string _claimProperty;

        /// <summary>
        /// The property name added to enriched log events.
        /// </summary>
        private readonly string _logEventProperty;

        public HttpRequestClaimValueEnricher(string claimProperty) : this(claimProperty, null)
        {
            _claimProperty = claimProperty;
        }

        public HttpRequestClaimValueEnricher(string claimProperty, string logEventProperty)
        {
            _claimProperty = claimProperty;
            _logEventProperty = logEventProperty ?? claimProperty;
        }

        public void Enrich(LogEvent logEvent, ILogEventPropertyFactory propertyFactory)
        {
            if (logEvent == null) throw new ArgumentNullException(nameof(logEvent));

            if (HttpContext.Current == null)
                return;

            if (HttpContextCurrent.Request == null)
                return;

            var user = HttpContext.Current.User;
            if (user == null)
                return;

            var claims = ((ClaimsIdentity) user.Identity).Claims;

            var value = claims?.FirstOrDefault(c => c.Type == _claimProperty)?.Value;
            if (string.IsNullOrWhiteSpace(value))
                return;

            var claimProperty = new LogEventProperty(_logEventProperty, new ScalarValue(value));
            logEvent.AddPropertyIfAbsent(claimProperty);
        }
    }

?

nblumhardt commented 7 years ago

Thanks for the suggestion! This looks good. Could we add it without requiring the package to take on any additional dependencies outside the ones it already has? (Can't check right now myself, sorry :-)). If so, a PR would be very welcome. Cheers!

FSou1 commented 7 years ago

Yes, sure. There aren't any required external dependencies (in my opinion).

I'd like to make a pull request, but can't push my changes into separate branch (403).

Done :)

FSou1 commented 7 years ago

PR #38

nblumhardt commented 7 years ago

Done! Thank you.