test(test-service-load): log driverTenantName to telemetry from stress tests - #28010
test(test-service-load): log driverTenantName to telemetry from stress tests#28010shlevari wants to merge 2 commits into
Conversation
github-actions
Bot
added
area: tools
area: repo
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (48 lines, 3 files), I've queued these reviewers:
How this works
|
…s tests Stress tests did not log the driverTenantName telemetry dimension that E2E tests and perf benchmarks already emit, so there was no way to tell which ODSP tenant a stress run exercised. That makes it hard to investigate whether a particular tenant is being hit too hard, which is a likely source of throttling errors. Add the dimension to the orchestrator and to each child runner. The runner now creates its test driver before the logger so the tenant name is available as a dimension; driver creation is wrapped so a failure there still reports runnerFailed telemetry instead of exiting silently. ADO: 66983 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6d0e2d2 to
e0acfb3
Compare
There was a problem hiding this comment.
Pull request overview
Adds driverTenantName as a telemetry dimension for test-service-load stress tests so runs can be attributed to the ODSP tenant being exercised (matching what E2E/perf tests already emit).
Changes:
- Emit
driverTenantNamefrom the orchestrator logger (main.ts). - Create the test driver before the runner logger so child runners can include
driverTenantNamein all telemetry (runner.ts). - Extend the shared
createLoggerdimension type contract to includedriverTenantName(FileLogger.ts).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/test/test-service-load/src/runner.ts | Creates the test driver earlier to log tenant name; adds setup-time failure telemetry path. |
| packages/test/test-service-load/src/main.ts | Adds driverTenantName to orchestrator telemetry dimensions. |
| packages/test/test-service-load/src/FileLogger.ts | Updates logger dimension typing to include driverTenantName. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Flush before rethrowing: the top-level handler exits the process immediately. | ||
| await flushSetup(); | ||
| throw error; |
Alex Villarreal (alexvy86)
left a comment
Alex Villarreal (alexvy86)
left a comment
There was a problem hiding this comment.
The Copilot suggestion seems good, otherwise LGTM. Ideally let's verify this before merging by pushing it to a test/ branch in the upstream repo and doing a manual run of Build - client packages -> Real Service Stress test and checking that the new field shows up in Kusto telemetry for that run.
Fleet Review — In progressRunning reviewers: correctness, security, api-compatibility, performance, testing |
A rejected flush would replace the original createTestDriver failure being rethrown, hiding the root cause of the runner exiting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bundle size comparisonBase commit: Pending — |
Stress tests did not log the driverTenantName telemetry dimension that E2E tests and perf benchmarks already emit, so there was no way to tell which ODSP tenant a stress run exercised. That makes it hard to investigate whether a particular tenant is being hit too hard, which is a likely source of throttling errors.
Add the dimension to the orchestrator and to each child runner. The runner now creates its test driver before the logger so the tenant name is available as a dimension; driver creation is wrapped so a failure there still reports runnerFailed telemetry instead of exiting silently.
ADO: 66983