Security: Add new endpoint and DLS properties#5374
Conversation
|
Following you can find the validation changes against the target branch for the APIs.
You can validate these APIs yourself by using the |
|
let me fix validate failure ^ in next PR to keep PRs scoped to one purpose, i have it passing locally |
|
Thank you for this!
I'm glad that you got |
|
nope change is quite small i can put it into this PR if you prefer, i was thinking cleaner to have two PRs change: diff --git a/specification/xpack/usage/types.ts b/specification/xpack/usage/types.ts
index 085fff1ed..e5ad6ac55 100644
--- a/specification/xpack/usage/types.ts
+++ b/specification/xpack/usage/types.ts
@@ -320,9 +320,42 @@ export class SecurityRolesDls {
}
export class SecurityRolesDlsBitSetCache {
+ /** Number of entries in the cache. */
count: integer
+ /** Human-readable amount of memory taken up by the cache. */
memory?: ByteSize
+ /** Memory taken up by the cache in bytes. */
memory_in_bytes: ulong
+ /**
+ * Total number of cache hits.
+ * @availability stack since=9.2.0
+ * @availability serverless
+ */
+ hits: ulong
+ /**
+ * Total number of cache misses.
+ * @availability stack since=9.2.0
+ * @availability serverless
+ */
+ misses: ulong
+ /**
+ * Total number of cache evictions.
+ * @availability stack since=9.2.0
+ * @availability serverless
+ */
+ evictions: ulong
+ /**
+ * Total combined time spent in cache for hits in milliseconds.
+ * @availability stack since=9.2.0
+ * @availability serverless
+ */
+ hits_time_in_millis: ulong
+ /**
+ * Total combined time spent in cache for misses in milliseconds.
+ * @availability stack since=9.2.0
+ * @availability serverless
+ */
+ misses_time_in_millis: ulong
}
|
|
Including this change is fine, thank you! It's quite valuable to see the green report. Plus the SecurityRolesDlsBitSetCache changes may affect other APIs, which could affect this PR too. |
pquentin
left a comment
There was a problem hiding this comment.
This looks great, thank you! A few minor adjustments are needed.
specification/xpack/usage/types.ts
Outdated
| * @availability stack since=9.2.0 | ||
| * @availability serverless | ||
| */ | ||
| hits: ulong |
There was a problem hiding this comment.
I realize the above uses ulong, but let's still stick to long for the new values since this is the actual Java type.
| hits: ulong | |
| hits: long |
There was a problem hiding this comment.
changed
i was thinking ulong to indicate this can't be < 0 even tho java type is long; but it all gets turned to a number i believe so probably doesn't matter
There was a problem hiding this comment.
Yeah, I discussed it with the team and it's... complicated. We only have 4 of those right now, and hundreds of long that are also going to be positive-only. Their handling in the different generators is unclear. It's very minor, so let's not get blocked on this.
I did revert the change to the existing ulong in 6c507dee since that would have been a breaking change.
|
just flagging in case but when changing we already use this everywhere so assuming whatever we use to actually generate the docs does this correctly |
Wow, nice catch! I alerted the docs team. |
pquentin
left a comment
There was a problem hiding this comment.
Thank you for your attention to detail! LGTM.
* Add `/_security/stats` specification * Add new properties to DLS cache stats * quentin comments * Revert breaking ulong change --------- Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co> (cherry picked from commit 3e3c203)
* Add `/_security/stats` specification * Add new properties to DLS cache stats * quentin comments * Revert breaking ulong change --------- (cherry picked from commit 3e3c203) Co-authored-by: Szymon Bialkowski <szymon.bialkowski@elastic.co> Co-authored-by: Quentin Pradet <quentin.pradet@elastic.co>
Server PRs: elastic/elasticsearch#134835 + elastic/elasticsearch#135271