leekeiabstraction commented on code in PR #619: URL: https://github.com/apache/fluss-rust/pull/619#discussion_r3416249898
########## 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? If not you may want to consider raising a short FIP (potentially including more new client side metrics that might be valuable) and discussion for the definitions and implementation of these metrics. Metrics are user facing, releasing and then removing these metrics at later release would be a breaking change so we'd want to be mindful about what we introduce. I think we would benefit from discussing and gathering feedback from the community. -- 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]
