You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "panbingkun (via GitHub)" <gi...@apache.org> on 2024/04/04 08:23:12 UTC
[PR] [SPARK-47593][CORE] Connector module: Migrate logWarn with variables to structured logging framework [spark]
panbingkun opened a new pull request, #45877:
URL: https://github.com/apache/spark/pull/45877
### What changes were proposed in this pull request?
### Why are the changes needed?
### Does this PR introduce _any_ user-facing change?
### How was this patch tested?
### Was this patch authored or co-authored using generative AI tooling?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47592][CORE] Connector module: Migrate logError with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #45877:
URL: https://github.com/apache/spark/pull/45877#discussion_r1554370875
##########
common/utils/src/main/scala/org/apache/spark/internal/LogKey.scala:
##########
@@ -43,20 +43,24 @@ object LogKey extends Enumeration {
val EXECUTOR_ID = Value
val EXECUTOR_STATE_CHANGED = Value
val EXIT_CODE = Value
+ val FILE_NAME = Value
Review Comment:
Sure
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47592][CORE] Connector module: Migrate logError with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #45877:
URL: https://github.com/apache/spark/pull/45877#issuecomment-2037849333
@panbingkun This task should cover the whole connector directory.
```
avro/src/main/scala/org/apache/spark/sql/v2/avro/AvroPartitionReaderFactory.scala: logError("Exception while opening DataFileReader", e)
avro/src/main/scala/org/apache/spark/sql/avro/AvroFileFormat.scala: logError("Exception while opening DataFileReader", e)
connect/server/src/test/scala/org/apache/spark/sql/connect/ProtoToParsedPlanTestSuite.scala: logError(s"Skipping $fileName")
connect/server/src/main/scala/org/apache/spark/sql/connect/utils/ErrorUtils.scala: logError(
connect/server/src/main/scala/org/apache/spark/sql/connect/service/SparkConnectServer.scala: logError("Error starting Spark Connect server", e)
docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/DockerJDBCIntegrationSuite.scala: logError(s"Failed to initialize Docker container for ${this.getClass.getName}", e)
kafka-0-10/src/test/scala/org/apache/spark/streaming/kafka010/DirectKafkaStreamSuite.scala: logError("commit failed", e)
kafka-0-10/src/test/scala/org/apache/spark/streaming/kafka010/KafkaTestUtils.scala: logError("Found a leak KafkaTestUtils.", exception)
kafka-0-10/src/main/scala/org/apache/spark/streaming/kafka010/KafkaUtils.scala: logError(s"${ConsumerConfig.GROUP_ID_CONFIG} is null, you should probably set it")
kafka-0-10/src/main/scala/org/apache/spark/streaming/kafka010/KafkaDataConsumer.scala: logError("Error closing oldest Kafka consumer", x)
kafka-0-10/src/main/scala/org/apache/spark/streaming/kafka010/KafkaRDD.scala: logError("Kafka ConsumerRecord is not serializable. " +
kafka-0-10/src/main/scala/org/apache/spark/streaming/kafka010/DirectKafkaInputDStream.scala: logError("Kafka ConsumerRecord is not serializable. " +
kafka-0-10-sql/src/test/scala/org/apache/spark/sql/kafka010/KafkaTestUtils.scala: logError("Found a leak KafkaTestUtils.", exception)
kafka-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/consumer/InternalKafkaConsumerPool.scala: logError(s"Error closing Kafka consumer", e)
kafka-0-10-sql/src/main/scala/org/apache/spark/sql/kafka010/KafkaSourceRDD.scala: logError("Kafka ConsumerRecord is not serializable. " +
kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisCheckpointer.scala: logError(s"Exception: WorkerId $workerId encountered an exception while checkpointing" +
kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisRecordProcessor.scala: logError(s"Exception: WorkerId $workerId encountered and exception while storing " +
kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisRecordProcessor.scala: logError(s"Retryable Exception: Random backOffMillis=${backOffMillis}", e)
kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisRecordProcessor.scala: logError(s"ShutdownException: Caught shutdown exception, skipping checkpoint.", e)
kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisRecordProcessor.scala: logError(s"InvalidStateException: Cannot save checkpoint to the DynamoDB table used" +
kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisRecordProcessor.scala: logError(s"Unexpected, non-retryable exception.", e)
profiler/src/main/scala/org/apache/spark/executor/profiler/ExecutorJVMProfiler.scala: logError("JVM profiling aborted. Exception occurred in profiler native code: ", e)
profiler/src/main/scala/org/apache/spark/executor/profiler/ExecutorJVMProfiler.scala: logError("Failed to start JVM profiler", e)
profiler/src/main/scala/org/apache/spark/executor/profiler/ExecutorJVMProfiler.scala: case e: IOException => logError("Exception occurred while writing some profiler output: ", e)
profiler/src/main/scala/org/apache/spark/executor/profiler/ExecutorJVMProfiler.scala: logError("Some profiler output not written." +
profiler/src/main/scala/org/apache/spark/executor/profiler/ExecutorJVMProfiler.scala: case e: Exception => logError("Some profiler output not written. Unexpected exception: ", e)
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47592][CORE] Connector module: Migrate logError with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang closed pull request #45877: [SPARK-47592][CORE] Connector module: Migrate logError with variables to structured logging framework
URL: https://github.com/apache/spark/pull/45877
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47592][CORE] Connector module: Migrate logError with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #45877:
URL: https://github.com/apache/spark/pull/45877#discussion_r1554278479
##########
common/utils/src/main/scala/org/apache/spark/internal/LogKey.scala:
##########
@@ -43,20 +43,24 @@ object LogKey extends Enumeration {
val EXECUTOR_ID = Value
val EXECUTOR_STATE_CHANGED = Value
val EXIT_CODE = Value
+ val FILE_NAME = Value
Review Comment:
Unify `FILE_NAME` with `PATH`?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47592][CORE] Connector module: Migrate logError with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #45877:
URL: https://github.com/apache/spark/pull/45877#issuecomment-2041004296
Thanks, merging to master
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47592][CORE] Connector module: Migrate logError with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #45877:
URL: https://github.com/apache/spark/pull/45877#discussion_r1554528200
##########
connector/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisRecordProcessor.scala:
##########
@@ -166,15 +168,16 @@ private[kinesis] object KinesisRecordProcessor extends Logging {
if numRetriesLeft > 1 =>
val backOffMillis = Random.nextInt(maxBackOffMillis)
Thread.sleep(backOffMillis)
- logError(s"Retryable Exception: Random backOffMillis=${backOffMillis}", e)
+ logError(log"Retryable Exception: Random " +
+ log"backOffMillis=${MDC(MAX_BETWEEN_RETRIES, backOffMillis)}", e)
Review Comment:
Done
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47592][CORE] Connector module: Migrate logError with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #45877:
URL: https://github.com/apache/spark/pull/45877#discussion_r1554530484
##########
common/utils/src/main/scala/org/apache/spark/internal/LogKey.scala:
##########
@@ -81,10 +83,13 @@ object LogKey extends Enumeration {
val REDUCE_ID = Value
val REMOTE_ADDRESS = Value
val RETRY_COUNT = Value
+ val RETRY_INTERVAL_MILLISECOND = Value
Review Comment:
Okay, This is what i want, done.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47592][CORE] Connector module: Migrate logError with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #45877:
URL: https://github.com/apache/spark/pull/45877#discussion_r1554528345
##########
common/utils/src/main/scala/org/apache/spark/internal/LogKey.scala:
##########
@@ -81,10 +83,13 @@ object LogKey extends Enumeration {
val REDUCE_ID = Value
val REMOTE_ADDRESS = Value
val RETRY_COUNT = Value
+ val RETRY_INTERVAL_MILLISECOND = Value
Review Comment:
As per https://github.com/apache/spark/pull/45903/, the default unit will be MILLISECOND. We can ignore it in the name.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47592][CORE] Connector module: Migrate logError with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #45877:
URL: https://github.com/apache/spark/pull/45877#issuecomment-2038341155
Oh, OMG,
I `misunderstood` Jira's description and mistook the `connector` for `connect`.
Okay, I'll update it.
The same goes for the other PR.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-47592][CORE] Connector module: Migrate logError with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #45877:
URL: https://github.com/apache/spark/pull/45877#discussion_r1554523229
##########
connector/kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisRecordProcessor.scala:
##########
@@ -166,15 +168,16 @@ private[kinesis] object KinesisRecordProcessor extends Logging {
if numRetriesLeft > 1 =>
val backOffMillis = Random.nextInt(maxBackOffMillis)
Thread.sleep(backOffMillis)
- logError(s"Retryable Exception: Random backOffMillis=${backOffMillis}", e)
+ logError(log"Retryable Exception: Random " +
+ log"backOffMillis=${MDC(MAX_BETWEEN_RETRIES, backOffMillis)}", e)
Review Comment:
One last comment: Let's rename this to make it more clear.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org