Repository: elastic/synthetics
Pull request #514 (2022-05-26 19:51:35)

fix: only append 'Elastic/Synthetics' to default UA
When we originally implemented https://github.com/elastic/synthetics/issues/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.

CC @paulb-elastic @drewpost 

PS this is currently a draft because there are no tests, I'll add tests if we agree on this approach.

apmmachine: 
## :broken_heart: Tests Failed


<!-- BUILD BADGES-->
> _the below badges are clickable and redirect to their specific view in the CI or DOCS_
[![Pipeline View](https://img.shields.io/badge/pipeline-pipeline%20-green)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-514/detail/PR-514/1//pipeline) [![Test View](https://img.shields.io/badge/test-test-green)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-514/detail/PR-514/1//tests) [![Changes](https://img.shields.io/badge/changes-changes-green)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-514/detail/PR-514/1//changes) [![Artifacts](https://img.shields.io/badge/artifacts-artifacts-yellow)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-514/detail/PR-514/1//artifacts) [![preview](https://img.shields.io/badge/docs-preview-yellowgreen)](http://synthetics_514.docs-preview.app.elstc.co/diff)  [![preview](https://img.shields.io/badge/elastic-observability-blue)](https://ci-stats.elastic.co/app/apm/services/apm-ci/transactions/view?rangeFrom=2022-05-26T19:41:55.709Z&rangeTo=2022-05-26T20:01:55.709Z&transactionName=BUILD+apm-agent-rum%2Felastic-synthetics%2FPR-%7Bnumber%7D&transactionType=job&latencyAggregationType=avg&traceId=3e768eedf077d3e8c432c395a31a01be&transactionId=b1328f0ff2c97285)

<!-- BUILD SUMMARY-->
<details><summary>Expand to view the summary</summary>
<p>

#### Build stats




* Start Time: 2022-05-26T19:51:55.709+0000


* Duration: 8 min 38 sec




#### Test stats :test_tube:

| Test         | Results                         |
| ------------ | :-----------------------------: |
| Failed       | 2  |
| Passed       | 155  |
| Skipped      | 2 |
| Total        | 159   |


</p>
</details>

<!-- TEST RESULTS IF ANY-->

  ### Test errors [![2](https://img.shields.io/badge/2%20-red)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-514/detail/PR-514/1//tests)
<details><summary>Expand to view the tests failures</summary><p>
    
    
##### `Test / Gatherer Elastic UA identifier works with device emulation – Gatherer Elastic UA identifier works with device emulation`


<ul>
<details><summary>Expand to view the error details</summary><p>

```
 null 
 ```
</p></details>
<details><summary>Expand to view the stacktrace</summary><p>

```
 Error: expect(received).toContain(expected) // indexOf

Expected substring: "Elastic/Synthetics"
Received string:    "Mozilla/5.0 (iPhone; CPU iPhone OS 14_4 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.4 Mobile/15E148 Safari/604.1"
    at Object.<anonymous> (/var/lib/jenkins/workspace/nt-rum_elastic-synthetics_PR-514/src/github.com/elastic/synthetics/__tests__/core/gatherer.test.ts:135:25) 
 ```
</p></details>
</ul>
    
##### `Test / Gatherer Elastic UA identifier works with popup window – Gatherer Elastic UA identifier works with popup window`


<ul>
<details><summary>Expand to view the error details</summary><p>

```
 null 
 ```
</p></details>
<details><summary>Expand to view the stacktrace</summary><p>

```
 Error: expect(received).toContain(expected) // indexOf

Expected substring: " Elastic/Synthetics"
Received string:    "Mozilla/5.0 (Linux; Android 8.0.0; SM-G965U Build/R16NW) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/101.0.4929.0 Mobile Safari/537.36"
    at BrowserContext.<anonymous> (/var/lib/jenkins/workspace/nt-rum_elastic-synthetics_PR-514/src/github.com/elastic/synthetics/__tests__/core/gatherer.test.ts:149:49)
    at BrowserContext.emit (node:events:527:28)
    at BrowserContext._onRequest (/var/lib/jenkins/workspace/nt-rum_elastic-synthetics_PR-514/src/github.com/elastic/synthetics/node_modules/playwright-chromium/node_modules/playwright-core/lib/client/browserContext.js:172:10)
    at Proxy.<anonymous> (/var/lib/jenkins/workspace/nt-rum_elastic-synthetics_PR-514/src/github.com/elastic/synthetics/node_modules/playwright-chromium/node_modules/playwright-core/lib/client/browserContext.js:139:16)
    at Proxy.emit (node:events:527:28)
    at Connection.dispatch (/var/lib/jenkins/workspace/nt-rum_elastic-synthetics_PR-514/src/github.com/elastic/synthetics/node_modules/playwright-chromium/node_modules/playwright-core/lib/client/connection.js:210:21)
    at Immediate.<anonymous> (/var/lib/jenkins/workspace/nt-rum_elastic-synthetics_PR-514/src/github.com/elastic/synthetics/node_modules/playwright-chromium/node_modules/playwright-core/lib/inProcessFactory.js:50:83)
    at processImmediate (node:internal/timers:466:21) 
 ```
</p></details>
</ul>
    
</p></details>


<!-- STEPS ERRORS IF ANY -->












### Steps errors [![1](https://img.shields.io/badge/1%20-red)](https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-rum%2Felastic-synthetics%2FPR-514/detail/PR-514/1//pipeline)
  <details><summary>Expand to view the steps failures</summary>
  <p>


  
  
  
  
##### `Runs the tests`
<ul>
<li>Took 2 min 36 sec . View more details <a href="https://apm-ci.elastic.co//blue/rest/organizations/jenkins/pipelines/apm-agent-rum/pipelines/elastic-synthetics/pipelines/PR-514/runs/1/steps/341/log/?start=0">here</a></li>
<li>Description: <code>./node_modules/.bin/jest --ci --reporters=default --reporters=jest-junit</code></l1>
</ul>
  
  </p>
  </details>



## :robot: GitHub comments

To re-run your PR in the CI, just comment with:



- `/test` : Re-trigger the build.




<!--COMMENT_GENERATED_WITH_ID_comment.id-->

Patch:
diff --git a/src/core/gatherer.ts b/src/core/gatherer.ts
index 6d9639a..2c615cc 100644
--- 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(


Test patch:
diff --git a/__tests__/core/gatherer.test.ts b/__tests__/core/gatherer.test.ts
index bd3f6ec..c9c7512 100644
--- 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);