fix(tracing): Only record transaction name changes if data is different (#5728)

Makes sure either transaction name or source has actually been changed before recording a name-change entry.

Author: lobsterkatie

packages/tracing/src/transaction.ts

@@ -76,13 +76,18 @@ export class Transaction extends SpanClass implements TransactionInterface {
    * JSDoc
    */
   public setName(name: string, source: TransactionMetadata['source'] = 'custom'): void {
+    // `source` could change without the name changing if we discover that an unparameterized route is actually
+    // parameterized by virtue of having no parameters in its path
+    if (name !== this.name || source !== this.metadata.source) {
+      this.metadata.changes.push({
+        source,
+        timestamp: timestampInSeconds(),
+        propagations: this.metadata.propagations,
+      });
+    }
+
     this._name = name;
     this.metadata.source = source;
-    this.metadata.changes.push({
-      source,
-      timestamp: timestampInSeconds(),
-      propagations: this.metadata.propagations,
-    });
   }
 
   /**

packages/tracing/test/transaction.test.ts

@@ -24,7 +24,7 @@ describe('`Transaction` class', () => {
       expect(transaction.metadata.source).toEqual('custom');
     });
 
-    it('updates transaction metadata with correct variables needed', () => {
+    it('updates transaction name changes with correct variables needed', () => {
       const transaction = new Transaction({ name: 'dogpark' });
       expect(transaction.metadata.changes).toEqual([]);
 
@@ -62,6 +62,64 @@ describe('`Transaction` class', () => {
           propagations: 3,
         },
       ]);
+
+      // Only change `source`
+      transaction.setName('playground', 'route');
+
+      expect(transaction.metadata.changes).toEqual([
+        {
+          source: 'custom',
+          timestamp: expect.any(Number),
+          propagations: 0,
+        },
+        {
+          source: 'custom',
+          timestamp: expect.any(Number),
+          propagations: 3,
+        },
+        {
+          source: 'route',
+          timestamp: expect.any(Number),
+          propagations: 3,
+        },
+      ]);
+    });
+
+    it("doesn't update transaction name changes if no change in data", () => {
+      const transaction = new Transaction({ name: 'dogpark' });
+      expect(transaction.metadata.changes).toEqual([]);
+
+      transaction.name = 'ballpit';
+
+      expect(transaction.metadata.changes).toEqual([
+        {
+          source: 'custom',
+          timestamp: expect.any(Number),
+          propagations: 0,
+        },
+      ]);
+
+      transaction.name = 'ballpit';
+
+      // Still only one entry
+      expect(transaction.metadata.changes).toEqual([
+        {
+          source: 'custom',
+          timestamp: expect.any(Number),
+          propagations: 0,
+        },
+      ]);
+
+      transaction.setName('ballpit', 'custom');
+
+      // Still only one entry
+      expect(transaction.metadata.changes).toEqual([
+        {
+          source: 'custom',
+          timestamp: expect.any(Number),
+          propagations: 0,
+        },
+      ]);
     });
 
     describe('`setName` method', () => {
@@ -81,7 +139,7 @@ describe('`Transaction` class', () => {
         expect(transaction.metadata.source).toEqual('route');
       });
 
-      it('updates transaction metadata with correct variables needed', () => {
+      it('updates transaction name changes with correct variables needed', () => {
         const transaction = new Transaction({ name: 'dogpark' });
         expect(transaction.metadata.changes).toEqual([]);