last cleanup, one more test

Author: Lms24

packages/node-core/src/utils/outgoingFetchRequest.ts

@@ -147,10 +147,12 @@ function _deduplicateArrayHeader(headers: string[], headerName: string): void {
       continue;
     }
 
-    if (headerName === SENTRY_BAGGAGE_HEADER) {
-      // merge the initial entry into the later occurrence so that we keep the initial sentry- values around.
-      // all other non-sentry values are merged
-      const merged = mergeBaggageHeaders(headers[i + 1] as string, headers[firstIndex + 1] as string);
+    const firstHeaderValue = headers[firstIndex + 1];
+    if (headerName === SENTRY_BAGGAGE_HEADER && firstHeaderValue) {
+      // mergeBaggageHeaders always takes sentry- values from the new baggage (2nd param) and merges
+      // it with the existing one (1st param). Here, we want to keep the first header's existing
+      // sentry- values in favor of the new ones. Hence we swap the parameters.
+      const merged = mergeBaggageHeaders(headers[i + 1], firstHeaderValue);
       if (merged) {
         headers[firstIndex + 1] = merged;
       }

packages/node-core/src/utils/outgoingHttpRequest.ts

@@ -68,7 +68,11 @@ export function addTracePropagationHeadersToOutgoingRequest(
 
   const hasExistingSentryTraceHeader = !!request.getHeader('sentry-trace');
 
-  if (sentryTrace && !hasExistingSentryTraceHeader) {
+  if (hasExistingSentryTraceHeader) {
+    return;
+  }
+
+  if (sentryTrace) {
     try {
       request.setHeader('sentry-trace', sentryTrace);
       DEBUG_BUILD && debug.log(LOG_PREFIX, 'Added sentry-trace header to outgoing request');
@@ -82,7 +86,7 @@ export function addTracePropagationHeadersToOutgoingRequest(
     }
   }
 
-  if (traceparent && !hasExistingSentryTraceHeader && !request.getHeader('traceparent')) {
+  if (traceparent && !request.getHeader('traceparent')) {
     try {
       request.setHeader('traceparent', traceparent);
       DEBUG_BUILD && debug.log(LOG_PREFIX, 'Added traceparent header to outgoing request');
@@ -96,23 +100,9 @@ export function addTracePropagationHeadersToOutgoingRequest(
     }
   }
 
-  if (baggage && !hasExistingSentryTraceHeader) {
+  if (baggage) {
     const existingBaggage = request.getHeader('baggage');
-
-    let cleanedExistingBaggage = existingBaggage;
-
-    // In the edge case that a baggage header with sentry- keys was added
-    // BUT NO sentry-trace header, we overwrite the sentry- keys in the header we attach.
-    // Therefore, we clean the existing baggage header of all sentry- keys.
-    if (existingBaggage) {
-      const tmpBaggage = parseBaggageHeader(existingBaggage);
-      const baggageWithoutSentry = tmpBaggage
-        ? Object.fromEntries(Object.entries(tmpBaggage).filter(([key]) => !key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX)))
-        : {};
-      cleanedExistingBaggage = objectToBaggageHeader(baggageWithoutSentry);
-    }
-
-    const newBaggage = mergeBaggageHeaders(cleanedExistingBaggage, baggage);
+    const newBaggage = mergeBaggageHeaders(existingBaggage, baggage);
     if (newBaggage) {
       try {
         request.setHeader('baggage', newBaggage);

packages/node-core/test/utils/baggage.test.ts

@@ -158,4 +158,10 @@ describe('mergeBaggageHeaders', () => {
     const result = mergeBaggageHeaders(existing, newBaggage);
     expect(result).toBe('non-sentry=keep,sentry-trace_id=new,sentry-environment=new');
   });
+
+  it('preserves existing sentry entries when new baggage has no sentry entries', () => {
+    const result = mergeBaggageHeaders('sentry-release=1.0.0,foo=bar', 'baz=qux');
+
+    expect(result).toBe('sentry-release=1.0.0,foo=bar,baz=qux');
+  });
 });