Skip to content
This repository was archived by the owner on Dec 18, 2019. It is now read-only.

Prints graphite url in nagios dashboard for better visibility#13

Open
sanoojm wants to merge 2 commits into
etsy:masterfrom
sanoojm:master
Open

Prints graphite url in nagios dashboard for better visibility#13
sanoojm wants to merge 2 commits into
etsy:masterfrom
sanoojm:master

Conversation

@sanoojm

@sanoojm sanoojm commented Oct 13, 2017

Copy link
Copy Markdown

No description provided.

@sanoojm

sanoojm commented Mar 25, 2019

Copy link
Copy Markdown
Author

@avleen @bugi @edgewortheric @jedi4ever please review.

Comment thread check_graphite_data
print 'Current value: %s, lower band: %s, upper band: %s' % \
(graphite_data, graphite_lower, graphite_upper)
print 'Current value: %s, lower band: %s, upper band: %s, url: %s' % \
(graphite_data, graphite_lower, graphite_upper, url)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not currently using nagios, but doesn't the UI allow you to view the command used to run a check, and thus would include the URL? I feel like this adds visual clutter that isn't necessarily useful for everyone who uses this script, so I'd be more inclined to sign off on the change if the display of the URL were optional. (Mind you, I don't think I have commit grants on master, so I'm not sure how much my opinion matters here 😄 )

@bugi

bugi commented Mar 25, 2019

Copy link
Copy Markdown
Contributor

I agree with edgewortheric about clutter. To avoid that part of the display getting too busy, I'd put the url on a second line. IIRC, the classic nagios service dashboard would show the first line of output. Then to see second and later lines the user could click through for more details.

It's been quite awhile since I've used check_graphite_data.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants