[non urgent] uptime metric tracks whether --enable-preview is set #1548
Conversation
Generate changelog in
|
| @@ -80,6 +81,10 @@ private static void registerAttributes(InternalJvmMetrics metrics) { | |||
| .javaRuntimeVersion(System.getProperty("java.runtime.version", "unknown")) | |||
| .javaVendorVersion(System.getProperty("java.vendor.version", "unknown")) | |||
| .javaVmVendor(System.getProperty("java.vm.vendor", "unknown")) | |||
| .enablePreview( | |||
| runtimeMxBean.getInputArguments().contains("--enable-preview") | |||
There was a problem hiding this comment.
Seems like we don't have junit tests to prove this method gives us sensible values, but I did validate this:
$ jshell --enable-preview
| Welcome to JShell -- Version 15.0.7
| For an introduction type: /help intro
jshell> java.lang.management.ManagementFactory.getRuntimeMXBean().getInputArguments()
$1 ==> [-agentlib:jdwp=transport=dt_socket,address=localhost:61023, --enable-preview]
There was a problem hiding this comment.
If we wanted to unit test this, I think we'd need to bump source and library target to at least 12 and enable preview at compilation, effectively forcing consumers to JDK 15. I don't think we are quite ready to force that JDK dependency bump for tritium quite yet (though possibly soon).
There was a problem hiding this comment.
I don't think we ever want to --enable-preview for the compilation of published jars, because they can only be run by the same java version that was used to compile them (i.e. if we did this with java15 for :tritium-metrics-jvm then anyone running java17 in production would fail to start)... I think this would quickly lead to unsolveable dependency graphs and much angst in #dev-backend-infra.
We'd need to either invent a gradle API for setting --enable-preview on the test sourceset only (I didn't add support for this initially palantir/gradle-baseline#2322), or make up an entire separate gradle project that we would enable preview on.
iamdanfox
changed the title
| - name: enablePreview | ||
| docs: "Whether the JVM is running with --enable-preview (see JEP 12)" | ||
| values: [true, false] |
There was a problem hiding this comment.
palantir/metric-schema#832 recently added the runtime JDK version info as tags to all metrics. Including enablePreview tag on every metric produced is likely extraneous noise, so putting this on uptime seems reasonable and shouldn't change metric cardinality.
There was a problem hiding this comment.
👍 Interesting that we've added it in metric-schema - I guess that means it's slightly easier to split up a graph in datadog by javaversion only!
| @@ -80,6 +81,10 @@ private static void registerAttributes(InternalJvmMetrics metrics) { | |||
| .javaRuntimeVersion(System.getProperty("java.runtime.version", "unknown")) | |||
| .javaVendorVersion(System.getProperty("java.vendor.version", "unknown")) | |||
| .javaVmVendor(System.getProperty("java.vm.vendor", "unknown")) | |||
| .enablePreview( | |||
| runtimeMxBean.getInputArguments().contains("--enable-preview") | |||
There was a problem hiding this comment.
If we wanted to unit test this, I think we'd need to bump source and library target to at least 12 and enable preview at compilation, effectively forcing consumers to JDK 15. I don't think we are quite ready to force that JDK dependency bump for tritium quite yet (though possibly soon).
Co-authored-by: David Schlosnagle <schlosna@gmail.com>
Co-authored-by: David Schlosnagle <schlosna@gmail.com>
|
👍 to approve dschlosnagle's suggested commits - EDIT nvm I think my +1 is meaningless as I'm no longer an admin on this repo sorry
|
|
Released 0.52.0 |
Before this PR
I've been slowly trying to enable folks to use pattern-matching switch expressions instead of visitors for their conjure unions (see
#hackweek-2022-sealed-unions), and one of the requirements for this is to add the--enable-previewflag at runtime (palantir/sls-packaging#1365).It occurred to me that if folks ever actually started using this in production, we'd probably want a convenient way to be able to eyeball precisely which installations are using it... especially if we found some undesirable behaviour that only manifested in the --enable-preview case and we wanted to find usages and warn/remediate them.
After this PR
==COMMIT_MSG==
The
uptimemetric now has an additional tagenablePreviewwhich signifies if the--enable-previewflag is passed to java at runtime or not.==COMMIT_MSG==
Possible downsides?