diff --git a/hypertrace-trace-enricher/enriched-span-constants/src/main/java/org/hypertrace/traceenricher/enrichedspan/constants/utils/EnrichedSpanUtils.java b/hypertrace-trace-enricher/enriched-span-constants/src/main/java/org/hypertrace/traceenricher/enrichedspan/constants/utils/EnrichedSpanUtils.java index 4b431d35c..defe82ec7 100644 --- a/hypertrace-trace-enricher/enriched-span-constants/src/main/java/org/hypertrace/traceenricher/enrichedspan/constants/utils/EnrichedSpanUtils.java +++ b/hypertrace-trace-enricher/enriched-span-constants/src/main/java/org/hypertrace/traceenricher/enrichedspan/constants/utils/EnrichedSpanUtils.java @@ -2,6 +2,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; +import java.net.MalformedURLException; +import java.net.URL; import java.nio.ByteBuffer; import java.util.Collections; import java.util.List; @@ -301,13 +303,25 @@ public static Optional getHttpMethod(Event event) { } public static Optional getFullHttpUrl(Event event) { - if (event.getHttp() != null && event.getHttp().getRequest() != null) { - return Optional.ofNullable(event.getHttp().getRequest().getUrl()); + if (event.getHttp() != null + && event.getHttp().getRequest() != null + && event.getHttp().getRequest().getUrl() != null + && isAbsoluteUrl(event.getHttp().getRequest().getUrl())) { + return Optional.of(event.getHttp().getRequest().getUrl()); } - return Optional.empty(); } + private static boolean isAbsoluteUrl(String urlStr) { + try { + URL url = new URL(new URL("http://hypertrace.org"), urlStr); + return url.toString().equals(urlStr); + } catch (MalformedURLException e) { + // ignore + } + return false; + } + public static Optional getRequestSize(Event event) { Protocol protocol = EnrichedSpanUtils.getProtocol(event); if (protocol == null) { diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/SpanEventViewGenerator.java b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/SpanEventViewGenerator.java index d1e6fa40f..2c5fe7b91 100644 --- a/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/SpanEventViewGenerator.java +++ b/hypertrace-view-generator/hypertrace-view-generator/src/main/java/org/hypertrace/viewgenerator/generators/SpanEventViewGenerator.java @@ -6,12 +6,15 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; import org.apache.avro.Schema; import org.hypertrace.core.datamodel.Entity; import org.hypertrace.core.datamodel.Event; import org.hypertrace.core.datamodel.MetricValue; import org.hypertrace.core.datamodel.StructuredTrace; +import org.hypertrace.core.datamodel.eventfields.http.Http; +import org.hypertrace.core.datamodel.eventfields.http.Request; import org.hypertrace.core.datamodel.shared.SpanAttributeUtils; import org.hypertrace.traceenricher.enrichedspan.constants.EnrichedSpanConstants; import org.hypertrace.traceenricher.enrichedspan.constants.utils.EnrichedSpanUtils; @@ -292,7 +295,9 @@ String getRequestUrl(Event event, Protocol protocol) { switch (protocol) { case PROTOCOL_HTTP: case PROTOCOL_HTTPS: - return EnrichedSpanUtils.getFullHttpUrl(event).orElse(event.getHttp().getRequest().getPath()); + return Optional.ofNullable(event.getHttp()) + .map(Http::getRequest) + .map(Request::getUrl).orElse(null); case PROTOCOL_GRPC: return event.getEventName(); } diff --git a/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/SpanEventViewGeneratorTest.java b/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/SpanEventViewGeneratorTest.java index 8e89ce59d..4bb534a79 100644 --- a/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/SpanEventViewGeneratorTest.java +++ b/hypertrace-view-generator/hypertrace-view-generator/src/test/java/org/hypertrace/viewgenerator/generators/SpanEventViewGeneratorTest.java @@ -7,6 +7,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; diff --git a/span-normalizer/span-normalizer/src/main/java/org/hypertrace/core/spannormalizer/fieldgenerators/HttpFieldsGenerator.java b/span-normalizer/span-normalizer/src/main/java/org/hypertrace/core/spannormalizer/fieldgenerators/HttpFieldsGenerator.java index f4bc56b4b..8d8239add 100644 --- a/span-normalizer/span-normalizer/src/main/java/org/hypertrace/core/spannormalizer/fieldgenerators/HttpFieldsGenerator.java +++ b/span-normalizer/span-normalizer/src/main/java/org/hypertrace/core/spannormalizer/fieldgenerators/HttpFieldsGenerator.java @@ -446,7 +446,7 @@ private static void setUserAgent( .ifPresent(userAgent -> httpBuilder.getRequestBuilder().setUserAgent(userAgent)); } - private static void setPath( + static void setPath( Http.Builder httpBuilder, Map tagsMap) { if (httpBuilder.getRequestBuilder().hasPath()) { return; @@ -469,7 +469,7 @@ private static String removeTrailingSlash(String s) { return s.endsWith(SLASH) && s.length() > 1 ? s.substring(0, s.length() - 1) : s; } - private static Optional getPathFromUrlObject(String urlPath) { + static Optional getPathFromUrlObject(String urlPath) { try { URL url = getNormalizedUrl(urlPath); return Optional.of(url.getPath()); @@ -552,8 +552,6 @@ private void populateUrlParts(Request.Builder requestBuilder) { if (url.toString().equals(urlStr)) { // absolute URL requestBuilder.setScheme(url.getProtocol()); requestBuilder.setHost(url.getAuthority()); // Use authority so in case the port is specified it adds it to this - } else { // relative URL - requestBuilder.setUrl(null); // unset the URL as we only allow absolute/full URLs in the url field } setPathFromUrl(requestBuilder, url); if (!requestBuilder.hasQueryString()) { @@ -569,13 +567,28 @@ private static URL getNormalizedUrl(String url) throws MalformedURLException { return new URL(new URL(RELATIVE_URL_CONTEXT), url); } + /** + * If the requestBuilder already has a url, which is absolute do nothing + * if not, try building a url based on otel attributes + */ private void maybeSetHttpUrlForOtelFormat( Request.Builder requestBuilder, final Map attributeValueMap) { - if (requestBuilder.hasUrl()) { + if (requestBuilder.hasUrl() && isAbsoluteUrl(requestBuilder.getUrl())) { return; } + // if requestBuilder.getUrl() is not absolute try building the url from other attributes Optional url = HttpSemanticConventionUtils.getHttpUrlForOTelFormat(attributeValueMap); url.ifPresent(requestBuilder::setUrl); } + + static boolean isAbsoluteUrl(String urlStr) { + try { + URL url = getNormalizedUrl(urlStr); + return url.toString().equals(urlStr); + } catch (MalformedURLException e) { + // ignore + } + return false; + } } diff --git a/span-normalizer/span-normalizer/src/test/java/org/hypertrace/core/spannormalizer/fieldgenerators/HttpFieldsGeneratorTest.java b/span-normalizer/span-normalizer/src/test/java/org/hypertrace/core/spannormalizer/fieldgenerators/HttpFieldsGeneratorTest.java index 259238fe4..d0a7cb69e 100644 --- a/span-normalizer/span-normalizer/src/test/java/org/hypertrace/core/spannormalizer/fieldgenerators/HttpFieldsGeneratorTest.java +++ b/span-normalizer/span-normalizer/src/test/java/org/hypertrace/core/spannormalizer/fieldgenerators/HttpFieldsGeneratorTest.java @@ -38,6 +38,8 @@ import static org.hypertrace.core.span.constants.v1.OTSpanTag.OT_SPAN_TAG_HTTP_URL; import static org.hypertrace.core.spannormalizer.utils.TestUtils.createKeyValue; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.common.collect.Maps; import io.jaegertracing.api_v2.JaegerSpanInternalModel; @@ -46,9 +48,13 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; +import org.apache.http.client.methods.RequestBuilder; import org.hypertrace.core.datamodel.AttributeValue; import org.hypertrace.core.datamodel.Event; import org.hypertrace.core.datamodel.eventfields.http.Http; +import org.hypertrace.core.datamodel.eventfields.http.Request; +import org.hypertrace.core.semantic.convention.constants.span.OTelSpanSemanticConventions; import org.hypertrace.core.span.constants.RawSpanConstants; import org.hypertrace.core.semantic.convention.constants.http.OTelHttpSemanticConventions; import org.junit.jupiter.api.Assertions; @@ -589,7 +595,7 @@ public void testRelativeUrlNotSetsUrlField() { assertEquals("/dispatch/test?a=b&k1=v1", httpBuilder.getRequestBuilder().getUrl()); httpFieldsGenerator.populateOtherFields(eventBuilder, Maps.newHashMap()); // this should unset the url field - Assertions.assertNull(httpBuilder.getRequestBuilder().getUrl()); + Assertions.assertEquals("/dispatch/test?a=b&k1=v1", httpBuilder.getRequestBuilder().getUrl()); } @Test @@ -696,7 +702,7 @@ public void testPopulateOtherFields() { eventBuilder.getHttpBuilder().getRequestBuilder().setUrl("/apis/5673/events?a1=v1&a2=v2"); httpFieldsGenerator.populateOtherFields(eventBuilder, Maps.newHashMap()); - Assertions.assertNull(eventBuilder.getHttpBuilder().getRequestBuilder().getUrl()); + Assertions.assertEquals("/apis/5673/events?a1=v1&a2=v2", eventBuilder.getHttpBuilder().getRequestBuilder().getUrl()); Assertions.assertNull(eventBuilder.getHttpBuilder().getRequestBuilder().getScheme()); Assertions.assertNull(eventBuilder.getHttpBuilder().getRequestBuilder().getHost()); assertEquals("/apis/5673/events", eventBuilder.getHttpBuilder().getRequestBuilder().getPath()); @@ -836,10 +842,9 @@ public void testPopulateOtherFieldsOTelSpan() { eventBuilder = Event.newBuilder(); eventBuilder.getHttpBuilder().getRequestBuilder().setUrl("/apis/5673/events?a1=v1&a2=v2"); map = Maps.newHashMap(); - map.put(OTelHttpSemanticConventions.HTTP_URL.getValue(), buildAttributeValue(url)); httpFieldsGenerator.populateOtherFields(eventBuilder, map); - Assertions.assertNull(eventBuilder.getHttpBuilder().getRequestBuilder().getUrl()); + Assertions.assertEquals("/apis/5673/events?a1=v1&a2=v2", eventBuilder.getHttpBuilder().getRequestBuilder().getUrl()); Assertions.assertNull(eventBuilder.getHttpBuilder().getRequestBuilder().getScheme()); Assertions.assertNull(eventBuilder.getHttpBuilder().getRequestBuilder().getHost()); assertEquals("/apis/5673/events", eventBuilder.getHttpBuilder().getRequestBuilder().getPath()); @@ -876,6 +881,41 @@ public void testPopulateOtherFieldsOTelSpan() { "/some-test-path", eventBuilder.getHttpBuilder().getRequestBuilder().getPath()); assertEquals( "some-query-str=v1", eventBuilder.getHttpBuilder().getRequestBuilder().getQueryString()); + + // originally set url is a relative url, should be overridden + eventBuilder = Event.newBuilder(); + eventBuilder.getHttpBuilder().getRequestBuilder().setUrl("/api/v1/gatekeeper/check?url=%2Fpixel%2Factivities%3Fadvertisable%3DTRHRT&method=GET&service=pixel"); + map = Maps.newHashMap(); + map.put(OTelHttpSemanticConventions.HTTP_SCHEME.getValue(), buildAttributeValue("http")); + map.put(OTelSpanSemanticConventions.SPAN_KIND.getValue(), buildAttributeValue("server")); + map.put(OTelHttpSemanticConventions.HTTP_NET_HOST_NAME.getValue(), buildAttributeValue("example.internal.com")); + map.put(OTelHttpSemanticConventions.HTTP_NET_HOST_PORT.getValue(), buildAttributeValue("50850")); + map.put(OTelHttpSemanticConventions.HTTP_TARGET.getValue(), buildAttributeValue("/api/v1/gatekeeper/check?url=%2Fpixel%2Factivities%3Fadvertisable%3DTRHRT&method=GET&service=pixel")); + httpFieldsGenerator.populateOtherFields(eventBuilder, map); + assertEquals("http://example.internal.com:50850/api/v1/gatekeeper/check?url=%2Fpixel%2Factivities%3Fadvertisable%3DTRHRT&method=GET&service=pixel", eventBuilder.getHttpBuilder().getRequestBuilder().getUrl()); + } + + @Test + public void testIsAbsoluteUrl() { + assertTrue(HttpFieldsGenerator.isAbsoluteUrl("http://example.com/abc/xyz")); + assertFalse(HttpFieldsGenerator.isAbsoluteUrl("/abc/xyz")); + } + + @Test + public void testGetPathFromUrl() { + Optional path = HttpFieldsGenerator.getPathFromUrlObject( + "/api/v1/gatekeeper/check?url=%2Fpixel%2Factivities%3Fadvertisable%3DTRHRT&method=GET&service=pixel"); + assertEquals(path.get(), "/api/v1/gatekeeper/check"); + } + + @Test + public void testSetPath() { + Map tagsMap = new HashMap<>(); + tagsMap.put(OTelHttpSemanticConventions.HTTP_TARGET.getValue(), + createKeyValue("/api/v1/gatekeeper/check?url=%2Fpixel%2Factivities%3Fadvertisable%3DTRHRT&method=GET&service=pixel")); + Http.Builder builder = Http.newBuilder().setRequestBuilder(Request.newBuilder()); + HttpFieldsGenerator.setPath(builder, tagsMap); + assertEquals(builder.getRequestBuilder().getPath(), "/api/v1/gatekeeper/check"); } private static AttributeValue buildAttributeValue(String value) {