Skip to content

Standardized metrics export - feature flagged#125

Merged
chrishagglund-ship-it merged 25 commits into
mainfrom
gated-metrics-standardization
Jun 22, 2026
Merged

Standardized metrics export - feature flagged#125
chrishagglund-ship-it merged 25 commits into
mainfrom
gated-metrics-standardization

Conversation

@chrishagglund-ship-it

@chrishagglund-ship-it chrishagglund-ship-it commented May 4, 2026

Copy link
Copy Markdown
Contributor

Provides cross-sdk standardized metrics output format, enabled by setting WORKER_CANONICAL_METRICS=true, see standardized metrics catalog for the proposed standard metrics catalog

see TOOL-34 for more info

@codecov

codecov Bot commented May 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.76318% with 134 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...rc/sdk/worker/metrics/MetricsCollectorInterface.ts 0.00% 54 Missing ⚠️
...rc/sdk/worker/metrics/CanonicalMetricsCollector.ts 91.88% 43 Missing ⚠️
src/sdk/clients/workflow/WorkflowExecutor.ts 14.28% 24 Missing ⚠️
src/sdk/worker/metrics/LegacyMetricsCollector.ts 89.33% 8 Missing ⚠️
.../sdk/worker/metrics/CanonicalPrometheusRegistry.ts 98.21% 3 Missing ⚠️
...dk/createConductorClient/helpers/fetchWithRetry.ts 97.29% 2 Missing ⚠️
Flag Coverage Δ
integration-v4 64.62% <36.13%> (-2.41%) ⬇️
integration-v5 69.94% <39.41%> (-2.91%) ⬇️
unit 49.33% <88.99%> (+3.23%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/sdk/builders/tasks/pullWorkflowMessages.ts 100.00% <100.00%> (ø)
src/sdk/clients/worker/Poller.ts 98.63% <100.00%> (+<0.01%) ⬆️
src/sdk/clients/worker/TaskRunner.ts 100.00% <100.00%> (ø)
src/sdk/clients/worker/events/EventDispatcher.ts 100.00% <100.00%> (ø)
src/sdk/createConductorClient/constants.ts 100.00% <100.00%> (ø)
...sdk/createConductorClient/createConductorClient.ts 100.00% <100.00%> (ø)
...eateConductorClient/helpers/metricsInterceptors.ts 100.00% <100.00%> (ø)
...reateConductorClient/helpers/resolveOrkesConfig.ts 100.00% <100.00%> (ø)
src/sdk/worker/core/TaskHandler.ts 73.84% <100.00%> (+0.04%) ⬆️
src/sdk/worker/metrics/MetricsServer.ts 100.00% <100.00%> (ø)
... and 9 more

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bradyyie

Copy link
Copy Markdown

One thing on the metrics wiring: the recordApiRequestTime calls in wrapFetchWithRetry (both the success .then and the error handler) aren't guarded. If a metric emit ever throws, the success path turns a good response into a rejected promise, and on the error path it throws over the original network error before throw error ever runs. Metrics shouldn't be able to fail a real request.

Can we wrap the emit in a try/catch (or a small safeEmit helper) so a collector blowing up just gets logged and the request still goes through? Same goes for the other observer calls added on the request path, e.g. recordWorkflowStartError in the start-workflow catch.

Comment thread src/sdk/worker/metrics/LegacyMetricsCollector.ts Outdated
…cessively flaky tests to maybe not need to be run five+ times to pass
@chrishagglund-ship-it chrishagglund-ship-it merged commit f919ead into main Jun 22, 2026
75 of 89 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants