Skip to content

Commit 7a5cd68

Browse files
sarahchen6sarahchen6
committed
Tweaks
1 parent b32874c commit 7a5cd68

File tree

9 files changed

+75
-93
lines changed

9 files changed

+75
-93
lines changed

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ public ServletOutputStream getOutputStream() throws IOException {
6767
this::onInjected,
6868
bytes -> RumInjector.getTelemetryCollector().onInjectionResponseSize("3", bytes));
6969
} catch (Exception e) {
70+
injectionStartTime = -1;
7071
RumInjector.getTelemetryCollector().onInjectionFailed("3", contentEncoding);
7172
throw e;
7273
}
@@ -96,6 +97,7 @@ public PrintWriter getWriter() throws IOException {
9697
this::onInjected);
9798
printWriter = new PrintWriter(wrappedPipeWriter);
9899
} catch (Exception e) {
100+
injectionStartTime = -1;
99101
RumInjector.getTelemetryCollector().onInjectionFailed("3", contentEncoding);
100102
throw e;
101103
}
@@ -109,7 +111,7 @@ public void setHeader(String name, String value) {
109111
String lowerName = name.toLowerCase();
110112
if (lowerName.startsWith("content-security-policy")) {
111113
RumInjector.getTelemetryCollector().onContentSecurityPolicyDetected("3");
112-
} else if (lowerName.equals("content-encoding")) {
114+
} else if (lowerName.contains("content-encoding")) {
113115
this.contentEncoding = value;
114116
}
115117
}
@@ -122,7 +124,7 @@ public void addHeader(String name, String value) {
122124
String lowerName = name.toLowerCase();
123125
if (lowerName.startsWith("content-security-policy")) {
124126
RumInjector.getTelemetryCollector().onContentSecurityPolicyDetected("3");
125-
} else if (lowerName.equals("content-encoding")) {
127+
} else if (lowerName.contains("content-encoding")) {
126128
this.contentEncoding = value;
127129
}
128130
}
@@ -169,7 +171,7 @@ public void resetBuffer() {
169171
public void onInjected() {
170172
RumInjector.getTelemetryCollector().onInjectionSucceed("3");
171173

172-
// report injection time
174+
// calculate total injection time
173175
if (injectionStartTime != -1) {
174176
long nanoseconds = System.nanoTime() - injectionStartTime;
175177
long milliseconds = nanoseconds / 1_000_000L;

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ class RumHttpServletResponseWrapperTest extends AgentTestRunner {
106106
wrapper.setHeader("X-Content-Security-Policy", "test")
107107

108108
then:
109-
0 * mockTelemetryCollector.onContentSecurityPolicyDetected()
109+
0 * mockTelemetryCollector.onContentSecurityPolicyDetected("3")
110110
1 * mockResponse.setHeader("X-Content-Security-Policy", "test")
111111
}
112112

@@ -115,7 +115,7 @@ class RumHttpServletResponseWrapperTest extends AgentTestRunner {
115115
wrapper.addHeader("X-Content-Security-Policy", "test")
116116

117117
then:
118-
0 * mockTelemetryCollector.onContentSecurityPolicyDetected()
118+
0 * mockTelemetryCollector.onContentSecurityPolicyDetected("3")
119119
1 * mockResponse.addHeader("X-Content-Security-Policy", "test")
120120
}
121121

@@ -161,16 +161,14 @@ class RumHttpServletResponseWrapperTest extends AgentTestRunner {
161161

162162
void 'injection timing is reported when injection is successful'() {
163163
setup:
164-
wrapper.setContentType("text/html")
164+
// set the injection start time to simulate timing
165+
wrapper.@injectionStartTime = System.nanoTime() - 2_000_000L
165166

166167
when:
167-
try {
168-
wrapper.getOutputStream() // set injectionStartTime
169-
} catch (Exception ignored) {} // expect failure due to improper setup
170-
Thread.sleep(1) // ensure some time passes
171-
wrapper.onInjected() // report timing when injection "is successful"
168+
wrapper.onInjected() // report timing when injection is successful
172169

173170
then:
174-
1 * mockTelemetryCollector.onInjectionTime("3", { it >= 0 })
171+
1 * mockTelemetryCollector.onInjectionSucceed("3")
172+
1 * mockTelemetryCollector.onInjectionTime("3", { it > 0 })
175173
}
176174
}

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ public ServletOutputStream getOutputStream() throws IOException {
4949
this::onInjected,
5050
bytes -> RumInjector.getTelemetryCollector().onInjectionResponseSize("5", bytes));
5151
} catch (Exception e) {
52+
injectionStartTime = -1;
5253
RumInjector.getTelemetryCollector().onInjectionFailed("5", contentEncoding);
5354
throw e;
5455
}
@@ -64,7 +65,7 @@ public PrintWriter getWriter() throws IOException {
6465
RumInjector.getTelemetryCollector().onInjectionSkipped("5");
6566
return super.getWriter();
6667
}
67-
// start timing installation
68+
// start timing injection
6869
if (injectionStartTime == -1) {
6970
injectionStartTime = System.nanoTime();
7071
}
@@ -77,6 +78,7 @@ public PrintWriter getWriter() throws IOException {
7778
this::onInjected);
7879
printWriter = new PrintWriter(wrappedPipeWriter);
7980
} catch (Exception e) {
81+
injectionStartTime = -1;
8082
RumInjector.getTelemetryCollector().onInjectionFailed("5", contentEncoding);
8183
throw e;
8284
}
@@ -90,7 +92,7 @@ public void setHeader(String name, String value) {
9092
String lowerName = name.toLowerCase();
9193
if (lowerName.startsWith("content-security-policy")) {
9294
RumInjector.getTelemetryCollector().onContentSecurityPolicyDetected("5");
93-
} else if (lowerName.equals("content-encoding")) {
95+
} else if (lowerName.contains("content-encoding")) {
9496
this.contentEncoding = value;
9597
}
9698
}
@@ -103,7 +105,7 @@ public void addHeader(String name, String value) {
103105
String lowerName = name.toLowerCase();
104106
if (lowerName.startsWith("content-security-policy")) {
105107
RumInjector.getTelemetryCollector().onContentSecurityPolicyDetected("5");
106-
} else if (lowerName.equals("content-encoding")) {
108+
} else if (lowerName.contains("content-encoding")) {
107109
this.contentEncoding = value;
108110
}
109111
}
@@ -146,7 +148,7 @@ public void resetBuffer() {
146148
public void onInjected() {
147149
RumInjector.getTelemetryCollector().onInjectionSucceed("5");
148150

149-
// report injection time
151+
// calculate total injection time
150152
if (injectionStartTime != -1) {
151153
long nanoseconds = System.nanoTime() - injectionStartTime;
152154
long milliseconds = nanoseconds / 1_000_000L;

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ class RumHttpServletResponseWrapperTest extends AgentTestRunner {
106106
wrapper.setHeader("X-Content-Security-Policy", "test")
107107

108108
then:
109-
0 * mockTelemetryCollector.onContentSecurityPolicyDetected()
109+
0 * mockTelemetryCollector.onContentSecurityPolicyDetected("5")
110110
1 * mockResponse.setHeader("X-Content-Security-Policy", "test")
111111
}
112112

@@ -115,7 +115,7 @@ class RumHttpServletResponseWrapperTest extends AgentTestRunner {
115115
wrapper.addHeader("X-Content-Security-Policy", "test")
116116

117117
then:
118-
0 * mockTelemetryCollector.onContentSecurityPolicyDetected()
118+
0 * mockTelemetryCollector.onContentSecurityPolicyDetected("5")
119119
1 * mockResponse.addHeader("X-Content-Security-Policy", "test")
120120
}
121121

@@ -161,16 +161,14 @@ class RumHttpServletResponseWrapperTest extends AgentTestRunner {
161161

162162
void 'injection timing is reported when injection is successful'() {
163163
setup:
164-
wrapper.setContentType("text/html")
164+
// set the injection start time to simulate timing
165+
wrapper.@injectionStartTime = System.nanoTime() - 2_000_000L
165166

166167
when:
167-
try {
168-
wrapper.getOutputStream() // set injectionStartTime
169-
} catch (Exception ignored) {} // expect failure due to improper setup
170-
Thread.sleep(1) // ensure some time passes
171-
wrapper.onInjected() // report timing when injection "is successful"
168+
wrapper.onInjected() // report timing when injection is successful
172169

173170
then:
174-
1 * mockTelemetryCollector.onInjectionTime("5", { it >= 0 })
171+
1 * mockTelemetryCollector.onInjectionSucceed("5")
172+
1 * mockTelemetryCollector.onInjectionTime("5", { it > 0 })
175173
}
176174
}

‎internal-api/src/main/java/datadog/trace/api/rum/RumInjector.java‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ public final class RumInjector {
2929
private final DDCache<String, byte[]> markerCache;
3030
private final Function<String, byte[]> snippetBytes;
3131

32-
// telemetry collector defaults to NO_OP
3332
private static volatile RumTelemetryCollector telemetryCollector = RumTelemetryCollector.NO_OP;
3433

3534
RumInjector(Config config, InstrumenterConfig instrumenterConfig) {
@@ -126,7 +125,11 @@ public byte[] getMarkerBytes(String encoding) {
126125
return this.markerCache.computeIfAbsent(encoding, MARKER_BYTES);
127126
}
128127

129-
// start telemetry collection and report metrics via the given StatsDClient
128+
/**
129+
* Starts telemetry collection and reports metrics via StatsDClient.
130+
*
131+
* @param statsDClient The StatsDClient to report metrics to.
132+
*/
130133
public static void enableTelemetry(datadog.trace.api.StatsDClient statsDClient) {
131134
if (statsDClient != null) {
132135
RumInjectorMetrics metrics = new RumInjectorMetrics(statsDClient);
@@ -140,18 +143,26 @@ public static void enableTelemetry(datadog.trace.api.StatsDClient statsDClient)
140143
}
141144
}
142145

143-
// shutdown telemetry and reset to NO_OP
146+
/** Shuts down telemetry collection and resets the telemetry collector to NO_OP. */
144147
public static void shutdownTelemetry() {
145148
telemetryCollector.close();
146149
telemetryCollector = RumTelemetryCollector.NO_OP;
147150
}
148151

149-
// set the telemetry collector
152+
/**
153+
* Sets the telemetry collector.
154+
*
155+
* @param collector The telemetry collector to set or {@code null} to reset to NO_OP.
156+
*/
150157
public static void setTelemetryCollector(RumTelemetryCollector collector) {
151158
telemetryCollector = collector != null ? collector : RumTelemetryCollector.NO_OP;
152159
}
153160

154-
// get the telemetry collector. this is used to directly report telemetry
161+
/**
162+
* Gets the telemetry collector.
163+
*
164+
* @return The telemetry collector used to report telemetry.
165+
*/
155166
public static RumTelemetryCollector getTelemetryCollector() {
156167
return telemetryCollector;
157168
}

‎internal-api/src/main/java/datadog/trace/api/rum/RumInjectorMetrics.java‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,12 @@
22

33
import datadog.trace.api.StatsDClient;
44
import java.util.concurrent.atomic.AtomicLong;
5-
import org.slf4j.Logger;
6-
import org.slf4j.LoggerFactory;
75

86
// This class implements the RumTelemetryCollector interface, which is used to collect telemetry
97
// from the RumInjector. Metrics are then reported via StatsDClient with tagging. See:
108
// https://github.com/DataDog/dd-go/blob/prod/trace/apps/tracer-telemetry-intake/telemetry-metrics/static/common_metrics.json
119
// for common metrics and tags.
1210
public class RumInjectorMetrics implements RumTelemetryCollector {
13-
private static final Logger log = LoggerFactory.getLogger(RumInjectorMetrics.class);
14-
15-
private static final String[] NO_TAGS = new String[0];
16-
1711
// Use static tags for common combinations so that we don't have to build them for each metric
1812
private static final String[] CSP_SERVLET3_TAGS =
1913
new String[] {
@@ -79,8 +73,7 @@ public RumInjectorMetrics(final StatsDClient statsd) {
7973
// Get RUM config values (applicationId and remoteConfigUsed) for tagging
8074
RumInjector rumInjector = RumInjector.get();
8175
if (rumInjector.isEnabled()) {
82-
datadog.trace.api.Config config = datadog.trace.api.Config.get();
83-
RumInjectorConfig injectorConfig = config.getRumInjectorConfig();
76+
RumInjectorConfig injectorConfig = datadog.trace.api.Config.get().getRumInjectorConfig();
8477
if (injectorConfig != null) {
8578
this.applicationId = injectorConfig.applicationId;
8679
this.remoteConfigUsed = injectorConfig.remoteConfigurationId != null ? "true" : "false";

‎internal-api/src/main/java/datadog/trace/api/rum/RumTelemetryCollector.java‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,30 +36,22 @@ public String summary() {
3636
}
3737
};
3838

39-
// call when RUM injection succeeds
4039
void onInjectionSucceed(String integrationVersion);
4140

42-
// call when RUM injection fails
4341
void onInjectionFailed(String integrationVersion, String contentEncoding);
4442

45-
// call when RUM injection is skipped
4643
void onInjectionSkipped(String integrationVersion);
4744

48-
// call when RUM injector initialization succeeds
4945
void onInitializationSucceed();
5046

51-
// call when a Content Security Policy header is detected
5247
void onContentSecurityPolicyDetected(String integrationVersion);
5348

54-
// call to get the response size before RUM injection
5549
void onInjectionResponseSize(String integrationVersion, long bytes);
5650

57-
// call to report the time it takes to inject the RUM SDK
5851
void onInjectionTime(String integrationVersion, long milliseconds);
5952

6053
default void close() {}
6154

62-
// human-readable summary of the current health metrics
6355
default String summary() {
6456
return "";
6557
}

0 commit comments

Comments
 (0)