Skip to content
This repository was archived by the owner on Nov 30, 2020. It is now read-only.

Implement std::fmt::Debug for eventstore::Connection #36

Open
wingertge opened this issue Jan 9, 2020 · 7 comments
Open

Implement std::fmt::Debug for eventstore::Connection #36

wingertge opened this issue Jan 9, 2020 · 7 comments

Comments

@wingertge
Copy link

I'm trying to use the tracing crate to monitor my functions that access eventstore and detect any potential bottlenecks, but its instrumentation requires all parameters to implement fmt::Debug. Connection doesn't implement that right now, forcing me to wrap and unwrap the connection to enable proper instrumentation. Implementing Debug on the Connection struct would solve this issue.

I'm not making a pull request because I'm not sure about the internals of the crate, so I don't know what should be output in the Debug fmt.

@YoEight
Copy link
Owner

YoEight commented Jan 9, 2020

Hi @wingertge,

Thanks for reporting this issue. I see nothing preventing an std::fmt::Debug implementation. I can cut a new release ASAP if needed.

@wingertge
Copy link
Author

The project is still in very early stages and have a workaround for now so no rush. I'm waiting for futures-0.3 update as well (tried updating it myself but I'm not super familiar with Rust yet and got hung up on lifetime stuff). So as long as that change is in the futures-0.3 release I'm good.

@YoEight
Copy link
Owner

YoEight commented Jan 10, 2020

I’m working on the futures 0.3 migration. Still a lot to do but at least, I have no blocker, unlike 2 weeks ago.

@wingertge
Copy link
Author

Just a quick update, I'm not sure if this is possible with the way the architecture works, but if the Debug output could log the connection type (single/cluster), as well as the SocketAddr of the current node, that would be ideal. Knowing the exact server can really help for tracing in distributed services.

If I remember the code correctly this might mean implementing Debug for the Driver as well.

@YoEight
Copy link
Owner

YoEight commented Jan 21, 2020

I could easily expose if a connection is in single-node or cluster mode. However, getting the current SocketAddr could be challenging. Will see.

@wingertge
Copy link
Author

I can also just do a quick PR once you're done with new-futures. Btw I'm already using that branch (I don't use subscriptions atm) and it's working perfectly in a real world scenario 👍

@YoEight
Copy link
Owner

YoEight commented Jan 25, 2020

@wingertge new-futures got merged. Feel free to propose a patch or give me a list of what you'll like to see in the Connection itself.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants