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

Init passenger plugin #522

Closed
wants to merge 1 commit into from
Closed

Init passenger plugin #522

wants to merge 1 commit into from

Conversation

v9n
Copy link
Contributor

@v9n v9n commented Jan 15, 2016

Need unit test and document. Will add later

@v9n v9n mentioned this pull request Jan 15, 2016
@@ -46,6 +46,7 @@ github.com/wvanbergen/kafka 1a8639a45164fcc245d5c7b4bd3ccfbd1a0ffbf3
github.com/wvanbergen/kazoo-go 0f768712ae6f76454f987c3356177e138df258f8
golang.org/x/crypto 3760e016850398b85094c4c99e955b8c3dea5711
golang.org/x/net 99ca920b6037ef77af8a11297150f7f0d8f4ef80
golang.org/x/text cf4986612c83df6c55578ba198316d1684a9a287
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This package is need to parse the XML with a a charset reader. The XML output has an attribute encoding=ISO88591 and I cannot simply parse it with xml.Unmarshal because go lag complains about it like this:

xml: encoding "iso8859-1" declared but Decoder.CharsetReader is nil

However, If I simply remove that attribute from XML then it works just fine. I'm not sure if we should just stripped out that attribute to remove this dependency

Copy link
Contributor

Choose a reason for hiding this comment

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

no problem, thanks for adding it to Godeps

@v9n
Copy link
Contributor Author

v9n commented Jan 15, 2016

@sparrc I added the doc, and unit test. Also I added some comment in the code so that you can review

"get_wait_list_size": sg.Get_wait_list_size,
"capacity_used": sg.Capacity_used,
}
acc.AddFields("passenger", fields, tags)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this have a different name? maybe passenger_supergroup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I used passenger_supergroup, map them directly to the xml so user can guess the metric easily.

@sparrc
Copy link
Contributor

sparrc commented Jan 17, 2016

lgtm, thanks @kureikain, is this ready to merge?

Gather metric by parsing XMLoutput of `passenger-status` utility.
More information of this utility:
https://www.phusionpassenger.com/library/admin/apache/overall_status_report.html
@v9n
Copy link
Contributor Author

v9n commented Jan 17, 2016

Yes, I rebased it with current master.

@sparrc sparrc closed this in 1388b1b Jan 18, 2016
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