Get control over Telemetry construction

[srogovtsev]

The code in ApplicationInsightsSink.ForwardLogEventPropertiesToTelemetryProperties always forwards all the properties (both built-in, like LogLevel or RenderedMessage, and custom) to the AI object, which leads to duplication. All examples below are valid for TraceTelemetry.

• LogLevel and RenderedMessage. I, frankly, see no point in having these custom properties at all, because LogLevel is also forwarded to TraceTelemetry.SeverityLevel (I understand that this mapping is not 1-to-1), and RenderedMessage goes to the TraceTelemetry.Message.
• Enricher properties. For example, I have HttpRequestIdEnricher to help me collate logs in non-AI case (such as Seq). But AI has its own OperationCorrelationTelemetryInitializer, which provides data for the same purpose, so I don't want the HttpRequestId property showing up in AI logs (and I don't want to turn off OperationCorrelationTelemetryInitializer because (a) there's some AI info that's collected by AI directly and (b) AI provides UI support for collation on Operations only). And the same applies to, for example, user ID.

So in general I'd like to be able to:

1. Totally skip some properties when mapping them to Telemetry custom properties.
2. Be able to map some properties not to custom properties, but to some part of TelemetryContext

And if we generalize this a bit more, we can come to the ability of custom transformation from LogEvent to Telemetry so that I can, for example, transform log messages coming from Serilog Timings/Metrics to DependencyTelemetry and MetricTelemetry.

What do you think of it?

[joergbattermann]

Hi Serg - thanks for the feedback / idea.. and I do like it :+1: I've cooked up a sample proposal for this (see #16) - would this be useful to you? Basically it allows you to point to your own, custom LogEvent > AI Telemetry forwarding implementation (and uses the old one as default).. it hands in every LogEvent and the (I)Telemetry + its Properties.

Thereby you can a) control what/how properties are sent and have access to the TelemetryContext to forward some of them (i.e. like your mentioned UserIds, Http Request Ids etc) to the AI Telemetry Context.

[joergbattermann]

Another option would be using / taking a Func<ITelemetry, ..> and check what it returns (Trace-/Event-/Exception/Dependency- or MetricTelemetry) and send that over to AI..

Hrm - that might even be a tad better.

[srogovtsev]

I was thinking more along the lines of Func<LogEvent,string,ITelemetry> for event creation, then some kind of property filter, returning bool to indicate whether the property should be forwarded, and for advanced cases - Action<LogEvent,string,ITelemetry> to allow any possible mapping scenario. We also have to keep in mind the exception transformation and property flattening.

So, all in all it comes down to:

• Func<LogEvent,string,ITelemetry> telemetryCreator, which allows to create ITelemetry based on event and formatted message
• bool trackExceptionsSeparately, which, when set to false, puts events with exceptions to "normal" pipeline (allowing them to be created by the telemetryCreator)
• bool populateTelemetry, which indicates whether the ITelemetry from telemetryCreator was already populated, or we have to put it through pipeline
• Action<LogEvent,string,ITelemetry> propertyForwarder, which allows to populate passed in ITelemetry from event and formatted message (maybe we should also pass in property flattener)
• Func<string,object,ITelemetry> propertyFilter, which allows to check whether passed in property (name + value) should be forwarded to ITelemetry

Obviously, the last two options are mutually exclusive, we either use our own forwarder, or just filter the forwarding in normal forwarder.

[joergbattermann]

Already working on an updated Action<LogEvent,IFormatProvider,ITelemetry> sample.. will take just a couple more minutes :)

I'd rather not make the interface too noisy with too many parameters and options of filtering etc but provide a raw constructor hook so the few who do want and need to customize the telemetry can do so, and everyone else can keep using the existing default ones via Trace- or Event Telemetries.

[olegKoshmeliuk]

I like idea for controlling telemetry construction. :+1: Now when https://github.com/serilog/serilog/pull/772 is merged we can implement 2nd optional behavior - property json serlialzation, and allow users to select from flatern, json serialization and user implementation.

[joergbattermann]

Alright.. updated #16 - now you can specify a Func<LogEvent, IFormatProvider, ITelemetry> logEventToTelemetryConverter when adding the ApplicationInsights sink, i.e. like this:

var log = new LoggerConfiguration()
    .WriteTo
    .ApplicationInsights("<MyApplicationInsightsInstrumentationKey>", LogEventsToMetricTelemetryConverter)
    .CreateLogger();

// ....

private static ITelemetry LogEventsToMetricTelemetryConverter(LogEvent serilogLogEvent, IFormatProvider formatProvider)
{
    var metricTelemetry = new MetricTelemetry(/* ...*/);
    // forward properties from logEvent or ignore them altogether...
    return metricTelemetry;
}

.. which gives you full control over what ITelemetry you want to create and more importantly.. how :+1:

There's one caveat though, it must be one of the TelemetryClient.Track* -able types, but I think that's managable.

What do you think.. :+1: or :-1: ?

[srogovtsev]

The problem with this approach is that if I simply want to exclude one or two properties from the telemetry, I'll have to copy the whole code from the original implementation.

I'll try to put my own thought in code tomorrow.

There's one caveat though, it must be one of the TelemetryClient.Track* -able types, but I think that's managable.

Why don't you simply use Track method? I understand that it is marked as "for internal use", but it's still public and I don't see any problem with it.

[joergbattermann]

The default LogEvent > Event- or TraceTelemetry are public extension methods to LogEvent instances (see Line #80 and following or if you only want to use the default Property forwarding, it's similarly an extension method) - so in your own code you can use those and post-process / remove the properties you don't want as AI properties and i.e place some in the TelemetryContext.

The handful of developers who do / might want to customize the Telemetries can do it with this rather raw but simple approach, if we added more overloads that take predicates/filters and selectors it would add extra code to maintain.

If more users would ask for a simpler approach over time I'd revisit it in the future, but for now and because you opened the ticket (and were the first one to miss telemetry customization), can/could you do everything you wanted with the approach in #16?

Re: (not) calling .Track(..): Someone of t he AI team took the time to explicitly state 'Do not call' in the documentation for the .Track() method, and while it may do little harm, I therefore decided against it.. I think while the current Track* implementation is not elegant per se, it will do for now and when AI gets new Telemetry types, I'll add them.

[srogovtsev]

If more users would ask for a simpler approach over time I'd revisit it in the future, but for now and because you opened the ticket (and were the first one to miss telemetry customization), can/could you do everything you wanted with the approach in #16?

Let's try it on simple thing that brought me here. Can you provide an example of code doing the following: I want everything done "as before" (i.e., LogEvent maps to TraceTelemetry, and so on, and so forth), but properties corresponding to log level and formatted message are not forwarded, the property "HttpRequestId" is not forwarded, and property "UserId" is forwarded to telemetry.Context.User.Id?

Someone of t he AI team took the time to explicitly state 'Do not call' in the documentation for the .Track() method, and while it may do little harm, I therefore decided against it..

The same code states

This method needs to be public so that we can build and ship new telemetry types without having to ship core.

So it seems just like something we need. May be we can go and ask AI developers if it is appropriate to use this generic approach?

[joergbattermann]

Sure, here's the example converter and usage assuming you want them as TraceTelemetry instances:

public static void Main()
        {
            var log = new LoggerConfiguration()
                .WriteTo
                .ApplicationInsights("<MyApplicationInsightsInstrumentationKey>", ConvertLogEventsToCustomTraceTelemetry)
                .CreateLogger();
        }

        private static ITelemetry ConvertLogEventsToCustomTraceTelemetry(LogEvent logEvent, IFormatProvider formatProvider)
        {
            // first create a (sink) default Trace Telemetry but without the log level, and (rendered) message (template)
            var telemetry = logEvent.ToDefaultTraceTelemetry(
                formatProvider,
                includeLogLevelAsProperty: false,
                includeRenderedMessageAsProperty: false,
                includeMessageTemplateAsProperty: false);

            // then go ahead and post-process the telemetry's context to contain the user id as desired
            if (logEvent.Properties.ContainsKey("UserId"))
            {
                telemetry.Context.User.Id = logEvent.Properties["UserId"].ToString();
            }

            // and remove UserId and HttpRequestId from the telemetry properties
            if (telemetry.Properties.ContainsKey("UserId"))
            {
                telemetry.Properties.Remove("UserId");
            }

            if (telemetry.Properties.ContainsKey("HttpRequestId"))
            {
                telemetry.Properties.Remove("HttpRequestId");
            }

            return telemetry;
        }

(The default implementations check whether there's a LogEvent.Exception and if so, return / build an ExceptionTelemetry instance.. but you could decide whether you'd want that, too).

Regarding usage of TelemetryClient.Track(..) I've opened an issue with the AI folks, see above.

[srogovtsev]

I admit, this look nice.

[joergbattermann]

Alright, thanks for the idea & feedback :+1: Build is running and an updated package will be pushed to nuget shortly thereafter.

Regarding TelemetryClient.Track(ITelemetry) ... usage - I'll update this separately once / if the AI team replies to my issue over there.

Cheers!

[srogovtsev]

There seems to be a problem.

Here's the config:

                .WriteTo.ApplicationInsights(TelemetryConfiguration.Active,
                    (logEvent, formatProvider) => logEvent
                        .ToDefaultTraceTelemetry(formatProvider, includeLogLevelAsProperty: false)
                        .Do(telemetry =>
                        {
                            telemetry.Properties.Remove(HttpRequestIdEnricher.HttpRequestIdPropertyName);
                        }))

And here's the exception in SelfLog

2016-06-23T14:11:34 Caught exception System.ArgumentNullException: Value cannot be null.
Parameter name: formatProvider
   at Serilog.ExtensionMethods.LogEventExtensions.ForwardPropertiesToTelemetryProperties(LogEvent logEvent, ISupportProperties telemetryProperties, IFormatProvider formatProvider, Boolean includeLogLevel, Boolean includeRenderedMessage, Boolean includeMessageTemplate)
   at Serilog.ExtensionMethods.LogEventExtensions.ToDefaultTraceTelemetry(LogEvent logEvent, IFormatProvider formatProvider, Boolean includeLogLevelAsProperty, Boolean includeRenderedMessageAsProperty, Boolean includeMessageTemplateAsProperty)

Explicitly passing IFormatProvider to .WriteTo.ApplicationInsights helps.

[joergbattermann]

Yep that is/was a regression - thanks for catching it! I unfortunately only tested it with an actual formatProvider instance :frowning: Fixed build is running / package coming shortly thereafter

[srogovtsev]

Its seems to work now, thank you!