Skip to content

Conversation

@szybia
Copy link
Contributor

@szybia szybia commented Sep 26, 2025

  • Should've just done in previous PR
  • Required more code changes when we were still adding file/native stats, but after we dropped them only requires this change
  • small human quality-of-life improvements, means all properties under bit_set_cache will be ordered across nodes (check pictures)
  • nothing changes in _xpack/usage

@szybia szybia added >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC labels Sep 26, 2025
@szybia szybia marked this pull request as ready for review September 26, 2025 17:12
@elasticsearchmachine elasticsearchmachine added the Team:Security Meta label for security team label Sep 26, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

@joegallo joegallo changed the title Deterministic response order of _security/stats/ Deterministic response order of _security/stats Sep 26, 2025
Copy link
Contributor

@joegallo joegallo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a commit that accomplishes the same thing a different way (I think).

What kind of test should we add that shows that this PR had the desired effect, and that it continues to after my changes? That'll also be a good guard against somebody breaking things accidentally in six months or three years since we're relying on the types of the things to result in the correct ordering.

* upstream/main: (22 commits)
  Fix InternalCategorizationAggregationTests.testReduceRandom (elastic#135533)
  [DOCS] GeoIP processor: add clarification about using a reverse proxy endpoint (elastic#135534)
  Move `ProjectRoutingInfo` and related classes (elastic#135586)
  Refactor IndexAbstractionResolver (elastic#135587)
  Simplify returnLocalAll handling in ES|QL (elastic#135353)
  Reapply "Add an option to return early from an allocate call"  (elastic#135589)
  Mute org.elasticsearch.xpack.esql.qa.single_node.GenerativeIT test elastic#134407
  Mute org.elasticsearch.xpack.esql.heap_attack.HeapAttackIT testAggTooManyMvLongs elastic#135585
  Mute org.elasticsearch.multiproject.test.XpackWithMultipleProjectsClientYamlTestSuiteIT test {yaml=esql/60_usage/Basic ESQL usage output (telemetry) snapshot version} elastic#135579
  Mute org.elasticsearch.search.ccs.KnnVectorQueryBuilderCrossClusterSearchIT testKnnQueryWithCcsMinimizeRoundTripsFalse elastic#135573
  Mute org.elasticsearch.xpack.esql.inference.textembedding.TextEmbeddingOperatorTests testSimpleCircuitBreaking elastic#135569
  Add telemetry for `TS` command (elastic#135471)
  Mute org.elasticsearch.cluster.routing.allocation.decider.RestoreInProgressAllocationDeciderTests testCanAllocatePrimaryExistingInRestoreInProgress elastic#135566
  allocation: clarify RestoreInProgressAllocationDecider failure message (elastic#132307)
  [ES|QL] Register AggregateMetricDoubleLiteral (elastic#135054)
  Validate Logstash pipeline ID when creating. (elastic#135378)
  Migrate transport versions 8841050 through 8841041 (elastic#135555)
  Mute org.elasticsearch.search.ccs.SparseVectorQueryBuilderCrossClusterSearchIT testSparseVectorQueryWithCcsMinimizeRoundTripsFalse elastic#135559
  Mute org.elasticsearch.action.admin.cluster.stats.SearchUsageStatsTests testToXContent elastic#135558
  Testing indices query cache memory stats (elastic#135298)
  ...
Copy link
Contributor Author

@szybia szybia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added some tests which should gate non-orderness in the future

Comment on lines +674 to +676
final Map<String, Object> usageStats = Maps.newLinkedHashMapWithExpectedSize(1);
usageStats.put("dls", Map.of("bit_set_cache", dlsBitsetCache.usageStats()));
return usageStats; // return LinkedHashMap for deterministic order in transport
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really needed right now since only one key so order is preserved

but added to leave a stronger note for anyone touching this code in the future to consider ordering

@szybia szybia requested a review from joegallo September 29, 2025 14:08
Copy link
Contributor

@joegallo joegallo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

@szybia szybia merged commit d2da68e into elastic:main Sep 29, 2025
40 checks passed
@szybia szybia deleted the dls-stats-ordering branch September 29, 2025 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>non-issue :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants