fix(rum-core): fix for `merge` utility function #1555

david-luna · 2024-12-11T16:00:50Z

The merge utility function was producing bogus values when certain payload is passed. This PR fixes it

vigneshshanmugam

Thanks @david-luna code changes LGTM, Need to fix some of the tests as well.

vigneshshanmugam · 2024-12-11T20:54:17Z

package-lock.json

@@ -55694,7 +55694,7 @@
    },
    "packages/rum": {
      "name": "@elastic/apm-rum",
-      "version": "5.16.0",
+      "version": "5.16.1",


weird why you got this change?

I got it just after npm i on a clean checkout

david-luna · 2024-12-12T17:26:46Z

Thanks @david-luna code changes LGTM, Need to fix some of the tests as well.

the intent of the change was to provide exactly the same behaviour & without changing the API but the internal implementation (so no need to change the tests). I haven't tested locally and relied on the CI since the docker compose command referred in the docs did not work.

NODEJS_VERSION=18.16.0 STACK_VERSION=8.6.1 docker compose -f ./dev-utils/docker-compose.yml up -d apm-server
WARN[0000] The "USER_ID" variable is not set. Defaulting to a blank string. 
WARN[0000] The "BUILD_NUMBER" variable is not set. Defaulting to a blank string. 
WARN[0000] The "BRANCH_NAME" variable is not set. Defaulting to a blank string. 
WARN[0000] The "CI" variable is not set. Defaulting to a blank string. 
WARN[0000] The "REPORT_FILE" variable is not set. Defaulting to a blank string. 
WARN[0000] The "REPORT_FILE" variable is not set. Defaulting to a blank string. 
WARN[0000] The "SCOPE" variable is not set. Defaulting to a blank string. 
WARN[0000] The "APM_SERVER_URL" variable is not set. Defaulting to a blank string. 
WARN[0000] The "KIBANA_URL" variable is not set. Defaulting to a blank string. 
WARN[0000] The "BUILD_NUMBER" variable is not set. Defaulting to a blank string. 
WARN[0000] The "BRANCH_NAME" variable is not set. Defaulting to a blank string. 
WARN[0000] The "CI" variable is not set. Defaulting to a blank string. 
WARN[0000] The "SAUCE_USERNAME" variable is not set. Defaulting to a blank string. 
WARN[0000] The "SAUCE_ACCESS_KEY" variable is not set. Defaulting to a blank string. 
WARN[0000] The "MODE" variable is not set. Defaulting to a blank string. 
WARN[0000] The "GOAL" variable is not set. Defaulting to a blank string. 
WARN[0000] The "WORKSPACE" variable is not set. Defaulting to a blank string. 
WARN[0000] The "BASE_DIR" variable is not set. Defaulting to a blank string. 
WARN[0000] The "USER_ID" variable is not set. Defaulting to a blank string. 
WARN[0000] The "BUILD_NUMBER" variable is not set. Defaulting to a blank string. 
WARN[0000] The "BRANCH_NAME" variable is not set. Defaulting to a blank string. 
WARN[0000] The "CI" variable is not set. Defaulting to a blank string. 
WARN[0000] The "REPORT_FILE" variable is not set. Defaulting to a blank string. 
WARN[0000] The "REPORT_FILE" variable is not set. Defaulting to a blank string. 
WARN[0000] The "USER_ID" variable is not set. Defaulting to a blank string. 
WARN[0000] /Users/david/Documents/repos/el/apm-agent-rum-js/dev-utils/docker-compose.yml: the attribute `version` is obsolete, it will be ignored, please remove it to avoid potential confusion 
no such service: apm-server

am I doing something wrong? Does the documentation need an update?

vigneshshanmugam · 2024-12-12T18:26:53Z

@david-luna Unfortunately I haven't tested anything in the midterm as well, But seems to happen on other PR's as well - #1546. @v1v Have you seen this one before?

v1v · 2024-12-12T18:36:56Z

not really, maybe the current docker-compose in the GH runner is not supporting the version set in the docker-compose.yml? or the other way around, the version installed locally is too new?

david-luna · 2024-12-13T14:45:59Z

there was a problem in the code, so CI caught the defects on the fix. With the last commit I made some progress but now it seems CI has trouble connection to saucelabs
https://github.com/elastic/apm-agent-rum-js/actions/runs/12317196104/job/34379154854?pr=1555

excerpt of the logs

node-puppeteer  | Running: runSauceTests(packages/rum, true, build:e2e, test:unit, test:e2e:supported, test:e2e:failsafe) 
node-puppeteer  | [Static Server] - serving on:  /app/packages/rum 8000
node-puppeteer  | [Backend Server] - serving on:  8003
node-puppeteer  | [APM Server Proxy] - For http://localhost:8200 on: 8001
node-puppeteer  | Opening local tunnel using Sauce Connect
node-puppeteer  | Starting sc with args: -u XXXXXXXX -k XXXXXXXX --no-ssl-bump-domains all --tunnel-identifier  --readyfile /tmp/sc-launcher-readyfile -x https://saucelabs.com/rest/v1
node-puppeteer  | Could not start Sauce Connect. Exit code 1 signal: null
node-puppeteer  | Opening local tunnel using Sauce Connect
node-puppeteer  | Starting sc with args: -u XXXXXXXX -k XXXXXXXX --no-ssl-bump-domains all --tunnel-identifier  --readyfile /tmp/sc-launcher-readyfile -x https://saucelabs.com/rest/v1
node-puppeteer  | Could not start Sauce Connect. Exit code 1 signal: null
node-puppeteer  | Opening local tunnel using Sauce Connect
node-puppeteer  | Starting sc with args: -u XXXXXXXX -k XXXXXXXX --no-ssl-bump-domains all --tunnel-identifier  --readyfile /tmp/sc-launcher-readyfile -x https://saucelabs.com/rest/v1
node-puppeteer  | Could not start Sauce Connect. Exit code 1 signal: null
node-puppeteer  | Opening local tunnel using Sauce Connect
node-puppeteer  | Starting sc with args: -u XXXXXXXX -k XXXXXXXX --no-ssl-bump-domains all --tunnel-identifier  --readyfile /tmp/sc-launcher-readyfile -x https://saucelabs.com/rest/v1
node-puppeteer  | Sauce connect Error Error: Could not start Sauce Connect. Exit code 1 signal: null
node-puppeteer  |     at ChildProcess.<anonymous> (/app/node_modules/sauce-connect-launcher/lib/sauce-connect-launcher.js:566:12)
node-puppeteer  |     at ChildProcess.emit (node:events:517:28)
node-puppeteer  |     at ChildProcess.emit (node:domain:489:12)
node-puppeteer  |     at ChildProcess._handle.onexit (node:internal/child_process:292:12)
node-puppeteer  | Running cleanups: 1
node-puppeteer  | ERROR: "test:sauce" exited with 1.
node-puppeteer  | lerna ERR! npm run test exited 1 in '@elastic/apm-rum'
Aborting on container exit...

vigneshshanmugam · 2024-12-13T18:21:09Z

@v1v We rotated the saucelabs tokens, may be the tests are not working be due to that? I believe we have the apm-ci user on the saucelabs organization, If not I can add that which should unblock us here. Please suggest.

v1v · 2024-12-16T08:24:37Z

@v1v We rotated the saucelabs tokens, may be the tests are not working be due to that? I believe we have the apm-ci user on the saucelabs organization, If not I can add that which should unblock us here. Please suggest.

Let's sync up and update the relevant GitHub secrets, I don't recall we have updated them after rotating the secrets of saucelabs. I'll raise an internal GH issue so we can track what's needed and unblock this

v1v · 2024-12-17T11:58:04Z

I've just rotated the GH secrets, let's see if the CI is happy about it now

david-luna · 2024-12-18T08:49:54Z

last run succeeded in SauceLabs tests for @elastic/apm-rum but failed for others with the message

node-puppeteer  | 17 12 2024 18:30:39.516:INFO [launcher]: Trying to start chrome 84  on SauceLabs again (2/2).
node-puppeteer  | 17 12 2024 18:32:37.039:ERROR [SaucelabsLauncher]: Error: Failed to create session.
node-puppeteer  | Redirected 10 times. Aborting.
node-puppeteer  |     at Object.startWebDriverSession (/app/node_modules/webdriver/build/utils.js:34:15)
node-puppeteer  |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
node-puppeteer  |     at async WebDriver.newSession (/app/node_modules/webdriver/build/index.js:35:45)

seems a SauceLabs issue. I'm going to retry

vigneshshanmugam

LGTM, Lets merge once the sauce tests are green.

david-luna · 2025-01-10T10:52:56Z

@vigneshshanmugam all green 🎉

david-luna added 2 commits December 11, 2024 13:26

chore: update package-loc file

a9c3ef6

fix(rum-core): fix issue when merging RUM config

f08f6e0

david-luna requested a review from vigneshshanmugam December 11, 2024 16:03

vigneshshanmugam reviewed Dec 11, 2024

View reviewed changes

fix(rum-core): fix merge of array props

a9f17b7

david-luna requested a review from vigneshshanmugam December 13, 2024 14:46

david-luna added 3 commits January 7, 2025 11:00

Merge branch 'main' of github.com:elastic/apm-agent-rum-js

2dfdda5

Merge branch 'main' into dluna-fix-merge-props-fun

68cdaa2

Merge branch 'main' into dluna-fix-merge-props-fun

3041889

vigneshshanmugam approved these changes Jan 9, 2025

View reviewed changes

david-luna merged commit 0d1eda5 into main Jan 10, 2025
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(rum-core): fix for `merge` utility function #1555

fix(rum-core): fix for `merge` utility function #1555

david-luna commented Dec 11, 2024 •

edited

Loading

vigneshshanmugam left a comment

vigneshshanmugam Dec 11, 2024

david-luna Dec 12, 2024

david-luna commented Dec 12, 2024

vigneshshanmugam commented Dec 12, 2024

v1v commented Dec 12, 2024 •

edited

Loading

david-luna commented Dec 13, 2024

vigneshshanmugam commented Dec 13, 2024

v1v commented Dec 16, 2024

v1v commented Dec 17, 2024

david-luna commented Dec 18, 2024

vigneshshanmugam left a comment

david-luna commented Jan 10, 2025

fix(rum-core): fix for merge utility function #1555

fix(rum-core): fix for merge utility function #1555

Conversation

david-luna commented Dec 11, 2024 • edited Loading

vigneshshanmugam left a comment

Choose a reason for hiding this comment

vigneshshanmugam Dec 11, 2024

Choose a reason for hiding this comment

david-luna Dec 12, 2024

Choose a reason for hiding this comment

david-luna commented Dec 12, 2024

vigneshshanmugam commented Dec 12, 2024

v1v commented Dec 12, 2024 • edited Loading

david-luna commented Dec 13, 2024

vigneshshanmugam commented Dec 13, 2024

v1v commented Dec 16, 2024

v1v commented Dec 17, 2024

david-luna commented Dec 18, 2024

vigneshshanmugam left a comment

Choose a reason for hiding this comment

david-luna commented Jan 10, 2025

fix(rum-core): fix for `merge` utility function #1555

fix(rum-core): fix for `merge` utility function #1555

david-luna commented Dec 11, 2024 •

edited

Loading

v1v commented Dec 12, 2024 •

edited

Loading