google / mtail

extract internal monitoring data from application logs for collection in a timeseries database
Apache License 2.0
3.84k stars 378 forks source link

Initialise metrics at startup #180

Open roidelapluie opened 6 years ago

roidelapluie commented 6 years ago

Just like in awk BEGIN,

I would like to be able to initiate metrics.

counter http_requests_duration_seconds_sum by code

BEGIN {
  http_requests_duration_seconds_sum["200"] = 0
}
roidelapluie commented 6 years ago

A workaround:

hidden counter init

init == 0 {
    http_requests_duration_seconds_sum["200"] += 0
    init++
}

But then we need to check init each time, which is not performant.

jnovack commented 6 years ago

I think this runs DANGEROUSLY close to confusing zero (0) with null. If you are initiating it with zero (0), then you are effectively skewing results when averaged over time. If a metric cannot be collected, it SHOULD be considered missing, not none.

I would not want this metric reading to be 0 if I just restarted the program before the first poll. image

However, doing some digging. Is this related to why you need it set? https://github.com/prometheus/prometheus/issues/1853

Per the Prometheus Best Practices:

Avoid missing metrics Time series that are not present until something happens are difficult to deal with, as the usual simple operations are no longer sufficient to correctly handle them. To avoid this, export 0 (or NaN, if 0 would be misleading) for any time series you know may exist in advance.

Most Prometheus client libraries (including Go, Java, and Python) will automatically export a 0 for you for metrics with no labels.

roidelapluie commented 6 years ago

Yes, I want to initiate counters with labels to 0. It is different from setting gauges, which makes less sense.

jaqx0r commented 6 years ago

Counters without dimensions are initialized to zero internally already. The dimensioned counters are not, because (maybe obviously) the values are not known until the log line is read (and mtail doesn't checkpoint this state, deliberately.)

I've been considering a BEGIN like awk for a while but not really convinced until now. Stlil a bit uneasy about the syntax; we only want to allow counters to be initialized? I can imagine that other things might want to be set up at start, but I don't know what those are. I worry though that this feature (like all features) creates an explosion of possibility and thus grants mtail programs the power to become very slow at processing log lines.

On Tue, 25 Sep 2018 at 04:07, Julien Pivotto notifications@github.com wrote:

Yes, I want to initiate counters with labels to 0. It is different from setting gauges, which makes less sense.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/google/mtail/issues/180#issuecomment-424070163, or mute the thread https://github.com/notifications/unsubscribe-auth/AC5b-0qagqVxRhozGUWCilVjJiUKpzd3ks5ueR9OgaJpZM4W2PBC .

jnovack commented 6 years ago

Stlil a bit uneasy about the syntax; we only want to allow counters to be initialized?

I can come up with a contrived example of why gauges should be initialized, but I do not have an actual example. I would want to set the gauge to zero when a gauge is measured as an offset of another number without a log file line stating as such (If we're using mtail, the application is probably not logging or metricing (verb) the way we want to, and we have to give intelligence to it, anyway). On application start, if I'm measuring drift of something that only complains when drift is NOT zero, I would like to initialize it as zero on my metrics so I can watch it drift from zero, rather than all of a sudden SEEING it at ±1 after a "missing" period.

I worry though that this feature (like all features) creates an explosion of possibility and thus grants mtail programs the power to become very slow at processing log lines.

Acknowledging feature creep, initializing is just for startup (runonce), it is just to initialize variables (no logic), and it is recommended practice by Prometheus.

jaqx0r commented 6 years ago

These seem like legit use cases. Let's build it.

On Wed, 26 Sep 2018 at 05:45, Justin J. Novack notifications@github.com wrote:

Stlil a bit uneasy about the syntax; we only want to allow counters to be initialized?

I can come up with a contrived example of why gauges should be initialized, but I do not have an actual example. I would want to set the gauge to zero when a gauge is measured as an offset of another number without a log file line stating as such. If we're using mtail, the application is probably not logging or metricing (verb) the way we want to, and we have to give intelligence to it. On application start, if I'm measuring drift of something that only complains when drift is NOT zero, I would like to initialize it as zero on my metrics so I can watch it drift from zero, rather than all of a sudden SEEING it at ±1 after a "missing" period.

I worry though that this feature (like all features) creates an explosion of possibility and thus grants mtail programs the power to become very slow at processing log lines.

Acknowledging feature creep, initializing is just for startup (runonce), it is just to initialize variables (no logic), and it is recommended practice by Prometheus.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/google/mtail/issues/180#issuecomment-424701588, or mute the thread https://github.com/notifications/unsubscribe-auth/AC5b--jeP5lTPx2ZtR5m3Ni9t_0h7z59ks5ue3cDgaJpZM4W2PBC .

jaqx0r commented 5 years ago

I think this is easier to implement without a BEGIN block but by allowing an initialisation expression after the metric declaration, e.g.:

counter foo = 0

just like in C++.

jaqx0r commented 5 years ago

Huh, that doesn't work for the case in your first comment, with a dimensioned metric. :/

roidelapluie commented 5 years ago

yeah because the counters are already 0 when they are not dimensional in mtail (IIRC)

jaqx0r commented 5 years ago

Facepalm. Thanks for the reminder :-)

darkl0rd commented 5 years ago

I've just started using mtail, but this one of the first things that confused me. I noticed all my counters in prometheus went missing when there are no matches (thus value = 0) and they will show up again when the value > 0. I can understand what the technical reasons are for this, but from a practical point of view, it is rather inconvenient that certain metrics simply disappear when their values are 0 (or in reality, probably null?).

I feel that when there are no matches, the value should always be set to '0' rather than omitting it.

SuperQ commented 5 years ago

One reason for initializing any gauge/counter/histogram is where you have by labels. The standard example is when you have counter http_requests_total by status. You want to be able to init status="500" to make sure you don't have a zero count for 500 errors. This is especially important when you do matching things in Prometheus.

sum without (status) (rate(http_requests_total{status="500"}[1m])) /
sum without (status) (rate(http_requests_total[1m]))

This will return no value if "500" is missing as there are no results on the left side of the expression.

praetp commented 4 years ago

Any progress on this ? We are using mtail based metrics to detect anomalies. However, we currently hitting https://github.com/prometheus/prometheus/issues/1673. We would not hit this problem if the metric would be initialized to zero.

roidelapluie commented 4 years ago

@praetp did you try https://github.com/google/mtail/issues/180#issuecomment-423908628?

praetp commented 4 years ago

@praetp did you try #180 (comment)?

Our counters are in fact dimensionless, so I just needed to upstep to a newer version of mtail :)

roidelapluie commented 3 years ago

There is even no syntax I know of that can initialize histograms (regardless of BEGIN or not).