StackExchange / StackExchange.Redis

General purpose redis client
https://stackexchange.github.io/StackExchange.Redis/
Other
5.85k stars 1.5k forks source link

Common performance counters for client operations #1081

Open Yonisha opened 5 years ago

Yonisha commented 5 years ago

There's no indication on how long the client operations took. It'll be very helpful to introduce common performance counters (operation duration/count/failures etc.)for the various operations to understand how it impacts application layer

mgravell commented 5 years ago

some of the overall counters already exist via GetCounters; timing, however, is relatively expensive to gather and quite hard to accurately summarise - I'd be cautious of that one; it also isn't particularly useful, as often you have 2000000 fast ops and 3 slow ones that took "too long" (for whatever that means), but against the 2000000 they won't show up. Happy to explore options, but it isn't trivial to design.

Yonisha commented 5 years ago

I am referring to client performance counters which are very useful in high throughput applications. AFAIK GetCounters provides server side metrics. Am I missing anything?

mgravell commented 5 years ago

GetCounters is client metrics, not server metrics - where (just to be 100% clear) the library (and whatever you're doing with it) is a client in this context, and the actual redis-server is the server. Are we talking past eachother?

sleemer commented 5 years ago

@mgravell have you looked into App.Metrics? e.g. using their histogram metric would help to put all the timings into buckets and you won't miss those 3 slow ones...

mgravell commented 5 years ago

I'm not familiar. Link?

On Tue, 4 Jun 2019, 15:22 Vlad Kovalev, notifications@github.com wrote:

@mgravell https://github.com/mgravell have you looked into App.Metrics? e.g. using their histogram metric would help to put all the timings into buckets and you won't miss those 3 slow ones...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/StackExchange/StackExchange.Redis/issues/1081?email_source=notifications&email_token=AAAEHMHYIOWDP3P6I6ONBELPYZ3B7A5CNFSM4G4CTDI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW4XLYA#issuecomment-498693600, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAEHMBQMDRAY7BQ3BEGPGDPYZ3B7ANCNFSM4G4CTDIQ .

sleemer commented 5 years ago

@mgravell, here is the link https://www.app-metrics.io/getting-started/metric-types/histograms/. Another option could be https://github.com/HdrHistogram/HdrHistogram.NET... but it seems like it hasn't been updated for a while...

NickCraver commented 4 years ago

Fundamentally, all of these options incur an overhead cost for all operations and lower efficiency. Given the rare ask for this, I don't think we'd add this overhead to the library. It's something to keep in mind for the future, but since there's no cheap way to do it - it's likely not going to be worth adding.

avireddy02 commented 3 years ago

@NickCraver @mgravell Any chance of exposing below internal fields to Public (readonly)? This will allow apps to build their metrics as needed. In Current version below data is only available when an exception is raised.

NickCraver commented 2 years ago

@avireddy02 missed this request - indeed that sounds reasonable to expose (not as they are, but in some way). The other bits I want to add here are for example "last 2 connection logs" and similar - going to do a pass at better telemetry (not sent anywhere) that's accessible with a call to get a "package" of helpful data like some counters, config, last few errors, last few connection logs, last reasons to reconnect, etc.

rkarthick commented 2 years ago

Hi @NickCraver Is this something you are actively working on? Could you share any docs around this if it is present? If not, do you mind if I take a stab at this?

NickCraver commented 2 years ago

@rkarthick I have a branch started, but likely won't back to it for a week or two (other priorities), have it worked out in my head on approach but not documented down anywhere. Probably still easiest for me to get the initial counters in - help would be appreciated in thinking of counters though - for sure!

rkarthick commented 2 years ago

Ah, that makes sense. Thank you so much for your work here :)

On Mon, Jun 27, 2022 at 11:26 AM Nick Craver @.***> wrote:

@rkarthick https://github.com/rkarthick I have a branch started, but likely won't back to it for a week or two (other priorities), have it worked out in my head on approach but not documented down anywhere. Probably still easiest for me to get the initial counters in - help would be appreciated in thinking of counters though - for sure!

— Reply to this email directly, view it on GitHub https://github.com/StackExchange/StackExchange.Redis/issues/1081#issuecomment-1167719887, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH4RGT6WCUKDABAYBKTY5DVRHW5NANCNFSM4G4CTDIQ . You are receiving this because you were mentioned.Message ID: @.***>

-- Karthick Ramachandran

thompson-tomo commented 6 months ago

Given the recent focus by Microsoft on supporting OpenTelemetry ie heavy theme in net 8 & the release of Aspire. What is the thought about recording this telemetry using System.Diagnostics.Metrics so that they can they be exported using an open Telemetry collector?

Key thing for me to be recording would be: