Skip to content

Comments

[SPARK-47590][SQL] Hive-thriftserver: Migrate logWarn with variables to structured logging framework#45923

Closed
itholic wants to merge 11 commits intoapache:masterfrom
itholic:hive-ts-logwarn
Closed

[SPARK-47590][SQL] Hive-thriftserver: Migrate logWarn with variables to structured logging framework#45923
itholic wants to merge 11 commits intoapache:masterfrom
itholic:hive-ts-logwarn

Conversation

@itholic
Copy link
Contributor

@itholic itholic commented Apr 8, 2024

What changes were proposed in this pull request?

This PR proposes to migrate logWarning with variables of Hive-thriftserver module to structured logging framework.

Why are the changes needed?

To improve the existing logging system by migrating into structured logging.

Does this PR introduce any user-facing change?

No API changes, but the SQL catalyst logs will contain MDC(Mapped Diagnostic Context) from now.

How was this patch tested?

Run Scala auto formatting and style check. Also the existing CI should pass.

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

No.

@itholic
Copy link
Contributor Author

itholic commented Apr 8, 2024

cc @gengliangwang as you're working on logError for Hive-thriftserver IIRC.

Also cc @panbingkun @pan3793

@pan3793
Copy link
Member

pan3793 commented Apr 8, 2024

LGTM. Actually, there are lot of Java code copied from Hive in this module, I'm not sure if we want to do some refactoring to adapt the structured logging framework.

itholic and others added 3 commits April 16, 2024 08:59
…/thriftserver/SparkSQLCLIDriver.scala

Co-authored-by: Gengliang Wang <gengliang@apache.org>
…/thriftserver/SparkSQLCLIDriver.scala

Co-authored-by: Gengliang Wang <gengliang@apache.org>
…/thriftserver/ui/HiveThriftServer2Listener.scala

Co-authored-by: Gengliang Wang <gengliang@apache.org>
@gengliangwang
Copy link
Member

Thanks, merging to master

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.

4 participants