monitoring: eagerly register apm-server metric instruments

[carsonip]

Motivation/summary

apm-server's monitoring tree (the /stats HTTP endpoint and the libbeat monitoring snapshot) is the source we use to generate three sets of upstream field mappings:

• the metricbeat beat module's _meta/fields.yml and stats/_meta/fields.yml
• the Elasticsearch stack-monitoring templates monitoring-beats.json and monitoring-beats-mb.json
• the elastic_agent integration package's beat-stats-fields.yml

Mappings can include fields that aren't always emitted at runtime — the problem is the tooling we use to derive those mappings. The stats-to-mapping helper from the closed #13638 (and any equivalent script) reads a single /stats snapshot and infers field types from what's there. If a counter only appears in /stats after some specific code path has fired, it's not in the snapshot and silently gets dropped from the regenerated mappings.

After #15094 "Translate otel metrics to libbeat monitoring", that's exactly what happens: the OpenTelemetry instruments backing the new monitoring path are lazy. An Int64Counter only contributes a data point — and therefore only appears in /stats — once .Add() has been called on it. Counters whose code path hasn't fired yet (most error responses, OTLP variants we haven't seen traffic for, agent-config cache hits/misses, etc.) are absent from the snapshot. Pre-#15094, libbeat monitoring counters were registered up front and emitted as 0 by default, which gave /stats an exhaustive enumeration of every counter the server defines.

This PR restores that property under the OpenTelemetry path so the mapping-generation tooling can regenerate field mappings from a single /stats capture without needing to drive every error path.

What this changes

Eager zero-init helper

Adds a small internal/otelmetric package whose constructors (NewInt64Counter, NewInt64UpDownCounter) call .Add(0) at instrument creation, ensuring each instrument has at least one data point so the SDK exports it. Gauges and histograms are intentionally not in the helper: a synthetic Record(0) would falsely declare a gauge's current value as 0 and would inflate a histogram's observation count.

Per-package eager registration

Each registration site that defines apm-server.* metrics is converted to use the helpers:

• internal/agentcfg/elasticsearch.go (6 counters under apm-server.agentcfg.elasticsearch.* plus the cache-entries gauge converted to an Int64UpDownCounter with delta tracking, since the cumulative sum of its deltas equals the current cache size)
• internal/beater/api/intake/handler.go (3 counters under apm-server.processor.stream.*)
• internal/beater/beater.go (apm-server.sampling.transactions_dropped)
• internal/model/modelprocessor/eventcounter.go (apm-server.processor.<event-type>.transformations)
• x-pack/apm-server/sampling/processor.go (6 counters under apm-server.sampling.tail.events.*)
• x-pack/apm-server/sampling/groups.go (apm-server.sampling.tail.dynamic_service_groups)

HTTP middleware and gRPC interceptor

MonitoringMiddleware and metricsInterceptor produce counter names that depend on a request.ResultID chosen per-request. A new request.AllResultIDs slice is iterated at construction so every (prefix, ResultID) pair has its counter created up front. This is the largest single contributor — it covers apm-server.acm.*, apm-server.root.*, apm-server.server.*, apm-server.otlp.{grpc,http}.* error counters that previously only appeared after a matching error response.

The interceptor's and middleware's per-request counter storage was previously a sync.Map (added in #16182 to fix a concurrent-map race). Since every (prefix, ResultID) pair is now created in advance, the maps are never written after construction and sync.Map's concurrent-mutation guarantees aren't needed. Both refactor to a plain map populated at startup and read-only at request time. The map keys avoid the per-call prefix + string(id) string concat (interceptor uses a {prefix, ResultID} struct key; middleware uses two map[ResultID]Counter since the prefix is fixed per-instance), so the hot path becomes allocation-free. A missing entry now logs "BUG: monitoring counter missing from eager registration" instead of silently creating a counter on first use — drift becomes runtime-loud.

The OTLP gRPC dispatch (three method names → three legacy prefixes) is consolidated into a single otlpGRPCLegacyMetricsPrefixes map keyed by gRPC method. The same map drives both per-call dispatch in Interceptor() and the eager-registration loop in Metrics(); adding a new OTLP signal is one map entry.

Single source of truth for the canonical ResultID set

internal/beater/request/result.go previously declared the same set of ResultIDs in three places: a const block, the MapResultIDToStatus map, and a ResultID-slice literal in AllResultIDs. A new ID could land in the const block and any inc() call site without ever being added to AllResultIDs, silently breaking the eager-registration property.

The file now has a single resultIDStatus map keyed by ResultID; init() derives both AllResultIDs and MapResultIDToStatus from it. Adding a ResultID is one new entry; forgetting the entry surfaces immediately because the new ID isn't in AllResultIDs, the eager loop skips it, and the first inc() logs the BUG line which the strict tests fail on.

Two dead constants (IDEventReceivedCount, IDEventDroppedCount — leftovers from the Jaeger removal in #14791) are removed in the same change since they had no remaining callers.

Float64 translator gap (fixes #20996)

apm-server.sampling.tail.storage.disk_usage_threshold_pct is a Float64Gauge introduced in #20464 (present in 9.2.0+) and has never appeared in /stats, even with TBS enabled and the storage manager actively recording values. addAPMServerMetricsToMap in internal/beatcmd/beat.go only handled Sum[int64] / Gauge[int64]; float64 instruments fell through the else. The translator is extended with a getScalarFloat64 path so float-typed apm-server.* instruments are reported via monitoring.ReportFloat. The validation rule for "single dimensionless data point" is hoisted into a generic helper scalarValue[T int64 | float64] shared by both getters. TestMonitoringApmServer is extended with a Float64Gauge to lock the float branch in.

unsupported_dropped guard removal

apm-server.otlp.{grpc,http}.metrics.consumer.unsupported_dropped was guarded by if stats.UnsupportedMetricsDropped > 0 inside the observable-counter callback, so it never appeared until at least one unsupported metric was dropped. The guard is removed; the callback now always observes the (possibly zero) counter.

libbeat.output / libbeat.pipeline lazy emission

The docappender → libbeat translation in addDocappenderLibbeatOutputMetrics and addDocappenderLibbeatPipelineMetrics (internal/beatcmd/beat.go) used to report each libbeat.output.events.* / libbeat.output.write.bytes / libbeat.pipeline.events.total field only inside the matching metric-name switch arm. Before docappender's first publish — i.e., at process start with ES output configured — none of the source metric names had data points, so eight fields were missing from /stats:

libbeat.output.events.{acked,active,batches,failed,toomany,total}
libbeat.output.write.bytes
libbeat.pipeline.events.total

Two earlier attempts to address this were rejected: an always-emit-zero adapter (10a8d1a, reverted in 702bc90) fabricated zeros even when docappender stopped reporting an instrument, masking renames; an upstream Add(0) PR in elastic/go-docappender#317 (now closed) put the eager init at the right architectural layer but couldn't seed the three status-attributed fields without changing docappender's test contract for events.processed.

This PR converts the two NewFunc callbacks to use per-instance persistent state structs (libbeatOutputState, libbeatPipelineState) backed by atomic.Bool / atomic.Int64 fields. The structs zero-initialise at process start, so every field surfaces in /stats from the first scrape, before any indexing has occurred. Each scrape updates only the fields docappender currently exports and reports the full set; a rename or drop of an upstream instrument leaves the field at its last-known value rather than snapping back to a fabricated zero. The if writeBytes > 0 guard is removed in the same change, since writeBytes is now a struct field rather than a per-call local. "type": "elasticsearch" and the field emission are gated on having ever observed the docappender scope, so console-output runs keep an empty libbeat.output sub-tree as before.

Drift in docappender's instrument names is caught by TestLibbeatMetrics, which exercises real traffic and asserts specific cumulative values for every field — a rename fails that test loudly. TestLibbeatMetricsEagerZero locks the eager-zero contract directly: build a Beat with output.elasticsearch, create a docappender with no traffic, assert every field appears at zero in the snapshot.

Walker performance

addAPMServerMetricsToMap walks each metric's dotted path. The original strings.Split allocated a []string per call; replaced with strings.SplitAfterSeq which yields each segment with its trailing . preserved, and strings.CutSuffix(seg, ".") distinguishes intermediate (descend) from leaf (assign) without lookahead state. Benchstat over a 3-metric input:

                            │ split        │ splitafterseq                  │
                            │ sec/op       │ sec/op     vs base             │
AddAPMServerMetricsToMap-16   1.175µ ± 1%   1.078µ ± 2%  -8.22% (p=0.000)
                              2.492Ki ± 0%  2.305Ki ± 0% -7.52% (p=0.000) [B/op]
                              18.00 ± 0%    15.00 ± 0% -16.67% (p=0.000) [allocs]

Interceptor and middleware performance

The sync.Map → plain map swap, combined with the struct/ID-keyed lookup, takes both hot paths to zero allocations:

                            │ syncmap       │ plain map                          │
                            │ sec/op        │ sec/op       vs base               │
Interceptor-16                738.1n ± 0%    405.2n ± 2%   -45.11% (p=0.000)
                              304.0 ± 0%     0.0 ± 0%     -100.00% (p=0.000) [B/op]
                              9.000 ± 0%     0.000 ± 0%   -100.00% (p=0.000) [allocs]

MonitoringMiddleware-16       906.6n ± 2%    432.2n ± 1%   -52.32% (p=0.000)
                              320.0 ± 0%     0.0 ± 0%     -100.00% (p=0.000) [B/op]
                              11.00 ± 0%     0.00 ± 0%    -100.00% (p=0.000) [allocs]

Tests

Strict-match assertions stay strict. monitoringtest/opentelemetry.go is byte-identical to main. TestMonitoringHandler (middleware) and TestMetrics (interceptor) build the full expected map in-place — prefix × request.AllResultIDs → int64(0) plus the per-test non-zero overrides — before calling the existing ExpectOtelMetrics. Tests with an unbounded eager-init footprint (mux api tests, otlp http/grpc tests, eventcounter test) stay on ExpectContainOtelMetrics (subset semantics), which is what they used before #15094.

TestMonitoringApmServer extended with a Float64Gauge so the getScalarFloat64 path is verified.

Two benchmarks added so future regressions are caught:

• BenchmarkAddAPMServerMetricsToMap in internal/beatcmd/beat_test.go.
• BenchmarkInterceptor in internal/beater/interceptors/metrics_test.go.
• BenchmarkMonitoringMiddleware in internal/beater/middleware/monitoring_middleware_test.go.

How to test these changes

# Build and run with no traffic.
go build -o /tmp/apm-server ./x-pack/apm-server
/tmp/apm-server -e -c apm-server.yml &
sleep 4
curl -s http://127.0.0.1:5066/stats | jq '."apm-server"'

Before this change, apm-server.* shows only metrics that have actually been incremented (typically just the few sampling.tail.storage.* int64 gauges if TBS is enabled, plus whatever traffic has triggered). After this change, the full enumerated set (acm, agentcfg, otlp, processor, root, sampling, server) is present at zero — and disk_usage_threshold_pct shows up under sampling.tail.storage for the first time.

Unit tests:

go test -race ./internal/... ./x-pack/...

Related issues

Unblocks the mapping-regeneration tooling in #13638 and the periodic upstream-mapping-sync work in #15533 and #13475.

Part of #20991.
Fixes #20996 as a side effect (float64 translator gap).

[github-actions]

🤖 GitHub comments

Just comment with:

• run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

[mergify]

This pull request does not have a backport label. Could you fix it @carsonip? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-8.19 is the label to automatically backport to the 8.19 branch.
• backport-9./d is the label to automatically backport to the 9./d branch. /d is the digit.
• backport-active-all is the label that automatically backports to all active branches.
• backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

[carsonip]

i'll review the code again tomorrow, but it looks right to me at a high level. At the very least it is fixing a bug.

[ericywl]

Looks good overall, just minor nits.

[carsonip]

[to reviewer] new test that catches missing float reporting

[carsonip]

[to reviewer] Single source of truth for method name to metric prefix. A slight overhead in map lookup compared to switch case string comparison but negligible in the grand scheme of things

[carsonip]

[to reviewer] sorry for the major refactor here. sync.Map was added in #16182 to workaround a concurrent map access. Now that we change the paradigm to always create up front, we can cut the sync.Map overhead and benefit from metric appearing in /stats from registration time

with this change, see benchstat

                 │ /tmp/syncmap.txt │          /tmp/plainmap.txt          │                                                                                                                                           
                 │      sec/op      │   sec/op     vs base                │
  Interceptor-16        738.1n ± 0%   405.2n ± 2%  -45.11% (p=0.000 n=10)                                                                                                                                             
                                                                     
                 │ /tmp/syncmap.txt │         /tmp/plainmap.txt         │                                                                                                                                             
                 │       B/op       │   B/op    vs base                 │
  Interceptor-16         304.0 ± 0%   0.0 ± 0%  -100.00% (p=0.000 n=10)                                                                                                                                               
                                                                                                                                                                                                                      
                 │ /tmp/syncmap.txt │          /tmp/plainmap.txt          │
                 │    allocs/op     │ allocs/op   vs base                 │                                                                                                                                           
  Interceptor-16         9.000 ± 0%   0.000 ± 0%  -100.00% (p=0.000 n=10)    

[carsonip]

[to reviewer] this cuts the string prefix and name concat alloc per inc call

[carsonip]

[to reviewer] strings.Split -> strings.SplitAfterSeq perf gain

                              │   split.txt    │       splitafterseq.txt            │
                              │     sec/op     │   sec/op     vs base               │
  AddAPMServerMetricsToMap-16    1.175µ ± 1%    1.078µ ± 2%  -8.22%  (p=0.000 n=10)

                              │   split.txt    │       splitafterseq.txt            │
                              │      B/op      │     B/op      vs base              │
  AddAPMServerMetricsToMap-16    2.492Ki ± 0%   2.305Ki ± 0%  -7.52%  (p=0.000 n=10)

                              │   split.txt    │       splitafterseq.txt            │
                              │   allocs/op    │ allocs/op   vs base                │
  AddAPMServerMetricsToMap-16     18.00 ± 0%    15.00 ± 0%  -16.67%  (p=0.000 n=10)

[carsonip]

[to reviewer] this gives us

                            │ syncmap         │ plain map                          │                                                                                                                                  
                            │ sec/op          │ sec/op     vs base                 │                                                                                                                                  
  MonitoringMiddleware-16     906.6n ± 2%      432.2n ± 1%  -52.32% (p=0.000 n=10)                                                                                                                                    
                              320.0 ± 0%       0.0 ± 0%    -100.00% (p=0.000 n=10) [B/op]
                              11.00 ± 0%       0.00 ± 0%   -100.00% (p=0.000 n=10) [allocs]          

[carsonip]

[to reviewer] according to Claude: IDEventReceivedCount / IDEventDroppedCount removed — dead consts since the Jaeger removal in #14791, no callers

[carsonip]

[to reviewer] sorry for major refactor here again. This is to allow eager registration to be aware of all the result IDs and make the code drift-proof.

[ericywl]

Would it be a good idea to have some indication that these metrics are being dropped?

[carsonip]

TODO

• double check if addDocappenderLibbeatOutputMetrics and addDocappenderLibbeatPipelineMetrics are working

[elasticmachine]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: 2578b13

Failed CI Steps

• Package x86_64

History

• 💚 Build #10484 succeeded 722c2a6
• 💚 Build #10481 succeeded 10a8d1a
• 💚 Build #10465 succeeded 76c97c0
• 💚 Build #10464 succeeded d70a918
• 💚 Build #10460 succeeded a18311d
• 💚 Build #10451 succeeded 06f6902