Stats Accesslogger: support the scope singleton for limiting cardinality by TAOXUY · Pull Request #43625 · envoyproxy/envoy

TAOXUY · 2026-02-24T20:14:07Z

Commit Message:

support the scope singleton which can be dynamic configured through new resource
make stats accesslogger support the scope singleton so that multiple accesslogger can share the same scope globally for limiting cardinality

Risk Level: lo
Testing: added
Docs Changes: updated
Release Notes: updated
Platform Specific Features: no

repokitteh-read-only · 2026-02-24T20:14:18Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @wbpcode
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #43625 was opened by TAOXUY.

see: more, trace.

wbpcode

Thanks for the contribution.

I always want to make the API simple and also easy to access for most general users. So, I won't prefer the way that need custom support from control plane (and I think this will not be supported in control plane, like Istio/EG, in a long time, that's say, it cannot make sense for most general users in long time)

So, if you like this feature, you can add a stats_prefix in the message Scope and use it in the stats logger directly, and document it clearly it will created a shared scope across multiple listeners and the hash or the stat_prefix of message Scope will be the unique identify.

TAOXUY · 2026-02-25T02:54:44Z

Thanks for the contribution.

I always want to make the API simple and also easy to access for most general users. So, I won't prefer the way that need custom support from control plane (and I think this will not be supported in control plane, like Istio/EG, in a long time, that's say, it cannot make sense for most general users in long time)

So, if you like this feature, you can add a stats_prefix in the message Scope and use it in the stats logger directly, and document it clearly it will created a shared scope across multiple listeners and the hash or the stat_prefix of message Scope will be the unique identify.

Hi @wbpcode

Thanks for the review and the feedback! I completely understand your point about keeping the API simple and accessible for general users, as well as the concerns regarding the timeline for control plane (like Istio/EG) support.

However, we actually had an extensive discussion regarding this exact design choice in PR #40992. During that PR, we made decision on moving forward with the current design, namely adding a new resource for the global singleton registration/management. We agreed that directly configuring the singleton redundantly isn't ideal and as for istio, we can make changes to support those singleton resources.

Given that this was previously thoroughly discussed and agreed upon there, I'd prefer to stick to the aligned approach unless there are new critical concerns that weren't covered. Let me know what you think!

wbpcode · 2026-02-25T04:21:59Z

I I compromised at #40992 because you have paid too much time at that solution. TBH, until today, I still didn't see how can normal users benefit from #40992.

We agreed that directly configuring the singleton redundantly isn't ideal

I agree it's not ideal but useful, simple and practical. And it actually be widely used in our code base. Even the dynamic config provider self are shared across listeners by this way. The tracer instances also use this way to be shared across listeners. We only need to ensure this way is not abused.

In my personal opinion, practical and easy for users to use is always take the highest priority. We are design this feature as a tool to resolve problems rather than to design an artwork.

Sure, it's only my personal opinion and preference, if we can show actual progress about this, as for istio, we can make changes to support those singleton resources, it's fine to accept your current API.

TAOXUY · 2026-02-25T17:52:44Z

I I compromised at #40992 because you have paid too much time at that solution. TBH, until today, I still didn't see how can normal users benefit from #40992.

We agreed that directly configuring the singleton redundantly isn't ideal

I agree it's not ideal but useful, simple and practical. And it actually be widely used in our code base. Even the dynamic config provider self are shared across listeners by this way. The tracer instances also use this way to be shared across listeners. We only need to ensure this way is not abused.

In my personal opinion, practical and easy for users to use is always take the highest priority. We are design this feature as a tool to resolve problems rather than to design an artwork.

Sure, it's only my personal opinion and preference, if we can show actual progress about this, as for istio, we can make changes to support those singleton resources, it's fine to accept your current API.

Hi @wbpcode

I wanted to clarify that the current PR actually supports a file-based subscription approach, which provides a practical path forward for OSS control planes like Istio or Envoy Gateway. Those control planes can use file-based(also dynamic configuring) xDS resources for these singletons today and I’ve even included a test case for this file-based approach in the integration tests to demonstrate how it works. The OSS control planes can extend with ADS-based configuring approach when needed.

Let me know if this helps alleviate your concerns, or if you still feel a different approach would be better.

CC @kyessenov @markdroth

TAOXUY · 2026-02-25T18:46:29Z

/retest

TAOXUY · 2026-02-26T05:24:51Z

/retest

wbpcode · 2026-02-28T01:15:49Z

Sorry, Tao. I still don't think it's a good idea before I see actual progress in control plane. File based xDS sure works good, but in actual practices, we may cannot mount the file into Envoy pod. That's say, the users need to pre-mounted the configuration file and cannot actually configure it dynamically.

In another word, I didn't see the advantage except bring complexity and users' chaos.

But I am not tough in this type technical debating because IMO, any solution can resolve problem is good solution. I try to keep my mind open.

Let's ping @ggreenway for some input or suggestions. @ggreenway is initial designer of this extension and should have a clear understanding about the extension and how it should work for users.

wbpcode · 2026-02-28T01:19:51Z

Another idea in my mind is that we should make loggers shared across all listeners based on singleton, like what we did to tracers. Then loggers and tracers will works in same way

TAOXUY · 2026-02-28T04:55:20Z

Another idea in my mind is that we should make loggers shared across all listeners based on singleton, like what we did to tracers. Then loggers and tracers will works in same way

Thx for suggestions. Users can have different stats accesslogger configs for different listeners so this isn't ideal.

TAOXUY · 2026-03-04T05:41:47Z

@wbpcode PTAL on API

TAOXUY · 2026-03-04T15:40:28Z

@ggreenway ready to review

TAOXUY · 2026-03-04T17:35:19Z

/retest

TAOXUY · 2026-03-10T17:08:01Z

@ggreenway can you take another look on the implementation(the API shouldn't change too much)?

@wbpcode can you help review the API? thx!

paul-r-gall · 2026-03-16T14:02:54Z

@TAOXUY please merge main.

TAOXUY · 2026-03-16T16:16:49Z

@TAOXUY please merge main.

DONE

TAOXUY · 2026-03-16T21:29:13Z

/retest

TAOXUY · 2026-03-16T21:47:24Z

/retest

wbpcode

LGTM overall with only three minor comments. Thanks so much for your update. 🙇

wbpcode

LGTM with nit reply

Signed-off-by: Xuyang Tao <taoxuy@google.com>

TAOXUY · 2026-04-06T18:27:55Z

/retest

TAOXUY · 2026-04-06T18:59:35Z

/retest

TAOXUY · 2026-04-08T15:04:51Z

@ggreenway could you take another look? Thx!

ggreenway

/wait

ggreenway · 2026-04-08T19:43:31Z

-  string stat_prefix = 1 [(validate.rules).string = {min_len: 1}];
+  // Deprecated: please use ``stats_scope.prefix`` instead.
+  // It will override ``stats_scope.prefix`` if non-empty.
+  string stat_prefix = 1;


As noted by @wbpcode you need to add an annotation to specify that it is deprecated

ggreenway · 2026-04-08T19:44:38Z

+message ScopeConfig {
+  // Max number of counters allowed in this scope. 0 means no limit.
+  // When the limit is reached, new counters will not be created.
+  uint32 max_counters = 1;


But why not allow limiting to zero? What if I want to have 0 histograms to reduce memory use? That may not make sense for the stats access logger, but you're defining a common message, so this needs to be thought through in the general case.

ggreenway · 2026-04-08T19:45:34Z

+  void setCleanupCallback(std::function<void()> callback) override {
+    cleanup_callback_ = std::move(callback);
+  }
+
+  ~IsolatedScopeImpl() override {
+    if (cleanup_callback_) {
+      cleanup_callback_();
+    }
+    prefix_.free(store_.symbolTable());
+  }


destructor should be ordered immediately after constructors.

ggreenway · 2026-04-08T19:46:02Z


 ScopeSharedPtr IsolatedStoreImpl::makeScope(StatName name, StatsMatcherSharedPtr matcher) {
-  return std::make_shared<IsolatedScopeImpl>(name, *this, std::move(matcher));
+  return ScopeSharedPtr(new IsolatedScopeImpl(name, *this, std::move(matcher)));


Why did you change this? Should this be reverted?

ggreenway · 2026-04-08T19:49:48Z

+// [#protodoc-title: Scope]
+
+// Configuration for limiting the number of stats in a scope.
+message ScopeConfig {


Why is this a separate message, and not part of Scope? I think all the fields of both seem logically like part of ScopeConfig

ggreenway · 2026-04-08T19:51:08Z

+
+  // The scope name, required for shared scopes.
+  // If empty, no sharing will happen.
+  string name = 4;


I don't think it makes sense to have both name and enable_sharing. I think just sharing_name or something would be enough, and empty means sharing is not enabled. That way there's no way to create an invalid config of enable_sharing is true and name is empty

ggreenway · 2026-04-08T19:52:12Z

+          [] { return std::make_shared<ScopeProviderSingleton>(); });
+
+  size_t hash = MessageUtil::hash(config);
+  absl::flat_hash_map<size_t, std::weak_ptr<Stats::Scope>>::iterator it =


Suggested change

absl::flat_hash_map<size_t, std::weak_ptr<Stats::Scope>>::iterator it =

auto it =

ggreenway · 2026-04-08T19:54:09Z


 ScopeSharedPtr TestStore::makeScope(StatName name, StatsMatcherSharedPtr matcher) {
-  return std::make_shared<TestScope>(name, *this, std::move(matcher));
+  return ScopeSharedPtr(new TestScope(name, *this, std::move(matcher)));


This didn't get reverted

Signed-off-by: Xuyang Tao <taoxuy@google.com>

TAOXUY · 2026-04-09T02:03:30Z

/retest

TAOXUY · 2026-04-09T14:56:45Z

/retest

Signed-off-by: Xuyang Tao <taoxuy@google.com>

TAOXUY · 2026-04-13T17:28:52Z

@ggreenway PTAL

phlax · 2026-04-16T13:51:25Z

i think this is waiting on you @ggreenway

wbpcode

/lgtm api

TAOXUY · 2026-04-21T15:25:15Z

@ggreenway PTAL

ggreenway

This mostly LGTM, just a few small cleanup items

/wait

ggreenway · 2026-04-24T23:38:17Z


 ScopeSharedPtr TestStore::makeScope(StatName name, StatsMatcherSharedPtr matcher) {
-  return std::make_shared<TestScope>(name, *this, std::move(matcher));
+  return ScopeSharedPtr(new TestScope(name, *this, std::move(matcher)));


This didn't get reverted

ggreenway · 2026-04-24T23:43:23Z

+      "Either 'stat_prefix' or 'stats_scope' must be configured, but not both.");
+}
+
+#ifndef ENVOY_DISABLE_DEPRECATED_FEATURES


Use DEPRECATED_FEATURE_TEST instead of ifdef

ggreenway · 2026-04-24T23:45:17Z

    - source/common/formatter/stream_info_formatter.cc
    - source/common/formatter/substitution_formatter.cc
    - source/common/stats/tag_extractor_impl.cc
+    - source/common/stats/scope_provider_singleton.cc


this path doesn't exist; revert

Signed-off-by: Xuyang Tao <taoxuy@google.com>

Co-authored-by: Greg Greenway <ggreenway@apple.com> Signed-off-by: Xuyang Tao <taoxuy@google.com>

TAOXUY requested review from ggreenway, kyessenov and wbpcode as code owners February 24, 2026 20:14

repokitteh-read-only Bot added the api label Feb 24, 2026

repokitteh-read-only Bot assigned wbpcode Feb 24, 2026

wbpcode requested changes Feb 25, 2026

View reviewed changes

wbpcode assigned ggreenway Feb 28, 2026

ggreenway requested changes Mar 1, 2026

View reviewed changes

Comment thread api/envoy/config/metrics/v3/scope.proto Outdated

Comment thread api/envoy/type/v3/scope.proto Outdated

Comment thread source/common/stats/scope_provider_singleton.h Outdated

ggreenway reviewed Mar 4, 2026

View reviewed changes

Comment thread api/envoy/type/v3/scope.proto

ggreenway reviewed Mar 4, 2026

View reviewed changes

Comment thread api/envoy/type/v3/scope.proto Outdated

wbpcode reviewed Mar 17, 2026

View reviewed changes

Comment thread api/envoy/type/v3/scope.proto Outdated

Comment thread source/extensions/access_loggers/stats/scope_provider_singleton.cc Outdated

Comment thread source/extensions/access_loggers/stats/stats.cc Outdated

wbpcode previously approved these changes Mar 17, 2026

View reviewed changes

repokitteh-read-only Bot removed the api label Mar 17, 2026

TAOXUY added 2 commits April 1, 2026 03:46

iterate

87ccd20

Signed-off-by: Xuyang Tao <taoxuy@google.com>

iterate

b96da1f

Signed-off-by: Xuyang Tao <taoxuy@google.com>

ggreenway mentioned this pull request Apr 1, 2026

stats: remove deprecated ScopeStatsLimitSettings #44211

Closed

TAOXUY added 2 commits April 6, 2026 17:00

Merge main into dynamicScope and resolve conflicts

cef145d

Signed-off-by: Xuyang Tao <taoxuy@google.com>

Rename evictable to enable_eviction in scope.proto

435e925

Signed-off-by: Xuyang Tao <taoxuy@google.com>

ggreenway requested changes Apr 8, 2026

View reviewed changes

repokitteh-read-only Bot added the waiting label Apr 8, 2026

iterate

a728f36

Signed-off-by: Xuyang Tao <taoxuy@google.com>

repokitteh-read-only Bot removed the waiting label Apr 8, 2026

TAOXUY added 2 commits April 8, 2026 22:12

iterate

141f968

Signed-off-by: Xuyang Tao <taoxuy@google.com>

iterate

cad18eb

Signed-off-by: Xuyang Tao <taoxuy@google.com>

TAOXUY added 2 commits April 9, 2026 16:12

fix test

82b2449

Signed-off-by: Xuyang Tao <taoxuy@google.com>

fix test

7441db2

Signed-off-by: Xuyang Tao <taoxuy@google.com>

wbpcode reviewed Apr 20, 2026

View reviewed changes

repokitteh-read-only Bot removed the api label Apr 20, 2026

ggreenway requested changes Apr 24, 2026

View reviewed changes

repokitteh-read-only Bot added the waiting label Apr 24, 2026

TAOXUY added 2 commits April 25, 2026 21:48

Merge branch 'main' into dynamicScope

fc1d015

Signed-off-by: Xuyang Tao <taoxuy@google.com>

fix

ef0e000

Signed-off-by: Xuyang Tao <taoxuy@google.com>

repokitteh-read-only Bot removed the waiting label Apr 25, 2026

Update api/envoy/type/v3/scope.proto

38d1b55

Co-authored-by: Greg Greenway <ggreenway@apple.com> Signed-off-by: Xuyang Tao <taoxuy@google.com>

repokitteh-read-only Bot added the api label Apr 25, 2026

	absl::flat_hash_map<size_t, std::weak_ptr<Stats::Scope>>::iterator it =
	auto it =

Conversation

TAOXUY commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only Bot commented Feb 24, 2026

Uh oh!

wbpcode left a comment • 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!

TAOXUY commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wbpcode commented Feb 25, 2026

Uh oh!

TAOXUY commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TAOXUY commented Feb 25, 2026

Uh oh!

TAOXUY commented Feb 26, 2026

Uh oh!

wbpcode commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wbpcode commented Feb 28, 2026

Uh oh!

TAOXUY commented Feb 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TAOXUY commented Mar 4, 2026

Uh oh!

TAOXUY commented Mar 4, 2026

Uh oh!

TAOXUY commented Mar 4, 2026

Uh oh!

Uh oh!

Uh oh!

TAOXUY commented Mar 10, 2026

Uh oh!

paul-r-gall commented Mar 16, 2026

Uh oh!

TAOXUY commented Mar 16, 2026

Uh oh!

TAOXUY commented Mar 16, 2026

Uh oh!

TAOXUY commented Mar 16, 2026

Uh oh!

wbpcode left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wbpcode left a comment

Choose a reason for hiding this comment

Uh oh!

TAOXUY commented Apr 6, 2026

Uh oh!

TAOXUY commented Apr 6, 2026

Uh oh!

TAOXUY commented Apr 8, 2026

Uh oh!

ggreenway left a comment

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!

Choose a reason for hiding this comment

Uh oh!

TAOXUY commented Feb 24, 2026 •

edited

Loading

wbpcode left a comment •

edited

Loading

TAOXUY commented Feb 25, 2026 •

edited

Loading

TAOXUY commented Feb 25, 2026 •

edited

Loading

wbpcode commented Feb 28, 2026 •

edited

Loading