Create an activity more often than before

[davidfowl]

• Create an Activity if there's a diagnostic listener attached with the activity name or if logging is enabled.
• Add the activity id to the logging scope (and call the field ActivityId)

cc @lmolkova @SergeyKanzhelev

#5926
#5918

[benaadams]

Is _logger.IsEnabled(LogLevel.Critical) the right level to create an activity? (As generally it will always be on at critical or above)

[davidfowl]

In theory, there should always be an activity (I need to run some perf tests to see how this affects the default scenarios). This basically replaces the current request id. The current logic assumes if you're going to log a a scope, then the activity should be created.

We added this but there's kinda a chicken and egg problem as we need to make the activity to be a part of the scope before knowing if the scope if there.

[davidfowl]

There may also need to be another explicit way to turn it on or off outside logging but that's sorta why this is a draft.

[benaadams]

Resetable Activity on the connection using something like: #6895

Currently problematic as Activity doesn't support being cleared.

[davidfowl]

The server would need to own activity creation in that case. I'm not sure we'll come up with anything reasonable in the 3.0 time frame for #6895. I think coupling it to logging is the best bet mostly because the data is for the logging system and that scenario has a bunch of allocations already (the scope, the messages being logged etc).

Maybe the log level needs to be adjusted to something like information but the category is this internal category (which might be a bit weird).

[benaadams]

dotnet/extensions#1422

Then something like?

var scope = logger.BeginScope(
    (state) => new HostingLogScope(state.httpContext, state.activityId), 
    (httpContext, activityId));

if (!scope.IsNullScope() || (diagnosticListenerEnabled && _diagnosticListener.IsEnabled(ActivityName, httpContext)))
{
    context.Activity = StartActivity(httpContext);
}

[davidfowl]

This only helps when scopes are off. We could just add a DisableActivityCreation to the WebHostOptions if we wanted to remove the allocation in benchmarks.

[benaadams]

Is there anyway to do an inversion? Say for example AppInisights is set to sample 5% of activities rather than 100% is there a way to ask it "will you consume this activity" and create based on that; rather than create it 100% of the time and throw it away unobserved 95% of the time?

[davidfowl]

Yes there's a plan to support sampling but this feature needs to work with logging when app insights isn't on so that it's possible to correlate logs without too much setup.

See #9340 for the sampling PR. The activity still needs to be created though (so there's a chicken and egg problem)

[SergeyKanzhelev]

@benaadams we will need an Activity not only to track something, but also to propagate the decision that we decided NOT to track something. So head-based sampling would work across the components.

[pranavkm]

You could get rid of this type

[davidfowl]

@SergeyKanzhelev @lmolkova should we split this out into span id and trace id in the logs?

[SergeyKanzhelev]

This is what loggers are typically doing. Like in OpenCensus: https://github.com/census-instrumentation/opencensus-specs/blob/c3fd41b28fed96ceffc8c35af280d74cacea3ab2/trace/LogCorrelation.md#key-names-for-tracing-data or Elastic: elastic/ecs#99