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

Expose event counters for performance metrics #18016

Closed
roji opened this issue Sep 24, 2019 · 6 comments · Fixed by #21912
Closed

Expose event counters for performance metrics #18016

roji opened this issue Sep 24, 2019 · 6 comments · Fixed by #21912
Assignees
Labels
area-global area-perf closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@roji
Copy link
Member

roji commented Sep 24, 2019

.NET Core 3.0 introduced event counters, which are a way to efficiently expose performance metrics from the application. For example, the .NET runtime emits various GC, CPU and JIT statistics. These can be read via dotnet counters.

Here is what we intend to implement for 5.0:

  • Compiled query cache hit/miss statistics, allowing people to find bad queries which aren't getting cached for some reason. Could also provide some insight into query cache eviction (see Improve query cache eviction #12905).
  • Basic query (total queries, query rate per second, query/command cache hit rates)
  • SaveChanges
  • DbContexts currently in use (initialized but not yet disposed)
  • Execution strategy success/failure rate (per sec)
  • Optimistic concurrency success/failure rate (per sec)

Other counter ideas have been moved out to #21931

@unaizorrilla
Copy link

Hi @roji

I like the initial proposal for :

  • query executions ( current active connections, current transactions, number of query executions, number of query throws etc )
  • query cache hits/miss

But it could be very intersting to have dynamic counters to see query execution time for specify queries (¿annotated with Tag, with a new metadata?). These dynamic counters allow us verify how "main" queries are working.

The image below is a sample of dynamic counters ( the first two [darkmode] and [matchscore] )

imagen

Besides, I'd also like to see counter for most of the EventId,RelationEventId,CoreEventId warnings like query client evaluations, execution strategy retrying, optimistic concurrency exceptions..

Thanks

Unai

@roji
Copy link
Member Author

roji commented Sep 24, 2019

But it could be very intersting to have dynamic counters to see query execution time for specify queries (¿annotated with Tag, with a new metadata?). These dynamic counters allow us verify how "main" queries are working.

I'm not 100% sure, but from what I saw the design for event counters doesn't really allow for dynamic counters - you typically define a fixed set of counters that your component emits. What you're describing may be a better fit for a logging consolidation solution, which would ingest logs produced by EF Core (via DiagnosticSource), correlate events and produce the information you want. But it's worth thinking about.

Besides, I'd also like to see counter for most of the EventId,RelationEventId,CoreEventId warnings like query client evaluations, execution strategy retrying, optimistic concurrency exceptions..

Client evaluation is (mostly) no longer supported. Retries could be an interesting idea (added to the list), although it's a provider-specific feature which makes it a bit more complicated. I'm not sure what value there would be in counting optimistic concurrency exceptions, but it's certainly possible.

More generally, as I noted above, EF Core already generates lots of information via DiagnosticSource. Event counters are definitely not meant to replace that or mirror it - they're supposed to be a small set of well-defined standard statistics that can be emitted quickly and efficiently, and tracked in real time.

@unaizorrilla
Copy link

Hi @roji

I'm not 100% sure, but from what I saw the design for event counters doesn't really allow for dynamic counters - you typically define a fixed set of counters that your component emits. What you're describing may be a better fit for a logging consolidation solution, which would ingest logs produced by EF Core (via DiagnosticSource), correlate events and produce the information you want. But it's worth thinking about.

I do dynamic counters in my code and works well! I hope is supported :-)

https://github.com/Xabaril/Esquio/blob/master/src/Esquio/Diagnostics/EsquioEventSource.cs

@roji
Copy link
Member Author

roji commented Sep 24, 2019

I do dynamic counters in my code and works well! I hope is supported :-)

Interesting idea, not sure it's 100% supported/safe but worth taking a look! Thanks.

@ajcvickers
Copy link
Member

  • Creation and disposal of a DbContext
  • Calls to SaveChanges

Also, opening connections and using transactions would be useful, but should probably be at the ADO.NET level.

@roji
Copy link
Member Author

roji commented Jun 3, 2020

Suggestion by @AndriySvyryd: counter for the number of service providers, to help locate problematic cases. We already have a warning for this case, and it might not be that valuable to have a perf counter for something that is essentially "boolean" (is your program working correctly or not), but something to think about.

roji added a commit that referenced this issue Aug 4, 2020
@roji roji linked a pull request Aug 4, 2020 that will close this issue
roji added a commit that referenced this issue Aug 6, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-rc1 Aug 14, 2020
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 14, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0-rc1, 5.0.0 Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-global area-perf closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants