Skip to content

client: honor envoy.stat_sinks.* in --stats-sinks - #1649

Closed
oschaaf wants to merge 1 commit into
envoyproxy:mainfrom
We-Amp:stats-sink-registry-fix
Closed

oschaaf wants to merge 1 commit into
envoyproxy:mainfrom
We-Amp:stats-sink-registry-fix

Conversation

@oschaaf

@oschaaf oschaaf commented Nov 3, 2026

Copy link
Copy Markdown
Member

Fixes #1607

What

ProcessImpl::setupStatsSinks looked sinks up only in Nighthawk's own
NighthawkStatsSinkFactory registry. A bootstrap sink config naming a
linked Envoy stats sink (e.g. envoy.stat_sinks.statsd) therefore failed
with "Didn't find a registered implementation", even though the
--stats-sinks help text advertised exactly such configurations, so
users could not flush Nighthawk's benchmark metrics to any
Envoy-provided stats sink.

setupStatsSinks now consults Envoy's
Server::Configuration::StatsSinkFactory registry first — by typed
config type URL, the same lookup Envoy's own server performs — and falls
back to Nighthawk's registry when no Envoy factory matches. Configs
matching neither registry keep failing fast with an error naming the
sink. Errors returned by an Envoy sink factory's createStatsSink
propagate as an EnvoyException; they are never swallowed.

This change also links the envoy.stat_sinks.statsd extension via
extensions_build_config.bzl and points the documented example at the
cluster-free UDP address form.

Test evidence

New process_test cases (v4 and v6):

  • EnvoyStatsSinkHonored: a --stats-sinks config naming
    envoy.stat_sinks.statsd with a UDP address on a loopback listener;
    the run flushes datagrams containing benchmark. counter names to
    that listener. Before this change the run aborted in
    getAndCheckFactory<NighthawkStatsSinkFactory> with
    Didn't find a registered implementation for 'envoy.stat_sinks.statsd' with type URL: 'envoy.config.metrics.v3.StatsdSink'.
  • UnknownSinkNameFailsFast: sink name envoy.stat_sinks.nosuchsink
    fails the run with an error naming the sink.

Full process_test and service_test suites pass locally on macOS
except the pre-existing TestWithEncapsulation failures (a
setsockopt: Protocol not available platform limitation, reproducible
on main without this change).

DCO signed; CI will confirm on Linux.

Signed-off-by: Otto van der Schaaf oschaaf@we-amp.com

ProcessImpl::setupStatsSinks only consulted Nighthawk's own stats sink
factory registry, so sink configurations naming a linked Envoy stats
sink (e.g. envoy.stat_sinks.statsd) failed with "Didn't find a
registered implementation", even though the CLI documentation advertised
exactly such configurations. Users could not flush Nighthawk metrics to
any Envoy-provided stats sink (envoyproxy#1607).

Consult Envoy's Server::Configuration::StatsSinkFactory registry first,
by typed config type URL, which is how Envoy's own server resolves
stats sinks; fall back to Nighthawk's registry when no Envoy factory
matches. Configurations matching neither registry keep failing fast
with an error naming the sink. Errors returned by an Envoy sink
factory's createStatsSink propagate as an EnvoyException and are never
swallowed.

Link the envoy.stat_sinks.statsd extension via
extensions_build_config.bzl, and point the documented example at the
cluster-free UDP address form.

The statsd sink emits raw histogram samples when the benchmark client
records them; percentile summaries are not part of its statsd payload.

Fixes envoyproxy#1607

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
@oschaaf

oschaaf commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

Closing for now - we'll re-submit from our fork when the timing is right. Thanks!

@oschaaf oschaaf closed this Nov 3, 2026
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.

--stats-sinks aborts at startup: no stats sink implementation is registered

1 participant