fix: only append 'Elastic/Synthetics' to default UA (#514)

* Only append 'Elastic/Synthetics' to default UA When we originally implemented #232 it appears we didn't implement the third AC, allowing users to override our custom UA string "Elastic/Synthetics" which is appended. Some users need to fully replace the UA, this allows them to do that. Rather than adding a boolean, this assumes that if you're overriding the UA you want full control, which is more in line with the principle of least surprise. * fix tests and rebase Co-authored-by: vigneshshanmugam <[email protected]>
elastic · Jun 3, 2022 · f7a5c8f · f7a5c8f
1 parent a4c10b5
commit f7a5c8f
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 11 deletions.
diff --git a/__tests__/core/gatherer.test.ts b/__tests__/core/gatherer.test.ts
@@ -117,7 +117,7 @@ describe('Gatherer', () => {
   });
 
   describe('Elastic UA identifier', () => {
-    it('works on a single page', async () => {
+    it('present when UA is not overriden', async () => {
       const driver = await Gatherer.setupDriver({ wsEndpoint });
       expect(await driver.page.evaluate(() => navigator.userAgent)).toContain(
         ' Elastic/Synthetics'
@@ -126,29 +126,26 @@ describe('Gatherer', () => {
       await Gatherer.stop();
     });
 
-    it('works with device emulation', async () => {
+    it('not present when UA is modified - emulated', async () => {
       const driver = await Gatherer.setupDriver({
         wsEndpoint,
         playwrightOptions: { ...devices['iPhone 12 Pro Max'] },
       });
       const userAgent = await driver.page.evaluate(() => navigator.userAgent);
-      expect(userAgent).toContain('Elastic/Synthetics');
+      expect(userAgent).not.toContain('Elastic/Synthetics');
       expect(userAgent).toContain('iPhone');
       await Gatherer.dispose(driver);
       await Gatherer.stop();
     });
 
-    it('works with popup window', async () => {
+    it('present on popup windows', async () => {
       const driver = await Gatherer.setupDriver({
         wsEndpoint,
-        playwrightOptions: { ...devices['Galaxy S9+'] },
       });
       const { page, context } = driver;
       await page.goto(server.TEST_PAGE);
       context.on('request', request => {
-        expect(request.headers()['user-agent']).toContain(
-          ' Elastic/Synthetics'
-        );
+        expect(request.headers()['user-agent']).toContain('Elastic/Synthetics');
       });
       await page.setContent(
         '<a target=_blank rel=noopener href="/popup.html">popup</a>'
@@ -159,7 +156,7 @@ describe('Gatherer', () => {
       ]);
       await page1.waitForLoadState();
       expect(await page1.evaluate(() => navigator.userAgent)).toContain(
-        ' Elastic/Synthetics'
+        'Elastic/Synthetics'
       );
       expect(await page1.textContent('body')).toEqual('Not found');
       await Gatherer.dispose(driver);

diff --git a/src/core/gatherer.ts b/src/core/gatherer.ts
@@ -65,12 +65,12 @@ export class Gatherer {
   }
 
   static async getUserAgent(userAgent?: string) {
-    const syntheticsIdentifier = ' Elastic/Synthetics';
     if (!userAgent) {
       const session = await Gatherer.browser.newBrowserCDPSession();
       ({ userAgent } = await session.send('Browser.getVersion'));
+      return userAgent + ' Elastic/Synthetics';
     }
-    return userAgent + syntheticsIdentifier;
+    return userAgent;
   }
 
   static setNetworkConditions(