Skip to content

Commit 69cb881

Browse files
AugustyniakAugustyniak
authored
health dashboards: remove path templatization logic (#9)
1 parent 7294fc0 commit 69cb881

19 files changed

+76
-378
lines changed

‎platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/CaptureJniLibrary.kt

-5
Original file line numberDiff line numberDiff line change
@@ -263,9 +263,4 @@ internal object CaptureJniLibrary {
263263
* directly as it allows for centralized control over error flood controls.
264264
*/
265265
external fun reportError(message: String, stackTraceProvider: StackTraceProvider)
266-
267-
/**
268-
* Normalizes a URL path, replacing any high cardinality path substrings with "<id>".
269-
*/
270-
external fun normalizeUrlPath(urlPath: String): String?
271266
}

‎platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/network/HttpRequestInfo.kt

+4-11
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
package io.bitdrift.capture.network
99

10-
import io.bitdrift.capture.CaptureJniLibrary
1110
import io.bitdrift.capture.InternalFieldsMap
1211
import io.bitdrift.capture.events.span.SpanField
1312
import io.bitdrift.capture.providers.FieldValue
@@ -36,9 +35,6 @@ data class HttpRequestInfo @JvmOverloads constructor(
3635
) {
3736
internal val name: String = "HTTPRequest"
3837

39-
// Lazy since the creation of the `HTTPRequestInfo` can happen before the logger is initialized
40-
// and trying to call a native `CaptureJniLibrary.normalizeUrlPath` method before the `Capture`
41-
// library is loaded leads to unsatisfied linking error.
4238
internal val fields: InternalFieldsMap by lazy {
4339
// Do not put body bytes count as a common field since response log has a more accurate
4440
// measurement of request' body count anyway.
@@ -49,7 +45,7 @@ data class HttpRequestInfo @JvmOverloads constructor(
4945
}
5046

5147
internal val commonFields: InternalFieldsMap by lazy {
52-
val fields = buildMap {
48+
extraFields.toFields() + buildMap {
5349
put(SpanField.Key.ID, FieldValue.StringField(spanId.toString()))
5450
put(SpanField.Key.NAME, FieldValue.StringField("_http"))
5551
put(SpanField.Key.TYPE, FieldValue.StringField(SpanField.Value.TYPE_START))
@@ -58,14 +54,11 @@ data class HttpRequestInfo @JvmOverloads constructor(
5854
putOptional(HttpFieldKey.PATH, path?.value)
5955
putOptional(HttpFieldKey.QUERY, query)
6056
path?.let {
61-
@Suppress("SwallowedException")
62-
val normalized: String? = it.template?.let { it }
63-
?: try { CaptureJniLibrary.normalizeUrlPath(it.value) } catch (e: Throwable) { null }
64-
putOptional(HttpFieldKey.PATH_TEMPLATE, normalized)
57+
it.template?.let {
58+
put(HttpFieldKey.PATH_TEMPLATE, FieldValue.StringField(it))
59+
}
6560
}
6661
}
67-
68-
extraFields.toFields() + fields
6962
}
7063

7164
internal val matchingFields: InternalFieldsMap = headers?.let { HTTPHeaders.normalizeHeaders(it) }.toFields()

‎platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/network/HttpResponseInfo.kt

+64-47
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
package io.bitdrift.capture.network
99

10-
import io.bitdrift.capture.CaptureJniLibrary
1110
import io.bitdrift.capture.InternalFieldsMap
1211
import io.bitdrift.capture.events.span.SpanField
1312
import io.bitdrift.capture.providers.FieldValue
@@ -36,60 +35,78 @@ data class HttpResponseInfo @JvmOverloads constructor(
3635
) {
3736
internal val name: String = "HTTPResponse"
3837

39-
// Lazy since the creation of the `HTTPResponseInfo` can happen before the logger is initialized
40-
// and trying to call a native `CaptureJniLibrary.normalizeUrlPath` method before the `Capture`
41-
// library is loaded leads to unsatisfied linking error.
42-
internal val fields: InternalFieldsMap by lazy {
43-
val fields = buildMap {
44-
put(SpanField.Key.TYPE, FieldValue.StringField(SpanField.Value.TYPE_END))
45-
put(SpanField.Key.DURATION, FieldValue.StringField(durationMs.toString()))
46-
put(SpanField.Key.RESULT, FieldValue.StringField(response.result.name.lowercase()))
47-
putOptional("_status_code", response.statusCode)
48-
putOptional("_error_type", response.error) { it::javaClass.get().simpleName }
49-
putOptional("_error_message", response.error) { it.message.orEmpty() }
50-
putOptional(HttpFieldKey.HOST, response.host)
51-
putOptional(HttpFieldKey.PATH, response.path?.value)
52-
putOptional(HttpFieldKey.QUERY, response.query)
38+
internal val fields: InternalFieldsMap =
39+
run {
40+
// Collect out-of-the-box fields specific to HTTPResponse logs. The list consists of
41+
// HTTP specific fields such as host, path, or query and HTTP request performance
42+
// metrics such as DNS resolution time.
43+
val fields = buildMap {
44+
this.put(SpanField.Key.TYPE, FieldValue.StringField(SpanField.Value.TYPE_END))
45+
this.put(
46+
SpanField.Key.DURATION,
47+
FieldValue.StringField(durationMs.toString()),
48+
)
49+
this.put(
50+
SpanField.Key.RESULT,
51+
FieldValue.StringField(response.result.name.lowercase()),
52+
)
53+
putOptional("_status_code", response.statusCode)
54+
putOptional(
55+
"_error_type",
56+
response.error,
57+
) { it::javaClass.get().simpleName }
58+
putOptional(
59+
"_error_message",
60+
response.error,
61+
) { it.message.orEmpty() }
62+
putOptional(HttpFieldKey.HOST, response.host)
63+
putOptional(HttpFieldKey.PATH, response.path?.value)
64+
putOptional(HttpFieldKey.QUERY, response.query)
5365

54-
response.path?.let {
55-
val requestPathTemplate = if (request.path?.value == it.value) {
56-
// If the path between request and response did not change and an explicit path
57-
// template was provided as part of a request use it as path template on a response.
58-
request.path.template
59-
} else {
60-
null
61-
}
66+
response.path?.let {
67+
val requestPathTemplate =
68+
if (request.path?.value == it.value) {
69+
// If the path between request and response did not change and an explicit path
70+
// template was provided as part of a request use it as path template on a response.
71+
request.path.template
72+
} else {
73+
null
74+
}
6275

63-
@Suppress("SwallowedException")
64-
val normalized: String? = requestPathTemplate?.let { it }
65-
?: it.template?.let { it }
66-
?: try {
67-
CaptureJniLibrary.normalizeUrlPath(it.value)
68-
} catch (e: Throwable) {
69-
null
70-
}
76+
putOptional(
77+
HttpFieldKey.PATH_TEMPLATE,
78+
requestPathTemplate?.let { it } ?: it.template?.let { it },
79+
)
80+
}
7181

72-
putOptional(HttpFieldKey.PATH_TEMPLATE, normalized)
82+
metrics?.let<HttpRequestMetrics, Unit> {
83+
this.put(
84+
"_request_body_bytes_sent_count",
85+
FieldValue.StringField(it.requestBodyBytesSentCount.toString()),
86+
)
87+
this.put(
88+
"_response_body_bytes_received_count",
89+
FieldValue.StringField(it.responseBodyBytesReceivedCount.toString()),
90+
)
91+
this.put(
92+
"_request_headers_bytes_count",
93+
FieldValue.StringField(it.requestHeadersBytesCount.toString()),
94+
)
95+
this.put(
96+
"_response_headers_bytes_count",
97+
FieldValue.StringField(it.responseHeadersBytesCount.toString()),
98+
)
99+
putOptional("_dns_resolution_duration_ms", it.dnsResolutionDurationMs)
100+
}
73101
}
74102

75-
metrics?.let {
76-
put("_request_body_bytes_sent_count", FieldValue.StringField(it.requestBodyBytesSentCount.toString()))
77-
put("_response_body_bytes_received_count", FieldValue.StringField(it.responseBodyBytesReceivedCount.toString()))
78-
put("_request_headers_bytes_count", FieldValue.StringField(it.requestHeadersBytesCount.toString()))
79-
put("_response_headers_bytes_count", FieldValue.StringField(it.responseHeadersBytesCount.toString()))
80-
putOptional("_dns_resolution_duration_ms", it.dnsResolutionDurationMs)
81-
}
103+
// Combine fields in the increasing order of their priority as the latter fields
104+
// override the former ones in the case of field name conflicts.
105+
extraFields.toFields() + request.commonFields + fields
82106
}
83107

84-
extraFields.toFields() + request.commonFields + fields
85-
}
86-
87-
// Lazy since the creation of the `HTTPResponseInfo` can happen before the logger is initialized
88-
// and trying to call a native `CaptureJniLibrary.normalizeUrlPath` method before the `Capture`
89-
// library is loaded leads to unsatisfied linking error.
90-
internal val matchingFields: InternalFieldsMap by lazy {
108+
internal val matchingFields: InternalFieldsMap =
91109
request.fields.mapKeys { "_request.${it.key}" } +
92110
request.matchingFields.mapKeys { "_request.${it.key}" } +
93111
response.headers?.let { HTTPHeaders.normalizeHeaders(it) }.toFields()
94-
}
95112
}

‎platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureLoggerTest.kt

-3
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,6 @@ class CaptureLoggerTest {
125125
"_host" to "api.bitdrift.io",
126126
"_method" to "GET",
127127
"_path" to "/my_path/12345",
128-
"_path_template" to "/my_path/<id>",
129128
"_query" to "my=query",
130129
"_span_id" to spanId.toString(),
131130
"_span_name" to "_http",
@@ -164,7 +163,6 @@ class CaptureLoggerTest {
164163
"_host" to "api.bitdrift.io",
165164
"_method" to "GET",
166165
"_path" to "/my_path/12345",
167-
"_path_template" to "/my_path/<id>",
168166
"_query" to "my=query",
169167
"_span_id" to spanId.toString(),
170168
"_span_name" to "_http",
@@ -181,7 +179,6 @@ class CaptureLoggerTest {
181179
"_request._host" to "api.bitdrift.io",
182180
"_request._method" to "GET",
183181
"_request._path" to "/my_path/12345",
184-
"_request._path_template" to "/my_path/<id>",
185182
"_request._span_id" to spanId.toString(),
186183
"_request._span_name" to "_http",
187184
"_request._span_type" to "start",

‎platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/CaptureOkHttpEventListenerFactoryTest.kt

-7
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,6 @@ class CaptureOkHttpEventListenerFactoryTest {
3131
private val clock: IClock = mock()
3232
private val logger: ILogger = mock()
3333

34-
init {
35-
CaptureJniLibrary.load()
36-
}
37-
3834
@Test
3935
fun testRequestAndResponseReuseCommonInfo() {
4036
// ARRANGE
@@ -98,9 +94,6 @@ class CaptureOkHttpEventListenerFactoryTest {
9894
assertThat(httpRequestInfo.fields["_path"].toString()).isEqualTo("/my_path/12345")
9995
assertThat(httpResponseInfo.fields["_path"].toString())
10096
.isEqualTo(httpRequestInfo.fields["_path"].toString())
101-
assertThat(httpRequestInfo.fields["_path_template"].toString()).isEqualTo("/my_path/<id>")
102-
assertThat(httpResponseInfo.fields["_path_template"].toString())
103-
.isEqualTo(httpRequestInfo.fields["_path_template"].toString())
10497
assertThat(httpRequestInfo.fields["_query"].toString()).isEqualTo("my_query=my_value")
10598
assertThat(httpResponseInfo.fields["_query"].toString())
10699
.isEqualTo(httpRequestInfo.fields["_query"].toString())

‎platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/HttpRequestInfoTest.kt

-37
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@ import org.junit.Test
1515
import java.util.UUID
1616

1717
class HttpRequestInfoTest {
18-
init {
19-
CaptureJniLibrary.load()
20-
}
2118

2219
@Test
2320
fun testFields() {
@@ -54,38 +51,4 @@ class HttpRequestInfoTest {
5451
).toFields(),
5552
)
5653
}
57-
58-
@Test
59-
fun testTemplateFallback() {
60-
val spanId = UUID.randomUUID()
61-
val requestInfo = HttpRequestInfo(
62-
host = "api.bitdrift.io",
63-
method = "GET",
64-
path = HttpUrlPath("/my_path/12345"),
65-
query = "my=query",
66-
headers = mapOf("content-type" to "json"),
67-
spanId = spanId,
68-
extraFields = mapOf("my_extra_key_1" to "my_extra_value_1"),
69-
)
70-
71-
assertThat(requestInfo.fields).isEqualTo(
72-
mapOf(
73-
"_host" to "api.bitdrift.io",
74-
"_method" to "GET",
75-
"_path" to "/my_path/12345",
76-
"_path_template" to "/my_path/<id>",
77-
"_query" to "my=query",
78-
"_span_id" to spanId.toString(),
79-
"_span_name" to "_http",
80-
"_span_type" to "start",
81-
"my_extra_key_1" to "my_extra_value_1",
82-
).toFields(),
83-
)
84-
85-
assertThat(requestInfo.matchingFields).isEqualTo(
86-
mapOf(
87-
"_headers.content-type" to "json",
88-
).toFields(),
89-
)
90-
}
9154
}

‎platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/HttpResponseInfoTest.kt

+2-8
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,9 @@ import org.junit.Test
1717
import java.util.UUID
1818

1919
class HttpResponseInfoTest {
20-
init {
21-
CaptureJniLibrary.load()
22-
}
2320

2421
@Test
25-
fun test_response_template_override() {
22+
fun testHTTPResponseTemplateOverride() {
2623
val spanId = UUID.randomUUID()
2724
val requestInfo = HttpRequestInfo(
2825
host = "foo.com",
@@ -67,7 +64,7 @@ class HttpResponseInfoTest {
6764
}
6865

6966
@Test
70-
fun test_http_request_explicit_path_template() {
67+
fun testHTTPRequestExplicitPathTemplate() {
7168
val spanId = UUID.randomUUID()
7269
val requestInfo = HttpRequestInfo(
7370
host = "foo.com",
@@ -129,7 +126,6 @@ class HttpResponseInfoTest {
129126
"_host" to "api.bitdrift.io",
130127
"_method" to "GET",
131128
"_path" to "/my_path/12345",
132-
"_path_template" to "/my_path/<id>",
133129
"_query" to "my=query",
134130
"_span_id" to spanId.toString(),
135131
"_span_name" to "_http",
@@ -156,7 +152,6 @@ class HttpResponseInfoTest {
156152
"_host" to "foo.com",
157153
"_method" to "GET",
158154
"_path" to "/foo_path/12345",
159-
"_path_template" to "/foo_path/<id>",
160155
"_query" to "foo_query",
161156
"_span_id" to spanId.toString(),
162157
"_span_name" to "_http",
@@ -175,7 +170,6 @@ class HttpResponseInfoTest {
175170
"_request._host" to "api.bitdrift.io",
176171
"_request._method" to "GET",
177172
"_request._path" to "/my_path/12345",
178-
"_request._path_template" to "/my_path/<id>",
179173
"_request._span_id" to spanId.toString(),
180174
"_request._span_name" to "_http",
181175
"_request._span_type" to "start",

‎platform/jvm/jni_symbols.lds

-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,5 @@ Java_io_bitdrift_capture_CaptureJniLibrary_flush
1919
Java_io_bitdrift_capture_CaptureJniLibrary_debugDebug
2020
Java_io_bitdrift_capture_CaptureJniLibrary_debugError
2121
Java_io_bitdrift_capture_CaptureJniLibrary_reportError
22-
Java_io_bitdrift_capture_CaptureJniLibrary_normalizeUrlPath
2322
Java_io_bitdrift_capture_Jni_isRuntimeEnabled
2423
JNI_OnLoad

‎platform/jvm/src/jni.rs

+1-22
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ use jni::signature::{Primitive, ReturnType};
4949
use jni::sys::{jboolean, jbyteArray, jdouble, jint, jlong, jobject, jvalue, JNI_TRUE};
5050
use jni::{JNIEnv, JavaVM};
5151
use platform_shared::metadata::Mobile;
52-
use platform_shared::{normalize_url_path, LoggerHolder, LoggerId};
52+
use platform_shared::{LoggerHolder, LoggerId};
5353
use std::borrow::{Borrow, Cow};
5454
use std::collections::HashMap;
5555
use std::ffi::c_void;
@@ -1072,27 +1072,6 @@ fn exception_stacktrace(
10721072
Ok(Some(stacktrace_str.to_string_lossy().to_string()))
10731073
}
10741074

1075-
#[no_mangle]
1076-
pub extern "system" fn Java_io_bitdrift_capture_CaptureJniLibrary_normalizeUrlPath<'a>(
1077-
env: JNIEnv<'a>,
1078-
_class: JClass<'_>,
1079-
url_path: JString<'a>,
1080-
) -> JString<'a> {
1081-
bd_client_common::error::with_handle_unexpected_or(
1082-
|| {
1083-
let url_path = unsafe { env.get_string_unchecked(&url_path) }?
1084-
.to_string_lossy()
1085-
.to_string();
1086-
1087-
let normalized_url_path = normalize_url_path(&url_path);
1088-
1089-
Ok(env.new_string(normalized_url_path)?)
1090-
},
1091-
unsafe { JString::from_raw(core::ptr::null_mut()) },
1092-
"jni normalize_url_path",
1093-
)
1094-
}
1095-
10961075
#[no_mangle]
10971076
pub extern "system" fn Java_io_bitdrift_capture_Jni_isRuntimeEnabled(
10981077
env: JNIEnv<'_>,

0 commit comments

Comments
 (0)