Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add scopes support to AI logger #491

Closed
pakrym opened this issue Jul 10, 2017 · 22 comments

Comments

Projects
None yet
@pakrym
Copy link
Member

commented Jul 10, 2017

No description provided.

@Tadimsky

This comment has been minimized.

Copy link
Member

commented Nov 29, 2017

Any update on this issue? BeginScope clearly is not doing anything :)

public IDisposable BeginScope<TState>(TState state)
{
    return null;
}
@davidfowl

This comment has been minimized.

Copy link

commented Mar 19, 2018

@SergeyKanzhelev can we get some movement on this? It means that you're missing a bunch of useful properties.

/cc @glennc

@cijothomas cijothomas added logging and removed configuration labels Mar 20, 2018

@cijothomas

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2018

@Dmitry-Matveev Lets triage and prioritize this issue in our next planning. There are a bunch of items in Logging area itself.
https://github.com/Microsoft/ApplicationInsights-aspnetcore/labels/logging

@davidfowl

This comment has been minimized.

Copy link

commented Mar 21, 2018

Feel free to pull in anyone from the ASP.NET Core team about if there are questions about why this is important.

@cijothomas

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2018

@davidfowl Sure! thanks.

@Dmitry-Matveev

This comment has been minimized.

Copy link
Member

commented Mar 21, 2018

@cijothomas, I'd add it to 2.3 due to high demand (apparently, half ASP.NET Core team up voted the issue :)). However, that would mean we've got only 9 days to take a look into this. So, I assume this can land in the beta we release in May instead. What's the amount of the work expected here?

@pharring

This comment has been minimized.

Copy link
Member

commented Mar 22, 2018

For the uninitiated, can someone (@pakrym, @davidfowl ?) write up a brief description of this feature and what user-scenarios it enables?

@davidfowl

This comment has been minimized.

Copy link

commented Mar 22, 2018

Logging scopes allow users to attach extra properties (key value pairs) to all logs within that scope. So for example ASP.NET Core itself creates a request scope with a bunch of properties that get attached to every log within a request (request id, correlation id, http connection id). ASP.NET Core MVC creates another scope to attach properties that correlate to the current action. ASP.NET Core SignalR (in 2.1) creates a scope that attaches metadata bout the websocket connection.

All of this to say, it's a way to add custom properties within an "async local scope". Today when you use this feature, AI doesn't get any of these extra properties resulting in a loss of rich logging information.

@cijothomas cijothomas added this to the 2.4.0 milestone May 23, 2018

@cijothomas

This comment has been minimized.

Copy link
Contributor

commented May 23, 2018

Tagging for 2.4 milestone, though not sure if we have man-hours available !

@ruimsft

This comment has been minimized.

Copy link

commented Jun 26, 2018

This is a very important feature to support ILogger. To return null means we cannot use AppInsights for our logger.

@gursin87

This comment has been minimized.

Copy link

commented Jul 2, 2018

Is this available in the latest beta of 2.4?

@cijothomas cijothomas modified the milestones: 2.4.0, 2.5.0 Aug 3, 2018

@cijothomas

This comment has been minimized.

Copy link
Contributor

commented Aug 22, 2018

@gursin87 Unfortunately this has not landed in 2.4.0 due to other commitments. Tagging for 2.5.0 release.

@profet23

This comment has been minimized.

Copy link

commented Sep 18, 2018

This is really important. Any updates?

@profet23

This comment has been minimized.

Copy link

commented Sep 19, 2018

Also curious what the implementation would look like for AppInsights.

I would expect that if you're sending an IDictionary it would simply append all key/value pairs as custom metrics to all telemetry in scope.

It might also be nice to have it create a new level to all telemetry below it by creating a new telemetry item with a new id and then giving telemetry in scope that as an operation_ParentId.

@zippy1981

This comment has been minimized.

Copy link

commented Sep 19, 2018

The Serilogger apender for ApplicationInsights handles scope by storing it in CustomDimensions. I'd like to move away from Serilog for my greenfield Asp.NET Core project work, because quite frankly your built in logging implementations are finally good enough, except for this one aspect.

Honestly, if you guys are unsure how to proceed (and I understand that some existing customers will see this change as breaking) with this, can you at least somehow expose scope to ITelemetryInitializer and let the users handle it?

However, being IncludeScope is a configurable boolean, I really doubt any customers will be affected in a truely bad way by this.

@cijothomas cijothomas modified the milestones: 2.5.0, 2.6.0 Oct 3, 2018

@profet23

This comment has been minimized.

Copy link

commented Oct 3, 2018

@cijothomas any reason this keeps getting pushed back? It would be amazing to have all my log scopes actually send info to Application Insights.

@cijothomas

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2018

@profet23 lack of bandwidth. Team is occupied with other deliverables :( Given the high demain, this item will be addressed as soon as I get some free cycles.

@2xPower

This comment has been minimized.

Copy link

commented Oct 9, 2018

Glad to hear - looking forward to progress

@RicoSuter

This comment has been minimized.

Copy link

commented Nov 26, 2018

It's really a shame that this important feature is still not supported. As a workaround we use Serilog with an Application Insights sink... The advantage is that it will even log correctly/in the same way when you switch to another sink (e.g. Seq).

@RamjotSingh

This comment has been minimized.

Copy link

commented Dec 31, 2018

PR microsoft/ApplicationInsights-dotnet-logging#239 has been merged in which introduces the support for Scopes once the package is out you can use it to log Scopes as well.

@bkaidbb

This comment has been minimized.

Copy link

commented Jan 7, 2019

@cijothomas

This comment has been minimized.

Copy link
Contributor

commented Jan 24, 2019

Resolved via -> microsoft/ApplicationInsights-dotnet-logging#239
Closing this.

Please post feedback/issues about Ilogger, use this repo:
https://github.com/Microsoft/ApplicationInsights-dotnet-logging

@cijothomas cijothomas closed this Jan 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.