Skip to content

[SPARK-48623][CORE] Structured Logging Migrations#46980

Closed
asl3 wants to merge 19 commits into
apache:masterfrom
asl3:logging-migrationscala
Closed

[SPARK-48623][CORE] Structured Logging Migrations#46980
asl3 wants to merge 19 commits into
apache:masterfrom
asl3:logging-migrationscala

Conversation

@asl3

@asl3 asl3 commented Jun 13, 2024

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR migrates Scala logging to comply with the scala style changes in #46979

Why are the changes needed?

This makes development and PR review of the structured logging migration easier.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Tested by ensuring dev/scalastyle checks pass

Was this patch authored or co-authored using generative AI tooling?

No

@asl3 asl3 changed the title [DRAFT] Scala Structured Logging Migration [SPARK-48623][INFRA] Structured Logging Framework Scala Style Migration Jun 13, 2024
Comment thread common/utils/src/main/scala/org/apache/spark/util/MavenUtils.scala Outdated
Comment thread core/src/main/scala/org/apache/spark/broadcast/Broadcast.scala Outdated
Comment thread core/src/main/scala/org/apache/spark/deploy/ExternalShuffleService.scala Outdated
logError(s"Failed to delete temporary checksum file " +
s"at ${checksumTmp.getAbsolutePath}")
logError(log"Failed to delete temporary checksum file at " +
log"${MDC(LogKeys.FILE_ABSOLUTE_PATH, checksumTmp.getAbsolutePath)}")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

let's just use PATH

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see LogKey.scala has this key which is used in other files: case object FILE_ABSOLUTE_PATH extends LogKey

should we migrate these other occurrences to PATH ? @gengliangwang

Comment thread mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala Outdated
Comment thread mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala Outdated
Comment thread mllib/src/main/scala/org/apache/spark/ml/classification/LogisticRegression.scala Outdated
Comment thread scalastyle-config.xml Outdated
Comment thread resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala Outdated
Comment thread resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala Outdated
@github-actions github-actions Bot removed the BUILD label Jun 14, 2024
@asl3 asl3 requested a review from gengliangwang June 14, 2024 14:45
Comment thread streaming/src/main/scala/org/apache/spark/streaming/dstream/DStream.scala Outdated
Comment thread streaming/src/main/scala/org/apache/spark/streaming/dstream/DStream.scala Outdated
Comment thread common/utils/src/main/scala/org/apache/spark/internal/LogKey.scala Outdated
Comment thread common/utils/src/main/scala/org/apache/spark/internal/LogKey.scala Outdated
Comment thread common/utils/src/main/scala/org/apache/spark/internal/LogKey.scala Outdated
Comment thread common/utils/src/main/scala/org/apache/spark/internal/LogKey.scala Outdated
Comment thread common/utils/src/main/scala/org/apache/spark/internal/LogKey.scala Outdated
Comment thread common/utils/src/main/scala/org/apache/spark/internal/LogKey.scala Outdated
Comment thread common/utils/src/main/scala/org/apache/spark/internal/LogKey.scala Outdated
Comment thread common/utils/src/main/scala/org/apache/spark/internal/LogKey.scala Outdated
@gengliangwang

Copy link
Copy Markdown
Member

Thanks, merging to master

@gengliangwang gengliangwang changed the title [SPARK-48623][INFRA] Structured Logging Framework Scala Style Migration [SPARK-48623][CORE] Structured Logging Migrations Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants