Skip to content

feat: add prometheus exporter and grafana dashboard#38

Open
zUZWqEHF wants to merge 2 commits intograelo:mainfrom
zUZWqEHF:feature/prometheus-exporter
Open

feat: add prometheus exporter and grafana dashboard#38
zUZWqEHF wants to merge 2 commits intograelo:mainfrom
zUZWqEHF:feature/prometheus-exporter

Conversation

@zUZWqEHF
Copy link

No description provided.

@graelo
Copy link
Owner

graelo commented Dec 16, 2025

Hi @crackerben99, thanks for this contribution! A Prometheus exporter is a valuable addition to pumas.

Before we can merge this, there are a few things that need attention:

PR Description

Could you add a description explaining:

  • The motivation for this feature
  • How to use it (example commands)
  • Any design decisions you made

Code Quality Issues

  1. Error handling

The PR has many .unwrap() calls that will panic in production. We've recently cleaned up similar issues across the codebase (see #40, #42). Please replace these with proper error handling:

// Instead of:
registry.register(Box::new(cpu_active_ratio.clone())).unwrap();
let server = Server::http(format!("0.0.0.0:{}", port)).unwrap();
request.respond(response).unwrap();
rx.recv().unwrap()

// Use ? with Result, or at minimum .expect("descriptive message")
  1. Duplicate metric

pumas_GPU_active_ratio and pumas_GPU_sm_activity are set to the same value (metrics.GPU.active_ratio). Please remove one or clarify if they should track different things.

  1. Spawned thread without error handling

The HTTP server thread's JoinHandle is discarded. If that thread panics, the server silently stops responding while the main loop continues. Please either store the handle or add error handling.

  1. Hardcoded bind address

0.0.0.0 is hardcoded. Consider making it configurable via --bind flag (users may want 127.0.0.1 for local-only access).

Rebase Required

The codebase has changed since this PR was opened. Please rebase on current main. Notable changes:

  • Error handling improvements throughout
  • New MetricKey enum replacing string-based history keys
  • MSRV bumped to 1.80

Nice to have

  • Basic tests for the new server code
  • Constants or comments for thermal pressure magic numbers (0.0, 1.0, 2.0...)

Let me know if you have questions or need help with any of this!

Thanks again for your effort!
Cheers

@graelo
Copy link
Owner

graelo commented Dec 16, 2025

I almost forgot: i think it's worth considering putting the prometheus server in a separate crate. I'll let you provide your opinion on this idea. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants