INFOPLAT-13349: feat(beholder): track beholder.export.* metrics per export via custom gRPC stats handler

[kirqz23]

What

Adds a beholder.export.bytes and beholder.export.duration metrics to the beholder gRPC client, restoring
per-node log volume visibility and export duration (labelled by csa_public_key and signal_type logs/metrics/traces)

According to these docs, rpc.client.request.* metrics are totally deprecated without any successors in otelgrpc semconv v1.40.0
RPC semantic convention stability migration guide

Semantic conventions for RPC metrics

Additionally, new beholder.export.* metrics are not recorded on each separate message, but on the whole batch at once. In terms of beholder.export.bytes it shows real number of exported bytes only on success without cumulative number on all retries that rpc.client.request.size was producing. This was misleading in terms of how many logs/metrics are coming through the gateway and downstream consumers.

Changes

• Implements exportStatsHandler (gRPC stats.Handler) to capture outbound message sizes via OutPayload events
• Adds new metric beholder.export.bytes with attributes otel_signal and csa_public_key
• Adds new metric beholder.export.duration (histogram, unit s) with attributes otel_signal, csa_public_key and error
  • Uses explicit second-scaled bucket boundaries (0.005–60); the SDK defaults are millisecond-scaled, so nearly every export would land in the first bucket
• Groups both instruments into a shared exportMetrics struct, attached to the metric exporter once the MeterProvider exists (attachMetrics)
• Wraps log and metric exporters with metering logic that:
  • Uses per-call context to isolate concurrent exports
  • Records bytes only on successful exports
  • Records duration on both successful and failed exports, labelled error={true,false}
  • Handles retries correctly (stores size, doesn't accumulate)
  • Measures duration once per logical batch, covering all retry attempts and backoff
• Comprehensive test coverage including concurrency and retry scenarios

Notes

• Both metrics are batch-scoped: one observation per logical export, never per gRPC message or per retry attempt.
• Size is captured in HandleRPC because the uncompressed proto length is only observable inside the gRPC stack. Duration is measured in the wrapper instead, so it also covers proto marshaling, connection setup and inter-attempt backoff and is still recorded when an export fails before any RPC is issued (e.g. an already-expired context, which produces no stats events at all).

Requires

Supports

[github-actions]

👋 kirqz23, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

[github-actions]

📊 API Diff Results

No changes detected for module github.com/smartcontractkit/chainlink-common

View full report

[Copilot]

Pull request overview

Adds a custom metric to restore per-node visibility into OTLP log export volume by capturing the uncompressed outbound gRPC payload size and emitting it as beholder.logs.export.bytes (labelled by csa_public_key).

Changes:

• Introduces a gRPC stats.Handler (sizeCaptureHandler) to capture stats.OutPayload.Length.
• Wraps the OTLP logs exporter with meteredLogsExporter to increment a bytes counter on successful exports (to avoid retry inflation).
• Wires the stats handler + metered exporter into NewGRPCClient’s shared log exporter connection.

Reviewed changes

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

File	Description
pkg/beholder/metered_exporter.go	Adds size-capture stats handler and metered exporter wrapper emitting beholder.logs.export.bytes.
pkg/beholder/client.go	Wires the shared capture + exporter wrapper and attaches the gRPC stats handler(s) to the log exporter dial options.

---

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

[kirqz23]

gRPC builds istats.NewCombinedHandler(...) from the registered slice of multiple handlers, that is exactly the delegating handler proposed by the Copilot. This proposal is exactly what gRPC already builds for us internally. The downside of having multiple stats handlers however is that gRPC fires every handler on every event, in registration order. So for one OutPayload event there will be two HandleRPC(...) calls which might be an overhead. We might consider either keeping both statsHandlers if we need them, or dropping grpc.WithStatsHandler(otelgrpc.NewClientHandler(otelOpts...)).

This old handler:

1. Emits rpc.client.* metrics (which part of them are going to be removed),
2. Creates a trace span per RPC
3. Injects trace-context headers

[kirqz23]

So, we have two approaches here:

1. Keep two stats handlers as we have it now. exportSizeHandler shouldn't add much to the performance.
2. Drop grpc.WithStatsHandler(otelgrpc.NewClientHandler(otelOpts...)) taking into account that we loose 3 points mentioned in the above comment, however rpc.client.* metrics are totally deprecated anyway expect duration, which can be added to our custom handler alongside beholder.export.bytes, e.g. sth like beholder.export.duration

cc @pkcll

[kirqz23]

I decided to drop the otelgrpc stats handler and use only our custom beholderStatsHandler, cause it has now both size and duration metrics. We can later on expand it to produce more needed metrics.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

pkg/beholder/client.go:580

• In newMeterProvider, the previous implementation appended cfg.metricOptions() (which includes sdkmetric.WithCardinalityLimit(cfg.MetricCardinalityLimit)) but the new code constructs the MeterProvider without it. This silently drops the configured per-instrument cardinality limit, which can increase time-series cardinality and memory usage in production.

	return sdkmetric.NewMeterProvider(
		sdkmetric.WithReader(sdkmetric.NewPeriodicReader(metered, readerOpts...)),
		sdkmetric.WithResource(resource),
		sdkmetric.WithView(cfg.MetricViews...),
	), metered, nil

pkg/beholder/client.go:153

• The PR description states beholder.export.* should be labelled per signal type (logs/metrics/traces), but the implementation only meters log and metric exports. Trace exports created in newTracerProvider are not wrapped and do not use exportSizeHandler, so beholder.export.bytes/beholder.export.duration won't be emitted for otel_signal="traces".

	// Shared export instruments beholder.export.bytes and
	// beholder.export.duration, labelled per signal. They live on this
	// MeterProvider, so the metrics exporter can only be wired up once the
	// provider and its meter exist.
	expMetrics, err := newExportMetrics(meter)

[pkcll]

Code Review: beholder export metrics (high effort)

Ran line-by-line, removed-behavior, cross-file tracer, reuse, simplification, efficiency, altitude, and conventions passes, plus targeted verification on the highest-uncertainty candidates. One candidate (byte double-counting under gRPC-level transparent retries) was checked directly against vendored grpc-go v1.82.1 source and refuted — both grpc-go's transparent retries and the OTLP SDK's own retry loop create a fresh TagRPC/rpcState per attempt, so there's no accumulation-before-End risk.

Findings

1. pkg/beholder/client.go:583 — newLoggerOpts drops otelgrpc.NewClientHandler, removing all span and standard rpc.client.* metric instrumentation for the log-export gRPC channel with no replacement (the new exportStatsHandler only counts bytes on success, never creates spans or RPC status/latency metrics). Impact: any dashboard/alert built on rpc.client.duration or gRPC client spans for the log-export channel goes silently dark the moment this merges; no dual-emission or deprecation notice.

2. pkg/beholder/httpclient.go:283 — NewHTTPClient/newHTTPMeterProvider still builds its otlpmetrichttp exporter straight into sdkmetric.NewPeriodicReader with no meteredMetricExporter wrapping and no stats-handler equivalent, so beholder.export.bytes/beholder.export.duration are never emitted for HTTP-transport beholder clients. Impact: a service configured with cfg.OtelExporterHTTPEndpoint (a fully supported, non-deprecated path chosen in NewClient) gets zero beholder.export.* metrics despite the PR's stated goal of tracking these "per export" — a silent, undocumented gap between transport choices.

3. pkg/beholder/client.go:145 — tracerProvider and meterProvider (each holding a live gRPC connection; meterProvider also a periodic-reader goroutine) are constructed before three fallible steps this diff adds/keeps (newExportMetrics, newLoggerOpts, otlploggrpcNew), and no error path shuts the already-created providers down. Impact: if otlploggrpcNew (line 165) fails after tracerProvider/meterProvider succeeded, NewGRPCClient returns the error while leaking the metric periodic-reader goroutine and both gRPC connections. Pattern pre-dates this PR, but newExportMetrics (line 152) is a brand-new failure point widening the same window.

4. pkg/beholder/metered_exporter.go:58 — exportAttrs attaches the raw (possibly empty) cfg.AuthPublicKeyHex as csa_public_key, while newOtelResource (client.go:366-371) falls back to the literal "not-configured" for the same conceptual resource attribute when the key is empty. Impact: a client with no auth key configured emits beholder.export.bytes/duration with csa_public_key="", while every other resource-level attribute on the same telemetry reads csa_public_key="not-configured" — an inconsistent label for the same dimension within one export.

5. pkg/beholder/client.go:145 — sdkmetric.NewPeriodicReader(metered, ...) starts its background export goroutine synchronously inside newMeterProvider, before the subsequent attachMetrics calls (lines 156-158) wire real instruments into metricStatsHandler/meteredMetrics. Impact: cfg.MetricReaderInterval can be very short; if a periodic tick fires before attachMetrics executes, that export cycle silently records zero beholder.export.bytes/duration for the metrics signal with no error/log indicating a dropped observation.

6. pkg/beholder/client.go:141 — the "logs"/"metrics"/"traces" signal string is passed independently to newExportStatsHandler and to newMeteredLogExporter/newBaseExporter at separate call sites, with no shared constant or type tying the two together for a given signal. Impact: a future rename or addition of a signal updated in only one of the two call sites still compiles but silently produces mismatched otel_signal label values between beholder.export.bytes and beholder.export.duration for what should be the same signal — undetectable until someone diffs a dashboard.

7. pkg/beholder/metered_exporter.go:128 — lazyMetered.attachMetrics and exportStatsHandler.attachMetrics independently reimplement the identical "store instruments in an atomic.Pointer once the MeterProvider exists" pattern instead of sharing one helper. Impact: a future change to attach semantics (e.g. an idempotency guard or panic-on-double-attach) must be applied in both places and can silently drift out of sync.

8. pkg/beholder/metered_exporter.go:407 — meteredTraceExporter/newMeteredTraceExporter is fully implemented and unit-tested but never wired into newTracerProvider/NewGRPCClient — confirmed dead production code (only referenced from its own test file). Impact: the type and its atomic-pointer wiring carry maintenance weight for a feature with zero production callers; a later refactor of meteredExporter.record that breaks trace-specific behavior would go uncaught outside its own unit tests.

9. pkg/beholder/metered_exporter.go:89 — meteredExporter.record unconditionally calls m.metrics.duration.Record(...) with no nil-guard; a zero-value exportMetrics{} (both fields nil interfaces) would panic, unlike lazyMetered.record's explicit nil-check for the unattached state. Impact: not reachable via current NewGRPCClient wiring, but the type permits constructing this state (metered_exporter_test.go:682 does exactly this) — any future direct construction of newMeteredLogExporter/newBaseExporter with an unpopulated exportMetrics panics instead of degrading gracefully.

10. pkg/beholder/client.go:549 — otlpmetricgrpc.WithDialOption(dialOpts...) is only safe because it's called exactly once with the fully-combined dialOpts slice; the option-append semantics aren't obviously idempotent-safe against a second call. Impact: a future PR that adds a second otlpmetricgrpc.WithDialOption(...) call anywhere in this option chain risks silently discarding the stats handler and/or the rotating-auth credential dial option depending on the library's overwrite-vs-append semantics — a latent trap for maintainers unaware of this constraint.

---

Generated via automated code review (high effort, 8 finder angles + targeted verification).

[pkcll]

pkg/beholder/client.go:583 — newLoggerOpts drops otelgrpc.NewClientHandler, removing all span and standard rpc.client.* metric instrumentation for the log-export gRPC channel with no replacement (the new exportStatsHandler only counts bytes on success, never creates spans or RPC status/latency metrics). Impact: any dashboard/alert built on rpc.client.duration or gRPC client spans for the log-export channel goes silently dark the moment this merges; no dual-emission or deprecation notice.

I think its fine to drop otelgrpc.NewClientHandler since it was only used internally

[pkcll]

Correction to finding #1 above

Retracting part of my first finding — I filed it without reading the PR description properly, and it's unfair to the change as written.

What I got wrong. Finding #1 said dropping otelgrpc.NewClientHandler happens with "no dual-emission or deprecation notice." That's false: the description explains the reasoning, and the substantive half of it is correct. rpc.client.request.size accumulated bytes across retry attempts, so it overstated real export volume — beholder.export.bytes being success-only and batch-scoped is a better metric for the question being asked, not just a differently-named one. I don't think the handler should be restored, and I withdraw the "compose or delegate to the OTel handler" suggestion: doing that would also drag back the requirement that both MeterProvider and TracerProvider be fully constructed before newLoggerOpts runs, which is the coupling behind finding #3.

Two small factual notes on the rationale. The vendored stack here is otelgrpc v0.63.0 against go.opentelemetry.io/otel/semconv/v1.37.0/rpcconv (otel v1.44.0), not semconv v1.40.0. In that version all five client instruments — rpc.client.duration, .request.size, .response.size, .requests_per_rpc, .responses_per_rpc — are live and carry no Deprecated: marker in the Go source (rpcconv, e.g. ClientDuration.Name() at line 68). The migration direction is real, but they aren't deprecated in the version this repo compiles against today. Separately, NewClientHandler also builds a tracer and starts client spans (stats_handler.go:171, :222), so removing it drops those too; the description only addresses the metrics.

The narrower gap that I do think is worth closing. Of everything the old handler emitted, one signal isn't replaced by the new metrics: per-attempt gRPC status codes. rpc.client.duration carried rpc.grpc.status_code, whereas beholder.export.duration labels failures with error={true,false}. That boolean can tell you exports are failing but not whether the pipeline is hitting UNAVAILABLE, RESOURCE_EXHAUSTED, or DEADLINE_EXCEEDED — which is the distinction that actually drives triage when the telemetry path degrades. Suggest closing that inside exportStatsHandler (a status-code attribute, or a beholder.export.attempts counter keyed by gRPC code) rather than reinstating otelgrpc. HandleRPC already sees *stats.End, so the code is available at the point the duration/error decision is made.

On self-reference, since it came up while I was checking this. Worth recording that it isn't a differentiator between the two designs. exportStatsHandler is attached to the metric export channel and records beholder.export.bytes into the same MeterProvider those exports drain, so exporting metrics produces metrics that ride the next export cycle. That's bounded rather than a feedback loop — the periodic reader fires on a timer regardless of data volume and cardinality is fixed at two instruments — but the metrics signal will no longer go idle. The old otelgrpc pattern had the same shape with a wider surface (five histograms plus spans). Neither recurses, because in both cases the provider's own export path is uninstrumented. Not asking for a change here, just noting it so it's a known property rather than a surprise later.

Findings #2 through #10 in the comment above stand as written.

[pkcll]

Correction to finding #1 above

Retracting part of my first finding — I filed it without reading the PR description properly, and it's unfair to the change as written.

What I got wrong. Finding #1 said dropping otelgrpc.NewClientHandler happens with "no dual-emission or deprecation notice." That's false: the description explains the reasoning, and the substantive half of it is correct. rpc.client.request.size accumulated bytes across retry attempts, so it overstated real export volume — beholder.export.bytes being success-only and batch-scoped is a better metric for the question being asked, not just a differently-named one. I don't think the handler should be restored, and I withdraw the "compose or delegate to the OTel handler" suggestion: doing that would also drag back the requirement that both MeterProvider and TracerProvider be fully constructed before newLoggerOpts runs, which is the coupling behind finding #3.

Two small factual notes on the rationale. The vendored stack here is otelgrpc v0.63.0 against go.opentelemetry.io/otel/semconv/v1.37.0/rpcconv (otel v1.44.0), not semconv v1.40.0. In that version all five client instruments — rpc.client.duration, .request.size, .response.size, .requests_per_rpc, .responses_per_rpc — are live and carry no Deprecated: marker in the Go source (rpcconv, e.g. ClientDuration.Name() at line 68). The migration direction is real, but they aren't deprecated in the version this repo compiles against today. Separately, NewClientHandler also builds a tracer and starts client spans (stats_handler.go:171, :222), so removing it drops those too; the description only addresses the metrics.

The narrower gap that I do think is worth closing. Of everything the old handler emitted, one signal isn't replaced by the new metrics: per-attempt gRPC status codes. rpc.client.duration carried rpc.grpc.status_code, whereas beholder.export.duration labels failures with error={true,false}. That boolean can tell you exports are failing but not whether the pipeline is hitting UNAVAILABLE, RESOURCE_EXHAUSTED, or DEADLINE_EXCEEDED — which is the distinction that actually drives triage when the telemetry path degrades. Suggest closing that inside exportStatsHandler (a status-code attribute, or a beholder.export.attempts counter keyed by gRPC code) rather than reinstating otelgrpc. HandleRPC already sees *stats.End, so the code is available at the point the duration/error decision is made.

On self-reference, since it came up while I was checking this. Worth recording that it isn't a differentiator between the two designs. exportStatsHandler is attached to the metric export channel and records beholder.export.bytes into the same MeterProvider those exports drain, so exporting metrics produces metrics that ride the next export cycle. That's bounded rather than a feedback loop — the periodic reader fires on a timer regardless of data volume and cardinality is fixed at two instruments — but the metrics signal will no longer go idle. The old otelgrpc pattern had the same shape with a wider surface (five histograms plus spans). Neither recurses, because in both cases the provider's own export path is uninstrumented. Not asking for a change here, just noting it so it's a known property rather than a surprise later.

Findings #2 through #10 in the comment above stand as written.

Consider adding status_code to beholder.export.duration

[kirqz23]

1 (otelgrpc dropped): rpc.client.* is deprecated without successors in otelgrpc semconv v1.40.0, which is what this PR replaces; the span/trace-context trade-off was already discussed. It is safe to drop it.

2 (HTTP transport): Out of scope as it was done for gRPC-only by design; stats.Handler has no HTTP equivalent (a RoundTripper sees only the compressed body), and no consumer configures OtelExporterHTTPEndpoint. Follow-up if ever needed.

3 NewGRPCClient now shuts down tracerProvider/meterProvider on any later failure, no more leaked connections/reader goroutine.

4 exportAttrs falls back to "not-configured" for empty CSA key (shared constant with newOtelResource) + test.

5 (tick before attach): By design unattached handler/wrapper are tested no-ops recording nothing; the self-referential first cycle is empty anyway; the window is microseconds vs a 10s default interval.

6 shared signalLogs/signalMetrics/signalTraces constants; handler and wrapper labels can't drift now.

7 (two attachMetrics): Deliberate different payload types; a shared helper would need generics for zero behavioral gain, and the compiler catches mix-ups.

8 (newMeteredTraceProvider): implemented and the instrumentation used in the gRPC client; resolved by wiring the trace exporter

9 zero-value exportMetrics{} degrades to passthrough instead of panic + test.

10 (WithDialOption semantics): No code change needed, all three call sites already build one fully-combined dialOpts slice and call WithDialOption exactly once, so the replace-not-append semantics are satisfied by construction.