Skip to content

Conversation

@woshigaopp
Copy link
Contributor

This pull request introduces the new AutoMQ Log Uploader module, which provides asynchronous S3 log upload functionality for Log4j 1.x applications. It refactors and relocates key log uploading classes, adds configuration constants, and implements a robust configuration and startup process for the uploader. The changes are grouped into module introduction/documentation, configuration and integration, and codebase refactoring.

Module introduction & documentation:

  • Added a comprehensive README.md for the new automq-log-uploader module, detailing its purpose, integration steps, configuration options, and extension points for custom setups.

Configuration and integration:

  • Added build.gradle for the new module, specifying dependencies and repository settings for building and integrating the uploader.
  • Introduced LogConfigConstants.java to centralize all configuration keys and defaults for S3 log uploading, improving maintainability and clarity.
  • Added DefaultS3LogConfig.java, which loads and normalizes configuration from properties, constructs the S3 object storage URI, and sets up leader election strategies for log uploading and cleanup.

Codebase refactoring and improvements:

  • Refactored and relocated LogUploader.java and LogRecorder.java from the automq-shell submodule to the new automq-log-uploader package, decoupling them from application-specific dependencies and improving error messages in validation logic. [1] [2] [3]
  • Simplified the startup and shutdown logic in LogUploader.java, replacing the singleton/bean pattern with explicit configuration and thread management, and removed unused async/future initialization code. [1] [2]

@@ -0,0 +1,19 @@
plugins {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old automq-shell will be deprecated in future. So keep the new module as other kafka modules

Copy link
Collaborator

@superhx superhx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@woshigaopp woshigaopp requested a review from Copilot November 14, 2025 08:07
Copilot finished reviewing on behalf of woshigaopp November 14, 2025 08:09
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request introduces comprehensive OpenTelemetry and S3 log upload support for Kafka Connect workers, enabling unified observability across the AutoMQ Kafka ecosystem. The changes refactor telemetry infrastructure into reusable modules and integrate them with Connect's runtime.

Key changes:

  • New automq-metrics and automq-log-uploader modules for reusable telemetry functionality
  • OpenTelemetryMetricsReporter for Connect metrics export (Prometheus, OTLP, S3)
  • S3 log upload integration with leader-based coordination
  • AZ-aware client configuration for optimized cluster topology

Reviewed Changes

Copilot reviewed 90 out of 95 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
connect/runtime/src/main/java/org/apache/kafka/connect/automq/metrics/OpenTelemetryMetricsReporter.java Implements MetricsReporter bridge between Kafka Connect and OpenTelemetry with comprehensive metric type detection
connect/runtime/src/main/java/org/apache/kafka/connect/automq/log/ConnectLogUploader.java Initializes S3 log uploader for Connect workers with config provider integration
connect/runtime/src/main/java/org/apache/kafka/connect/automq/az/AzAwareClientConfigurator.java Adds availability-zone metadata to client configurations for rack-aware operations
tests/kafkatest/tests/connect/connect_distributed_test.py Adds comprehensive integration tests for OpenTelemetry metrics and S3 exports
core/src/main/java/kafka/server/TelemetrySupport.java Refactors telemetry initialization into reusable helper for broker/controller
build.gradle Adds new automq-metrics and automq-log-uploader modules with proper dependencies
bin/kafka-run-connect-class.sh New script for Connect-specific classpath management excluding heavy dependencies
config/connect-log4j.properties Switches to S3RollingFileAppender with ConnectS3LogConfigProvider
Comments suppressed due to low confidence (2)

tests/kafkatest/tests/connect/templates/connect-distributed.properties:84

  • Lines 74, 77, and 81 contain Chinese characters that appear to be comment placeholders or encoding issues. These should be replaced with proper English comments.
    automq-metrics/src/main/java/com/automq/opentelemetry/yammer/YammerMetricsProcessor.java:163
  • [nitpick] The documentation references a LinkedIn Cruise Control issue URL which may not be accessible or relevant to AutoMQ users. Consider replacing with a more general explanation or an internal documentation link.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +52 to +54
private static Logger getLogger() {
return LoggerFactory.getLogger(AbstractConnectCli.class);
}
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The logger is converted from a static final field to a static method without clear justification. This adds unnecessary method call overhead for every logging operation. If the intent is to defer logger initialization, consider using lazy initialization pattern or document the reasoning for this change.

Copilot uses AI. Check for mistakes.
if (!isBlank(env)) {
return env.trim();
}
String host = workerProps.getProperty("automq.log.s3.node.hostname");
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The configuration key automq.log.s3.node.hostname is used but not documented in LogConfigConstants or mentioned in the README. This undocumented configuration option should either be added to the constants class or documented in the README configuration section.

Copilot uses AI. Check for mistakes.
@woshigaopp woshigaopp merged commit dd70821 into 1.6 Nov 14, 2025
6 checks passed
@woshigaopp woshigaopp deleted the feature/connect_new_1.6 branch November 14, 2025 09:19
woshigaopp added a commit that referenced this pull request Nov 14, 2025
Extract metrics and logging features as standalone modules.
woshigaopp added a commit that referenced this pull request Nov 14, 2025
…3012)

Extract metrics and logging features as standalone modules.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants