Skip to content

CRE: Add support for user emitted wf metrics#1924

Merged
george-dorin merged 6 commits intomainfrom
DF-23780
Apr 3, 2026
Merged

CRE: Add support for user emitted wf metrics#1924
george-dorin merged 6 commits intomainfrom
DF-23780

Conversation

@karen-stepanyan
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 24, 2026

⚠️ API Diff Results - github.com/smartcontractkit/chainlink-common

⚠️ Breaking Changes (1)

pkg/workflows/wasm/host.ExecutionHelper (1)
  • EmitUserMetric — ➕ Added

✅ Compatible Changes (14)

pkg/settings/cresettings.Workflows (5)
  • UserMetricEnabled — ➕ Added

  • UserMetricLabelsPerMetric — ➕ Added

  • UserMetricLabelValueLength — ➕ Added

  • UserMetricNameLengthLimit — ➕ Added

  • UserMetricPayloadLimit — ➕ Added

pkg/workflows/wasm/host.ModuleConfig (9)
  • EnableUserMetrics — ➕ Added

  • MaxUserMetricLabelsPerMetric — ➕ Added

  • MaxUserMetricLabelsPerMetricLimiter — ➕ Added

  • MaxUserMetricLabelValueLength — ➕ Added

  • MaxUserMetricLabelValueLengthLimiter — ➕ Added

  • MaxUserMetricNameLength — ➕ Added

  • MaxUserMetricNameLengthLimiter — ➕ Added

  • MaxUserMetricPayloadBytes — ➕ Added

  • MaxUserMetricPayloadLimiter — ➕ Added


📄 View full apidiff report

Comment thread pkg/settings/cresettings/settings.go Outdated

if err := e.module.cfg.MaxMetricPayloadLimiter.Check(e.ctx, config.Size(ptrlen)); err != nil {
e.module.cfg.Logger.Warnf("metric payload too large: %d bytes - dropping: %s", ptrlen, err)
return -1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we return a better error code? cc @nolag

@karen-stepanyan karen-stepanyan marked this pull request as ready for review March 26, 2026 08:34
@karen-stepanyan karen-stepanyan requested review from a team as code owners March 26, 2026 08:34
Copilot AI review requested due to automatic review settings March 26, 2026 08:34
@karen-stepanyan karen-stepanyan requested review from alecgard, jmank88 and nolag and removed request for MStreet3 and pavel-raykov March 26, 2026 08:34
@karen-stepanyan karen-stepanyan changed the title draft version of user emitted wf metrics user emitted wf metrics Mar 26, 2026
@karen-stepanyan karen-stepanyan changed the title user emitted wf metrics CRe: Add support for user emitted wf metrics Mar 26, 2026
@karen-stepanyan karen-stepanyan changed the title CRe: Add support for user emitted wf metrics CRE: Add support for user emitted wf metrics Mar 26, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds initial support for user-emitted workflow metrics from WASM (NoDAG) modules, including host-side validation/limits, test coverage, and CRE settings schema/default updates.

Changes:

  • Link a new WASM host import (env.emit_metric) and implement host-side decoding + limit enforcement for user metrics.
  • Extend host execution plumbing (config + ExecutionHelper) and add NoDAG tests plus a new WASM test module for metrics.
  • Add new CRE settings fields and defaults for metric enablement and metric limits (payload/name/labels).

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
pkg/workflows/wasm/host/wasm_nodag_test.go Adds NoDAG tests for metric emission limits and the disabled behavior.
pkg/workflows/wasm/host/test/metric_limits/cmd/main_wasip1.go Adds a WASI test module that emits several user metrics for limit testing.
pkg/workflows/wasm/host/module.go Adds metric-related config/limiters, extends ExecutionHelper, and links the new emit_metric import.
pkg/workflows/wasm/host/mock_execution_helper_test.go Updates generated mock to include EmitUserMetric.
pkg/workflows/wasm/host/internal/rawsdk/helpers_wasip1.go Adds the raw WASM import declaration for emit_metric.
pkg/workflows/wasm/host/execution.go Implements emitMetric handler (proto decode + limit checks + forwarding).
pkg/settings/cresettings/settings.go Adds metric settings to the schema/types.
pkg/settings/cresettings/defaults.toml Adds default values for metric settings.
pkg/settings/cresettings/defaults.json Adds default values for metric settings.
pkg/settings/cresettings/README.md Documents metric settings in the limits diagram.
go.mod Bumps workflows protos dependency to a version that includes metric protos.
go.sum Updates checksums for the bumped workflows protos dependency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 139 to +140
EmitUserLog(log string) error

Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExecutionHelper is an exported interface in a public module (chainlink-common). Adding EmitUserMetric makes this a breaking API change for any downstream implementers. Consider introducing a separate optional interface (e.g., type UserMetricEmitter interface { EmitUserMetric(*wfpb.WorkflowUserMetric) error }) and using a type assertion at call sites, or providing a new wrapper type, to keep backward compatibility.

Suggested change
EmitUserLog(log string) error
EmitUserLog(log string) error
}
// UserMetricEmitter can be implemented optionally to support emitting user metrics.
type UserMetricEmitter interface {

Copilot uses AI. Check for mistakes.
Comment thread pkg/workflows/wasm/host/execution.go
Comment thread pkg/workflows/wasm/host/wasm_nodag_test.go Outdated
Comment thread pkg/workflows/wasm/host/test/metric_limits/cmd/main_wasip1.go Outdated
@jmank88
Copy link
Copy Markdown
Contributor

jmank88 commented Mar 31, 2026

Is there a core PR that uses these?

Comment thread pkg/settings/cresettings/settings.go Outdated
jmank88
jmank88 previously approved these changes Mar 31, 2026
@karen-stepanyan
Copy link
Copy Markdown
Contributor Author

@jmank88 here is the change in core repo
smartcontractkit/chainlink#21817
I used replace reference to this PR. If it looks ok, I can merge this first and rebase the core PR. We have breaking change in this PR so I want to avoid delaying core changes merge

@george-dorin george-dorin added this pull request to the merge queue Apr 3, 2026
Merged via the queue into main with commit b39dab3 Apr 3, 2026
32 of 34 checks passed
@george-dorin george-dorin deleted the DF-23780 branch April 3, 2026 09:39
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.

5 participants