feat(preprod): Add confluent producer handling behind flag#106624
feat(preprod): Add confluent producer handling behind flag#106624NicoHinderling wants to merge 2 commits intomasterfrom
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Which additional metrics do you need ? |
|
451360d to
f9b5d03
Compare
f9b5d03 to
e5a3d33
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| _preprod_confluent_producer = get_confluent_producer( | ||
| build_kafka_producer_configuration(default_config=cluster_options) | ||
| ) | ||
| return _preprod_confluent_producer |
There was a problem hiding this comment.
Confluent producer lacks shutdown flush, risking message loss
High Severity
The new _preprod_confluent_producer global variable lacks shutdown handling. Unlike SingletonProducer, which registers an atexit handler to flush pending messages and close the producer on process termination, the Confluent producer path only calls poll(0) (non-blocking) after each produce. Messages still in the producer's internal buffer when the process exits will be silently lost. This is especially concerning since the comment indicates this is for retryable tasks, but message loss would prevent retries.
| _preprod_confluent_producer = get_confluent_producer( | ||
| build_kafka_producer_configuration(default_config=cluster_options) | ||
| ) | ||
| return _preprod_confluent_producer |
There was a problem hiding this comment.
Confluent producer missing excluded config keys
Medium Severity
The original _get_preprod_producer explicitly excludes compression.type and message.max.bytes from the Kafka configuration, but _get_or_create_confluent_producer does not. The default cluster config sets compression.type: lz4 and message.max.bytes: 50000000. This means the new Confluent producer path will use different Kafka settings than the original producer, which could cause compatibility issues or unexpected behavior. This exclusion pattern is used by multiple other producers in the codebase, suggesting these keys cause problems for certain topics.
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |

Roll out switching traffic for preprod from the regular Producer class to the ConfluentProducer class in order to get extra producer metrics