Skip to content

Commit c529c39

Browse files
author
Rishabh
authored
Set Event.http.request.url on a best effort basis
1. Update HttpFieldsGenerator#maybeSetHttpUrlForOtelFormat to overwrite relative url (#104) 2. Update the logic to fetch http url in SpanEventViewGenerator
1 parent 6b774bf commit c529c39

File tree

4 files changed

+88
-5
lines changed

4 files changed

+88
-5
lines changed

‎hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/SpanEventViewGenerator.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,15 @@
66
import java.util.HashMap;
77
import java.util.List;
88
import java.util.Map;
9+
import java.util.Optional;
910
import java.util.stream.Collectors;
1011
import org.apache.avro.Schema;
1112
import org.hypertrace.core.datamodel.Entity;
1213
import org.hypertrace.core.datamodel.Event;
1314
import org.hypertrace.core.datamodel.MetricValue;
1415
import org.hypertrace.core.datamodel.StructuredTrace;
16+
import org.hypertrace.core.datamodel.eventfields.http.Http;
17+
import org.hypertrace.core.datamodel.eventfields.http.Request;
1518
import org.hypertrace.core.datamodel.shared.SpanAttributeUtils;
1619
import org.hypertrace.traceenricher.enrichedspan.constants.EnrichedSpanConstants;
1720
import org.hypertrace.traceenricher.enrichedspan.constants.utils.EnrichedSpanUtils;
@@ -292,7 +295,9 @@ String getRequestUrl(Event event, Protocol protocol) {
292295
switch (protocol) {
293296
case PROTOCOL_HTTP:
294297
case PROTOCOL_HTTPS:
295-
return EnrichedSpanUtils.getFullHttpUrl(event).orElse(event.getHttp().getRequest().getPath());
298+
return EnrichedSpanUtils.getFullHttpUrl(event).orElse(
299+
Optional.ofNullable(event.getHttp()).map(Http::getRequest).map(Request::getPath)
300+
.orElse(null));
296301
case PROTOCOL_GRPC:
297302
return event.getEventName();
298303
}

‎hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/SpanEventViewGeneratorTest.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,29 @@ public void test_getRequestUrl_grpcProctol_shouldReturnEventName() {
6464
spanEventViewGenerator.getRequestUrl(event, Protocol.PROTOCOL_GRPC)
6565
);
6666
}
67+
68+
@Test
69+
public void testGetRequestUrl_fullUrlIsAbsent() {
70+
Event event = mock(Event.class);
71+
when(event.getHttp()).thenReturn(Http.newBuilder()
72+
.setRequest(Request.newBuilder()
73+
.setPath("/api/v1/gatekeeper/check")
74+
.build()
75+
).build()
76+
);
77+
Assertions.assertEquals(
78+
"/api/v1/gatekeeper/check",
79+
spanEventViewGenerator.getRequestUrl(event, Protocol.PROTOCOL_HTTP));
80+
}
81+
82+
@Test
83+
public void testGetRequestUrl_urlAndPathIsAbsent() {
84+
Event event = mock(Event.class);
85+
when(event.getHttp()).thenReturn(Http.newBuilder()
86+
.setRequest(Request.newBuilder()
87+
.build()
88+
).build()
89+
);
90+
Assertions.assertNull(spanEventViewGenerator.getRequestUrl(event, Protocol.PROTOCOL_HTTP));
91+
}
6792
}

‎span-normalizer/span-normalizer/src/main/java/org/hypertrace/core/spannormalizer/fieldgenerators/HttpFieldsGenerator.java‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ private static void setUserAgent(
446446
.ifPresent(userAgent -> httpBuilder.getRequestBuilder().setUserAgent(userAgent));
447447
}
448448

449-
private static void setPath(
449+
static void setPath(
450450
Http.Builder httpBuilder, Map<String, JaegerSpanInternalModel.KeyValue> tagsMap) {
451451
if (httpBuilder.getRequestBuilder().hasPath()) {
452452
return;
@@ -469,7 +469,7 @@ private static String removeTrailingSlash(String s) {
469469
return s.endsWith(SLASH) && s.length() > 1 ? s.substring(0, s.length() - 1) : s;
470470
}
471471

472-
private static Optional<String> getPathFromUrlObject(String urlPath) {
472+
static Optional<String> getPathFromUrlObject(String urlPath) {
473473
try {
474474
URL url = getNormalizedUrl(urlPath);
475475
return Optional.of(url.getPath());
@@ -569,13 +569,27 @@ private static URL getNormalizedUrl(String url) throws MalformedURLException {
569569
return new URL(new URL(RELATIVE_URL_CONTEXT), url);
570570
}
571571

572+
/**
573+
* If the requestBuilder already has absolute url, do nothing
574+
* if not, try building url based on otel attributes and overwrite
575+
*/
572576
private void maybeSetHttpUrlForOtelFormat(
573577
Request.Builder requestBuilder,
574578
final Map<String, AttributeValue> attributeValueMap) {
575-
if (requestBuilder.hasUrl()) {
579+
if (requestBuilder.hasUrl() && isAbsoluteUrl(requestBuilder.getUrl())) {
576580
return;
577581
}
578582
Optional<String> url = HttpSemanticConventionUtils.getHttpUrlForOTelFormat(attributeValueMap);
579583
url.ifPresent(requestBuilder::setUrl);
580584
}
585+
586+
static boolean isAbsoluteUrl(String urlStr) {
587+
try {
588+
URL url = getNormalizedUrl(urlStr);
589+
return url.toString().equals(urlStr);
590+
} catch (MalformedURLException e) {
591+
// ignore
592+
}
593+
return false;
594+
}
581595
}

‎span-normalizer/span-normalizer/src/test/java/org/hypertrace/core/spannormalizer/fieldgenerators/HttpFieldsGeneratorTest.java‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838
import static org.hypertrace.core.span.constants.v1.OTSpanTag.OT_SPAN_TAG_HTTP_URL;
3939
import static org.hypertrace.core.spannormalizer.utils.TestUtils.createKeyValue;
4040
import static org.junit.jupiter.api.Assertions.assertEquals;
41+
import static org.junit.jupiter.api.Assertions.assertFalse;
42+
import static org.junit.jupiter.api.Assertions.assertTrue;
4143

4244
import com.google.common.collect.Maps;
4345
import io.jaegertracing.api_v2.JaegerSpanInternalModel;
@@ -46,9 +48,12 @@
4648
import java.util.HashMap;
4749
import java.util.List;
4850
import java.util.Map;
51+
import java.util.Optional;
4952
import org.hypertrace.core.datamodel.AttributeValue;
5053
import org.hypertrace.core.datamodel.Event;
5154
import org.hypertrace.core.datamodel.eventfields.http.Http;
55+
import org.hypertrace.core.datamodel.eventfields.http.Request;
56+
import org.hypertrace.core.semantic.convention.constants.span.OTelSpanSemanticConventions;
5257
import org.hypertrace.core.span.constants.RawSpanConstants;
5358
import org.hypertrace.core.semantic.convention.constants.http.OTelHttpSemanticConventions;
5459
import org.junit.jupiter.api.Assertions;
@@ -836,7 +841,6 @@ public void testPopulateOtherFieldsOTelSpan() {
836841
eventBuilder = Event.newBuilder();
837842
eventBuilder.getHttpBuilder().getRequestBuilder().setUrl("/apis/5673/events?a1=v1&a2=v2");
838843
map = Maps.newHashMap();
839-
map.put(OTelHttpSemanticConventions.HTTP_URL.getValue(), buildAttributeValue(url));
840844
httpFieldsGenerator.populateOtherFields(eventBuilder, map);
841845

842846
Assertions.assertNull(eventBuilder.getHttpBuilder().getRequestBuilder().getUrl());
@@ -876,6 +880,41 @@ public void testPopulateOtherFieldsOTelSpan() {
876880
"/some-test-path", eventBuilder.getHttpBuilder().getRequestBuilder().getPath());
877881
assertEquals(
878882
"some-query-str=v1", eventBuilder.getHttpBuilder().getRequestBuilder().getQueryString());
883+
884+
// originally set url is a relative url, should be overridden
885+
eventBuilder = Event.newBuilder();
886+
eventBuilder.getHttpBuilder().getRequestBuilder().setUrl("/api/v1/gatekeeper/check?url=%2Fpixel%2Factivities%3Fadvertisable%3DTRHRT&method=GET&service=pixel");
887+
map = Maps.newHashMap();
888+
map.put(OTelHttpSemanticConventions.HTTP_SCHEME.getValue(), buildAttributeValue("http"));
889+
map.put(OTelSpanSemanticConventions.SPAN_KIND.getValue(), buildAttributeValue("server"));
890+
map.put(OTelHttpSemanticConventions.HTTP_NET_HOST_NAME.getValue(), buildAttributeValue("example.internal.com"));
891+
map.put(OTelHttpSemanticConventions.HTTP_NET_HOST_PORT.getValue(), buildAttributeValue("50850"));
892+
map.put(OTelHttpSemanticConventions.HTTP_TARGET.getValue(), buildAttributeValue("/api/v1/gatekeeper/check?url=%2Fpixel%2Factivities%3Fadvertisable%3DTRHRT&method=GET&service=pixel"));
893+
httpFieldsGenerator.populateOtherFields(eventBuilder, map);
894+
assertEquals("http://example.internal.com:50850/api/v1/gatekeeper/check?url=%2Fpixel%2Factivities%3Fadvertisable%3DTRHRT&method=GET&service=pixel", eventBuilder.getHttpBuilder().getRequestBuilder().getUrl());
895+
}
896+
897+
@Test
898+
public void testIsAbsoluteUrl() {
899+
assertTrue(HttpFieldsGenerator.isAbsoluteUrl("http://example.com/abc/xyz"));
900+
assertFalse(HttpFieldsGenerator.isAbsoluteUrl("/abc/xyz"));
901+
}
902+
903+
@Test
904+
public void testGetPathFromUrl() {
905+
Optional<String> path = HttpFieldsGenerator.getPathFromUrlObject(
906+
"/api/v1/gatekeeper/check?url=%2Fpixel%2Factivities%3Fadvertisable%3DTRHRT&method=GET&service=pixel");
907+
assertEquals(path.get(), "/api/v1/gatekeeper/check");
908+
}
909+
910+
@Test
911+
public void testSetPath() {
912+
Map<String, JaegerSpanInternalModel.KeyValue> tagsMap = new HashMap<>();
913+
tagsMap.put(OTelHttpSemanticConventions.HTTP_TARGET.getValue(),
914+
createKeyValue("/api/v1/gatekeeper/check?url=%2Fpixel%2Factivities%3Fadvertisable%3DTRHRT&method=GET&service=pixel"));
915+
Http.Builder builder = Http.newBuilder().setRequestBuilder(Request.newBuilder());
916+
HttpFieldsGenerator.setPath(builder, tagsMap);
917+
assertEquals(builder.getRequestBuilder().getPath(), "/api/v1/gatekeeper/check");
879918
}
880919

881920
private static AttributeValue buildAttributeValue(String value) {

0 commit comments

Comments
 (0)