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

MDCs are missing in logs produced by DefaultErrorHandler. #3151

Closed
mkoralewski opened this issue Mar 22, 2024 · 1 comment
Closed

MDCs are missing in logs produced by DefaultErrorHandler. #3151

mkoralewski opened this issue Mar 22, 2024 · 1 comment

Comments

@mkoralewski
Copy link

In what version(s) of Spring for Apache Kafka are you seeing this issue?

3.1.3
In 3.1.2 works fine.

Describe the bug

For ConcurrentKafkaListenerContainerFactory we set CommonErrorHandler that you can see below:

  @Bean
  public ConcurrentKafkaListenerContainerFactory<String, SalesOrderOutbound> kafkaListenerContainerFactory(
      ConsumerFactory<String, SalesOrderOutbound> consumerFactory, DefaultErrorHandler defaultErrorHandler) {
    ConcurrentKafkaListenerContainerFactory<String, SalesOrderOutbound> factory =
        new ConcurrentKafkaListenerContainerFactory<>();

    factory.getContainerProperties().setObservationEnabled(true);
    factory.setConsumerFactory(consumerFactory);
    factory.setCommonErrorHandler(defaultErrorHandler);

    return factory;
  }
  @Bean
  public DefaultErrorHandler errorHandler() {
    BackOff fixedBackOff = new FixedBackOff(0, 0);
    return new DefaultErrorHandler(
        (consumerRecord, exception) -> log.error("Failed to process message from topic:{}. key:{}",
            consumerRecord.topic(), consumerRecord.key(), exception),
        fixedBackOff);
  }

When we use spring-kafka 3.1.2 logs produced by this handler had MDCs (traceId, spanId and one custom BaggageField). After upgrade to spring-kafka 3.1.3 MDCs are gone.

To Reproduce

Send event that will cause exception handled by errorHandler.

Expected behavior

Logs should have MDCs as before.

Sample

I'm not able to share sample now, hope that description above is enough.

@artembilan
Copy link
Member

Looks like this is a regression after #3049.

We will look shortly what can be done.
So, looks like we have to restore the previous logic with a try..catch inside observation.observe(), but at the same time use observation.error() in the catch block.

spring-builds pushed a commit that referenced this issue Mar 22, 2024
Fixes: #3151

After fixing #3049 we are missing an `ErrorHandler` part within an observation.
This even cause a retryable topic logic ot be out of an observation scope.

* Restore the previous behavior and add `observation.error(e)` when it is not re-thrown in case of `this.commonErrorHandler` presence

(cherry picked from commit c24575c)
spring-builds pushed a commit that referenced this issue Mar 22, 2024
Fixes: #3151

After fixing #3049 we are missing an `ErrorHandler` part within an observation.
This even cause a retryable topic logic ot be out of an observation scope.

* Restore the previous behavior and add `observation.error(e)` when it is not re-thrown in case of `this.commonErrorHandler` presence

(cherry picked from commit c24575c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants