Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Fix Prometheus recording rules to not use legacy metric names. #13718

Merged
merged 6 commits into from
Sep 8, 2022
Merged

Fix Prometheus recording rules to not use legacy metric names. #13718

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e708b05
Fix up missed bad legacy metric names with colons in the names
reivilibre Sep 5, 2022
8027fff
Fix up Prometheus recording rules
reivilibre Sep 5, 2022
8ddb4dd
Remove Prometheus v1 rules
reivilibre Sep 5, 2022
3bd3255
Newsfile
reivilibre Sep 5, 2022
9deaf2f
Merge branch 'develop' into rei/prometheus_use_modern_metric_names
reivilibre Sep 6, 2022
2d62260
Catch a few more metrics: MAU metrics (last ones that match "synapse_…
reivilibre Sep 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/13718.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add experimental configuration option to allow disabling legacy Prometheus metric names.
21 changes: 0 additions & 21 deletions contrib/prometheus/synapse-v1.rules
View file
Open in desktop

This file was deleted.

22 changes: 11 additions & 11 deletions contrib/prometheus/synapse-v2.rules
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,18 @@ groups:
expr: 'sum(synapse_http_server_request_count:by_method) by (servlet)'

- record: 'synapse_cache:hit_ratio_5m'
expr: 'rate(synapse_util_caches_cache:hits[5m]) / rate(synapse_util_caches_cache:total[5m])'
expr: 'rate(synapse_util_caches_cache_hits[5m]) / rate(synapse_util_caches_cache[5m])'
- record: 'synapse_cache:hit_ratio_30s'
expr: 'rate(synapse_util_caches_cache:hits[30s]) / rate(synapse_util_caches_cache:total[30s])'
expr: 'rate(synapse_util_caches_cache_hits[30s]) / rate(synapse_util_caches_cache[30s])'

- record: 'synapse_federation_client_sent'
labels:
type: "EDU"
expr: 'synapse_federation_client_sent_edus + 0'
expr: 'synapse_federation_client_sent_edus_total + 0'
- record: 'synapse_federation_client_sent'
labels:
type: "PDU"
expr: 'synapse_federation_client_sent_pdu_destinations:count + 0'
expr: 'synapse_federation_client_sent_pdu_destinations_count_total + 0'
- record: 'synapse_federation_client_sent'
labels:
type: "Query"
Expand All @@ -40,11 +40,11 @@ groups:
- record: 'synapse_federation_server_received'
labels:
type: "EDU"
expr: 'synapse_federation_server_received_edus + 0'
expr: 'synapse_federation_server_received_edus_total + 0'
- record: 'synapse_federation_server_received'
labels:
type: "PDU"
expr: 'synapse_federation_server_received_pdus + 0'
expr: 'synapse_federation_server_received_pdus_total + 0'
- record: 'synapse_federation_server_received'
labels:
type: "Query"
Expand All @@ -60,19 +60,19 @@ groups:
expr: 'synapse_federation_transaction_queue_pending_pdus + 0'

- record: synapse_storage_events_persisted_by_source_type
expr: sum without(type, origin_type, origin_entity) (synapse_storage_events_persisted_events_sep{origin_type="remote"})
expr: sum without(type, origin_type, origin_entity) (synapse_storage_events_persisted_events_sep_total{origin_type="remote"})
labels:
type: remote
- record: synapse_storage_events_persisted_by_source_type
expr: sum without(type, origin_type, origin_entity) (synapse_storage_events_persisted_events_sep{origin_entity="*client*",origin_type="local"})
expr: sum without(type, origin_type, origin_entity) (synapse_storage_events_persisted_events_sep_total{origin_entity="*client*",origin_type="local"})
labels:
type: local
- record: synapse_storage_events_persisted_by_source_type
expr: sum without(type, origin_type, origin_entity) (synapse_storage_events_persisted_events_sep{origin_entity!="*client*",origin_type="local"})
expr: sum without(type, origin_type, origin_entity) (synapse_storage_events_persisted_events_sep_total{origin_entity!="*client*",origin_type="local"})
labels:
type: bridges
- record: synapse_storage_events_persisted_by_event_type
expr: sum without(origin_entity, origin_type) (synapse_storage_events_persisted_events_sep)
expr: sum without(origin_entity, origin_type) (synapse_storage_events_persisted_events_sep_total)
- record: synapse_storage_events_persisted_by_origin
expr: sum without(type) (synapse_storage_events_persisted_events_sep)
expr: sum without(type) (synapse_storage_events_persisted_events_sep_total)

4 changes: 2 additions & 2 deletions synapse/federation/sender/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@
logger = logging.getLogger(__name__)

sent_pdus_destination_dist_count = Counter(
"synapse_federation_client_sent_pdu_destinations:count",
"synapse_federation_client_sent_pdu_destinations_count",
"Number of PDUs queued for sending to one or more destinations",
)

sent_pdus_destination_dist_total = Counter(
"synapse_federation_client_sent_pdu_destinations:total",
"synapse_federation_client_sent_pdu_destinations",
"Total number of PDUs queued for sending across all destinations",
Comment on lines -65 to 71
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(N.B. I also fix a metric that I forgot to rename in #13540)

Any way to check if we've missed any others? I guess grep for "synapse_.*:.*"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should have thought of that; suppose I never really twigged that they all have a convenient synapse_ prefix that makes them greppable instead of 'string with colon' in :-)

Found another pair of sneaky ones ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never really twigged that they all have a convenient synapse_ prefix

I'm just guessing here!!

Could also grep for something like Counter\(\n?.*".*: and Gauge\(\n?.*".*: if you can get the multiline regex stuff working

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conventionally we definitely are meant to use an easily-recognised prefix like synapse_. I don't think even we would ignore that...

)

Expand Down
2 changes: 2 additions & 0 deletions synapse/metrics/_legacy_exposition.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ def sample_line(line: Sample, name: str) -> str:
"synapse_util_caches_response_cache_hits": "synapse_util_caches_response_cache:hits",
"synapse_util_caches_response_cache_evicted_size": "synapse_util_caches_response_cache:evicted_size",
"synapse_util_caches_response_cache": "synapse_util_caches_response_cache:total",
"synapse_federation_client_sent_pdu_destinations": "synapse_federation_client_sent_pdu_destinations:total",
"synapse_federation_client_sent_pdu_destinations_count": "synapse_federation_client_sent_pdu_destinations:count",
}


Expand Down