Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit histogram metric for received message sizes per protocol #12342

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

ansd
Copy link
Member

@ansd ansd commented Sep 19, 2024

This PR:

Histogram
curl --silent -u guest:guest localhost:15692/metrics | ag rabbitmq_message_size_bytes
# TYPE rabbitmq_message_size_bytes histogram
# HELP rabbitmq_message_size_bytes Size of messages received from publishers
rabbitmq_message_size_bytes_bucket{protocol="mqtt311",le="100"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt311",le="1000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt311",le="10000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt311",le="100000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt311",le="1000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt311",le="10000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt311",le="50000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt311",le="100000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt311",le="+Inf"} 0
rabbitmq_message_size_bytes_count{protocol="mqtt311"} 0
rabbitmq_message_size_bytes_sum{protocol="mqtt311"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp091",le="100"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp091",le="1000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp091",le="10000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp091",le="100000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp091",le="1000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp091",le="10000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp091",le="50000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp091",le="100000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp091",le="+Inf"} 0
rabbitmq_message_size_bytes_count{protocol="amqp091"} 0
rabbitmq_message_size_bytes_sum{protocol="amqp091"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt50",le="100"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt50",le="1000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt50",le="10000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt50",le="100000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt50",le="1000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt50",le="10000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt50",le="50000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt50",le="100000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt50",le="+Inf"} 0
rabbitmq_message_size_bytes_count{protocol="mqtt50"} 0
rabbitmq_message_size_bytes_sum{protocol="mqtt50"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt310",le="100"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt310",le="1000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt310",le="10000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt310",le="100000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt310",le="1000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt310",le="10000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt310",le="50000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt310",le="100000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="mqtt310",le="+Inf"} 0
rabbitmq_message_size_bytes_count{protocol="mqtt310"} 0
rabbitmq_message_size_bytes_sum{protocol="mqtt310"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp10",le="100"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp10",le="1000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp10",le="10000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp10",le="100000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp10",le="1000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp10",le="10000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp10",le="50000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp10",le="100000000"} 0
rabbitmq_message_size_bytes_bucket{protocol="amqp10",le="+Inf"} 0
rabbitmq_message_size_bytes_count{protocol="amqp10"} 0
rabbitmq_message_size_bytes_sum{protocol="amqp10"} 0

@ansd ansd added this to the 4.1.0 milestone Sep 19, 2024
@mergify mergify bot added the bazel label Sep 19, 2024
@ansd ansd force-pushed the message-size-histogram branch 5 times, most recently from afce614 to bee7598 Compare September 23, 2024 16:20
@ansd ansd changed the title Add global histogram metrics for received message sizes per-protocol Emit histogram metric for received message sizes per protocol Sep 23, 2024
@ansd ansd force-pushed the message-size-histogram branch from bee7598 to f775310 Compare September 23, 2024 16:36
gomoripeti and others added 2 commits September 24, 2024 08:20
fixup: add new files to bazel

fixup: expose message_size_bytes as prometheus classic histogram type

`rabbit_msg_size_metrics` does not use `seshat` any more, but
`counters` directly.

fixup: add msg_size_metrics unit test
1.
Avoid unnecessary time series emitted for stream protocol
The stream protocol cannot observe message sizes.
This commit ensures that the following time series are omitted:
```
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="64"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="256"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="1024"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="4096"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="16384"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="65536"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="262144"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="1048576"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="4194304"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="16777216"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="67108864"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="268435456"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="+Inf"} 0
rabbitmq_global_message_size_bytes_count{protocol="stream"} 0
rabbitmq_global_message_size_bytes_sum{protocol="stream"} 0
```

This reduces the number of time series by 15.

2.
Further reduce the number of time series by reducing the number of
buckets. Instead of 13 bucktes, emit only 9 buckets. Buckets are not
free, each is an extra time series stored.

Prior to this commit:
```
curl -s -u guest:guest localhost:15692/metrics | ag message_size | wc -l
      92
```

After this commit:
```
curl -s -u guest:guest localhost:15692/metrics | ag message_size | wc -l
      57
```

3.
The emitted metric should be called
`rabbitmq_message_size_bytes_bucket` instead of `rabbitmq_global_message_size_bytes_bucket`.
The latter is poor naming. There is no need to use `global` in
the metric name given that this metric doesn't exist in the old flawed
aggregated metrics.

4.
This commit simplies module `rabbit_global_counters`.

5.
Avoid garbage collecting the 10-elements list of buckets per message
being received.
@ansd ansd force-pushed the message-size-histogram branch from f775310 to 1ec2152 Compare September 24, 2024 06:30
@ansd ansd marked this pull request as ready for review September 24, 2024 06:31
@mkuratczyk
Copy link
Contributor

The dashboard on https://grafana.com/grafana/dashboards/10991-rabbitmq-overview/ has not been updated yet, but the metric works well - I was able to put together a visualization like this:

Screenshot 2024-09-24 at 17 10 11

@mkuratczyk mkuratczyk merged commit 960808e into main Sep 24, 2024
199 checks passed
@mkuratczyk mkuratczyk deleted the message-size-histogram branch September 24, 2024 16:08
michaelklishin pushed a commit that referenced this pull request Sep 25, 2024
* Add global histogram metrics for received message sizes per-protocol

fixup: add new files to bazel

fixup: expose message_size_bytes as prometheus classic histogram type

`rabbit_msg_size_metrics` does not use `seshat` any more, but
`counters` directly.

fixup: add msg_size_metrics unit test

* Improve message size histogram

1.
Avoid unnecessary time series emitted for stream protocol
The stream protocol cannot observe message sizes.
This commit ensures that the following time series are omitted:
```
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="64"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="256"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="1024"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="4096"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="16384"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="65536"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="262144"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="1048576"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="4194304"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="16777216"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="67108864"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="268435456"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="+Inf"} 0
rabbitmq_global_message_size_bytes_count{protocol="stream"} 0
rabbitmq_global_message_size_bytes_sum{protocol="stream"} 0
```

This reduces the number of time series by 15.

2.
Further reduce the number of time series by reducing the number of
buckets. Instead of 13 bucktes, emit only 9 buckets. Buckets are not
free, each is an extra time series stored.

Prior to this commit:
```
curl -s -u guest:guest localhost:15692/metrics | ag message_size | wc -l
      92
```

After this commit:
```
curl -s -u guest:guest localhost:15692/metrics | ag message_size | wc -l
      57
```

3.
The emitted metric should be called
`rabbitmq_message_size_bytes_bucket` instead of `rabbitmq_global_message_size_bytes_bucket`.
The latter is poor naming. There is no need to use `global` in
the metric name given that this metric doesn't exist in the old flawed
aggregated metrics.

4.
This commit simplies module `rabbit_global_counters`.

5.
Avoid garbage collecting the 10-elements list of buckets per message
being received.

---------

Co-authored-by: Péter Gömöri <[email protected]>
mergify bot pushed a commit that referenced this pull request Sep 25, 2024
* Add global histogram metrics for received message sizes per-protocol

fixup: add new files to bazel

fixup: expose message_size_bytes as prometheus classic histogram type

`rabbit_msg_size_metrics` does not use `seshat` any more, but
`counters` directly.

fixup: add msg_size_metrics unit test

* Improve message size histogram

1.
Avoid unnecessary time series emitted for stream protocol
The stream protocol cannot observe message sizes.
This commit ensures that the following time series are omitted:
```
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="64"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="256"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="1024"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="4096"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="16384"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="65536"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="262144"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="1048576"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="4194304"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="16777216"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="67108864"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="268435456"} 0
rabbitmq_global_message_size_bytes_bucket{protocol="stream",le="+Inf"} 0
rabbitmq_global_message_size_bytes_count{protocol="stream"} 0
rabbitmq_global_message_size_bytes_sum{protocol="stream"} 0
```

This reduces the number of time series by 15.

2.
Further reduce the number of time series by reducing the number of
buckets. Instead of 13 bucktes, emit only 9 buckets. Buckets are not
free, each is an extra time series stored.

Prior to this commit:
```
curl -s -u guest:guest localhost:15692/metrics | ag message_size | wc -l
      92
```

After this commit:
```
curl -s -u guest:guest localhost:15692/metrics | ag message_size | wc -l
      57
```

3.
The emitted metric should be called
`rabbitmq_message_size_bytes_bucket` instead of `rabbitmq_global_message_size_bytes_bucket`.
The latter is poor naming. There is no need to use `global` in
the metric name given that this metric doesn't exist in the old flawed
aggregated metrics.

4.
This commit simplies module `rabbit_global_counters`.

5.
Avoid garbage collecting the 10-elements list of buckets per message
being received.

---------

Co-authored-by: Péter Gömöri <[email protected]>
(cherry picked from commit 1e3f4e5)

# Conflicts:
#	deps/rabbit/app.bzl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants