[telemetry] Detect service reachability issues #3426
Conversation
|
|
There was a problem hiding this comment.
This is a great observability addition, thank you @muhamadazmy! Left one minor naming comment which you should feel free to ignore.
| pub const INVOKER_TASKS_IN_FLIGHT: &str = "restate.invoker.inflight_tasks"; | ||
| pub const INVOKER_JOURNAL_REPLAY_TIME: &str = "restate.invoker.journal_replay_time.seconds"; | ||
| pub const INVOKER_SERVICE_DOWN_ERRORS: &str = "restate.invoker.service_down_errors.total"; |
There was a problem hiding this comment.
Nitpicky naming observation: Wondering if "unavailable" or "unreachable" might not be more accurate than "down" - since we can't tell authoritatively that it's really down, just that it's not available from our point of view.
There was a problem hiding this comment.
Ah very nice. Thank you. Will apply :)
| @@ -21,6 +21,8 @@ pub const INVOKER_TASK_DURATION: &str = "restate.invoker.task_duration.seconds"; | |||
| pub const INVOKER_SERVICE_RESPONSE_TIME: &str = "restate.invoker.service_response_time.seconds"; | |||
| pub const INVOKER_TASKS_IN_FLIGHT: &str = "restate.invoker.inflight_tasks"; | |||
| pub const INVOKER_JOURNAL_REPLAY_TIME: &str = "restate.invoker.journal_replay_time.seconds"; | |||
| pub const INVOKER_SERVICE_UNREACHABLE_ERRORS: &str = | |||
| "restate.invoker.service_unreachable_errors.total"; | |||
There was a problem hiding this comment.
I guess you mean deployment
crates/invoker-impl/src/lib.rs
Outdated
| @@ -1090,6 +1090,11 @@ where | |||
| .remove_invocation_with_epoch(partition, &invocation_id, invocation_epoch) | |||
| { | |||
| debug_assert_eq!(invocation_epoch, ism.invocation_epoch); | |||
|
|
|||
| if self.is_service_down_error(&error) { | |||
| counter!(INVOKER_SERVICE_UNREACHABLE_ERRORS, "service" => ism.invocation_target.service_name().to_string()).increment(1); | |||
There was a problem hiding this comment.
I think we need to have both the deployment id and the service name. The risk is that it this will be a high cardinality metric.
muhamadazmy
changed the title
This is used in the next PR to associate the deployment unreachable failure with the deployment id
Summary: Introducing a counter for number of "rechability" issues for a service that can detect a service is down or un-responsive by visualizing `rate(restate.invoker.deployment_unreachable_errors.total)` coupled with alerts, operator can know when a service is facing connectivity problems
|
@AhmedSoliman Thank you so much for your review. I applied the required changes, mainly to include the deployment id in the metric. This was not available on the error itself this is why I created the change at #3439 to be able do this. Would be great if you can also review it. While this metric might have a high cardinality, I don't believe it will be too crazy. Specially when older deployments are completely gone |
| describe_counter!( | ||
| INVOKER_DEPLOYMENT_UNREACHABLE_ERRORS, | ||
| Unit::Count, | ||
| "Number of deployment down errors" |
There was a problem hiding this comment.
| "Number of deployment down errors" | |
| "Number of service deployment unreachable errors" |
| .deployment_id | ||
| .map(|id| Cow::Owned(id.to_string())) | ||
| .unwrap_or_else(|| Cow::Borrowed("unknown")); | ||
| counter!(INVOKER_DEPLOYMENT_UNREACHABLE_ERRORS, "service" => ism.invocation_target.service_name().to_string(), "deployment" => deployment_id).increment(1); |
There was a problem hiding this comment.
Is this something we can already detect with the new fttb metric, or the invocation task status metric or does it require a new metric?
There was a problem hiding this comment.
Hmm, good idea. I will check if it can be done as part of the ttfb
[telemetry] Detect service reachability issues
Summary:
Introducing a counter for number of "rechability" issues for a service
that can detect a service is down or un-responsive
by visualizing
rate(restate.invoker.deployment_unreachable_errors.total)coupled with alerts, operatorcan know when a service is facing connectivity problems
Stack created with Sapling. Best reviewed with ReviewStack.