Add/Update Metrics TCK #186

yasmin-aumeeruddy · 2024-06-12T22:28:31Z

Uses the logging exporter instead of the in-memory exporter.
Updates metrics TCK to include the JVM metrics
Changes the in-memory Metrics exporter to filter by metric names instead of metric types. This was done as multiple JVM metrics would be exported with the same metric type so it was no longer a unique identifier for the test metrics.

Emily-Jiang · 2024-06-13T13:23:14Z

@yasmin-aumeeruddy please resolve the conflicts

Emily-Jiang · 2024-06-17T09:39:56Z

@yasmin-aumeeruddy please fix the build error?

Emily-Jiang

The build error needs to be resolved.

tck/logs/README.adoc

tck/metrics/src/main/java/org/eclipse/microprofile/telemetry/metrics/tck/jvm/JvmCpuTest.java

...ics/src/main/java/org/eclipse/microprofile/telemetry/metrics/tck/http/HttpHistogramTest.java

...c/main/java/org/eclipse/microprofile/telemetry/metrics/tck/jvm/JvmGarbageCollectionTest.java

tck/metrics/src/main/java/org/eclipse/microprofile/telemetry/metrics/tck/jvm/JvmMemoryTest.java

...c/main/java/org/eclipse/microprofile/telemetry/metrics/tck/jvm/JvmGarbageCollectionTest.java

donbourne

LGTM

donbourne · 2024-06-25T21:13:07Z

tck/logs/src/main/java/org/eclipse/microprofile/telemetry/logs/tck/application/JulTest.java

+    private static final String JUL_INFO_MESSAGE = "a very distinguishable info message";
+    private static final String JUL_WARN_MESSAGE = "a very distinguishable warning message";
+
+    private static final String SYS_OUT = "SystemOut";


This only applies to Open Liberty, so should be removed

donbourne · 2024-06-25T21:14:10Z

tck/logs/src/main/java/org/eclipse/microprofile/telemetry/logs/tck/application/JulTest.java

+    void julInfoTest() throws IOException {
+        julLogger.log(Level.INFO, JUL_INFO_MESSAGE);
+        try {
+            Assert.assertTrue(checkMessage(SYS_OUT + ".*" + "INFO" + ".*" + JUL_INFO_MESSAGE));


remove SYS_OUT, but you'll need to add something else in the line to avoid matching with the logs from the runtime (since you actually want to match against the logs from OTel bridge).

donbourne · 2024-06-25T21:14:44Z

tck/logs/src/main/java/org/eclipse/microprofile/telemetry/logs/tck/application/JulTest.java

+
+    @Test
+    void julWarnTest() throws IOException {
+        julLogger.log(Level.WARNING, JUL_INFO_MESSAGE);


JUL_INFO_MESSAGE -> JUL_WARN_MESSAGE

donbourne · 2024-06-25T21:14:58Z

tck/logs/src/main/java/org/eclipse/microprofile/telemetry/logs/tck/application/JulTest.java

+    void julWarnTest() throws IOException {
+        julLogger.log(Level.WARNING, JUL_INFO_MESSAGE);
+        try {
+            Assert.assertTrue(checkMessage(SYS_OUT + ".*" + "WARN" + ".*" + JUL_INFO_MESSAGE));


JUL_INFO_MESSAGE -> JUL_WARN_MESSAGE

donbourne · 2024-06-25T21:15:41Z

tck/logs/src/main/java/org/eclipse/microprofile/telemetry/logs/tck/application/JulTest.java

+                BufferedReader reader = new BufferedReader(new FileReader(logFilePath));
+                String line;
+                while ((line = reader.readLine()) != null) {
+                    if (line.contains(logMessage)) {


String contains method doesn't do regex matching -- need to use matches instead.

donbourne · 2024-06-25T21:26:11Z

...metrics/src/main/java/org/eclipse/microprofile/telemetry/metrics/tck/jvm/JvmClassesTest.java

+                .addAsLibrary(TestLibraries.AWAITILITY_LIB)
+                .addAsResource(new StringAsset(
+                        "otel.sdk.disabled=false\notel.logs.exporter=none\notel.traces.exporter=none"),
+                        "META-INF/microprofile-config.properties")
+                .addAsManifestResource(EmptyAsset.INSTANCE, "beans.xml");


I don't think you need any of these lines anymore

donbourne · 2024-06-25T21:26:46Z

...c/main/java/org/eclipse/microprofile/telemetry/metrics/tck/jvm/JvmGarbageCollectionTest.java

+                .addAsLibrary(TestLibraries.AWAITILITY_LIB)
+                .addAsResource(new StringAsset(
+                        "otel.sdk.disabled=false\notel.logs.exporter=none\notel.traces.exporter=none"),
+                        "META-INF/microprofile-config.properties")
+                .addAsManifestResource(EmptyAsset.INSTANCE, "beans.xml");


I don't think you need any of these lines anymore

donbourne · 2024-06-25T21:26:59Z

tck/metrics/src/main/java/org/eclipse/microprofile/telemetry/metrics/tck/jvm/JvmMemoryTest.java

+                .addAsLibrary(TestLibraries.AWAITILITY_LIB)
+                .addAsResource(new StringAsset(
+                        "otel.sdk.disabled=false\notel.logs.exporter=none\notel.traces.exporter=none"),
+                        "META-INF/microprofile-config.properties")
+                .addAsManifestResource(EmptyAsset.INSTANCE, "beans.xml");


I don't think you need any of these lines anymore

donbourne · 2024-06-25T21:27:18Z

tck/metrics/src/main/java/org/eclipse/microprofile/telemetry/metrics/tck/jvm/JvmThreadTest.java

+                .addAsLibrary(TestLibraries.AWAITILITY_LIB)
+                .addAsResource(new StringAsset(
+                        "otel.sdk.disabled=false\notel.logs.exporter=none\notel.traces.exporter=none"),
+                        "META-INF/microprofile-config.properties")
+                .addAsManifestResource(EmptyAsset.INSTANCE, "beans.xml");


I don't think you need any of these lines anymore

donbourne · 2024-06-25T21:30:54Z

tck/metrics/README.adoc

-If you want to run the metrics tests, you can specify all tests in the `tck-suite.xml`. E.g. 
+== Running the tests
+
+The jvm metrics tests require runtime configuration to enable metric reading at a runtime level. The metrics must be sent to stdout in the tests. Ensure logs written to stdout are captured in a file and set the system property `log.file.path` to the file containing the log output when running the logs TCK. Configure the runtime with `otel.metrics.exporter=console`/`OTEL_METRICS_EXPORTER=CONSOLE` and `otel.sdk.disabled=false`/`OTEL_SDK_DISABLED=FALSE` as a system property or environment variable.  For example:


donbourne · 2024-06-26T14:03:59Z

tck/logs/src/main/java/org/eclipse/microprofile/telemetry/logs/tck/application/JulTest.java

+    void julWarnTest() throws IOException {
+        julLogger.log(Level.WARNING, JUL_WARN_MESSAGE);
+        try {
+            Assert.assertTrue(checkMessage(".*INFO.*" + JUL_WARN_MESSAGE + ".*scopeInfo:.*"));


does OTel show "INFO" when we log a warning? would have expected WARN or WARNING

Add jvm metrics to TCK Remove log appender dependencies create test for http metrics (#197) Change exporter for jvm metrics separate metric tests for runtime tests Update logs tck

…ADME to include configuration of export interval

donbourne · 2024-06-26T19:09:40Z

tck/metrics/README.adoc

+The JVM metrics tests require runtime configuration to enable metric reading at a runtime level. The metrics must be sent to stdout in the tests. Ensure logs written to stdout are captured in a file and set the system property `log.file.path` to the file containing the log output when running the logs TCK. Configure the runtime with `otel.metrics.exporter=logging`/`OTEL_METRICS_EXPORTER=LOGGING` and `otel.sdk.disabled=false`/`OTEL_SDK_DISABLED=FALSE` as a system property or environment variable.
+The metric export interval `otel.metric.export.interval`/`OTEL_METRIC_EXPORT_INTERVAL` should be configured as well and it is recommended to be set to a value of `3000` (i.e., 3 seconds).
+Note: it is recommended to set `otel.traces.exporter` /  `OTEL_TRACES_EXPORTER` and `otel.logs.exporter` / `OTEL_LOGS_EXPORTER` to `none` for the execution of the metric tests. For example:


i'd suggest combining these, since they all must be configured with system property or env var...

The JVM metrics tests require runtime configuration to enable metric reading at a runtime level. The metrics must be sent to stdout in the tests. Ensure logs written to stdout are captured in a file and set the system property log.file.path to the file containing the log output when running the logs TCK. Configure the runtime with the following as system properties / environment variables:

otel.sdk.disabled=false/OTEL_SDK_DISABLED=FALSE

otel.metrics.exporter=logging/OTEL_METRICS_EXPORTER=LOGGING

otel.traces.exporter=none/OTEL_TRACES_EXPORTER=none

otel.logs.exporter=none/OTEL_LOGS_EXPORTER=none

otel.metric.export.interval=3000/OTEL_METRIC_EXPORT_INTERVAL=3000

Emily-Jiang approved these changes Jun 13, 2024

View reviewed changes

yasmin-aumeeruddy force-pushed the logs-tck branch 3 times, most recently from 923577a to 296c59b Compare June 17, 2024 09:07

yasmin-aumeeruddy changed the title ~~Remove in-memory exporter~~ Update TCK Jun 17, 2024

yasmin-aumeeruddy force-pushed the logs-tck branch from 1c91f0c to 61e30d6 Compare June 17, 2024 09:29

Emily-Jiang self-requested a review June 17, 2024 09:40

Emily-Jiang requested changes Jun 17, 2024

View reviewed changes

yasmin-aumeeruddy force-pushed the logs-tck branch from 61e30d6 to 24ba3b1 Compare June 17, 2024 09:49

donbourne requested changes Jun 19, 2024

View reviewed changes

donbourne reviewed Jun 20, 2024

View reviewed changes

...c/main/java/org/eclipse/microprofile/telemetry/metrics/tck/jvm/JvmGarbageCollectionTest.java Outdated Show resolved Hide resolved

yasmin-aumeeruddy force-pushed the logs-tck branch from c733559 to 701aa1f Compare June 20, 2024 13:41

donbourne approved these changes Jun 20, 2024

View reviewed changes

Emily-Jiang approved these changes Jun 24, 2024

View reviewed changes

Emily-Jiang changed the title ~~Update TCK~~ Add/Update Metrics TCK Jun 24, 2024

yasmin-aumeeruddy force-pushed the logs-tck branch 5 times, most recently from 5e9c298 to 53a23d4 Compare June 25, 2024 20:14

donbourne requested changes Jun 25, 2024

View reviewed changes

yasmin-aumeeruddy force-pushed the logs-tck branch 2 times, most recently from f3fcf17 to 5b84e7c Compare June 26, 2024 13:48

donbourne reviewed Jun 26, 2024

View reviewed changes

yasmin-aumeeruddy force-pushed the logs-tck branch 3 times, most recently from 495a4c9 to b472847 Compare June 26, 2024 14:19

donbourne approved these changes Jun 26, 2024

View reviewed changes

Update logs tck to remove in-memory exporter

4a7ded7

Add jvm metrics to TCK Remove log appender dependencies create test for http metrics (#197) Change exporter for jvm metrics separate metric tests for runtime tests Update logs tck

yasmin-aumeeruddy force-pushed the logs-tck branch from b472847 to 4a7ded7 Compare June 26, 2024 15:32

Channyboy added 2 commits June 26, 2024 14:47

Update metric type and metric unit for JVM related metrics; Update RE…

9bcb08d

…ADME to include configuration of export interval

formatting, wording

8c08a11

donbourne reviewed Jun 26, 2024

View reviewed changes

Channyboy added 2 commits June 26, 2024 15:35

Use review suggestion

5fca9c6

code syntax

9358ac6

donbourne approved these changes Jun 26, 2024

View reviewed changes

donbourne merged commit 0db1488 into main Jun 26, 2024
6 of 7 checks passed

yasmin-aumeeruddy mentioned this pull request Jul 1, 2024

Check severity of logs in TCK #174

Closed

Emily-Jiang mentioned this pull request Jul 4, 2024

Rename log.file.path property #214

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add/Update Metrics TCK #186

Add/Update Metrics TCK #186

yasmin-aumeeruddy commented Jun 12, 2024 •

edited

Loading

Emily-Jiang commented Jun 13, 2024

Emily-Jiang commented Jun 17, 2024

Emily-Jiang left a comment

donbourne left a comment

donbourne Jun 25, 2024

donbourne Jun 25, 2024

donbourne Jun 25, 2024

donbourne Jun 25, 2024

donbourne Jun 25, 2024

donbourne Jun 25, 2024

donbourne Jun 25, 2024

donbourne Jun 25, 2024

donbourne Jun 25, 2024

donbourne Jun 25, 2024

donbourne Jun 26, 2024

donbourne Jun 26, 2024

Add/Update Metrics TCK #186

Add/Update Metrics TCK #186

Conversation

yasmin-aumeeruddy commented Jun 12, 2024 • edited Loading

Emily-Jiang commented Jun 13, 2024

Emily-Jiang commented Jun 17, 2024

Emily-Jiang left a comment

Choose a reason for hiding this comment

donbourne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yasmin-aumeeruddy commented Jun 12, 2024 •

edited

Loading