-
Notifications
You must be signed in to change notification settings - Fork 454
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
Make prometheus monitoring optional #29
Conversation
Pull Request Test Coverage Report for Build 293
💛 - Coveralls |
config/config.go
Outdated
@@ -89,6 +89,7 @@ func (c *Config) fillDefaultValues() { | |||
"pitaya.metrics.statsd.rate": 1, | |||
"pitaya.metrics.prometheus.port": 9090, | |||
"pitaya.metrics.tags": map[string]string{}, | |||
"pitaya.metrics.prometheus.enabled": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking of changing this default value to false and logging that monitoring is disabled... this would make development easier IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it enabled by default to avoid breaking existing applications, but I agree that leaving it disabled by default would avoid some pain when running locally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we just need to ensure to add the env var to turn it on on the current production applications and then it will be safe to turn it off by default :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabled it by default now
No description provided.