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

metrics refactoring #231

Closed
Max-Meldrum opened this issue Jul 29, 2021 · 0 comments
Closed

metrics refactoring #231

Max-Meldrum opened this issue Jul 29, 2021 · 0 comments
Assignees
Labels
chore domain: metrics Anything related to Arcon metrics

Comments

@Max-Meldrum
Copy link
Member

Move metrics registration out of NodeMetrics/SourceMetrics

This should happen in the construction of SourceNode/Node (in new(...)).

The structs should only hold data structures used for metrics (e.g., Meter).

Change how we use metrics macro

Rather than combining strings each time, we can use the macro key/labels.

// old
register_histogram!(format!("{}_{}", node_name, "batch_execution_time"));
// new
register_histogram!(
  "batch_execution_time",
   Unit::Microseconds,
   "execution time per events batch",
   "node" => self.descriptor.clone(),
);

// old
histogram!(
  format!("{}_{}", &self.descriptor, "batch_execution_time"),
  elapsed.as_micros() as f64
);
// new
histogram!("batch_execution_time",  elapsed.as_micros() as f64, "node" => self.descriptor.clone());


// new counter
increment_counter!("watermark_counter", "node" => self.descriptor.clone());

Source throughput per batch

Currently, we are marking and reporting the Meter metric per Polled record. Change the process function to return ArconResult where usize is the counter variable indicating how many records were polled.

match self.process() {
#[cfg(not(feature = "metrics")]
Ok(_) => (),
#[cfg(feature = "metrics")]
Ok(polled_records) => {
   self.metrics.incoming_message_rate.mark_n(polled_records);
   gauge!("incoming_message_rate",  self.metrics.incoming_message_rate.one_min_rate(), "source" => self.descriptor().clone());
}
Err(error) => {
 // fatal error, must shutdown..
 // TODO: coordinate shutdown of the application..
 error!(self.logger, "{}", error);
}

Update HardwareMetricGroup

function names contain term gauge.

I think we can remove both of the functions if we are moving the registration to Node::new() and using the new macro approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore domain: metrics Anything related to Arcon metrics
Projects
None yet
Development

No branches or pull requests

2 participants