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

statsd config #779

Merged
merged 1 commit into from
Dec 6, 2023
Merged

statsd config #779

merged 1 commit into from
Dec 6, 2023

Conversation

dpep
Copy link
Collaborator

@dpep dpep commented Dec 4, 2023

following up on our discussion in #778 to enable

Flipper.configure do |conf|
  conf.statsd = my_client
end

this would simplify statsd integration and abstract away the subscriber logic. I don't love the embedded requires and have an idea to rework that if we'd like. we may want to put ActiveSupport::Notifications.subscribe into a method and pair it with a way to unsubscribe, so that it can be done multiple times (especially for testing)

Copy link
Collaborator

@jnunemaker jnunemaker left a comment

Choose a reason for hiding this comment

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

Works for me. Thanks for making this easier for people.

@jnunemaker jnunemaker merged commit 4c3821a into main Dec 6, 2023
64 checks passed
@jnunemaker jnunemaker deleted the statsd-config branch December 6, 2023 13:23
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