feat: generate "X-Request-ID" header for each browser request #819

DudaGod · 2023-12-25T12:29:42Z

What is done

For each browser request hermione generates uniq X-Request-ID header which consists of ${TEST_X_REQ_ID}${DELIMITER}$BROWSER_X_REQ_ID}, where:

TEST_X_REQ_ID - uniq uuid for each test (different from every other test retry), allows you to find all requests related to a single test run using logs
DELIMITER - __ separator between test and request uuids
BROWSER_X_REQ_ID - uniq uuid for each browser request

Real-world example: 2f31ffb7-369d-41f4-bbb8-77744615d2eb__e8d011d8-bb76-42b9-b80e-02f03b8d6fe1.

Moreover revert previous decision in beta version with generate the same X-Request-ID for each request in whole hermione run.

This reverts commit 35df5cc.

DudaGod · 2023-12-25T12:57:54Z

src/browser/browser.js

@@ -19,13 +20,14 @@ const CUSTOM_SESSION_OPTS = [
 ];

 module.exports = class Browser {
-    static create(config, id, version) {


Изменил api, чтобы не докидывать testXReqId отдельным параметром

DudaGod · 2023-12-25T12:59:48Z

src/browser-pool/basic-pool.js

@@ -23,8 +23,7 @@ module.exports = class BasicPool extends Pool {
    }

    async getBrowser(id, opts = {}) {
-        const { version } = opts;
-        const browser = Browser.create(this._config, id, version);
+        const browser = Browser.create(this._config, { ...opts, id });


Изменил api, чтобы не докидывать testXReqId отдельным параметром. Похожий подход присутствует и в других файлах, поэтому их отдельно комментировать не буду.

DudaGod · 2023-12-25T14:12:15Z

src/browser/browser.js

@@ -1,5 +1,6 @@
 "use strict";

+const crypto = require("crypto");


Для генерации uuid юзаю crypto. randomUUID, его же предлагают юзать в пакете uuid - https://github.com/uuidjs/uuid если юзается только uuid v4

DudaGod · 2023-12-25T14:13:25Z

src/browser/browser.js

+                        req.headers["X-Request-ID"] = `${this.testXReqId}_${crypto.randomUUID()}`;
+                    }
+
+                    return !_.isNull(this._config[optName]) ? this._config[optName](req) : req;


Если юзер указал тоже transformRequest, то я его должен вызвать после своих изменений в req

DudaGod · 2023-12-25T14:13:54Z

src/browser/browser.js

-            if (!_.isNull(this._config[optName])) {
+            if (optName === "transformRequest") {
+                options[optName] = req => {
+                    if (!req.headers["X-Request-ID"]) {


Юзер мог самостоятельно указать в конфиге хедер X-Request-ID и в этом случае я ничего делать не должен.

DudaGod · 2023-12-25T14:14:30Z

src/browser/browser.js

+            if (optName === "transformRequest") {
+                options[optName] = req => {
+                    if (!req.headers["X-Request-ID"]) {
+                        req.headers["X-Request-ID"] = `${this.testXReqId}_${crypto.randomUUID()}`;


В качестве разделителя между uuid теста и uuid запроса юзаю _.

DudaGod · 2023-12-25T14:18:50Z

src/browser/existing-browser.js

            ...this._config.meta,
        };
+
+        if (!meta.testXReqId && !_.get(this._config.headers, "X-Request-ID")) {
+            meta.testXReqId = this.testXReqId;


Докидываю в мету если пользователь сам не переопределил testXReqId в мете конфига или не указал хедер X-Request_ID в конфиге самостоятельно.

KuznetsovRoman

В остальном /ок

KuznetsovRoman · 2023-12-26T09:21:15Z

README.md

@@ -1098,6 +1098,20 @@ Allows to intercept [HTTP request options](https://github.com/sindresorhus/got#o
 (RequestOptions) => RequestOptions
 ```

+In runtime a unique `X-Request-ID` header is generated for each browser request which consists of `${TEST_X_REQ_ID}__${BROWSER_X_REQ_ID}`, where:
+- `TEST_X_REQ_ID` - unique uuid for each test (different for each retry), allows to find all requests related to a single test run;
+- `BROWSER_X_REQ_ID` - unique uuid for each browser request, allows to find one unique request.


Тут нейминг не нравится. Кажется, что это уникальное значение браузера, по аналогии с тестом: у обоих REQ_ID.

Давай напишем, что строим этот requestId из testId (отличный для каждого ретрая, или вообще можно явно указать, из каких частей он строится) и uuidv4(). Так будет понятно, что это и есть та рандомная на каждый запрос часть, и не будет казаться, что оно как то связано с браузером

Поправил, чтобы было понятнее.

DudaGod · 2023-12-27T09:35:23Z

src/runner/test-runner/regular-test-runner.js

@@ -64,6 +65,7 @@ module.exports = class RegularTestRunner extends Runner {
            sessionCaps: this._browser.capabilities,
            sessionOpts: this._browser.publicAPI.options,
            file: this._test.file,
+            testXReqId: this._browser.testXReqId,


Прокидываю x-req-id теста в worker, чтобы затем добросить его до браузера в воркере (existing-browser). По другому данные по нормальному не передать.

DudaGod · 2023-12-27T09:35:39Z

src/runner/test-runner/regular-test-runner.js

@@ -80,7 +82,7 @@ module.exports = class RegularTestRunner extends Runner {

    async _getBrowser() {
        try {
-            this._browser = await this._browserAgent.getBrowser();
+            this._browser = await this._browserAgent.getBrowser({ testXReqId: crypto.randomUUID() });


Вот тут генерится x-req-id для теста и прокидывается в браузер (запущенный в мастере, т.е. в new-browser)

DudaGod · 2023-12-27T09:36:26Z

README.md

@@ -1098,6 +1098,20 @@ Allows to intercept [HTTP request options](https://github.com/sindresorhus/got#o
 (RequestOptions) => RequestOptions
 ```

+In runtime a unique `X-Request-ID` header is generated for each browser request which consists of `${TEST_X_REQ_ID}__${BROWSER_X_REQ_ID}`, where:
+- `TEST_X_REQ_ID` - unique uuid for each test (different for each retry), allows to find all requests related to a single test run;
+- `BROWSER_X_REQ_ID` - unique uuid for each browser request, allows to find one unique request.


Поправил, чтобы было понятнее.

KuznetsovRoman · 2023-12-27T10:46:08Z

@DudaGod А давай явно писать, из чего строится первый хэш.
Как-то так: ${hash(fullName, browserId, retryCount)}__${requestId}

DudaGod · 2023-12-27T11:01:13Z

А давай явно писать, из чего строится первый хэш.
Как-то так: ${hash(fullName, browserId, retryCount)}__${requestId}

Первый кусок хеша это тоже просто uuidv4, так как иначе нужно придумывать какой-то свой алгоритм с учетом данных теста + timestamp + соль, чтобы при параллельных запусках точно не было дубликатов. В итоге я не понял зачем мне тут велосипедить если уже все сделали за меня и нужно просто юзать uuidv4.

DudaGod · 2023-12-27T11:02:53Z

src/runner/test-runner/high-priority-browser-agent.js

-    getBrowser() {
-        return this._browserAgent.getBrowser({ highPriority: true });
+    getBrowser(opts = {}) {
+        return this._browserAgent.getBrowser({ ...opts, highPriority: true });


Нашел баг, что при ретрае теста в мету не прокидывался testXReqID. Пофиксил.

Revert "feat: generate "X-Request-ID" header"

2abd043

This reverts commit 35df5cc.

DudaGod commented Dec 25, 2023

View reviewed changes

DudaGod force-pushed the HERMIONE-1276.fix_x_req_id branch from b5a8ae4 to 13493e5 Compare December 25, 2023 20:26

KuznetsovRoman approved these changes Dec 26, 2023

View reviewed changes

DudaGod commented Dec 27, 2023

View reviewed changes

DudaGod force-pushed the HERMIONE-1276.fix_x_req_id branch from 13493e5 to f3fd1a1 Compare December 27, 2023 09:37

feat: generate "X-Request-ID" header for each browser request

d40cd6a

DudaGod force-pushed the HERMIONE-1276.fix_x_req_id branch from f3fd1a1 to d40cd6a Compare December 27, 2023 10:57

DudaGod commented Dec 27, 2023

View reviewed changes

DudaGod merged commit 22814e5 into master Dec 27, 2023
2 checks passed

DudaGod deleted the HERMIONE-1276.fix_x_req_id branch December 27, 2023 11:03

DudaGod mentioned this pull request Dec 28, 2023

feat: generate "X-Request-ID" header for each browser request (backport to v7) #823

Merged

DudaGod mentioned this pull request Jan 15, 2024

fix: correctly generate test x request id for each test in one browser #825

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: generate "X-Request-ID" header for each browser request #819

feat: generate "X-Request-ID" header for each browser request #819

DudaGod commented Dec 25, 2023 •

edited

Loading

DudaGod Dec 25, 2023

DudaGod Dec 25, 2023

DudaGod Dec 25, 2023

DudaGod Dec 25, 2023

DudaGod Dec 25, 2023

DudaGod Dec 25, 2023

DudaGod Dec 25, 2023

KuznetsovRoman left a comment

KuznetsovRoman Dec 26, 2023 •

edited

Loading

DudaGod Dec 27, 2023

DudaGod Dec 27, 2023

DudaGod Dec 27, 2023

DudaGod Dec 27, 2023

KuznetsovRoman commented Dec 27, 2023

DudaGod commented Dec 27, 2023

DudaGod Dec 27, 2023

		@@ -1,5 +1,6 @@
		"use strict";

		const crypto = require("crypto");

feat: generate "X-Request-ID" header for each browser request #819

feat: generate "X-Request-ID" header for each browser request #819

Conversation

DudaGod commented Dec 25, 2023 • edited Loading

What is done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KuznetsovRoman left a comment

Choose a reason for hiding this comment

KuznetsovRoman Dec 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KuznetsovRoman commented Dec 27, 2023

DudaGod commented Dec 27, 2023

Choose a reason for hiding this comment

DudaGod commented Dec 25, 2023 •

edited

Loading

KuznetsovRoman Dec 26, 2023 •

edited

Loading