-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
adding tomcat plugin #3112
adding tomcat plugin #3112
Conversation
plugins/inputs/tomcat/README.md
Outdated
[[inputs.tomcat]] | ||
# A Tomcat status URI to gather stats. | ||
# Default is "http://127.0.0.1:8080/manager/status/all?XML=true". | ||
url = http://127.0.0.1:8080/manager/status/all?XML=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.
Quote string
plugins/inputs/tomcat/tomcat.go
Outdated
var sampleconfig = ` | ||
## A Tomcat status URI to gather stats. | ||
## Default is "http://127.0.0.1:8080/manager/status/all?XML=true". | ||
url = http://127.0.0.1:8080/manager/status/all?XML=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.
Quote string
plugins/inputs/tomcat/tomcat.go
Outdated
## Cedentials for status URI. | ||
## Default is tomcat/s3cret. | ||
username = "tomcat" | ||
password = "s3cret" |
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.
Fix up this indention because it is used directly in the sample config output. You can test with either of these commands:
telegraf --usage tomcat
telegraf config
plugins/inputs/tomcat/tomcat.go
Outdated
## A Tomcat status URI to gather stats. | ||
## Default is "http://127.0.0.1:8080/manager/status/all?XML=true". | ||
url = http://127.0.0.1:8080/manager/status/all?XML=true | ||
## Cedentials for status URI. |
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.
typo
plugins/inputs/tomcat/README.md
Outdated
# A Tomcat status URI to gather stats. | ||
# Default is "http://127.0.0.1:8080/manager/status/all?XML=true". | ||
url = http://127.0.0.1:8080/manager/status/all?XML=true | ||
# Cedentials for status URI. |
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.
typo
updated comments / corrected typo. |
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 just noticed we need to check the status code, also need to add to plugins/inputs/all/all.go
.
if err != nil { | ||
return fmt.Errorf("Unable to call URL '%s': %s", s.URL, err) | ||
} | ||
defer resp.Body.Close() |
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.
After checking for errors, we should make sure the response code was a 200 status and if not return an error.
@mlindes I'll will do the change to address my last comment tomorrow. |
Required for all PRs: