Add DLS stats to _security/stats

[szybia]

• Add roles with DLS stats to security stats, using the same code and format as /_xpack/usage:

{
  "nodes": {
    "NzgeBRW-RyOeVOwGogV3lQ": {
      "roles": {
        "dls": {
          "bit_set_cache": {
            "count": 0,
            "memory": "0b",
            "memory_in_bytes": 0,
            "hits": 0,
            "misses": 0,
            "evictions": 0,
            "hits_time_in_millis": 0,
            "misses_time_in_millis": 0
          }
        }
      }
    }
  }
}

This is a follow up to #134835, which added the (empty) _security/stats API itself.

[elasticsearchmachine]

Hi @szybia, I've created a changelog YAML for you.

[elasticsearchmachine]

Pinging @elastic/es-security (Team:Security)

[tvernum]

This will block a transport thread and NativeRolesStore.usageStats is potentially expensive.

Rather than using a future, we should override nodeOperationAsync instead and make use of the listener.

[szybia]

hmm, unless i'm missing something, from my reading of the code and testing, nodeOperation doesn't run on transport threads, but the threadpool that's defined at the top (management thread pool):

    @Override
    protected GetSecurityStatsNodeResponse nodeOperation(final GetSecurityStatsNodeRequest request, final Task task) {
        if (Transports.isTransportThread(Thread.currentThread())) {
            LOG.info("szy: transport thread");
            throw new IllegalArgumentException("transport thread");
        } else {
            LOG.info("szy: not transport thread");
        }

[2025-09-24T13:48:33,922][INFO ][o.e.x.s.a.s.TransportSecurityStatsAction] [runTask-0] szy: not transport thread: elasticsearch[runTask-0][management][T#1]
[2025-09-24T13:48:33,925][INFO ][o.e.x.s.a.s.TransportSecurityStatsAction] [runTask-1] szy: not transport thread: elasticsearch[runTask-1][management][T#2]
[2025-09-24T13:48:33,924][INFO ][o.e.x.s.a.s.TransportSecurityStatsAction] [runTask-2] szy: not transport thread: elasticsearch[runTask-2][management][T#2]

i also think this is backed up by the fact that there are no overrides in ES of nodeOperationAsync, everything overrides nodeOperation, and assuming some of those do heavy things, and if they were in transport this would've been spotted

based on javadoc, my final understanding of nodeOperationAsync is that it's just to get access to the listener, maybe leading to cleaner code, but i've tried it out and don't see the hype over what i currently have:

    @Override
    protected GetSecurityStatsNodeResponse nodeOperation(final GetSecurityStatsNodeRequest request, final Task task) {
        return null; // never called because we override nodeOperationAsync
    }

    @Override
    protected void nodeOperationAsync(final GetSecurityStatsNodeRequest request,
                                      final Task task,
                                      final ActionListener<GetSecurityStatsNodeResponse> listener) {
        if (rolesStore == null) {
            listener.onResponse(new GetSecurityStatsNodeResponse(clusterService.localNode(), null));
        } else {
            rolesStore.usageStats(
                ActionListener.wrap(
                    stats -> listener.onResponse(new GetSecurityStatsNodeResponse(clusterService.localNode(), stats)),
                    listener::onFailure
                )
            );
        }
    }

thoughts?

[tvernum]

You're right it will be a management thread not a transport thread, but we still end up blocking that thread while doing the work on another thread pool.

That risks contention on the management pool, potentially even with deadlocks if the management pool is completely full with threads that are waiting for work from other threads.

my final understanding of nodeOperationAsync is that it's just to get access to the listener, maybe leading to cleaner code

Not for cleaner code, but so you can perform asynchronous operations. You need the listener because you can't produce the required return value on the current thread, you have to return it asynchronously and for that you need the listener (because blocking on a future isn't something we want to do).

To my knowledge there are no other TransportNodesAction that need to do asynchronous work in the node operation which is why there are no implementations of nodeOperationAsync

However, that opens the question of whether we actually should be doing this work on every node. I hadn't really thought about that when I agreed with using the existing usage stats.
The file and dls stats make sense in a per-node response, but the native stats are inherently cluster-wide stats - they're returning information from the security index - and it doesn't make sense to calculate and return the same stats on every node in the cluster.

We should probably rethink that.

Our options (long term, not necessarily to be handled this PR):

1. Accept that it's OK for every node to calculate the same stats, and stick with what we have. There are two issues here:
   1. It's potentially a performance issue (not necessarily for native role stats, but if we continue down this path it could be)
   2. There's the chance for timing issues and having nodes return different stats. That's pretty confusing - what does it mean for 1 node to have 134 native roles and another node to have 135 native rules?
2. Calculate them once, but slot that identical value into the JSON response for each node, that overcomes the two issues above, but at the cost of:
   1. It's just weird to pretend these are node stats if they're cluster stats
   2. Now we have the problem of hiding (lying) if 1 node actually has an incorrect view of the stats (e.g. a caching bug). The response would tell you that the node has the same view of the data as all other nodes, but that's not actually the case.
3. Move some stats to the top level of the JSON (outside of "nodes"). That's harder for client to consume, but more accurate (we could calculate those stats on the coordinating node for the request).

I think option 3 is best, but maybe Data Management has a view on how stats APIs ought to work in these sorts of cases.

[szybia]

very fair points

after having a think, given i'm trying to ship DLS stats in 9.2 (FF 30th Sep)

i think best plan of attack is to just ship the DLS stats which don't seem to be contentious

and then have a think about structure for everything else

i've updated the code

• it should be extensible to all your ideas above
• follows _xpack/usage pattern
• shouldn't be blocking now so i've left it as nodeOperation

[joegallo]

re: long term options: I had a quick sanity check conversation with @dakrone, and after that conversation I feel safe saying that for the moment that list of options looks good enough that we aren't trapped in some bad state if we release this with only the dls cache stats now and then worry about how to handle (for example) the native stats later.

[tvernum]

LGTM.

Thanks for iterating - sometimes I wish that the thing that look easy might actually turn out to be easy.