Basic spdlog integration

tenstorrent / tt-umd

User-Mode Driver for Tenstorrent hardware

Apache License 2.0

9 stars 5 forks source link

Basic spdlog integration #176

Open joelsmithTT opened 3 weeks ago

joelsmithTT commented 3 weeks ago

Example of how it logs:

[2024-10-16 13:11:21.779] [info] [pci_device.cpp:362] Opened device /dev/tenstorrent/0
[2024-10-16 13:11:21.780] [info] [pci_device.cpp:400] Closed device /dev/tenstorrent/0

joelsmithTT commented 3 weeks ago

This is not intended to be merged, but to gather some input about how we want to use this.

Do we care about emitting a log file? spdlog can 'sink' to both stderr/stdout and a log file. The file can be rotated. I don't think we need an actual file at this point... stderr/stdout is fine.
Probably want some environment variables to control the behavior (log level threshold for what is emitted, possibly the formatting string. How is this invoked? Upon first message, or explicitly?
If spdlog use grows beyond UMD, do we want something that says "UMD" in every message that we emit (at least to stdout/stderr) for filtering purposes? (If we write to our own file, this isn't needed)
Existing logger have a notion of type - Metal has a bunch, UMD is almost all "LogSiliconDriver" although the logging macros are inconsistent here. I don't think we need this in the way Metal is using it.

blozano-tt commented 3 weeks ago

A log file for reviewing program trace, can be invaluable, especially debugging release builds.

blozano-tt commented 3 weeks ago

If spdlog use grows beyond UMD, do we want something that says "UMD" in every message that we emit (at least to stdout/stderr) for filtering purposes? (If we write to our own file, this isn't needed)

Yes, we should have an identifier, if nothing else to differentiate our log messages from other things that are printed to the screen.

blozano-tt commented 3 weeks ago

Existing logger have a notion of type - Metal has a bunch, UMD is almost all "LogSiliconDriver" although the logging macros are inconsistent here. I don't think we need this in the way Metal is using it.

I've not wrapped my head around why they do this yet.

pjanevskiTT commented 3 weeks ago

Existing logger have a notion of type - Metal has a bunch, UMD is almost all "LogSiliconDriver" although the logging macros are inconsistent here. I don't think we need this in the way Metal is using it.

Just to add, I think this is intended to be used in a same way that you are suggesting UMD messages should have something that says that message comes from UMD. I don't know how tt-metal uses it, but in Buda this made sense, since you had a lot of compilation components (compute, data movement) and messsages were clearly stating from which component the message came. That was original usage I think

Generally, spdlog looks cleaner to me as well than the logger we use right now, but I don't have strong opinions on it. I guesss @blozano-tt is the best person for these questions, in terms of what customers want to see, consistency across repos etc...