Repository: envoyproxy/envoy
File: source/extensions/filters/http/ext_proc/processor_state.cc (C++)

Code before:
                                          CallbackState callback_state) {
  ENVOY_STREAM_LOG(debug, "Start external processing call", *filter_callbacks_);
  callback_state_ = callback_state;

  // Skip starting timer For FULL_DUPLEX_STREAMED body mode.
  if (bodyMode() != ProcessingMode::FULL_DUPLEX_STREAMED) {
    if (message_timer_ == nullptr) {
      message_timer_ = filter_callbacks_->dispatcher().createTimer(cb);
    }
    message_timer_->enableTimer(timeout);
    ENVOY_STREAM_LOG(debug, "Traffic direction {}: {} ms timer enabled", *filter_callbacks_,
                     trafficDirectionDebugStr(), timeout.count());
  }

  call_start_time_ = filter_callbacks_->dispatcher().timeSource().monotonicTime();
  new_timeout_received_ = false;
}

void ProcessorState::onFinishProcessorCall(Grpc::Status::GrpcStatus call_status,
                                           CallbackState next_state) {
  ENVOY_STREAM_LOG(debug, "Finish external processing call", *filter_callbacks_);
  filter_.logStreamInfo();

  stopMessageTimer();

  if (call_start_time_.has_value()) {
    std::chrono::microseconds duration = std::chrono::duration_cast<std::chrono::microseconds>(
        filter_callbacks_->dispatcher().timeSource().monotonicTime() - call_start_time_.value());
    ExtProcLoggingInfo* logging_info = filter_.loggingInfo();
    if (logging_info != nullptr) {
      logging_info->recordGrpcCall(duration, call_status, callback_state_, trafficDirection());
    }
    call_start_time_ = absl::nullopt;
  }
  callback_state_ = next_state;
  new_timeout_received_ = false;
}

void ProcessorState::stopMessageTimer() {
  if (message_timer_) {
    ENVOY_STREAM_LOG(debug, "Traffic direction {}: timer disabled", *filter_callbacks_,
                     trafficDirectionDebugStr());
    message_timer_->disableTimer();
  }
}

// Server sends back response to stop the original timer and start a new timer.
// Do not change call_start_time_ since that call has not been responded yet.
// Do not change callback_state_ either.
bool ProcessorState::restartMessageTimer(const uint32_t message_timeout_ms) {
  if (message_timer_ && message_timer_->enabled() && !new_timeout_received_) {

Review comment (refactor):
Do/should we still rely on this `call_start_time_.has_value()` check here? 

call_start_time_.has_value() should always be true once `call_start_time_ = absl::nullopt;` is removed here Could you confirm that? If so, i think we can remove this check here, `callback_state_ != CallbackState::Idle` better reflect the purpose of this if branch.
(I guess we still need this check for the defensive programming purpose, inside of if branch before accessing call_start_time_.value())


Code after:
                                          CallbackState callback_state) {
  ENVOY_STREAM_LOG(debug, "Start external processing call", *filter_callbacks_);
  callback_state_ = callback_state;

  // Skip starting timer For FULL_DUPLEX_STREAMED body mode.
  if (bodyMode() != ProcessingMode::FULL_DUPLEX_STREAMED) {
    if (message_timer_ == nullptr) {
      message_timer_ = filter_callbacks_->dispatcher().createTimer(cb);
    }
    message_timer_->enableTimer(timeout);
    ENVOY_STREAM_LOG(debug, "Traffic direction {}: {} ms timer enabled", *filter_callbacks_,
                     trafficDirectionDebugStr(), timeout.count());
  }

  call_start_time_ = filter_callbacks_->dispatcher().timeSource().monotonicTime();
  new_timeout_received_ = false;
}

void ProcessorState::onFinishProcessorCall(Grpc::Status::GrpcStatus call_status,
                                           CallbackState next_state) {
  ENVOY_STREAM_LOG(debug, "Finish external processing call", *filter_callbacks_);
  filter_.logStreamInfo();

  stopMessageTimer();

  if (call_start_time_.has_value() && callback_state_ != CallbackState::Idle) {
    std::chrono::microseconds duration = std::chrono::duration_cast<std::chrono::microseconds>(
        filter_callbacks_->dispatcher().timeSource().monotonicTime() - call_start_time_.value());
    ExtProcLoggingInfo* logging_info = filter_.loggingInfo();
    if (logging_info != nullptr) {
      logging_info->recordGrpcCall(duration, call_status, callback_state_, trafficDirection());
    }
  }
  callback_state_ = next_state;
  new_timeout_received_ = false;
}

void ProcessorState::stopMessageTimer() {
  if (message_timer_) {
    ENVOY_STREAM_LOG(debug, "Traffic direction {}: timer disabled", *filter_callbacks_,
                     trafficDirectionDebugStr());
    message_timer_->disableTimer();
  }
}

// Server sends back response to stop the original timer and start a new timer.
// Do not change call_start_time_ since that call has not been responded yet.
// Do not change callback_state_ either.
bool ProcessorState::restartMessageTimer(const uint32_t message_timeout_ms) {
  if (message_timer_ && message_timer_->enabled() && !new_timeout_received_) {
    ENVOY_STREAM_LOG(debug,