Fix some things

Author: sarahchen6

dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/RumHttpServletResponseWrapper.java

@@ -71,6 +71,7 @@ public PrintWriter getWriter() throws IOException {
                     rumInjector.getSnippetChars(),
                     this::onInjected));
       } catch (Exception e) {
+        injectionStartTime = -1;
         RumInjector.getTelemetryCollector().onInjectionFailed();
         throw e;
       }

dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/RumHttpServletResponseWrapperTest.groovy

@@ -92,13 +92,13 @@ class RumHttpServletResponseWrapperTest extends AgentTestRunner {
     1 * mockResponse.setHeader("Content-Security-Policy", "test")
   }
 
-  void 'addHeader with Content-Security-Policy-Report-Only reports CSP detected'() {
+  void 'addHeader with Content-Security-Policy reports CSP detected'() {
     when:
-    wrapper.addHeader("Content-Security-Policy-Report-Only", "test")
+    wrapper.addHeader("Content-Security-Policy", "test")
 
     then:
     1 * mockTelemetryCollector.onContentSecurityPolicyDetected()
-    1 * mockResponse.addHeader("Content-Security-Policy-Report-Only", "test")
+    1 * mockResponse.addHeader("Content-Security-Policy", "test")
   }
 
   void 'setHeader with non-CSP header does not report CSP detected'() {

dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/RumHttpServletResponseWrapper.java

@@ -71,6 +71,7 @@ public PrintWriter getWriter() throws IOException {
                     rumInjector.getSnippetChars(),
                     this::onInjected));
       } catch (Exception e) {
+        injectionStartTime = -1;
         RumInjector.getTelemetryCollector().onInjectionFailed();
         throw e;
       }

dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/RumHttpServletResponseWrapperTest.groovy

@@ -92,13 +92,13 @@ class RumHttpServletResponseWrapperTest extends AgentTestRunner {
     1 * mockResponse.setHeader("Content-Security-Policy", "test")
   }
 
-  void 'addHeader with Content-Security-Policy-Report-Only reports CSP detected'() {
+  void 'addHeader with Content-Security-Policy reports CSP detected'() {
     when:
-    wrapper.addHeader("Content-Security-Policy-Report-Only", "test")
+    wrapper.addHeader("Content-Security-Policy", "test")
 
     then:
     1 * mockTelemetryCollector.onContentSecurityPolicyDetected()
-    1 * mockResponse.addHeader("Content-Security-Policy-Report-Only", "test")
+    1 * mockResponse.addHeader("Content-Security-Policy", "test")
   }
 
   void 'setHeader with non-CSP header does not report CSP detected'() {

internal-api/src/test/groovy/datadog/trace/api/rum/RumInjectorMetricsTest.groovy

@@ -85,33 +85,7 @@ class RumInjectorMetricsTest extends Specification {
     metrics.close()
   }
 
-  def "test onInjectionResponseSize with multiple sizes"() {
-    when:
-    metrics.onInjectionResponseSize(256)
-    metrics.onInjectionResponseSize(512)
-    metrics.onInjectionResponseSize(2048)
-
-    then:
-    1 * statsD.distribution('rum.injection.response.bytes', 256, _)
-    1 * statsD.distribution('rum.injection.response.bytes', 512, _)
-    1 * statsD.distribution('rum.injection.response.bytes', 2048, _)
-    0 * _
-  }
-
-  def "test onInjectionTime with multiple durations"() {
-    when:
-    metrics.onInjectionTime(5L)
-    metrics.onInjectionTime(10L)
-    metrics.onInjectionTime(25L)
-
-    then:
-    1 * statsD.distribution('rum.injection.ms', 5L, _)
-    1 * statsD.distribution('rum.injection.ms', 10L, _)
-    1 * statsD.distribution('rum.injection.ms', 25L, _)
-    0 * _
-  }
-
-  def "test flushing multiple events"() {
+  def "test multiple events"() {
     setup:
     def latch = new CountDownLatch(4) // expecting 4 metric types
     def metrics = new RumInjectorMetrics(new Latched(statsD, latch), 10, TimeUnit.MILLISECONDS)
@@ -158,16 +132,16 @@ class RumInjectorMetricsTest extends Specification {
     metrics.close()
   }
 
-  def "test summary with multiple events"() {
+  def "test summary with multiple events in different order"() {
     when:
+    metrics.onContentSecurityPolicyDetected()
     metrics.onInjectionSucceed()
     metrics.onInjectionFailed()
     metrics.onInjectionSucceed()
     metrics.onInjectionFailed()
     metrics.onInjectionSucceed()
     metrics.onInjectionSkipped()
     metrics.onContentSecurityPolicyDetected()
-    metrics.onContentSecurityPolicyDetected()
     def summary = metrics.summary()
 
     then:
@@ -190,6 +164,33 @@ class RumInjectorMetricsTest extends Specification {
     0 * _
   }
 
+  // events below are reported immediately as distribution metrics and do not get summarized
+  def "test onInjectionResponseSize with multiple sizes"() {
+    when:
+    metrics.onInjectionResponseSize(256)
+    metrics.onInjectionResponseSize(512)
+    metrics.onInjectionResponseSize(2048)
+
+    then:
+    1 * statsD.distribution('rum.injection.response.bytes', 256, _)
+    1 * statsD.distribution('rum.injection.response.bytes', 512, _)
+    1 * statsD.distribution('rum.injection.response.bytes', 2048, _)
+    0 * _
+  }
+
+  def "test onInjectionTime with multiple durations"() {
+    when:
+    metrics.onInjectionTime(5L)
+    metrics.onInjectionTime(10L)
+    metrics.onInjectionTime(25L)
+
+    then:
+    1 * statsD.distribution('rum.injection.ms', 5L, _)
+    1 * statsD.distribution('rum.injection.ms', 10L, _)
+    1 * statsD.distribution('rum.injection.ms', 25L, _)
+    0 * _
+  }
+
   // taken from HealthMetricsTest
   private static class Latched implements StatsDClient {
     final StatsDClient delegate

internal-api/src/test/groovy/datadog/trace/api/rum/RumInjectorTest.groovy

@@ -82,9 +82,6 @@ class RumInjectorTest extends DDSpecification {
 
     then:
     RumInjector.getTelemetryCollector() == RumTelemetryCollector.NO_OP
-
-    cleanup:
-    RumInjector.setTelemetryCollector(RumTelemetryCollector.NO_OP)
   }
 
   void 'enable telemetry with StatsDClient'() {
@@ -104,9 +101,6 @@ class RumInjectorTest extends DDSpecification {
 
     then:
     RumInjector.getTelemetryCollector() == RumTelemetryCollector.NO_OP
-
-    cleanup:
-    RumInjector.shutdownTelemetry()
   }
 
   void 'shutdown telemetry'() {
@@ -128,17 +122,19 @@ class RumInjectorTest extends DDSpecification {
     // simulate reporting successful injection
     def telemetryCollector = RumInjector.getTelemetryCollector()
     telemetryCollector.onInjectionSucceed()
-    telemetryCollector.onInjectionSucceed()
     telemetryCollector.onInjectionFailed()
+    telemetryCollector.onInjectionSkipped()
     telemetryCollector.onContentSecurityPolicyDetected()
+    telemetryCollector.onInjectionResponseSize(256)
+    telemetryCollector.onInjectionTime(5L)
 
     // verify metrics are collected
     def summary = telemetryCollector.summary()
 
     then:
-    summary.contains("injectionSucceed=2")
+    summary.contains("injectionSucceed=1")
     summary.contains("injectionFailed=1")
-    summary.contains("injectionSkipped=0")
+    summary.contains("injectionSkipped=1")
     summary.contains("contentSecurityPolicyDetected=1")
 
     cleanup:
@@ -153,9 +149,9 @@ class RumInjectorTest extends DDSpecification {
     RumInjector.enableTelemetry(mockStatsDClient)
 
     def telemetryCollector = RumInjector.getTelemetryCollector()
+    telemetryCollector.onInjectionResponseSize(256)
     telemetryCollector.onInjectionResponseSize(512)
     telemetryCollector.onInjectionResponseSize(2048)
-    telemetryCollector.onInjectionResponseSize(256)
 
     then:
     // response sizes are reported immediately as distribution metrics
@@ -185,7 +181,7 @@ class RumInjectorTest extends DDSpecification {
     RumInjector.shutdownTelemetry()
   }
 
-  void 'concurrent telemetry calls are thread-safe'() {
+  void 'concurrent telemetry calls return an accurate summary'() {
     setup:
     RumInjector.enableTelemetry(mock(datadog.trace.api.StatsDClient))
     def telemetryCollector = RumInjector.getTelemetryCollector()
@@ -199,6 +195,8 @@ class RumInjectorTest extends DDSpecification {
         telemetryCollector.onInjectionFailed()
         telemetryCollector.onInjectionSkipped()
         telemetryCollector.onContentSecurityPolicyDetected()
+        telemetryCollector.onInjectionResponseSize(256)
+        telemetryCollector.onInjectionTime(5L)
       }
     }
     threads*.join()