Fix missing commas in logs

[ackintosh]

Issue Addressed

In beacon.log, commas are missing in case where multiple spans are present, for example:

Apr 05 07:03:07.646 DEBUG Sync's view on execution engine state updated  past_state: Online, new_state: Offline, service: "sync"service: "network_context"

A comma should be present between service: "sync" and service: "network_context" as shown in the example.

Proposed Changes

This PR:

• makes build_log_text() testable and adds unit tests for it
• fixes the missing commas issue

[macladson]

This has some of the same ideas as #7239, I'm wondering if you've had a chance to look at that PR? It would be good if we could make them compatible with each other.

[macladson]

I've written up a version of this PR on top of #7239
There's a few changes (commas are handled slightly differently and the span order for multiple spans is reversed).
I wanted to make sure you still get the credit for this fix so let me know what you think.

[mergify]

Hi @ackintosh, this pull request has been closed automatically due to 30 days of inactivity. If you’d like to continue working on it, feel free to reopen at any time.

[macladson]

Now that #7239 has been merged we can push this fix through. @ackintosh let me know if you want me to make the changes, or if you have time to fix it

[mergify]

This pull request has merge conflicts. Could you please resolve them @ackintosh? 🙏

[ackintosh]

@macladson Thanks for pointing that out! I believe the fix in this PR has already been addressed by #7239. On the current unstable branch, I checked the logs and confirmed that entries with multiple spans are now correctly formatted, with the missing commas resolved:

Jul 29 06:51:24.629 DEBUG Sync's view on execution engine state updated  past_state: Online, new_state: Online, service: "network_context", service: "sync"

[macladson]

Oh that's an interesting side-effect! I'll close this now then but we should keep an eye on the logs in case it still appears in very specific situations 👍