-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Rename Tomcat metrics, fix types and add help text #92
Conversation
convention of Prometheus, specifying type and help text
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.
Thanks for the PR. I don't think that listing every single mBean and attribute of interest is going to be very maintainable. Can you collapse it down to something more manageable?
example_configs/tomcat.yml
Outdated
- pattern: 'Catalina<type=GlobalRequestProcessor, name=\"(\w+-\w+)-(\d+)\"><>(\w+):' | ||
name: tomcat_$3_total | ||
|
||
whitelistObjectNames: ["Tomcat:*", "java.lang:*"] |
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.
Don't include java.lang, they're already taken care of.
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.
How are they taken care of?
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.
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.
Ok, I'll drop them all.
Some are important to me, but I understand it doesn't belong here.
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.
If there's some missing I'll take a PR over in client_java.
example_configs/tomcat.yml
Outdated
# maxTime | ||
- pattern: '^Tomcat<type=GlobalRequestProcessor, name=\"(.*)\"><>maxTime:' | ||
name: tomcat_global_processingtime_milliseconds_max | ||
help: 'Tomcat global maximum request processing time seen, in milliseconds.' |
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.
There was a recent change to allow multiplication, so you can convert this to seconds.
I doubt that max is the mertric you want though.
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.
Thanks for the reminder. I'll do the multiplication.
Do you mean the max is not very useful?
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.
Brian, the list is not exhaustive. Many were omitted, and others were just enumerated in comments. I also tried using regular expressions to catch many in one rule, but trying to balance terseness, useful help text, and maintainability. How do you propose to collapse it down? |
Help text isn't that important, 10ish general rules should be sufficient to cover most servers. |
example_configs/tomcat.yml
Outdated
# Number of active sessions at this moment (Tomcat<type=Manager, host=localhost, context=/><>activeSessions) | ||
- pattern: '^Tomcat<type=Manager, host=(.*), context=(.*)><>activeSessions:' | ||
type: GAUGE | ||
name: tomcat_sessions_active_total |
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.
_total
suffix should be reserved to counters.
example_configs/tomcat.yml
Outdated
# Total number of sessions created by this manager (Tomcat<type=Manager, host=localhost, context=/><>sessionCounter) | ||
- pattern: '^Tomcat<type=Manager, host=(.*), context=(.*)><>sessionCounter:' | ||
type: COUNTER | ||
name: tomcat_sessions_created |
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.
_total
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.
For the time being, I was using this configuration, maybe it can help.
Try to keep/add the _total
suffix for counters.
example_configs/tomcat.yml
Outdated
# Number of duplicated session ids generated (Tomcat<type=Manager, host=localhost, context=/><>duplicates) | ||
- pattern: '^Tomcat<type=Manager, host=(.*), context=(.*)><>duplicates:' | ||
type: COUNTER | ||
name: tomcat_sessions_duplicated |
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.
same _total
example_configs/tomcat.yml
Outdated
labels: | ||
context: $1 | ||
|
||
# ------------------------ java.lang ----------------------- |
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 not sure this should be hidden at the bottom of the tomcat example.
example_configs/tomcat.yml
Outdated
# ------------------------ WILDCARD ----------------------- | ||
|
||
# This ensures we export all metrics, even if one of the above regexes fail, or if new metrics are added. | ||
- pattern: ".*" |
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.
Same here, i'm 👎 having this here.
- pattern: '^Tomcat<type=StringCache><>cacheSize:' | ||
name: tomcat_stringcache_size | ||
valueFactor: 0.001 | ||
help: 'Tomcat: The number of entries in the string cache.' |
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.
What's the unit here? It's odd that a count would be divided by 1000
# Maximum processing time of a request (Tomcat<j2eeType=Servlet, WebModule=//localhost/, name=default, J2EEApplication=none, J2EEServer=none><>maxTime) | ||
# Minimum processing time of a request (Tomcat<j2eeType=Servlet, WebModule=//localhost/, name=default, J2EEApplication=none, J2EEServer=none><>minTime) | ||
- pattern: '^Tomcat<j2eeType=Servlet, WebModule=//localhost(.*), name=(.*), J2EEApplication=none, J2EEServer=none><>(max|min)Time:' | ||
name: tomcat_servlet_processing_$3_seconds |
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.
Be consistent, you've the min/max at the end above
type: COUNTER | ||
labels: | ||
context: $1 | ||
- pattern: '^Tomcat<j2eeType=WebModule, name=//localhost(.*), J2EEApplication=none, J2EEServer=none><>errorCount:' |
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.
This way of extracting context is different to the ones above. Will it always have the same content and meaning?
# Maximum execution time of all servlets in this context (Tomcat<j2eeType=WebModule, name=//localhost/, J2EEApplication=none, J2EEServer=none><>maxTime) | ||
# Minimum execution time of all servlets in this context (Tomcat<j2eeType=WebModule, name=//localhost/, J2EEApplication=none, J2EEServer=none><>minTime) | ||
- pattern: '^Tomcat<j2eeType=WebModule, name=//localhost(.*), J2EEApplication=none, J2EEServer=none><>(min|max)Time:' | ||
name: tomcat_webmodule_execution__$2 |
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.
Extra _
lowercaseOutputLabelNames: true | ||
lowercaseOutputName: true | ||
rules: |
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.
rules need comeback, or error:
Caused by: while parsing a block mapping
in 'reader', line 4, column 1:
lowercaseOutputLabelNames: true
^
expected , but found BlockEntry
in 'reader', line 12, column 1:
- pattern: '^Tomcat<type=GlobalR ...
This has gone stale, so closing for now. |
Rename most of the interesting Tomcat JMX metrics to follow naming convention of Prometheus, specifying type and help text.
This has been tested with Tomcat 8.