-
Notifications
You must be signed in to change notification settings - Fork 314
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
Add parameter support to telemetry devices #458
Add parameter support to telemetry devices #458
Conversation
With this commit we add a new command line flag `--telemetry-params` which allows users to inject parameters to telemetry devices. We also enhance the `jfr` telemetry device to choose the recording template based on such a parameter.
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.
LGTM! Left a few minor comments. New tests worked fine.
|
||
java_opts = self.java_opts(log_file) | ||
|
||
logger.info("jfr: Adding JVM arguments: [%s].", java_opts) |
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.
Could use the new .format() 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.
Ah silly me. It's the logger. Ignore.
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 think this is a different case here and I wanted to discuss this separately anyway. It looks as if we are using old style string formatting but we are actually not. This is the standard way the Python logging facility is used and I think it would make sense that we go with the default 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.
Ok, I'll merge then. :)
With this commit we add a new command line flag
--telemetry-params
which allows users to inject parameters to telemetry devices.
We also enhance the
jfr
telemetry device to choose the recordingtemplate based on such a parameter.