Skip to content

Commit 7387cc9

Browse files
xgouchetxgouchet
authored
Merge pull request #2496 from DataDog/xgouchet/RUM-8314/unredact_404
RUM-8314 allow disabling 404 span redaction
2 parents 6c54695 + 6a01d80 commit 7387cc9

13 files changed

+123
-25
lines changed

‎integrations/dd-sdk-android-okhttp/api/apiSurface

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,5 @@ open class com.datadog.android.okhttp.trace.TracingInterceptor : okhttp3.Interce
5858
fun setTraceSampleRate(Float): R
5959
fun setTraceSampler(com.datadog.android.core.sampling.Sampler<io.opentracing.Span>): R
6060
fun setTraceContextInjection(com.datadog.android.okhttp.TraceContextInjection): R
61+
fun set404ResourcesRedacted(Boolean): R
6162
abstract fun build(): T

‎integrations/dd-sdk-android-okhttp/api/dd-sdk-android-okhttp.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ public class com/datadog/android/okhttp/trace/TracingInterceptor : okhttp3/Inter
112112
public abstract class com/datadog/android/okhttp/trace/TracingInterceptor$BaseBuilder {
113113
public fun <init> (Ljava/util/Map;)V
114114
public abstract fun build ()Lcom/datadog/android/okhttp/trace/TracingInterceptor;
115+
public final fun set404ResourcesRedacted (Z)Lcom/datadog/android/okhttp/trace/TracingInterceptor$BaseBuilder;
115116
public final fun setSdkInstanceName (Ljava/lang/String;)Lcom/datadog/android/okhttp/trace/TracingInterceptor$BaseBuilder;
116117
public final fun setTraceContextInjection (Lcom/datadog/android/okhttp/TraceContextInjection;)Lcom/datadog/android/okhttp/trace/TracingInterceptor$BaseBuilder;
117118
public final fun setTraceSampleRate (F)Lcom/datadog/android/okhttp/trace/TracingInterceptor$BaseBuilder;

‎integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/DatadogInterceptor.kt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ open class DatadogInterceptor internal constructor(
7979
internal val rumResourceAttributesProvider: RumResourceAttributesProvider,
8080
traceSampler: Sampler<Span>,
8181
traceContextInjection: TraceContextInjection,
82+
redacted404ResourceName: Boolean,
8283
localTracerFactory: (SdkCore, Set<TracingHeaderType>) -> Tracer
8384
) : TracingInterceptor(
8485
sdkInstanceName,
@@ -87,6 +88,7 @@ open class DatadogInterceptor internal constructor(
8788
ORIGIN_RUM,
8889
traceSampler,
8990
traceContextInjection,
91+
redacted404ResourceName,
9092
localTracerFactory
9193
) {
9294

@@ -135,6 +137,7 @@ open class DatadogInterceptor internal constructor(
135137
rumResourceAttributesProvider = rumResourceAttributesProvider,
136138
traceSampler = traceSampler,
137139
traceContextInjection = TraceContextInjection.All,
140+
redacted404ResourceName = true,
138141
localTracerFactory = { sdkCore, tracingHeaderTypes ->
139142
AndroidTracer.Builder(sdkCore).setTracingHeaderTypes(tracingHeaderTypes).build()
140143
}
@@ -189,6 +192,7 @@ open class DatadogInterceptor internal constructor(
189192
rumResourceAttributesProvider = rumResourceAttributesProvider,
190193
traceSampler = traceSampler,
191194
traceContextInjection = TraceContextInjection.All,
195+
redacted404ResourceName = true,
192196
localTracerFactory = { sdkCore, tracingHeaderTypes ->
193197
AndroidTracer.Builder(sdkCore).setTracingHeaderTypes(tracingHeaderTypes).build()
194198
}
@@ -229,6 +233,7 @@ open class DatadogInterceptor internal constructor(
229233
rumResourceAttributesProvider = rumResourceAttributesProvider,
230234
traceSampler = traceSampler,
231235
traceContextInjection = TraceContextInjection.All,
236+
redacted404ResourceName = true,
232237
localTracerFactory = { sdkCore, tracingHeaderTypes ->
233238
AndroidTracer.Builder(sdkCore).setTracingHeaderTypes(tracingHeaderTypes).build()
234239
}
@@ -469,6 +474,7 @@ open class DatadogInterceptor internal constructor(
469474
rumResourceAttributesProvider,
470475
traceSampler,
471476
traceContextInjection,
477+
redacted404ResourceName,
472478
localTracerFactory
473479
)
474480
}

‎integrations/dd-sdk-android-okhttp/src/main/kotlin/com/datadog/android/okhttp/trace/TracingInterceptor.kt

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ internal constructor(
8484
internal val traceOrigin: String?,
8585
internal val traceSampler: Sampler<Span>,
8686
internal val traceContextInjection: TraceContextInjection,
87+
internal val redacted404ResourceName: Boolean,
8788
internal val localTracerFactory: (SdkCore, Set<TracingHeaderType>) -> Tracer
8889
) : Interceptor {
8990

@@ -130,18 +131,19 @@ internal constructor(
130131
tracedRequestListener: TracedRequestListener = NoOpTracedRequestListener(),
131132
traceSampler: Sampler<Span> = DeterministicTraceSampler(DEFAULT_TRACE_SAMPLE_RATE)
132133
) : this(
133-
sdkInstanceName,
134-
tracedHosts.associateWith {
134+
sdkInstanceName = sdkInstanceName,
135+
tracedHosts = tracedHosts.associateWith {
135136
setOf(
136137
TracingHeaderType.DATADOG,
137138
TracingHeaderType.TRACECONTEXT
138139
)
139140
},
140-
tracedRequestListener,
141-
null,
142-
traceSampler,
143-
localTracerFactory = DEFAULT_LOCAL_TRACER_FACTORY,
144-
traceContextInjection = TraceContextInjection.All
141+
tracedRequestListener = tracedRequestListener,
142+
traceOrigin = null,
143+
traceSampler = traceSampler,
144+
traceContextInjection = TraceContextInjection.All,
145+
redacted404ResourceName = true,
146+
localTracerFactory = DEFAULT_LOCAL_TRACER_FACTORY
145147
)
146148

147149
/**
@@ -175,13 +177,14 @@ internal constructor(
175177
tracedRequestListener: TracedRequestListener = NoOpTracedRequestListener(),
176178
traceSampler: Sampler<Span> = DeterministicTraceSampler(DEFAULT_TRACE_SAMPLE_RATE)
177179
) : this(
178-
sdkInstanceName,
179-
tracedHostsWithHeaderType,
180-
tracedRequestListener,
181-
null,
182-
traceSampler,
183-
localTracerFactory = DEFAULT_LOCAL_TRACER_FACTORY,
184-
traceContextInjection = TraceContextInjection.All
180+
sdkInstanceName = sdkInstanceName,
181+
tracedHosts = tracedHostsWithHeaderType,
182+
tracedRequestListener = tracedRequestListener,
183+
traceOrigin = null,
184+
traceSampler = traceSampler,
185+
traceContextInjection = TraceContextInjection.All,
186+
redacted404ResourceName = true,
187+
localTracerFactory = DEFAULT_LOCAL_TRACER_FACTORY
185188
)
186189

187190
/**
@@ -208,13 +211,14 @@ internal constructor(
208211
tracedRequestListener: TracedRequestListener = NoOpTracedRequestListener(),
209212
traceSampler: Sampler<Span> = DeterministicTraceSampler(DEFAULT_TRACE_SAMPLE_RATE)
210213
) : this(
211-
sdkInstanceName,
212-
emptyMap(),
213-
tracedRequestListener,
214-
null,
215-
traceSampler,
216-
localTracerFactory = DEFAULT_LOCAL_TRACER_FACTORY,
217-
traceContextInjection = TraceContextInjection.All
214+
sdkInstanceName = sdkInstanceName,
215+
tracedHosts = emptyMap(),
216+
tracedRequestListener = tracedRequestListener,
217+
traceOrigin = null,
218+
traceSampler = traceSampler,
219+
traceContextInjection = TraceContextInjection.All,
220+
redacted404ResourceName = true,
221+
localTracerFactory = DEFAULT_LOCAL_TRACER_FACTORY
218222
)
219223

220224
// region Interceptor
@@ -715,7 +719,7 @@ internal constructor(
715719
if (statusCode in HttpURLConnection.HTTP_BAD_REQUEST until HttpURLConnection.HTTP_INTERNAL_ERROR) {
716720
(span as? MutableSpan)?.isError = true
717721
}
718-
if (statusCode == HttpURLConnection.HTTP_NOT_FOUND) {
722+
if (statusCode == HttpURLConnection.HTTP_NOT_FOUND && redacted404ResourceName) {
719723
(span as? MutableSpan)?.resourceName = RESOURCE_NAME_404
720724
}
721725
onRequestIntercepted(sdkCore, request, span, response, null)
@@ -807,6 +811,7 @@ internal constructor(
807811
traceOrigin,
808812
traceSampler,
809813
traceContextInjection,
814+
redacted404ResourceName,
810815
localTracerFactory
811816
)
812817
}
@@ -825,6 +830,8 @@ internal constructor(
825830
internal var localTracerFactory = DEFAULT_LOCAL_TRACER_FACTORY
826831
internal var traceContextInjection = TraceContextInjection.All
827832

833+
internal var redacted404ResourceName = true
834+
828835
/**
829836
* Set the SDK instance name to bind to, the default value is null.
830837
* @param sdkInstanceName SDK instance name to bind to, the default value is null.
@@ -882,6 +889,18 @@ internal constructor(
882889
return getThis()
883890
}
884891

892+
/**
893+
* Set whether network requests returning a 404 status code should have their resource name redacted.
894+
* In order to reduce the cardinality of resource names in APM, 404 URLs are automatically redacted to
895+
* "404".
896+
* @param redacted if true, all 404 requests will have a resource name set to "404", else the resource name
897+
* will be the URL
898+
*/
899+
fun set404ResourcesRedacted(redacted: Boolean): R {
900+
redacted404ResourceName = redacted
901+
return getThis()
902+
}
903+
885904
internal fun setLocalTracerFactory(factory: (SdkCore, Set<TracingHeaderType>) -> Tracer): R {
886905
this.localTracerFactory = factory
887906
return getThis()

‎integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/DatadogInterceptorTest.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ internal class DatadogInterceptorTest : TracingInterceptorNotSendingSpanTest() {
9595
rumResourceAttributesProvider = mockRumAttributesProvider,
9696
traceSampler = mockTraceSampler,
9797
traceContextInjection = TraceContextInjection.All,
98+
redacted404ResourceName = fakeRedacted404Resources,
9899
localTracerFactory = factory
99100
)
100101
}

‎integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/DatadogInterceptorWithoutRumTest.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ internal class DatadogInterceptorWithoutRumTest : TracingInterceptorTest() {
6060
rumResourceAttributesProvider = mockRumAttributesProvider,
6161
traceSampler = mockTraceSampler,
6262
traceContextInjection = TraceContextInjection.All,
63+
redacted404ResourceName = fakeRedacted404Resources,
6364
localTracerFactory = factory
6465
)
6566
}

‎integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/DatadogInterceptorWithoutTracesTest.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import com.datadog.tools.unit.extensions.config.TestConfiguration
3131
import com.datadog.tools.unit.forge.BaseConfigurator
3232
import com.datadog.tools.unit.forge.exhaustiveAttributes
3333
import fr.xgouchet.elmyr.Forge
34+
import fr.xgouchet.elmyr.annotation.BoolForgery
3435
import fr.xgouchet.elmyr.annotation.Forgery
3536
import fr.xgouchet.elmyr.annotation.IntForgery
3637
import fr.xgouchet.elmyr.annotation.StringForgery
@@ -138,6 +139,8 @@ internal class DatadogInterceptorWithoutTracesTest {
138139
@StringForgery(type = StringForgeryType.HEXADECIMAL)
139140
lateinit var fakeTraceId: String
140141

142+
@BoolForgery
143+
var fakeRedacted404Resources: Boolean = true
141144
// endregion
142145

143146
@BeforeEach
@@ -162,6 +165,7 @@ internal class DatadogInterceptorWithoutTracesTest {
162165
tracedRequestListener = mockRequestListener,
163166
rumResourceAttributesProvider = mockRumAttributesProvider,
164167
traceSampler = mockTraceSampler,
168+
redacted404ResourceName = fakeRedacted404Resources,
165169
traceContextInjection = TraceContextInjection.All
166170
) { _, _ -> mockLocalTracer }
167171
whenever(rumMonitor.mockSdkCore.getFeature(Feature.TRACING_FEATURE_NAME)) doReturn mock()

‎integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/TracingInterceptorBuilderTest.kt

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import com.datadog.android.trace.TracingHeaderType
1515
import com.datadog.tools.unit.extensions.TestConfigurationExtension
1616
import com.datadog.tools.unit.forge.BaseConfigurator
1717
import fr.xgouchet.elmyr.Forge
18+
import fr.xgouchet.elmyr.annotation.BoolForgery
1819
import fr.xgouchet.elmyr.annotation.StringForgery
1920
import fr.xgouchet.elmyr.junit5.ForgeConfiguration
2021
import fr.xgouchet.elmyr.junit5.ForgeExtension
@@ -81,6 +82,7 @@ internal class TracingInterceptorBuilderTest {
8182
assertThat(interceptor.traceSampler).isInstanceOf(DeterministicTraceSampler::class.java)
8283
assertThat(interceptor.traceOrigin).isNull()
8384
assertThat(interceptor.localTracerFactory).isNotNull()
85+
assertThat(interceptor.redacted404ResourceName).isTrue()
8486
}
8587

8688
@Test
@@ -103,6 +105,7 @@ internal class TracingInterceptorBuilderTest {
103105
assertThat(interceptor.traceSampler).isInstanceOf(DeterministicTraceSampler::class.java)
104106
assertThat(interceptor.traceOrigin).isNull()
105107
assertThat(interceptor.localTracerFactory).isNotNull()
108+
assertThat(interceptor.redacted404ResourceName).isTrue()
106109
}
107110

108111
@Test
@@ -120,6 +123,7 @@ internal class TracingInterceptorBuilderTest {
120123
assertThat(interceptor.traceSampler).isInstanceOf(DeterministicTraceSampler::class.java)
121124
assertThat(interceptor.traceOrigin).isNull()
122125
assertThat(interceptor.localTracerFactory).isNotNull()
126+
assertThat(interceptor.redacted404ResourceName).isTrue()
123127
}
124128

125129
@Test
@@ -137,6 +141,7 @@ internal class TracingInterceptorBuilderTest {
137141
assertThat(interceptor.traceSampler).isInstanceOf(DeterministicTraceSampler::class.java)
138142
assertThat(interceptor.traceOrigin).isNull()
139143
assertThat(interceptor.localTracerFactory).isNotNull()
144+
assertThat(interceptor.redacted404ResourceName).isTrue()
140145
}
141146

142147
@Test
@@ -154,6 +159,7 @@ internal class TracingInterceptorBuilderTest {
154159
assertThat(interceptor.traceSampler).isInstanceOf(DeterministicTraceSampler::class.java)
155160
assertThat(interceptor.traceOrigin).isEqualTo(fakeOrigin)
156161
assertThat(interceptor.localTracerFactory).isNotNull()
162+
assertThat(interceptor.redacted404ResourceName).isTrue()
157163
}
158164

159165
@Test
@@ -171,6 +177,7 @@ internal class TracingInterceptorBuilderTest {
171177
assertThat(interceptor.traceSampler).isInstanceOf(DeterministicTraceSampler::class.java)
172178
assertThat(interceptor.traceOrigin).isNull()
173179
assertThat(interceptor.localTracerFactory).isNotNull()
180+
assertThat(interceptor.redacted404ResourceName).isTrue()
174181
}
175182

176183
@Test
@@ -188,5 +195,26 @@ internal class TracingInterceptorBuilderTest {
188195
assertThat(interceptor.traceSampler).isSameAs(mockSampler)
189196
assertThat(interceptor.traceOrigin).isNull()
190197
assertThat(interceptor.localTracerFactory).isNotNull()
198+
assertThat(interceptor.redacted404ResourceName).isTrue()
199+
}
200+
201+
@Test
202+
fun `M set redacted404Resources W build { setTraceSampler }`(
203+
@BoolForgery redacted: Boolean
204+
) {
205+
// When
206+
val interceptor = TracingInterceptor.Builder(fakeTracedHostsWithHeaderType)
207+
.set404ResourcesRedacted(redacted)
208+
.build()
209+
210+
// Then
211+
assertThat(interceptor.tracedHosts).isEqualTo(fakeTracedHostsWithHeaderType)
212+
assertThat(interceptor.traceContextInjection).isEqualTo(TraceContextInjection.All)
213+
assertThat(interceptor.sdkInstanceName).isNull()
214+
assertThat(interceptor.tracedRequestListener).isInstanceOf(NoOpTracedRequestListener::class.java)
215+
assertThat(interceptor.traceSampler).isInstanceOf(DeterministicTraceSampler::class.java)
216+
assertThat(interceptor.traceOrigin).isNull()
217+
assertThat(interceptor.localTracerFactory).isNotNull()
218+
assertThat(interceptor.redacted404ResourceName).isEqualTo(redacted)
191219
}
192220
}

‎integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/trace/TracingInterceptorContextInjectionSampledTest.kt

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import com.datadog.tools.unit.extensions.TestConfigurationExtension
2929
import com.datadog.tools.unit.extensions.config.TestConfiguration
3030
import com.datadog.tools.unit.setStaticValue
3131
import fr.xgouchet.elmyr.Forge
32+
import fr.xgouchet.elmyr.annotation.BoolForgery
3233
import fr.xgouchet.elmyr.annotation.Forgery
3334
import fr.xgouchet.elmyr.annotation.IntForgery
3435
import fr.xgouchet.elmyr.annotation.StringForgery
@@ -127,6 +128,9 @@ internal class TracingInterceptorContextInjectionSampledTest {
127128
@Mock
128129
lateinit var mockInternalLogger: InternalLogger
129130

131+
@BoolForgery
132+
var fakeRedacted404Resources: Boolean = true
133+
130134
// endregion
131135

132136
// region Fakes
@@ -200,6 +204,7 @@ internal class TracingInterceptorContextInjectionSampledTest {
200204
.setTraceSampler(mockTraceSampler)
201205
.setTraceContextInjection(TraceContextInjection.Sampled)
202206
.setLocalTracerFactory(factory)
207+
.set404ResourcesRedacted(fakeRedacted404Resources)
203208
.build()
204209
}
205210

@@ -1037,7 +1042,11 @@ internal class TracingInterceptorContextInjectionSampledTest {
10371042
verify(mockSpan).setTag("http.method", fakeMethod)
10381043
verify(mockSpan).setTag("http.status_code", 404)
10391044
verify(mockSpan as MutableSpan).setError(true)
1040-
verify(mockSpan as MutableSpan).setResourceName(TracingInterceptor.RESOURCE_NAME_404)
1045+
if (fakeRedacted404Resources) {
1046+
verify(mockSpan as MutableSpan).setResourceName(TracingInterceptor.RESOURCE_NAME_404)
1047+
} else {
1048+
verify(mockSpan as MutableSpan, never()).setResourceName(TracingInterceptor.RESOURCE_NAME_404)
1049+
}
10411050
verify(mockSpan).setTag(Tags.SPAN_KIND, Tags.SPAN_KIND_CLIENT)
10421051
verify(mockSpan).finish()
10431052
assertThat(response).isSameAs(fakeResponse)

‎integrations/dd-sdk-android-okhttp/src/test/kotlin/com/datadog/android/okhttp/trace/TracingInterceptorNonDdTracerNotSendingSpanTest.kt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import com.datadog.tools.unit.extensions.config.TestConfiguration
2929
import com.datadog.tools.unit.forge.BaseConfigurator
3030
import com.datadog.tools.unit.setStaticValue
3131
import fr.xgouchet.elmyr.Forge
32+
import fr.xgouchet.elmyr.annotation.BoolForgery
3233
import fr.xgouchet.elmyr.annotation.Forgery
3334
import fr.xgouchet.elmyr.annotation.IntForgery
3435
import fr.xgouchet.elmyr.annotation.StringForgery
@@ -161,6 +162,9 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest {
161162

162163
lateinit var fakeLocalHosts: Map<String, Set<TracingHeaderType>>
163164

165+
@BoolForgery
166+
var fakeRedacted404Resources: Boolean = true
167+
164168
// endregion
165169

166170
@BeforeEach
@@ -216,6 +220,7 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest {
216220
traceOrigin = fakeOrigin,
217221
traceSampler = mockTraceSampler,
218222
traceContextInjection = TraceContextInjection.All,
223+
redacted404ResourceName = fakeRedacted404Resources,
219224
localTracerFactory = factory
220225
) {
221226
override fun canSendSpan(): Boolean {

0 commit comments

Comments
 (0)