charlesdong1991 commented on code in PR #619: URL: https://github.com/apache/fluss-rust/pull/619#discussion_r3423044887
########## crates/fluss/src/metrics.rs: ########## @@ -295,6 +363,27 @@ pub const WRITER_BYTES_SEND_TOTAL: &str = "fluss.client.writer.bytes_send.total" /// Counter: total records re-enqueued for retry. pub const WRITER_RECORDS_RETRY_TOTAL: &str = "fluss.client.writer.records_retry.total"; +/// Counter: total batches that terminally failed, labeled by `error_kind`: +/// * `non_retriable` — server returned a non-retriable error. +/// * `max_retries_exceeded` — a retriable error that exhausted its retries. +/// * `writer_id_changed` — idempotent retry abandoned after a writer-id reset. +/// * `local_build` — batch failed to build locally (never sent). +/// +/// Counts terminal failures only; retries are tracked by +/// `WRITER_RECORDS_RETRY_TOTAL`, so there is no double counting. +/// Carries only `error_kind` (no table label), matching the +/// unlabeled writer-metric convention. +pub const WRITER_ERRORS_TOTAL: &str = "fluss.client.writer.errors.total"; + +/// `error_kind` value for a non-retriable server error. +pub(crate) const WRITER_ERROR_KIND_NON_RETRIABLE: &str = "non_retriable"; +/// `error_kind` value for a retriable error that exhausted its retries. +pub(crate) const WRITER_ERROR_KIND_MAX_RETRIES_EXCEEDED: &str = "max_retries_exceeded"; +/// `error_kind` value for an idempotent retry abandoned after a writer-id reset. +pub(crate) const WRITER_ERROR_KIND_WRITER_ID_CHANGED: &str = "writer_id_changed"; +/// `error_kind` value for a batch that failed to build locally (never sent). +pub(crate) const WRITER_ERROR_KIND_LOCAL_BUILD: &str = "local_build"; Review Comment: > Do all of these metrics exist on Java client side? No, they are not as i mentioned in the PR description, Java has things similar in server side, but rust client cannot get those... it is mainly inspired by those java side metrics, and i come up with some simple metrics for alerting accordingly. > Metrics are user facing, releasing and then removing these metrics at later release would be a breaking change You are right, i thought it's mainly on client side, so can be more flexible, but indeed better to share there to collect feedback first... I will think about how to draft discussion properly -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
