Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flow.log, possibly flow.async et al: Shorten Async_file_logger thread name (+other thread-name work?). #71

Open
ygoldfeld opened this issue Mar 10, 2024 · 0 comments
Labels
bug Something isn't working from-akamai-pre-open Issue origin is Akamai, before opening source

Comments

@ygoldfeld
Copy link
Contributor

Filed by @echan-dev pre-open-source:

Async_file_logger appends the address of the logger to the a prefix result in a thread name of the format "Async_file_logger[POINTER_ADDRESS]", where POINTER_ADDRESS is the hexadecimal address of the object. As a result, the thread is shown as something like "0x558e341dc010]" due to truncation to 15 characters, which is the maximum thread name limit. We should rename it to be something shorter, possibly "async_file_log".


Addendum by @ygoldfeld:

Indeed, unfortunate. Some minor clarifications:

  • There are a couple things going on -- when naming a thread or thread pool in flow.async, one passes a String_view nickname.
    • It actually calls another public flow.log API; this can be called by itself too; e.g., various programs do it to nickname the native main thread spawned by OS.
  • The nickname is then printed in all flow.log lines in that thread (at least, in existing loggers; generally a Logger impl can do whatever it wants, but as of this writing all existing ones use Ostream_log_msg_writer, which as of this writing prints various stuff including thread ID or nickname (the latter if set else the former).
  • Also, a native Linux API is called whenever this nickname is set; that's the thing that shows up in top and various other useful utilities. That one is 15 characters max. If nickname is >15 characters, we truncate it before calling the native API. The general approach is to truncate it from the front, because the differentiating thing is often at the end, not the start. But, if something - like Async_file_logger - happens to shove the object address into the nickname, then this truncation is counterproductive.

Would suggesting doing something like:

  • At a minimum, reverse the order of things in Async_file_logger nickname; as in its case "Async_file_logger" part is more useful than the hex addr.
  • Then, go through all threads started in Flow - from memory there are very few of those actually, Async_file_logger being a prominent one - and do whatever makes sense for each one.
  • Repeat for Flow-IPC.

That said, the following might make sense:

  • Look into providing an OS-thread-nickname arg, perhaps added in a backward-compatible way, for flow.async and the flow.log thread-namer APIs. The native thread name, in Linux at least, is quite commonly used in debugging, so it would make sense to focus on it a little more. (Flow is open-source and could be extended to macOS/BSD/Darwin/whatever, possibly Windows - an earlier version ran on Android and Windows and iOS for example - so a glance at other OS' thread-naming APIs/length limitations would be worthwhile too.) If not provided then use the long name + truncation like now; otherwise use this particular OS one (or maybe do so in those OS where there is a length limitation).
  • There is or will be a ticket about normalizing ostream-output operators in Flow-IPC and Flow; some conventions like maybe hex address first, use certain formatting... whatever. Often (not always) thread nickname is taken from that. So thinking through the decisions in that work, versus this work, would make sense.

Anyway! If we're going to fix this up to be nice, might as well really consider it. But even without that, Async_file_logger thread name is a pretty easy one-liner fix (probably hex address first, something like Async_file_log last).

@ygoldfeld ygoldfeld added bug Something isn't working from-akamai-pre-open Issue origin is Akamai, before opening source labels Mar 10, 2024
@ygoldfeld ygoldfeld changed the title flow.log, possibly flow.async et al: Shorten Async_file_logger thread name. flow.log, possibly flow.async et al: Shorten Async_file_logger thread name (+other thread-name work?). Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working from-akamai-pre-open Issue origin is Akamai, before opening source
Projects
None yet
Development

No branches or pull requests

1 participant