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.

Sign up for GitHub

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

Jump to bottom

Network-level telemetry via sink stats #203

Merged
merged 5 commits into from
Mar 21, 2024
Merged

Network-level telemetry via sink stats #203

merged 5 commits into from
Mar 21, 2024

Conversation

mlowicki
Copy link
Contributor

@mlowicki mlowicki commented Mar 18, 2024 •
edited

@mlowicki mlowicki mentioned this pull request Mar 18, 2024
Closed
56quarters
56quarters reviewed Mar 19, 2024
View reviewed changes
Copy link
Owner

@56quarters 56quarters left a comment •
edited

Choose a reason for hiding this comment

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

This looks pretty good to me. Nicely done adding the counters without needing to go through the Mutex around the writer.

cadence/src/sinks/unix.rs Outdated Show resolved Hide resolved
cadence/src/sinks/unix.rs Outdated Show resolved Hide resolved
@mlowicki mlowicki force-pushed the mlowicki/network_level_telemetry branch from 4c0ff34 to a991935 Compare March 19, 2024 14:45
@mlowicki mlowicki marked this pull request as ready for review March 19, 2024 14:47
56quarters
56quarters reviewed Mar 19, 2024
View reviewed changes
Copy link
Owner

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

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

Very nice, just a few small changes.

cadence/src/sinks/core.rs Outdated Show resolved Hide resolved
cadence/src/sinks/udp.rs Outdated Show resolved Hide resolved
cadence/src/sinks/core.rs Outdated Show resolved Hide resolved
56quarters
56quarters approved these changes Mar 20, 2024
View reviewed changes
Copy link
Owner

@56quarters 56quarters left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

cadence/src/sinks/core.rs Show resolved Hide resolved
@56quarters 56quarters merged commit 4c147ad into 56quarters:master Mar 21, 2024
6 checks passed
@mlowicki
Copy link
Contributor Author

LGTM, thanks!

thanks! any plans to release new version anytime soon?

@56quarters
Copy link
Owner

LGTM, thanks!

thanks! any plans to release new version anytime soon?

Yep, probably in the next few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@56quarters 56quarters 56quarters approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mlowicki @56quarters