Clair & ClairCore Metrics Pass 1

[ldelossa]

Clair V4 Metrics Specification

This specification outlines how Clair V4 must implement metrics.

Metric collection is assumed to be driven by Prometheus and thus influences some key behaviors:

• Histograms are cumulative
• Implicit labels providing a dimension for "host" and "scrape job" are always present and are not mentioned at each metric definition.

Clair and ClairCore

Clair implements the http layer, notifier subsystem, and authentication.

ClairCore implement the internal API consisting of business logic and data store queries.

Metric instrumentation rules for both components must be defined.

Clair

HTTP Layer

Each API endpoint Clair exposes will be associated with the following key metrics:
- Request Count
- Request Size
- Response Size
- Request Duration

A per-endpoint metric strategy is chosen to combat a multiplicative increase in time-series cardinality as future development adds endpoints to Clair.

Request Count

Metric Name Format: clair_http_{endpoint}_request_total

Metric Type: Counter

Labels:

• code: http status code returned to the client
• method: the method of the http request

Observables:

• total counts of endpoint requests
• rates of endpoint requests

Request Size

Metric Name Format: clair_http_{endpoint}_request_size_bytes

// <= 250 bytes, <= 500 bytes, <= 1kb, <= 250kb, <= 500kb, <= 1mb, <= 250mb, <= 500mb
Metric Type: Histogram(-inf, 250, 500, 1000, 25e4, 5e5, 1e6, 2.5e8, 5e+8, +inf)

Unit: bytes

Observables:

• Average request content length
• Quantile ranking of request content length

Response Size

Metric Name Format: clair_http_{endpoint}_response_size_bytes

Metric Type: Histogram(-inf, 250, 500, 1000, 25e4, 5e5, 1e6, 2.5e8, 5e+8, +inf)

Unit: bytes

Observables:

• Average response content length
• Quantile ranking of response content length

Request Duration

Metric Name Format: clair_http_{endpoint}_request_duration_seconds

// <= 30ms, <= 50ms, <= 1 second, <= 5 seconds, <= 30 seconds, <= 1 minute, <= 5 minutes, <= 10 minute
Metric Type: Histogram(-inf, .030, .1, .5, 1, 5, 10, 30, 60, 300, 600, +inf)

Unit: seconds

Observables:

• Average response content length
• Quantile ranking of response content length

Authentication

Error Count

Metric Name Format: clair_auth_error_total

Metric Type: Counter

Labels:

• method: the requested method
• path: the path of the requested end point
• type: the type of authentication (assumed PSK for current Clair V4)

Observables:

• total count of errors
• rates of errors

Notifier Subsystem

Notification Status Counts

Metric Name Format: "clair_notifier_notification_status_total"

Metric Type: Gauge

Observables:

• Current count of notifications in a specific status.

Labels:

• status: created|delivered|delivery_failed|deleted

Notification Counts

Metric Name Format: "clair_notifier_notification_total"

Metric Type: Counter

Observables:

• Total notifications created, deleted, failed to be delivered, and delivered.
• Rate of notifications being created, deleted, failed to delivered, and delivered.

Labels:

• status: created|delivered|delivery_failed|deleted

Notifier Polls

Metric Name Format: "clair_notifier_poll_total"

Metric Type: Counter

Observables:

• Total number of times the notifier polled the matcher for new update operations
• The rate of polling
• The rate of any polling errors

Labels:

• success: true|false

ClairCore

General API

Function Count

Metric Name Format: claircore_{component}_function_total

Metric Type: Counter

Observables:

• Total count of calls to a component
• Total count of calls to a component's specific function
• Rate of a components function calls
• Rate of a component's specific function calls

Labels:

• func: instrumented function of the given component

Function Duration

Metric Name Format: claircore_{component}_function_duration_seconds

// <= 50ns, <= 250ns, <= 500ns, <= 750ns, <=1m, <= 50ms, <= 75ms, <= 1second, <= 30 seconds, <= 60 seconds
Metric Type: Histogram(5e-8, 2.5e-7, 5e-7, 7.5e-7, 0.001, 0.05, 0.25, 0.5, 0.75, 1, 15, 30, 60)

Observables:
-Average duration of a component's function calls

• Quantile ranking of the duration of a component's function calls.

Labels:

• func: instrumented function of the given component

The above section was deemed to be a tracing concern and the pprof server will be used instead

General Database

Query Count

Metric Name Format: claircore_{store}{function}total || clair{store}{function}_total

Metric Type: Counter

Observables:

• Total count of calls to a component
• Total count of calls to a component's specific function
• Rate of a components function calls
• Rate of a component's specific function calls

Labels:

• query: the specific query ran within a store's api method.

Query Duration

Metric Name Format: claircore_{store}{function}duration_seconds || clair{store}{function}_total

// prometheus default bucket size
Metric Type: Histogram(.005, .01, .025, .05, .075, .1, .25, .5, .75, 1, 2.5, 5, 7.5, 10)

Observables:

• Average duration of a component's function calls
• Quantile ranking of the duration of a component's function calls.

Labels:

• query: the specific query ran within a store's api method.

[ldelossa]

Above is a round 1 metrics pass of Clair and ClairCore.

A round 2 metrics pass will follow with component specific instrumentations such as "layer fetcher fetcher counts", "indexer counts", etc...

[hdonnay]

Would that second pass be the time to involve custom profiles?

[ldelossa]

I don't see why not, I didn't have this in mind, but sounds like a good addition to monitoring efforts.

[hdonnay]

Clair

HTTP Layer

These look good, with the proviso that these {endpoint} bits can't have the whole path in it. I'd like to be able to see if the response was served from the cache, but we may not expose that information out of ClairCore.

Authentication

👍

Notifier Subsystem

I'll defer to you on these. They seem like generally useful operation stats.

ClairCore

General API

All of these seem more like tracing concerns rather than metrics concern.
It feels bad to put the semistable ClairCore API into a public metrics API. Do we need to knock up a few sentences in the documentation, something along the lines, "metrics with this prefix are subject to drastic change on minor revisions, as they expose internal library details for debugging and diagnosing purposes."

General Database

I can see a better argument for exposing timing around database timings, though. I think the operator use case is for bisecting "slowness" between the application and database, and helping us with query hotspots.

[ldelossa]

Clair

HTTP Layer

These look good, with the proviso that these {endpoint} bits can't have the whole path in it. I'd like to be able to see if the response was served from the cache, but we may not expose that information out of ClairCore.

So this gets a little tricky, since the allowed characters in a metric name are pretty slim. Basically the only "separator" that is usable is "_" and ":", but ":" is reserved lol. see: https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels

we could make the metrics like "clair_http_affected_manifests_request_duration". If you wanted to add up all request durations for some reason, something like "clair_http_*_request_duration" would have to be employed.

Authentication

+1

Notifier Subsystem

I'll defer to you on these. They seem like generally useful operation stats.

ClairCore

General API

All of these seem more like tracing concerns rather than metrics concern.
It feels bad to put the semistable ClairCore API into a public metrics API. Do we need to knock up a few sentences in the documentation, something along the lines, "metrics with this prefix are subject to drastic change on minor revisions, as they expose internal library details for debugging and diagnosing purposes."

This is a good point, I am looking at these metrics as a way for US to optimize, not just a way for ops to understand what's going on. IMO I'd like to scope out which API calls are super slow and try to find out why.

That being said - I think I'm okay with some disclaimer. I am also okay to say "hey ops, you only care about 'clair_' metrics".

General Database

I can see a better argument for exposing timing around database timings, though. I think the operator use case is for bisecting "slowness" between the application and database, and helping us with query hotspots.

agreed.

[ldelossa]

I think we need to make a decision then, on whether "clair_{component}_function_*" metrics are useful to us or not. In my mind, its a quick way for us to identify which funcs are taking a ton of time, but in a run-time specific way (unlike when you run pprof with contrived inputs). My reasoning is, say for some reason everything is super slow, and we can see that layerfetcher.Fetch is taking a very long time, we can immediately begin diagnosing that area of code.

[hdonnay]

So this gets a little tricky, since the allowed characters in a metric name are pretty slim. Basically the only "separator" that is usable is "_" and ":", but ":" is reserved lol. see: https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels

we could make the metrics like "clair_http_affected_manifests_request_duration". If you wanted to add up all request durations for some reason, something like "clair_http_*_request_duration" would have to be employed.

That sounds right. I think we can't use the path directly because there are object references in there. It seems like we'd actually want clair_http_request_duration and a label indicating which handler is actually called, according to that linked documentation.

That being said - I think I'm okay with some disclaimer. I am also okay to say "hey ops, you only care about 'clair_' metrics".

Yeah, I think defining the _claircore (and maybe _clair) prefixes to be subject to a disclaimer from the jump is a good policy. I think we can add as many metrics as we find useful and graduate the ones we'll put effort into keeping around. I'm thinking claircore may want to intentionally expose some metrics, e.g. the database layer.

[hdonnay]

I think we need to make a decision then, on whether "clair_{component}function*" metrics are useful to us or not. In my mind, its a quick way for us to identify which funcs are taking a ton of time, but in a run-time specific way (unlike when you run pprof with contrived inputs). My reasoning is, say for some reason everything is super slow, and we can see that layerfetcher.Fetch is taking a very long time, we can immediately begin diagnosing that area of code.

I think we'd be better off by taking a profile in that case, as we already expose the pprof server. And again, it seems like a tracing concern.

[ldelossa]

So this gets a little tricky, since the allowed characters in a metric name are pretty slim. Basically the only "separator" that is usable is "" and ":", but ":" is reserved lol. see: https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels
we could make the metrics like "clair_http_affected_manifests_request_duration". If you wanted to add up all request durations for some reason, something like "clair_http*_request_duration" would have to be employed.

That sounds right. I think we can't use the path directly because there are object references in there. It seems like we'd actually want clair_http_request_duration and a label indicating which handler is actually called, according to that linked documentation.

I actually had that at first but then as noted by the

A per-endpoint metric strategy is chosen to combat a multiplicative increase in time-series cardinality as future development adds endpoints to Clair.

I moved away from that idea, to ensure that our prometheus metrics scale with our application.

HTTP endpoints are known for "cardinality sprawl". Where the sprawl is equal to:
{n_hosts} * {n_methods} * { n_statuscodes } * { n_paths } where n is the max possible number of label values encountered.

If we remove "paths" from the label set, we remove a multiplicative cardinality increase which would occur with every new endpoint.

Its particularly ugly for histograms which create a time series for each bucket range you specify, thus the cardinality becomes equal to {n_hosts} * { n_paths } * { n_buckets } for histograms ( where we don't record method or status codes, and even larger where you may want to.)

TLDR; I think keeping a metric per path, vs placing paths in the labels of a generic metric, will help us scale prometheus in the long run.

The linked article has a counter-point article here: https://www.robustperception.io/cardinality-is-key which formed my thinking over this.

That being said - I think I'm okay with some disclaimer. I am also okay to say "hey ops, you only care about 'clair_' metrics".

Yeah, I think defining the _claircore (and maybe _clair) prefixes to be subject to a disclaimer from the jump is a good policy. I think we can add as many metrics as we find useful and graduate the ones we'll put effort into keeping around. I'm thinking claircore may want to intentionally expose some metrics, e.g. the database layer.

That makes sense to me.

[ldelossa]

@hdonnay

just to confirm, any timing data we would have gotten by instrumenting function durations, can be obtained on a live instance via pprof?

If thats the case ill nix the "function" instrumentations from the spec. Just want to ensure we will be able to see function call timing during run time.

[hdonnay]

Yeah, we should be able to collect the trace profile.

[ldelossa]

okay I will need to check with app-sre to see if we could do this in prod.

[hdonnay]

All it needs technically is the equivalent of kubectl proxy. I'd be surprised if some RBAC can't be cooked up for it.

[ldelossa]

Okay spec updated with function instrumentation changed to pprof