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

Add a connection listener that emits telemetry events #311

Merged
merged 6 commits into from
Jul 1, 2024

Conversation

v0idpwn
Copy link
Member

@v0idpwn v0idpwn commented Jun 24, 2024

Alternative to #310. Would have to write documentation, etc.

@v0idpwn v0idpwn force-pushed the telemetry-from-listener branch from eeb93f5 to 9a1783b Compare June 24, 2024 23:18
@v0idpwn v0idpwn requested a review from josevalim June 25, 2024 09:28
Copy link
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this! We need to improve the docs to say exactly how to use and the events emitted but it works for me!

@v0idpwn
Copy link
Member Author

v0idpwn commented Jun 27, 2024

I added two documentation sections. I'm afraid it's a bit hidden in the documentation, but the DBConnection module doc is already quite big :)

I also considered starting one of these on DBConnection supervision tree, with a name, to make it easier to configure (no need to start a process), but I guess it's not worth it, since most people won't use it.

Copy link
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One tiny comment and ship it!

@v0idpwn
Copy link
Member Author

v0idpwn commented Jul 1, 2024

Almost merged it as DbConnection instead of DBConnection 😄

@v0idpwn v0idpwn merged commit f0bbc4b into master Jul 1, 2024
2 checks passed
@v0idpwn v0idpwn deleted the telemetry-from-listener branch July 1, 2024 15:01
@v0idpwn
Copy link
Member Author

v0idpwn commented Jul 1, 2024

@josevalim I would love if this could be released soon, it would help us at @coingaming to improve an alarm.

@josevalim
Copy link
Member

Done!

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

Successfully merging this pull request may close these issues.

2 participants