mnadel / metrics.net.influxdb

An InfluxDB 0.9+ Reporter for Metrics.NET
Apache License 2.0
13 stars 2 forks source link

Selective writes? #9

Open peske opened 8 years ago

peske commented 8 years ago

Me again, with a question / feature request.

Here's the thing: it would be great to have an option to disable writing some metrics data to the db. Probably at MetricsContext level. For example, let's assume that we have three contexts:

It would be great to be able to disable writing of "NotImportant" context, but still to continue to write root and "Important". I'm aware that there is an option to disable the context, but this can't be used if metrics are needed - we only don't want them to be saved in db. (In my case the app uses metric values to "decide" what to do next...)

I believe that there is another option - to create "detached" metrics (created by calling the constructor directly instead of calling MetricsContext methods; i.e. HistogramMetric h = new HistogramMetric();) Although I haven't tried yet, I believe that such metrics wouldn't be reported anywhere (including db). This can solve my current problem, but has one significant drawback: there's no runtime on/off feature. The decision if a metrics will be written into db or not has to be made before the metrics is created.

I've had a doubt if the feature should be implemented in parent project (Metrics.NET) or in "appender" projects like Metrics.NET.InfluxDB, but I've concluded that the second option is much better choice for simple reason: it may happen that we want to have complete reporting on some "channels", but not on another. For example, we can decide that "NotImportant" metrics can be tracked locally (through some visualization appender), but not saved to db...

Finally, I'll suggest (in my opinion) the simplest way to implement this: introduce another optional parameter of type Func<string,bool> to configuration method (WithInflux). If the parameter is provided (not null), you will call the function before db writes to check if the write should be performed. If the function returns true - you'll write; if not you'll discard particular context.

Please let me know what you think about that?

Thanks!

(Btw. If you want me I can do the change. I don't have too much time to help you a lot, but every now and than I can make some changes. Since it is free, open source project I'm aware that "put up or shut up" principle makes sense :))

mnadel commented 8 years ago

Hi again :) Curious -- what's the harm in exporting too much? Other than it being unnecessary, are you seeing downstream issues?

My initial reaction is the same as the doubt that you raised -- does this belong in Metrics.NET? If the goal is to suppress an entire context from being exported anywhere, then obviously Metrics.NET is the best-suited home. But that could be too inflexible to meet your use case, since you want some, but not all, metrics to go to InfluxDB. Ideally, Metrics.NET's reporting subsystem could be enhanced to add flexible filtering.

Short of that, though, I think your proposed implementation sounds like a good approach. Further, it should be easy enough to deprecate it should Metrics.NET provide this functionality.

It'll likely be a bit before I get a chance to implement this. Feel free to submit a PR if you get impatient. :)

peske commented 8 years ago

Thanks for the answer.

Let me first explain "what's the harm in exporting too much". The solution I'm working on is extremely complex one, running on dozens of servers (hundreds of virtual machines). It is quantitative betting solution which (among other things) collects odds (betting prices) from dozens of bookies. Across the environment there are many "Odds" instances (millions) in memory. Every "Odds" instance will contain two HistagramMetric instances (one with long-term, and other with favour-recent sampling). It means that there be at least few millions active HistogramMetric instances. Do you still wonder "what's the harm"? :)

I will preserve some aggregate metrics but definitely not all of them.

Right now I will try with "detached" HistogramMetric instances, as proof-of-concept. Hopefully I will make pull request soon. I'll let you know.

Anyway, if you are interested, I will show you the crazy system I'm talking about (once there's something to show).

Btw. I have no doubt anymore that such filtering features should not be added at parent (Metrics.NET) level, but to particular appenders for simple reason I've explained in the original question: It may happen that you want to gather the reports through one channel (to visualise on the local machine), but not to through other channel (to save to db).

mnadel commented 8 years ago

I stand corrected :) Sounds like an interesting use case.

Have you seen this?

https://github.com/HdrHistogram/HdrHistogram/

peske commented 8 years ago

Yes, but it looks that .net part is stalling there. I've considered https://github.com/LeeCampbell/HdrHistogram.NET (I believe that it is ran by a guy who was working on the original one.)

Anyway, since there is HdrHistogram embedded in Metrics.NET (and it looks fine to me for now), I didn't wanted to add more dependancies, especially not redundant ones.

Thanks for helping! Hopefully soon will I have more time to help you...

peske commented 8 years ago

Btw. wouldn't be nice to have an option, for example, to temporary turn on odds from one match and watch them almost live through InfluxDb -> Grafana?

Don't worry! It is not yet another request - it can be done through the mechanism I've already mentioned, by temporary changing some parameters thus causing the function to return true...