feat(runtime): add support for buffered metrics#1922
Conversation
mjameswh
force-pushed
the
buffered-metrics
branch
from
ffb8a42 to
14d3115
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds experimental support for buffered metrics, allowing metrics emitted by the Core SDK to be exported through a language-side exporter. This provides an alternative to the existing Prometheus and OpenTelemetry exporters by buffering metric events in memory and exposing them via the Runtime.retrieveBufferedMetrics() method.
Changes:
- Added
BufferedMetricsExporterconfiguration option with buffer size and duration unit settings - Implemented
Runtime.retrieveBufferedMetrics()method to drain buffered metric updates - Extended metric interfaces with
kindandvalueTypeproperties for runtime type information
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/workflow/src/metrics.ts | Added kind and valueType properties to workflow metric implementations |
| packages/worker/src/runtime.ts | Added retrieveBufferedMetrics() method and updated imports |
| packages/worker/src/runtime-options.ts | Added BufferedMetricsExporter configuration type and compilation logic |
| packages/worker/src/runtime-metrics.ts | Added BufferedMetricUpdate interface and kind/valueType properties to runtime metrics |
| packages/test/src/test-runtime-otel.ts | New test file for OTEL metrics export functionality |
| packages/test/src/test-runtime-buffered-metrics.ts | New test file for buffered metrics functionality |
| packages/test/src/test-otel.ts | Refactored by moving OTEL test helpers to dedicated test file |
| packages/core-bridge/ts/native.ts | Added native types for buffered metrics support |
| packages/core-bridge/src/runtime.rs | Implemented buffered metrics buffer management in Rust runtime |
| packages/core-bridge/src/metrics.rs | Implemented buffered metrics collection and conversion logic |
| packages/core-bridge/src/helpers/try_into_js.rs | Added MemoizedHandle and OptionAsUndefined helper types |
| packages/core-bridge/bridge-macros/src/derive_tryintojs.rs | Enhanced macro to support enum-to-string conversions |
| packages/common/src/metrics.ts | Refactored metric types and added kind/valueType properties |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chris-olszewski
left a comment
chris-olszewski
left a comment
There was a problem hiding this comment.
LGTM.
Clippy lint suggestions seem sensible.
| res.write( | ||
| // This is a raw gRPC response, of length 0 | ||
| Buffer.from([ | ||
| new Uint8Array([ |
There was a problem hiding this comment.
Reason for this change other than general Buffer -> Uint8Array migration?
There was a problem hiding this comment.
For some reason, that usage of Buffer was showing as an error in my editor, even though that was building fine.
There was a problem hiding this comment.
I wonder if your editor is using TS 5.9: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-9.html#libdts-changes
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
packages/worker/src/runtime-metrics.ts:1
- The documentation appears to be cut off mid-sentence at line 364. The sentence starting with 'Note that the SDK may miss deduplication opportunities' is incomplete.
import {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 18 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2 participants
What was changed