Skip to content

feat: add spdlog dependency to implement logging#140

Merged
Xuanwo merged 1 commit into
apache:mainfrom
wgtmac:logger
Jul 14, 2025
Merged

feat: add spdlog dependency to implement logging#140
Xuanwo merged 1 commit into
apache:mainfrom
wgtmac:logger

Conversation

@wgtmac

@wgtmac wgtmac commented Jul 5, 2025

Copy link
Copy Markdown
Member

Added spdlog as a third-party dependency to implement logger support. My plan is to add a logger interface so we don't expose spdlog symbols externally (just like what we did with nanoarrow and nlohman-json).

@Kelvinyu1117

Kelvinyu1117 commented Jul 6, 2025

Copy link
Copy Markdown

I would like to suggest an alternative:
https://github.com/odygrd/quill

I used this logger professionally for ultra low latency application, maybe we can have a look.

@wgtmac

wgtmac commented Jul 6, 2025

Copy link
Copy Markdown
Member Author

Perhaps we can design a pluggable logger interface so that downstream projects can plug in their own logger of choice.

@lidavidm lidavidm left a comment

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.

Adding the logger interface makes sense. I'm not sure this library will log so much that logger performance will be a concern?

@zhjwpku

zhjwpku commented Jul 7, 2025

Copy link
Copy Markdown
Collaborator

+1 for adding the logger interface.
It seems easier for iceberg, but for iceberg-bundle, the logger needs to handle not just iceberg but also libraries like Arrow, Avro, and others that have their own logging interfaces.

@yingcai-cy

Copy link
Copy Markdown
Contributor

+1 to pluggable logging interface,I'll take a look into this later.

@Xuanwo Xuanwo left a comment

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.

Thank you for working on this!

@Xuanwo Xuanwo merged commit be514fc into apache:main Jul 14, 2025
7 checks passed
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.

7 participants