feat: Add Prometheus metrics endpoint and Grafana dashboard #2282
No reviewers
Labels
No labels
Abandoned
Blocked
Bug
Changelog
Added
Changelog
Missing
Changelog
None
Cherry-picking
Database
Dependencies
Dependencies/Renovate
Difficulty
Easy
Difficulty
Hard
Difficulty
Medium
Documentation
Enhancement
Good first issue
Help wanted
Inherited
Matrix/Administration
Matrix/Appservices
Matrix/Auth
Matrix/Client
Matrix/Core
Matrix/E2EE
Matrix/Federation
Matrix/Hydra
Matrix/MSC
Matrix/Media
Matrix/T&S
Merge
Merge/Manual
Merge/Squash
Meta
Meta/CI
Meta/Packaging
Priority
Blocking
Priority
High
Priority
Low
Security
Status
Confirmed
Status
Duplicate
Status
Invalid
Status
Needs Investigation
Support
bob the builder
No milestone
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
continuwuation/continuwuity!2282
Loading…
Reference in a new issue
No description provided.
Delete branch "eleboucher/continuwuity:feat/prometheus-metrics"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Adds an opt-in, token-protected /_continuwuity/metrics endpoint covering HTTP, federation, the sender, RocksDB, caches and the tokio runtime, plus a ready-to-import Grafana dashboard.
Pull request checklist:
mainbranch, and the branch is named something other thanmain.myself, if applicable. This includes ensuring code compiles.
Happy to see folks working on metrics!
That said, could you please take a look at #811, particularly this comment? There are some metrics requested there, including by maintainers, that don't seem to be covered here (yet), and I still think it would be a good idea to support OpenTelemetry for metrics since we already do for traces.
@julian45 wrote in #2282 (comment):
Done we should be good
563b13a1766a80d3b69b6a80d3b69bd59b503eecThanks! Better, but I think there are some further improvements you might want to look at:
One bit of existing work that involves a some of both is opentelemetry-instrumentation-tower; see e.g. their axum worked example. This crate alone may give you a bunch of the Tower/Axum-based stuff you're looking for right out of the gate.
a71f8d160642583b947342583b947366b595e33066b595e330b81fbb1649b81fbb1649b19e4b384ab19e4b384a0a367954790a3679547919a942d8acMostly OK, some small issues
@ -78,0 +96,4 @@scheme.eq_ignore_ascii_case("bearer") && constant_time_eq(provided, token)});if !authorized {401 Unauthorizedis probably the correct code@ -154,0 +161,4 @@.metrics.instruments.federation_incoming_pdus.add(1, &[KeyValue::new("outcome", "unparsable")]);Might wanna also set the type
@ -0,0 +133,4 @@fn size_buckets() -> Vec<f64> { (0..11).map(|i| 1024.0 * f64::from(4_u32.pow(i))).collect() }fn register_process(meter: &Meter) {let started = SystemTime::now();Instant::now()?@ -0,0 +245,4 @@let busy_metrics = metrics.clone();meter.f64_observable_counter("continuwuity.tokio.worker.busy_time")This should only be enabled when compiled with
tokio_unstable@ -0,0 +35,4 @@#[cfg(target_os = "linux")]pub(super) fn open_fds() -> Option<u64> {let count = std::fs::read_dir("/proc/self/fd").ok()?.count();This probably counts itself :3
@ -90,3 +89,3 @@.build().expect("Failed to create OTLP gRPC exporter"),| "http" => opentelemetry_otlp::SpanExporter::builder()| _ => opentelemetry_otlp::SpanExporter::builder()Why remove the warning?
@ -0,0 +103,4 @@"RocksDB block cache usage by column family","By",|services, observer| {let Ok(usage) = services.db.db.memory_usage_stats() else {Might wanna only call this once across both
continuwuity.databasegauges@ -0,0 +153,4 @@self.server.runtime().spawn(async move {let mut interval = tokio::time::interval(COUNT_INTERVAL);loop {interval.tick().await;Set
missed_tick_behaviorto something other thanBurst@ -0,0 +162,4 @@}let local = services.users.count().await;let remote = services.users.stream_remote_users().count().await;I don't think reading every single remote user in the database, unconditionally, is a great idea for performance. Same for other places you have used .count on a database stream.
@ -213,1 +177,16 @@media_files: AtomicU64,struct MemoryUsageCache(Mutex<Option<(Instant, Arc<MemoryUsage>)>>);impl MemoryUsageCache {fn get(&self, services: &Services) -> Option<Arc<MemoryUsage>> {let mut cached = self.0.lock().expect("locked");if let Some((at, usage)) = &*cached {if at.elapsed() < MEMORY_USAGE_TTL {return Some(usage.clone());}}let usage = Arc::new(services.db.db.memory_usage_stats().ok()?);*cached = Some((Instant::now(), usage.clone()));Some(usage)}}This is... significantly worse. When responding to review feedback, either push back when it's a bad idea, or look for a better solution. In this case, you could have gone to definition to see that you can get the mem_table & row_cache metrics separately from the col_caches metrics
f5996eb0b887404f542087404f542053aa7f10baHave you tested this on your server?
Testing locally spews a bunch of errors:
Plus debug info at a log level that's too high
That seems to be an issue solely where metrics are disabled entirely btw
Working in my setup!
@ -0,0 +33,4 @@```toml[global]allow_otlp_metrics = trueotlp_protocol = "http" # or "grpc"I'll update this to match the last commit when I open my IDE again
https://github.com/open-telemetry/opentelemetry-rust-contrib/issues/839
3695f90fc63975d31b91