strip out query and fragment in extractAPIPathValue logic

aws-observability · Aug 29, 2024 · 44c4d22 · 44c4d22
1 parent 98d5f83
commit 44c4d22
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 1 deletion.
diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-span-processing-util.ts b/aws-distro-opentelemetry-node-autoinstrumentation/src/aws-span-processing-util.ts
@@ -76,7 +76,31 @@ export class AwsSpanProcessingUtil {
     if (httpTarget == null || httpTarget === '') {
       return '/';
     }
-    const paths: string[] = httpTarget.split('/');
+
+    // Divergence from Java/Python
+    // https://github.com/open-telemetry/semantic-conventions/blob/4e7c42ee8e4c3a39a899c4c85c64df28cd543f78/docs/attributes-registry/http.md#deprecated-http-attributes
+    // According to OTel Spec, httpTarget may include query and fragment:
+    // - `/search?q=OpenTelemetry#SemConv`
+    // We do NOT want the `?` or `#` parts, so let us strip it out,
+    // because HTTP (ingress) instrumentation was observed to include the query (`?`) part
+    // - https://github.com/open-telemetry/opentelemetry-js/blob/b418d36609c371d1fcae46898e9ede6278aca917/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts#L502-L504
+    // This is a fix that can be applied here since this is the central location for generating API Path Value
+    // TODO: Possibly contribute fix to upstream for this diff between langauges. However, the current attribute value in JS is according to spec.
+    //
+    // Interestingly, according to Spec, Java/Python should be affected, but they are not.
+    let apiPathValueEndIndex: number = httpTarget.indexOf('?');
+    if (apiPathValueEndIndex === -1) {
+      apiPathValueEndIndex = httpTarget.indexOf('#');
+    }
+
+    let httpTargetWithoutQuery;
+    if (apiPathValueEndIndex === -1) {
+      httpTargetWithoutQuery = httpTarget;
+    } else {
+      httpTargetWithoutQuery = httpTarget.substring(0, apiPathValueEndIndex);
+    }
+
+    const paths: string[] = httpTargetWithoutQuery.split('/');
     if (paths.length > 1) {
       return '/' + paths[1];
     }

diff --git a/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-span-processing-util.test.ts b/aws-distro-opentelemetry-node-autoinstrumentation/test/aws-span-processing-util.test.ts
@@ -171,6 +171,20 @@ describe('AwsSpanProcessingUtilTest', () => {
     expect(pathValue).toEqual('/users');
   });
 
+  it('testExtractAPIPathValidPathSingleSlash', () => {
+    let validTarget: string = '/users?query#fragment';
+    let pathValue: string = AwsSpanProcessingUtil.extractAPIPathValue(validTarget);
+    expect(pathValue).toEqual('/users');
+
+    validTarget = '/users?query';
+    pathValue = AwsSpanProcessingUtil.extractAPIPathValue(validTarget);
+    expect(pathValue).toEqual('/users');
+
+    validTarget = '/users#fragment';
+    pathValue = AwsSpanProcessingUtil.extractAPIPathValue(validTarget);
+    expect(pathValue).toEqual('/users');
+  });
+
   it('testIsKeyPresentKeyPresent', () => {
     attributesMock[SEMATTRS_HTTP_TARGET] = 'target';
     expect(AwsSpanProcessingUtil.isKeyPresent(spanDataMock, SEMATTRS_HTTP_TARGET)).toBeTruthy();