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/16 08:51:55 UTC
[PR] [SPARK-47596][DSTREAMS] Streaming: Migrate logWarn with variables to structured logging framework [spark]
panbingkun opened a new pull request, #46079:
URL: https://github.com/apache/spark/pull/46079
### What changes were proposed in this pull request?
### Why are the changes needed?
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Pass GA.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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] [WIP][SPARK-47596][DSTREAMS] Streaming: Migrate logWarn with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46079:
URL: https://github.com/apache/spark/pull/46079#discussion_r1570335191
##########
streaming/src/main/scala/org/apache/spark/streaming/dstream/DStream.scala:
##########
@@ -884,14 +885,16 @@ abstract class DStream[T: ClassTag] (
val alignedToTime = if ((toTime - zeroTime).isMultipleOf(slideDuration)) {
toTime
} else {
- logWarning(s"toTime ($toTime) is not a multiple of slideDuration ($slideDuration)")
+ logWarning(log"toTime (${MDC(TO_TIME, toTime)}) is not a multiple of slideDuration " +
+ log"(${MDC(SLIDE_DURATION, slideDuration)})")
Review Comment:
The output time units are `milliseconds`.
--
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-47596][DSTREAMS] Streaming: Migrate logWarn with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #46079:
URL: https://github.com/apache/spark/pull/46079#issuecomment-2065758227
@panbingkun Thanks again for the works! 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-47596][DSTREAMS] Streaming: Migrate logWarn with variables to structured logging framework [spark]
Posted by "smileyboy2019 (via GitHub)" <gi...@apache.org>.
smileyboy2019 commented on PR #46079:
URL: https://github.com/apache/spark/pull/46079#issuecomment-2068510767
Structured Streaming supports writing Spark SQL and using SQL to write stream processing logic. Is this possible, similar to the syntax of Flink. SQL can satisfy the flow processing process.
--
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-47596][DSTREAMS] Streaming: Migrate logWarn with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #46079:
URL: https://github.com/apache/spark/pull/46079#issuecomment-2063629099
cc @gengliangwang
--
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-47596][DSTREAMS] Streaming: Migrate logWarn with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #46079:
URL: https://github.com/apache/spark/pull/46079#discussion_r1571792309
##########
streaming/src/main/scala/org/apache/spark/streaming/scheduler/ReceivedBlockTracker.scala:
##########
@@ -247,7 +247,8 @@ private[streaming] class ReceivedBlockTracker(
true
} catch {
case NonFatal(e) =>
- logWarning(s"Exception thrown while writing record: $record to the WriteAheadLog.", e)
+ logWarning(log"Exception thrown while writing record: " +
+ log"${MDC(RECEIVED_BLOCK_TRACKER_LOG_EVENT, record)} to the WriteAheadLog.", e)
Review Comment:
Using the class name `RECEIVED_BLOCK_TRACKER_LOG_EVENT` is also fine
--
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-47596][DSTREAMS] Streaming: Migrate logWarn with variables to structured logging framework [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang closed pull request #46079: [SPARK-47596][DSTREAMS] Streaming: Migrate logWarn with variables to structured logging framework
URL: https://github.com/apache/spark/pull/46079
--
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] [WIP][SPARK-47596][DSTREAMS] Streaming: Migrate logWarn with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46079:
URL: https://github.com/apache/spark/pull/46079#discussion_r1570330149
##########
streaming/src/main/scala/org/apache/spark/streaming/scheduler/ReceivedBlockTracker.scala:
##########
@@ -247,7 +247,8 @@ private[streaming] class ReceivedBlockTracker(
true
} catch {
case NonFatal(e) =>
- logWarning(s"Exception thrown while writing record: $record to the WriteAheadLog.", e)
+ logWarning(log"Exception thrown while writing record: " +
+ log"${MDC(RECEIVED_BLOCK_TRACKER_LOG_EVENT, record)} to the WriteAheadLog.", e)
Review Comment:
I'm not sure if it's called `RECORD`, maybe it's more `general`.
--
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] [WIP][SPARK-47596][DSTREAMS] Streaming: Migrate logWarn with variables to structured logging framework [spark]
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #46079:
URL: https://github.com/apache/spark/pull/46079#discussion_r1570306473
##########
streaming/src/main/scala/org/apache/spark/streaming/dstream/InputDStream.scala:
##########
@@ -91,8 +93,8 @@ abstract class InputDStream[T: ClassTag](_ssc: StreamingContext)
} else {
// Time is valid, but check it is more than lastValidTime
if (lastValidTime != null && time < lastValidTime) {
- logWarning(s"isTimeValid called with $time whereas the last valid time " +
- s"is $lastValidTime")
+ logWarning(log"isTimeValid called with ${MDC(TIME, time)} whereas the last valid time " +
+ log"is ${MDC(LAST_VALID_TIME, lastValidTime)}")
Review Comment:
Their types are `Time`, so the output time units are `milliseconds`
--
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