Add Beholder OTel SDK metric cardinality limit by pkcll · Pull Request #2225 · smartcontractkit/chainlink-common

pkcll · 2026-07-07T02:11:56Z

Summary

Add OTel SDK WithCardinalityLimit to Beholder meter providers (default 100,000, 0 disables).
Wire CL_TELEMETRY_METRIC_CARDINALITY_LIMIT through loop EnvConfig → server.go → beholder gRPC/HTTP/writer clients.
Use *int in EnvConfig so unset vs explicit-disable propagate correctly across LOOP parent/child processes.

Attribute-filter metric views are split to a follow-up PR (beholder-otel-metric-filter-views).

Test plan

go test ./pkg/beholder/... ./pkg/loop

github-actions · 2026-07-07T02:13:12Z

✅ API Diff Results - `github.com/smartcontractkit/chainlink-common`

✅ Compatible Changes (3)

`pkg/beholder.Config` (1)

MetricCardinalityLimit — ➕ Added

`pkg/beholder.writerClientConfig` (1)

MetricCardinalityLimit — ➕ Added

`pkg/loop.EnvConfig` (1)

TelemetryMetricCardinalityLimit — ➕ Added

📄 View full apidiff report

Copilot

Pull request overview

This PR reduces Beholder/OTel metric cardinality by introducing default SDK metric views that filter high-cardinality attributes before export, and by adding an SDK-level per-instrument attribute-set cardinality limit (defaulting to 10,000, configurable via CL_TELEMETRY_METRIC_CARDINALITY_LIMIT, with 0 disabling it). It also wires these metric-provider options consistently across the gRPC, HTTP, and writer/noop Beholder clients.

Changes:

Add default metric views that deny/allow specific attribute keys to prevent high-cardinality label export.
Add/configure an OTel SDK metric cardinality limit (default 10000, 0 disables) and plumb it through loop env config into Beholder config.
Centralize metric provider option construction (mergeMetricViews + appendMeterProviderOptions) and apply it across Beholder client variants.

Reviewed changes

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

Show a summary per file

File	Description
pkg/loop/server.go	Passes the env-configured metric cardinality limit into Beholder config during server startup.
pkg/loop/config.go	Adds parsing/defaulting for `CL_TELEMETRY_METRIC_CARDINALITY_LIMIT` and exports it via `AsCmdEnv()`.
pkg/loop/config_test.go	Updates full env config + `AsCmdEnv` test expectations for the new env var.
pkg/beholder/config.go	Extends Beholder config with `MetricViewsDisabled` and `MetricCardinalityLimit`; defaults limit to 10000 and disables in test defaults.
pkg/beholder/config_test.go	Updates golden config example struct to include the new config fields.
pkg/beholder/testdata/config-example.json	Updates golden JSON with `MetricViewsDisabled` and `MetricCardinalityLimit`.
pkg/beholder/metricviews/views.go	Introduces default attribute-filtering views (global denylist + base-trigger allowlists).
pkg/beholder/metricviews/views_test.go	Adds tests validating default views drop intended attributes.
pkg/beholder/meter_provider.go	Adds centralized view-merge + meter provider option builder (views + cardinality limit).
pkg/beholder/meter_provider_test.go	Adds tests for cardinality limit behavior and view merging.
pkg/beholder/client.go	Applies centralized meter provider options to the gRPC exporter path.
pkg/beholder/httpclient.go	Applies centralized meter provider options to the HTTP exporter path.
pkg/beholder/noop.go	Applies centralized meter provider options to the writer/noop client path.

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

pkcll · 2026-07-07T20:43:01Z

+	if e.TelemetryMetricCardinalityLimit != 0 {
+		add(envTelemetryMetricCardinalityLimit, strconv.Itoa(e.TelemetryMetricCardinalityLimit))
+	}


Already fixed by an earlier commit (switched the field to *int so unset vs. explicit-0 are distinguishable) — Copilot was reviewing a stale diff.

pkcll · 2026-07-07T20:43:02Z

+			limit, err := strconv.Atoi(v)
+			if err != nil {
+				return fmt.Errorf("failed to parse %s: %w", envTelemetryMetricCardinalityLimit, err)
+			}
+			e.TelemetryMetricCardinalityLimit = limit
+		} else {


Fixed — parse() now rejects negative values with an explicit error; added a test case.

pkcll · 2026-07-07T20:43:04Z

+	assert.Equal(t, int64(uniqueAttributes), total)
+}
+
+func TestMergeMetricViews_prependsDefaults(t *testing.T) {


Fixed — renamed to TestMergeMetricViews_appendsDefaultsAfterCallerViews.

pkcll · 2026-07-07T20:43:05Z

+	counter.Add(context.Background(), 1,
+		metric.WithAttributes(
+			attribute.String("capability_id", "cap-a"),
+			attribute.String("trigger_id", "trig-a"),
+			attribute.String("event_id", "ev-1"),
+		),
+	)
+
+	var rm metricdata.ResourceMetrics
+	require.NoError(t, reader.Collect(context.Background(), &rm))
+
+	keys := attributeKeysFromSum(t, rm)
+	assert.Contains(t, keys, attribute.Key("capability_id"))
+	assert.NotContains(t, keys, attribute.Key("trigger_id"))
+	assert.NotContains(t, keys, attribute.Key("event_id"))
+}


Fixed — added an attribute (attempts) that's absent from both the allow- and deny-lists, so the test only passes if the base-trigger view itself applied, not just the global catch-all.

INFOPLAT-16436 (see PR #2225 follow-up): trigger_id is needed on the default metric stream and should only be stripped where it's genuinely high-cardinality. Remove it from globalHighCardinalityDeny and add it to baseTriggerAllow so capabilities_base_trigger_* metrics keep passing it through, matching the existing stopped-resending allow-list.

pkcll · 2026-07-07T20:42:42Z

Response to Copilot review comments:

#	Finding	Status
1	AsCmdEnv drops 0 → child re-defaults to 10000	Already fixed by an earlier commit (switched the field to `*int` so unset vs. explicit-0 are distinguishable) — Copilot was reviewing a stale diff
2	Negative limit values silently accepted	Fixed — `parse()` now rejects negative values with an explicit error; added a test case
3	Test name says "prepends", code appends	Fixed — renamed to `TestMergeMetricViews_appendsDefaultsAfterCallerViews`
4	Base-trigger allowlist test doesn't prove the specific view matched	Fixed — added an attribute (`attempts`) that's absent from both the allow- and deny-lists, so the test only passes if the base-trigger view itself applied, not just the global catch-all

pavel-raykov · 2026-07-08T13:15:10Z

+)
+
+var (
+	globalHighCardinalityDeny = attribute.NewDenyKeysFilter(


this file reads as if it is applied globally to all metrics, instead these are specific limiters for capabilities - is this intended?

Yes this is global filter for labels which were added. Historically workflow_execution_id was added multiple times for metrics in multiple places. This view/filter attempts to prevent this from happening in the future for the top offenders. This is basically a black list for labels which we know for sure are not bounded.

pavel-raykov · 2026-07-08T13:16:44Z

+	)
+
+	baseTriggerAllow = attribute.NewAllowKeysFilter(
+		attribute.Key("capability_id"),


is there some beholder data showing that filtering based on these attributes is needed?

I dont have exact number for this label cardinality, ideally I would suggest to drop anything that has word ID in it. We should have only low cardinality labels to not put too much pressure on our Observability backends. But thats a long term goal. For now Im keeping this white list here for labels which do look suspicious.

is there a way to compute the traffic of _id* labels vs others now?

Not directly from the OTel pipeline, you'd need to either instrument the otel exporter on the node side or tap into it somewhere in the otel collector upstream. That would require custom code. New VictoriaMetrics backend has a native cardinality API and UI that shows number of time series for metrics per label: https://vm-ui-prod.ops.prod.cldev.sh/select/0/vmui/?#/cardinality?date=2026-07-09&topN=10&focusLabel=event_id

I think it supports wildcard too *_id

https://vm-ui-prod.ops.prod.cldev.sh/select/0/vmui/?#/cardinality?date=2026-07-09&topN=10&focusLabel=*_id

Some reports generated from querying VM API
https://vmselect-prod.ops.prod.cldev.sh:8481

Distinct label values ┌─────────────────────────────┬───────────────┬───────────────────────┐ │ Scope │ capability_id │ trigger_id │ ├─────────────────────────────┼───────────────┼───────────────────────┤ │ Global (all metrics) │ 160 │ 117 │ ├─────────────────────────────┼───────────────┼───────────────────────┤ │ capabilities_* │ 59 │ 117 │ ├─────────────────────────────┼───────────────┼───────────────────────┤ │ capabilities_base_trigger_* │ 59 │ 117 │ ├─────────────────────────────┼───────────────┼───────────────────────┤ │ platform_launcher_* │ 102 │ 0 (label not present) │ └─────────────────────────────┴───────────────┴───────────────────────┘ trigger_id only appears on capabilities_base_trigger_* metrics. capability_id also appears on platform_launcher_* (launcher metrics), which adds ~43 extra values beyond the capabilities family. ──────────────────────────────────────── Series count (key metrics) ┌─────────────────────────────────────────────────┬──────────────┬────────────────────────┬─────────────────────┐ │ Metric │ Total series │ Distinct capability_id │ Distinct trigger_id │ ├─────────────────────────────────────────────────┼──────────────┼────────────────────────┼─────────────────────┤ │ capabilities_base_trigger_ack_total │ 215,546 │ 52 │ 116 │ ├─────────────────────────────────────────────────┼──────────────┼────────────────────────┼─────────────────────┤ │ capabilities_base_trigger_retry_total │ 3,358 │ 36 │ 51 │ ├─────────────────────────────────────────────────┼──────────────┼────────────────────────┼─────────────────────┤ │ capabilities_base_trigger_active_registrations │ 543 │ 57 │ — │ ├─────────────────────────────────────────────────┼──────────────┼────────────────────────┼─────────────────────┤ │ capabilities_base_trigger_pending_events │ 457 │ 55 │ — │ ├─────────────────────────────────────────────────┼──────────────┼────────────────────────┼─────────────────────┤ │ capabilities_base_trigger_ack_attempts_bucket │ 2,155,460 │ 52 │ 116 │ ├─────────────────────────────────────────────────┼──────────────┼────────────────────────┼─────────────────────┤ │ capabilities_base_trigger_time_to_ack_ms_bucket │ 2,586,552 │ (timed out) │ 116 │ ├─────────────────────────────────────────────────┼──────────────┼────────────────────────┼─────────────────────┤ │ platform_launcher_* (all) │ 7,299 │ 102 │ — │ └─────────────────────────────────────────────────┴──────────────┴────────────────────────┴─────────────────────┘ Histogram buckets (*_bucket) dominate series count because of the le label — each capability_id/trigger_id is multiplied across bucket boundaries and other labels (donID, host_name, zone, platformEnv, etc.). ──────────────────────────────────────── Top cardinality drivers (ack_total) capability_id — up to ~10,912 series per value: • evm:ChainSelector:11344663589394136015@1.0.0 (56) • evm:ChainSelector:1523760397290643893@1.0.0 (5734951) trigger_id — up to ~9,960 series per value (hash-style trigger_reg_* IDs) ──────────────────────────────────────── Notes • VM has ~164M total series; global count({capability_id!=""}) and count({trigger_id!=""}) timed out at the 30s query limit. • Scoped queries and the label-values API completed successfully and are the reliable approach here. • trigger_id cardinality (117) is moderate; series explosion comes from label combinations (especially on histogram metrics), not from hundreds of thousands of unique trigger IDs.

event_id label cardinality — VictoriaMetrics Global
Source: https://vmselect-prod.ops.prod.cldev.sh:8481/select/0/prometheus

Distinct values ┌─────────────────────────────┬──────────────────────────┐ │ Scope │ Distinct event_id values │ ├─────────────────────────────┼──────────────────────────┤ │ Global │ 88,529 │ ├─────────────────────────────┼──────────────────────────┤ │ capabilities_* │ 88,529 │ ├─────────────────────────────┼──────────────────────────┤ │ capabilities_base_trigger_* │ 88,529 │ ├─────────────────────────────┼──────────────────────────┤ │ platform_launcher_* │ 0 (label not present) │ ├─────────────────────────────┼──────────────────────────┤ │ VM tsdb status (top labels) │ 72,955 │ └─────────────────────────────┴──────────────────────────┘ event_id is orders of magnitude higher than the other labels we checked: ┌───────────────┬────────────────────────┐ │ Label │ Global distinct values │ ├───────────────┼────────────────────────┤ │ event_id │ 88,529 │ ├───────────────┼────────────────────────┤ │ capability_id │ 160 │ ├───────────────┼────────────────────────┤ │ trigger_id │ 117 │ └───────────────┴────────────────────────┘ ──────────────────────────────────────── Where it appears • Only on capabilities_* metrics (same family as trigger_id) • Co-labeled with capability_id, trigger_id, donID, host_name, zone, platformEnv, etc. (42 labels total) • Not on platform_launcher_* or gauges like active_registrations / pending_events Values look like composite hashes, e.g. 00003f2477...:a873ff00...:0 ──────────────────────────────────────── Series count by metric ┌─────────────────────────────────────────────────┬──────────────────────┬───────────────────┐ │ Metric │ Series with event_id │ Distinct event_id │ ├─────────────────────────────────────────────────┼──────────────────────┼───────────────────┤ │ capabilities_base_trigger_ack_total │ 217,447 │ 82,391 │ ├─────────────────────────────────────────────────┼──────────────────────┼───────────────────┤ │ capabilities_base_trigger_retry_total │ 2,792 │ 2,792 (1:1) │ ├─────────────────────────────────────────────────┼──────────────────────┼───────────────────┤ │ capabilities_base_trigger_ack_attempts_bucket │ 2,174,470 │ 82,391 │ ├─────────────────────────────────────────────────┼──────────────────────┼───────────────────┤ │ capabilities_base_trigger_time_to_ack_ms_bucket │ 2,609,520 │ 82,398 │ ├─────────────────────────────────────────────────┼──────────────────────┼───────────────────┤ │ capabilities_base_trigger_active_registrations │ — │ — │ ├─────────────────────────────────────────────────┼──────────────────────┼───────────────────┤ │ capabilities_base_trigger_pending_events │ — │ — │ └─────────────────────────────────────────────────┴──────────────────────┴───────────────────┘ On ack_total, ~2.6 series per event_id on average (other labels multiply). Histogram buckets push total series into the millions. ──────────────────────────────────────── Takeaway event_id is the dominant high-cardinality label in this family — ~82k+ unique values vs ~117 trigger_id and ~59 capability_id on the same metrics. It's a strong candidate for aggregation, dropping at scrape/export, or scoping to debug-only metrics.

pavel-raykov · 2026-07-08T13:18:12Z

 	MetricRetryConfig    *RetryConfig
 	MetricViews          []metric.View
+	// MetricViewsDisabled skips DefaultViews attribute filtering (for tests).
+	MetricViewsDisabled bool


this is a strange approach - can't you default filters only in the place where they are created in prod? For example, do you also need these filters in staging?

Sorry, // MetricViewsDisabled skips DefaultViews attribute filtering (for tests). - this is confusing comment, its not just for tests (will fix that).

MetricViewsDisabled is a feature flag to disable the filters in case we need it for emergency situations. It will be added to core.toml config and passed from the core node, the same as other Beholder/ChIP config parameters.

Fixed 80de377

pavel-raykov · 2026-07-08T13:19:40Z

+// keeps only the first in registration order and drops the rest. If the default
+// "*" denylist ran first, caller histogram-bucket views would be silently dropped.
+//
+// A consequence is that filters do not compose: once a caller view wins for a


this I don't understand - what is the difficulty of making composable filters? (which are the most natural)

The difficulty is that OTel does not support chaining/composing filters out of the box. It picks the first match and applies it. So if you are adding a blacklist view to filter event_id and there is already a view passed from the core to Beholder for existing metrics (we have cases like that for configuring histogram buckets), the blacklist will not be applied.

jmank88 · 2026-07-09T19:03:06Z

+	return append(append([]sdkmetric.View{}, cfg.MetricViews...), metricviews.DefaultViews()...)
+}
+
+func appendMeterProviderOptions(cfg Config, opts ...sdkmetric.Option) []sdkmetric.Option {


I think it is unintuitive for an append func to reverse the order. What if we just had a simple method and caller did the append?

Suggested change

func appendMeterProviderOptions(cfg Config, opts ...sdkmetric.Option) []sdkmetric.Option {

func (cfg Config) metricOptions() []sdkmetric.Option {

Done — replaced appendMeterProviderOptions with zero-arg (cfg Config) metricOptions(); callers use append(cfg.metricOptions(), ...) for reader/resource options.

jmank88 · 2026-07-09T19:03:59Z

+	if cfg.MetricViewsDisabled {
+		return cfg.MetricViews
+	}
+	return append(append([]sdkmetric.View{}, cfg.MetricViews...), metricviews.DefaultViews()...)


Appending to empty?

Suggested change

return append(append([]sdkmetric.View{}, cfg.MetricViews...), metricviews.DefaultViews()...)

return append(cfg.MetricViews, metricviews.DefaultViews()...)

Addressed in #2238 after the PR split (metric filter views). metricViews() uses append(cfg.MetricViews, metricviews.DefaultViews()...) as suggested.

jmank88 · 2026-07-09T19:05:00Z

+// stream, the default attribute filter no longer applies to it. That is acceptable
+// because caller views target metrics that do not emit the denied labels; capability
+// trigger metrics carry no caller view and fall through to the defaults below.
+func mergeMetricViews(cfg Config) []sdkmetric.View {


This seems like it could be a method as well:

Suggested change

func mergeMetricViews(cfg Config) []sdkmetric.View {

func (cfg Config) metricViews() []sdkmetric.View {

Addressed in #2238 after the PR split. mergeMetricViews became (cfg Config) metricViews().

Wire CL_TELEMETRY_METRIC_CARDINALITY_LIMIT through loop EnvConfig and server into beholder clients. Default limit is 100,000; 0 disables it. Use *int in EnvConfig so unset vs explicit-disable propagate correctly across LOOP parent/child processes. Co-authored-by: Cursor <cursoragent@cursor.com>

Addresses jmank88 review: replace variadic metricOptions with a cfg-only method; call sites use append(cfg.metricOptions(), ...).

pkcll force-pushed the beholder-otel-limit-metric-cardinality branch from 0b3ca6d to ca0b2b4 Compare July 7, 2026 03:16

pkcll mentioned this pull request Jul 7, 2026

feat(beholder): reduce PerWorkflow histogram buckets 16→8 #2226

Draft

3 tasks

pkcll requested review from DylanTinianov, jmank88 and patrickhuie19 July 7, 2026 05:15

jmank88 requested a review from pavel-raykov July 7, 2026 13:33

pkcll marked this pull request as ready for review July 7, 2026 16:34

pkcll requested review from a team as code owners July 7, 2026 16:34

Copilot AI review requested due to automatic review settings July 7, 2026 16:34

product-security-plaid-production Bot requested a review from fouadkada July 7, 2026 16:34

Copilot started reviewing on behalf of pkcll July 7, 2026 16:35 View session

Copilot AI reviewed Jul 7, 2026

View reviewed changes

pkcll force-pushed the beholder-otel-limit-metric-cardinality branch from 1203e1b to 8231c54 Compare July 7, 2026 20:42

pavel-raykov reviewed Jul 8, 2026

View reviewed changes

jmank88 reviewed Jul 9, 2026

View reviewed changes

pkcll force-pushed the beholder-otel-limit-metric-cardinality branch from 80de377 to 9eaf479 Compare July 10, 2026 21:45

pkcll changed the title ~~Limit Beholder metric cardinality~~ Add Beholder OTel SDK metric cardinality limit Jul 10, 2026

pkcll mentioned this pull request Jul 10, 2026

Add Beholder default metric filter views #2238

Draft

1 task

pkcll force-pushed the beholder-otel-limit-metric-cardinality branch from 9eaf479 to a7dbabb Compare July 10, 2026 23:14

Use zero-arg Config.metricOptions(); callers append reader/resource.

1215879

Addresses jmank88 review: replace variadic metricOptions with a cfg-only method; call sites use append(cfg.metricOptions(), ...).

pkcll mentioned this pull request Jul 11, 2026

Wire Telemetry.MetricCardinalityLimit to Beholder smartcontractkit/chainlink#23084

Draft

1 task

	func appendMeterProviderOptions(cfg Config, opts ...sdkmetric.Option) []sdkmetric.Option {
	func (cfg Config) metricOptions() []sdkmetric.Option {

	return append(append([]sdkmetric.View{}, cfg.MetricViews...), metricviews.DefaultViews()...)
	return append(cfg.MetricViews, metricviews.DefaultViews()...)

	func mergeMetricViews(cfg Config) []sdkmetric.View {
	func (cfg Config) metricViews() []sdkmetric.View {

Uh oh!

Conversation

pkcll commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ API Diff Results - github.com/smartcontractkit/chainlink-common

✅ Compatible Changes (3)

pkg/beholder.Config (1)

pkg/beholder.writerClientConfig (1)

pkg/loop.EnvConfig (1)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pkcll commented Jul 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pkcll Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pkcll Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pkcll Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmank88 Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

pkcll commented Jul 7, 2026 •

edited

Loading

github-actions Bot commented Jul 7, 2026 •

edited

Loading

✅ API Diff Results - `github.com/smartcontractkit/chainlink-common`

`pkg/beholder.Config` (1)

`pkg/beholder.writerClientConfig` (1)

`pkg/loop.EnvConfig` (1)

pkcll Jul 9, 2026 •

edited

Loading

pkcll Jul 9, 2026 •

edited

Loading

pkcll Jul 9, 2026 •

edited

Loading

jmank88 Jul 9, 2026 •

edited

Loading