-
Notifications
You must be signed in to change notification settings - Fork 158
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
deprecate nagios monitor #5172
deprecate nagios monitor #5172
Conversation
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.
@pauljwil I think we need corresponding deprecation warnings on the docs https://docs.splunk.com/observability/en/gdi/monitors-monitoring/nagios.html
Unless @atoulme is tracking this some other way.
@@ -68,6 +68,8 @@ var ( | |||
// Configure and kick off internal metric collection | |||
func (m *Monitor) Configure(conf *Config) error { | |||
m.logger = logrus.WithFields(logrus.Fields{"monitorType": monitorType, "monitorID": conf.MonitorID}) | |||
m.logger.Warn("[NOTICE] The nagios monitor is deprecated and will be removed in a future release.") |
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 believe we use [WARNING]
in other places. Maybe better to be consistent if I'm not missing other places where we do [NOTICE]
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.
it's already logging at WARN level and therefore will show as a warning. We use [NOTICE] for all deprecation notices so far.
You are correct that we use [WARNING], but only in the concept of config converters where we use log.Println:
https://github.com/search?q=repo%3Asignalfx%2Fsplunk-otel-collector+%5BWARNING%5D&type=code
8f438d7
to
fc9d93a
Compare
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.
@aurbiztondo-splunk not sure if you recall but here is another deprecation that we did https://github.com/splunk/private-o11y-docs/pull/1699 - I am not sure if in the present case we go with a precise date as in the link. |
Thanks @pjanotti - will check with Aunsh as well! Docs updated in https://github.com/splunk/private-o11y-docs/pull/2292 |
Description:
Deprecate nagios monitor.