trace url bug fix #281
trace url bug fix #281
Conversation
Codecov Report
@@ Coverage Diff @@
## main #281 +/- ##
=========================================
Coverage 79.07% 79.07%
Complexity 1231 1231
=========================================
Files 110 110
Lines 4861 4861
Branches 440 440
=========================================
Hits 3844 3844
Misses 813 813
Partials 204 204
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
| @@ -184,7 +184,8 @@ public static boolean isAbsoluteUrl(String urlStr) { | |||
| */ | |||
| public static Optional<String> getHttpUrlForOTelFormat( | |||
| Map<String, AttributeValue> attributeValueMap) { | |||
| if (attributeValueMap.containsKey(HTTP_URL.getValue())) { | |||
| if (attributeValueMap.containsKey(HTTP_URL.getValue()) | |||
| && isAbsoluteUrl(attributeValueMap.get(HTTP_URL.getValue()).getValue())) { | |||
There was a problem hiding this comment.
What if this is not an absolute url but we don't have enough attributes to build the url as well?
There was a problem hiding this comment.
Fallback to present case only?
This comment has been minimized.
This comment has been minimized.
| @@ -196,6 +197,8 @@ public static boolean isAbsoluteUrl(String urlStr) { | |||
| attributeValueMap.get(HTTP_HOST.getValue()).getValue(), | |||
| attributeValueMap.get(HTTP_TARGET.getValue()).getValue()); | |||
| return Optional.of(url); | |||
| } else if (attributeValueMap.containsKey(HTTP_URL.getValue())) { | |||
There was a problem hiding this comment.
This is the fallback case, so should have the least priority?
This comment has been minimized.
This comment has been minimized.
| @@ -202,6 +203,8 @@ public static boolean isAbsoluteUrl(String urlStr) { | |||
| } else if (SpanSemanticConventionUtils.isServerSpanForOtelFormat(attributeValueMap) | |||
| || SpanSemanticConventionUtils.isServerSpanForOCFormat(attributeValueMap)) { | |||
| return getHttpUrlForOtelFormatServerSpan(attributeValueMap); | |||
| } else if (attributeValueMap.containsKey(HTTP_URL.getValue())) { | |||
There was a problem hiding this comment.
the above two cases would return empty optional, so the final case would not be reachable
This comment has been minimized.
This comment has been minimized.
In some of the cases, the full URL was not visible on traces so added conditions to handle that.