- 
                Notifications
    
You must be signed in to change notification settings  - Fork 168
 
Add jetty.yaml to JMX scraper #1517
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
Changes from 13 commits
bac90ea
              8b87c8f
              57a552f
              c2ace1a
              eacaeab
              9378dcf
              03b6536
              8a735ef
              643618b
              5df0512
              4d1989a
              bf569df
              3aba726
              a2a1c96
              187ca89
              5421f09
              7861f83
              ff8a540
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| 
     | 
||
| package io.opentelemetry.contrib.jmxscraper.target_systems; | ||
| 
     | 
||
| import static io.opentelemetry.contrib.jmxscraper.target_systems.MetricAssertions.assertGauge; | ||
| import static io.opentelemetry.contrib.jmxscraper.target_systems.MetricAssertions.assertGaugeWithAttributes; | ||
| import static io.opentelemetry.contrib.jmxscraper.target_systems.MetricAssertions.assertSumWithAttributes; | ||
| import static io.opentelemetry.contrib.jmxscraper.target_systems.MetricAssertions.assertSumWithAttributesMultiplePoints; | ||
| 
     | 
||
| import io.opentelemetry.contrib.jmxscraper.JmxScraperContainer; | ||
| import java.time.Duration; | ||
| import org.testcontainers.containers.GenericContainer; | ||
| import org.testcontainers.containers.wait.strategy.Wait; | ||
| import org.testcontainers.images.builder.ImageFromDockerfile; | ||
| 
     | 
||
| public class JettyIntegrationTest extends TargetSystemIntegrationTest { | ||
| 
     | 
||
| @Override | ||
| protected GenericContainer<?> createTargetContainer(int jmxPort) { | ||
| GenericContainer<?> container = | ||
| new GenericContainer<>( | ||
| new ImageFromDockerfile() | ||
| .withDockerfileFromBuilder( | ||
| builder -> | ||
| builder | ||
| .from("jetty:11") | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you consider upgrade to jetty 12? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, ideally we should test all those metrics on multiple versions but it's not the case for now, but it would be a nice improvement for later :-).  | 
||
| .run( | ||
| "java", | ||
| "-jar", | ||
| "/usr/local/jetty/start.jar", | ||
| "--add-to-startd=jmx,stats,http") | ||
| .run("mkdir -p /var/lib/jetty/webapps/ROOT/") | ||
| .run("touch /var/lib/jetty/webapps/ROOT/index.html") | ||
| .build())); | ||
| 
     | 
||
| container | ||
| .withEnv("JAVA_OPTIONS", genericJmxJvmArguments(jmxPort)) | ||
| .withStartupTimeout(Duration.ofMinutes(2)) | ||
| .waitingFor(Wait.forLogMessage(".*Started Server.*", 1)); | ||
| 
     | 
||
| return container; | ||
| } | ||
| 
     | 
||
| @Override | ||
| protected JmxScraperContainer customizeScraperContainer(JmxScraperContainer scraper) { | ||
| return scraper.withTargetSystem("jetty"); | ||
| } | ||
| 
     | 
||
| @Override | ||
| protected void verifyMetrics() { | ||
| waitAndAssertMetrics( | ||
| metric -> | ||
| assertSumWithAttributes( | ||
| metric, | ||
| "jetty.session.count", | ||
| "The number of sessions established in total.", | ||
| "{sessions}", | ||
                
      
                  SylvainJuge marked this conversation as resolved.
               
              
                Outdated
          
            Show resolved
            Hide resolved
         | 
||
| attrs -> attrs.containsKey("resource")), | ||
| metric -> | ||
| assertSumWithAttributes( | ||
| metric, | ||
| "jetty.session.time.total", | ||
| "The total time sessions have been active.", | ||
| "s", | ||
| attrs -> attrs.containsKey("resource")), | ||
| metric -> | ||
| assertGaugeWithAttributes( | ||
| metric, | ||
| "jetty.session.time.max", | ||
| "The maximum amount of time a session has been active.", | ||
| "s", | ||
| attrs -> attrs.containsKey("resource")), | ||
| metric -> | ||
| assertSumWithAttributesMultiplePoints( | ||
| metric, | ||
| "jetty.select.count", | ||
| "The number of select calls.", | ||
| "{operations}", | ||
                
      
                  SylvainJuge marked this conversation as resolved.
               
              
                Outdated
          
            Show resolved
            Hide resolved
         | 
||
| /* isMonotonic= */ true, | ||
| // minor divergence from jetty.groovy with extra metrics attributes | ||
| attrs -> attrs.containsKey("context").containsKey("id")), | ||
| metric -> | ||
| assertGaugeWithAttributes( | ||
| metric, | ||
| "jetty.thread.count", | ||
| "The current number of threads.", | ||
| "{threads}", | ||
                
      
                  SylvainJuge marked this conversation as resolved.
               
              
                Outdated
          
            Show resolved
            Hide resolved
         | 
||
| attrs -> attrs.containsEntry("state", "busy"), | ||
| attrs -> attrs.containsEntry("state", "idle")), | ||
| metric -> | ||
| assertGauge( | ||
| metric, | ||
| "jetty.thread.queue.count", | ||
| "The current number of threads in the queue.", | ||
| "{threads}")); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -92,6 +92,22 @@ static void assertSumWithAttributes( | |
| assertAttributedPoints(metric.getSum().getDataPointsList(), attributeGroupAssertions); | ||
| } | ||
| 
     | 
||
| @SafeVarargs | ||
| static void assertSumWithAttributesMultiplePoints( | ||
| Metric metric, | ||
| String name, | ||
| String description, | ||
| String unit, | ||
| boolean isMonotonic, | ||
| Consumer<MapAssert<String, String>>... attributeGroupAssertions) { | ||
| assertThat(metric.getName()).isEqualTo(name); | ||
| assertThat(metric.getDescription()).isEqualTo(description); | ||
| assertThat(metric.getUnit()).isEqualTo(unit); | ||
| assertThat(metric.hasSum()).isTrue(); | ||
| assertThat(metric.getSum().getIsMonotonic()).isEqualTo(isMonotonic); | ||
| assertAttributedMultiplePoints(metric.getSum().getDataPointsList(), attributeGroupAssertions); | ||
| } | ||
| 
     | 
||
| @SafeVarargs | ||
| static void assertGaugeWithAttributes( | ||
| Metric metric, | ||
| 
          
            
          
           | 
    @@ -127,6 +143,7 @@ private static void assertAttributedPoints( | |
| Arrays.stream(attributeGroupAssertions) | ||
| .map(assertion -> (Consumer<Map<String, String>>) m -> assertion.accept(assertThat(m))) | ||
| .toArray(Consumer[]::new); | ||
| 
     | 
||
| assertThat(points) | ||
| .extracting( | ||
| numberDataPoint -> | ||
| 
        
          
        
         | 
    @@ -136,4 +153,22 @@ private static void assertAttributedPoints( | |
| KeyValue::getKey, keyValue -> keyValue.getValue().getStringValue()))) | ||
| .satisfiesExactlyInAnyOrder(assertions); | ||
| } | ||
| 
     | 
||
| @SuppressWarnings("unchecked") | ||
| private static void assertAttributedMultiplePoints( | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [for reviewer] the difference with   | 
||
| List<NumberDataPoint> points, | ||
| Consumer<MapAssert<String, String>>... attributeGroupAssertions) { | ||
| 
     | 
||
| points.stream() | ||
| .map(NumberDataPoint::getAttributesList) | ||
| .forEach( | ||
| kvList -> { | ||
| Map<String, String> kvMap = | ||
| kvList.stream() | ||
| .collect( | ||
| Collectors.toMap(KeyValue::getKey, kv -> kv.getValue().getStringValue())); | ||
| Arrays.stream(attributeGroupAssertions) | ||
| .forEach(assertion -> assertion.accept(assertThat(kvMap))); | ||
| }); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -22,7 +22,9 @@ | |
| import java.util.ArrayList; | ||
| import java.util.Arrays; | ||
| import java.util.Collection; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.concurrent.BlockingQueue; | ||
| import java.util.concurrent.ExecutionException; | ||
| import java.util.concurrent.LinkedBlockingDeque; | ||
| 
          
            
          
           | 
    @@ -196,4 +198,27 @@ public void export( | |
| sb.http(0); | ||
| } | ||
| } | ||
| 
     | 
||
| protected static String genericJmxJvmArguments(int port) { | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good change! However it is a bit over-complicated in my opinion. Simple StringBuilder and additional private method would be enough, no need for maps and streams. It is just my opinion :-) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I think I agree...I would have just hard coded a bunch of string concats...but I don't think it's a huge problem.  | 
||
| Map<String, Object> args = new HashMap<>(); | ||
| args.put("com.sun.management.jmxremote.local.only", "false"); | ||
| args.put("com.sun.management.jmxremote.authenticate", "false"); | ||
| args.put("com.sun.management.jmxremote.ssl", "false"); | ||
| args.put("com.sun.management.jmxremote.port", port); | ||
| args.put("com.sun.management.jmxremote.rmi.port", port); | ||
| List<String> list = | ||
| args.entrySet().stream() | ||
| .map((e) -> toJvmArg(e.getKey(), e.getValue())) | ||
| .collect(Collectors.toList()); | ||
| return String.join(" ", list); | ||
| } | ||
| 
     | 
||
| private static String toJvmArg(String key, Object value) { | ||
| StringBuilder sb = new StringBuilder(); | ||
| sb.append("-D").append(key); | ||
| if (value != null) { | ||
| sb.append("=").append(value); | ||
| } | ||
| return sb.toString(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| --- | ||
| 
     | 
||
| rules: | ||
| 
     | 
||
| - bean: org.eclipse.jetty.io:context=*,type=managedselector,id=* | ||
| mapping: | ||
| selectCount: | ||
| metric: jetty.select.count | ||
| type: counter | ||
| unit: "{operations}" | ||
| desc: The number of select calls. | ||
| metricAttribute: | ||
| # minor divergence from jetty.groovy with extra attribute(s) | ||
| # 'id' is a numerical value in [0,9] by default | ||
| # 'context' is a high cardinality value like 'HTTP_1_1@7674f035' but likely stable for the | ||
| # duration of the jetty process lifecycle | ||
| context: param(context) | ||
| id: param(id) | ||
| 
     | 
||
| - bean: org.eclipse.jetty.server.session:context=*,type=sessionhandler,id=* | ||
| prefix: jetty.session. | ||
| metricAttribute: | ||
| resource: param(context) | ||
| mapping: | ||
| sessionsCreated: | ||
| metric: count | ||
| type: counter | ||
| unit: "{sessions}" | ||
| desc: The number of sessions established in total. | ||
| sessionTimeTotal: | ||
| metric: time.total | ||
| type: counter | ||
| unit: s | ||
| desc: The total time sessions have been active. | ||
| sessionTimeMax: | ||
| metric: time.max | ||
| # here a 'counter' seems more appropriate but gauge reflects jetty.groovy impl. | ||
| type: gauge | ||
| unit: s | ||
| desc: The maximum amount of time a session has been active. | ||
| 
     | 
||
| - bean: org.eclipse.jetty.util.thread:type=queuedthreadpool,id=* | ||
| # here the 'id' can be ignored as it's usually a single value equal to '0' | ||
| prefix: jetty.thread. | ||
| unit: "{threads}" | ||
| mapping: | ||
| busyThreads: | ||
| metric: count | ||
| # here an 'updowncounter' seems more appropriate but gauge reflects jetty.groovy impl. | ||
| type: gauge | ||
| desc: The current number of threads. | ||
| metricAttribute: | ||
| state: const(busy) | ||
| idleThreads: | ||
| metric: count | ||
| # here an 'updowncounter' seems more appropriate but gauge reflects jetty.groovy impl. | ||
| type: gauge | ||
| desc: The current number of threads. | ||
| metricAttribute: | ||
| state: const(idle) | ||
| queueSize: | ||
| metric: queue.count | ||
| # here an 'updowncounter' seems more appropriate but gauge reflects jetty.groovy impl. | ||
| type: gauge | ||
| desc: The current number of threads in the queue. | 
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.
nice 👍🏻