Add connection identifier to log lines for multi-client applications#63
Draft
Add connection identifier to log lines for multi-client applications#63
Conversation
Co-authored-by: baelter <1399369+baelter@users.noreply.github.com>
Co-authored-by: baelter <1399369+baelter@users.noreply.github.com>
Co-authored-by: baelter <1399369+baelter@users.noreply.github.com>
Co-authored-by: baelter <1399369+baelter@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add identifier to distinguish client log entries
Add connection identifier to log lines for multi-client applications
Oct 15, 2025
carlhoerberg
requested changes
Dec 15, 2025
| private def connection_identifier(io) : String | ||
| case io | ||
| when TCPSocket | ||
| io.local_address.port.to_s |
Member
There was a problem hiding this comment.
I guess this makes sense in LavinMQ, but from a clients perspective, remote_address.to_s would make more sense. Maybe users should be over to override it?
Suggested change
| io.local_address.port.to_s | |
| io.remote_address.to_s |
| # | ||
| # The *reason* might be logged by the server | ||
| def close(reason = "", no_wait = false) | ||
| Log.context.set connection_id: @connection_id |
Member
There was a problem hiding this comment.
this sets the metadata for the rest of the calling fiber's lifetime. You probably want to use Log.with_context(b: 2) do, that resets the metadata after the block
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When running multiple AMQP clients in a single application, it was impossible to distinguish which log lines belonged to which connection. This made debugging and monitoring difficult in production environments with multiple concurrent connections.
Solution
Added a unique connection identifier to all log lines using Crystal's
Log.contextfeature. The identifier is:54321)Using the local port number (as suggested in #issue-comment) provides an excellent identifier because:
Implementation Details
The connection identifier is:
@connection_idinstance variableread_loop)unsafe_write)close)Example Output
Before:
After (with two connections):
Testing
@connection_idis properly setexamples/connection_identifier_test.cr) demonstrating the feature with multiple connectionsFixes #[issue-number]
Original prompt
Fixes #62
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.