Measure bytes before limits #5601
Conversation
mapno
requested review from
carles-grafana,
electron0zero,
ie-pham,
javiermolinar,
joe-elliott,
mdisibio,
ruslan-mikhailov,
stoewer,
yvrhdn and
zalegrala
as code owners
| Name: "distributor_bytes_received_total", | ||
| Help: "The total number of proto bytes received per tenant", | ||
| }, []string{"tenant"}) | ||
| metricIngressBytes = promauto.NewCounterVec(prometheus.CounterOpts{ |
There was a problem hiding this comment.
Being a bit picky, it seems we have a convention where the unit of measurement goes first:
metricBytesIngested
Although this seems incorrect, following Prometheus naming conventions. https://prometheus.io/docs/practices/naming/#metric-names
Instead of Ingress and received which may be confusing maybe we could use something like:
metricBytesRecievedRaw
distributor_bytes_received_raw_total
There was a problem hiding this comment.
along with distributor_bytes_received_raw_total I want to throw in few more names into the mix:
tempo_distributor_bytes_attempted_totalsince these are the bytes that are attempted (may or may not be ingested)distributor_bytes_received_unfiltered_totalsince we are capturing the bytes before ingestion limits are applied (i.e. filtering the data)
There was a problem hiding this comment.
Being even nitpickier, ingress bytes can be considered the measurement unit in this metric. If we wanted a verb after the unit like in distributor_bytes_received_total it could be distributor_ingress_bytes_received_total or distributor_bytes_ingressed_total.
Ideally, we would rename distributor_bytes_received_total to ingested and use distributor_bytes_received_total for this new metric.
I don't have a strong opinion on this, I'm ok to go with whichever name sounds more clear for the team.
There was a problem hiding this comment.
agree, I don't want to bikeshade on the name and okay with tempo_distributor_ingress_bytes_total as well.
mapno
force-pushed
the
measure-ingress-bytes
branch
from
cd27945 to
7d542a5
Compare
mapno
force-pushed
the
measure-ingress-bytes
branch
from
7d542a5 to
788840b
Compare
3 participants
What this PR does:
Adds a new metric that measures bytes received before limits
Which issue(s) this PR fixes:
Fixes https://github.com/grafana/tempo-squad/issues/848
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]