Added the LDAP operation type and error name in the metric by ruchikajha95 · Pull Request #51491 · keycloak/keycloak

ruchikajha95 · 2026-08-06T10:52:44Z

Signed-off-by: Ruchika ruchika.jha1@ibm.com

keycloak-github-bot · 2026-08-07T00:42:23Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.actions.TermsAndConditionsTest#termsAccepted

Keycloak CI - Forms IT (firefox)

java.lang.RuntimeException: Could not create statement
	at org.jboss.arquillian.junit.Arquillian.methodBlock(Arquillian.java:307)
	at org.jboss.arquillian.junit.Arquillian$2.evaluate(Arquillian.java:160)
	at org.jboss.arquillian.junit.Arquillian.multiExecute(Arquillian.java:344)
	at org.jboss.arquillian.junit.Arquillian.access$200(Arquillian.java:48)
...

Report flaky test

keycloak-github-bot

Unreported flaky test detected, please review

Copilot

Pull request overview

Adds operation and error labels to LDAP request metrics.

Changes:

Adds LDAP operation names to execute metrics.
Adds exception class names to failed request metrics.
Classifies supported LDAP operations.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`LDAPOperationManager.java`	Adds operation and error metric tags.
`LDAPContextManager.java`	Adds error tags to connection metrics.

Suppressed comments (1)

federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java:865

The linked issue requires changing the LDAP execute trace spans, but these values are only added to the timer. The span is still named LDAPOperationManager.execute, still stores the full multi-line operation.toString(), and receives no structured operation/error/DN attributes, so closing #50958 would leave all of its stated trace problems unresolved; update the span instrumentation or stop closing that issue.

            recordLdapRequest("execute", operation.operationType(), success, startTimeNanos, errorName);

pruivo · 2026-08-10T09:52:27Z

@ruchikajha95 do we need both operation and operation_name?

ruchikajha95 · 2026-08-10T10:25:35Z

@ruchikajha95 do we need both operation and operation_name?

The idea was "operation" tag = "execute" or "authenticate" : this is the high-level category

"operation_name" tag = the specific LDAP operation type (e.g. "search", "create", "modify", "unknown") returned by operationType() so that the metrics can explain the type better.

pruivo · 2026-08-10T10:52:43Z

Can you list all possible values? if operation only has two values, it seems overkill to have 2 tags.

edit: I'm asking this because PromQL allows query aggregation, making the operation (the high category) tag irrelevant.

ruchikajha95 · 2026-08-10T10:57:42Z

Can you list all possible values? if operation only has two values, it seems overkill to have 2 tags.

The complete list of possible values are :

operation tag: only 2 values — "execute" and "authenticate"
- operation_name tag: "remove", "rename", "search", "searchPaginated", "lookupById", "modify", "create", "passwordModify", "unknown" (default)
I agree to your point . Do u suggest that to merge them into a single "operation" tag where "authenticate" is one value alongside the specific LDAP operation types

pruivo · 2026-08-10T11:02:47Z

Can you list all possible values? if operation only has two values, it seems overkill to have 2 tags.

The complete list of possible values are :

operation tag: only 2 values — "execute" and "authenticate"

operation_name tag: "remove", "rename", "search", "searchPaginated", "lookupById", "modify", "create", "passwordModify", "unknown" (default)

I agree to your point . Do u suggest that to merge them into a single "operation" tag where "authenticate" is one value alongside the specific LDAP operation types

Yes, that's my suggestion.

keycloak-github-bot · 2026-08-10T14:17:37Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#salesPostEncSignedAssertionsOnlyTest

Keycloak CI - Adapter IT Strict Cookies

org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
...

Report flaky test

keycloak-github-bot

Unreported flaky test detected, please review

pruivo

LGTM 👍

Closes keycloak#50958 Signed-off-by: Ruchika <ruchika.jha1@ibm.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java:865

The linked issue requires operation/error details on LDAPOperationManager.execute trace spans, but these values are only passed to the request timer here. The span is still named LDAPOperationManager.execute and still stores operation.toString() as one multiline attribute, so the reported tracing problem remains unchanged; apply the operation type and structured operation/error fields to the span as well.

            recordLdapRequest(operation.operationType(), success, startTimeNanos, errorName);

…51491) * Added the LDAP operation type and error name in the metric Closes keycloak#50958 Signed-off-by: Ruchika <ruchika.jha1@ibm.com> * Resolved the pr review comments Closes keycloak#50958 Signed-off-by: Ruchika <ruchika.jha1@ibm.com> * Resolved the pr review comments Closes keycloak#50958 Signed-off-by: Ruchika <ruchika.jha1@ibm.com> * changed the operation tags Closes keycloak#50958 Signed-off-by: Ruchika <ruchika.jha1@ibm.com> --------- Signed-off-by: Ruchika <ruchika.jha1@ibm.com> Signed-off-by: Michal Vavřík <dev@michalvavrik.net>

ruchikajha95 force-pushed the feature-50958/operation-type-and-error-name-to-LDAP branch 3 times, most recently from cd24bd9 to 1cf7c5a Compare August 6, 2026 20:13

keycloak-github-bot Bot added the flaky-test label Aug 7, 2026

keycloak-github-bot Bot reviewed Aug 7, 2026

View reviewed changes

ruchikajha95 self-assigned this Aug 7, 2026

ruchikajha95 requested a balanced review from Copilot August 7, 2026 08:59

Copilot started reviewing on behalf of ruchikajha95 August 7, 2026 09:00 View session

Copilot AI reviewed Aug 7, 2026

View reviewed changes

Comment thread ...ration/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPOperationManager.java Outdated

Comment thread federation/ldap/src/main/java/org/keycloak/storage/ldap/idm/store/ldap/LDAPContextManager.java Outdated

stianst added the team/core-iam label Aug 10, 2026

ruchikajha95 requested review from ahus1 and pruivo August 10, 2026 09:43

ruchikajha95 force-pushed the feature-50958/operation-type-and-error-name-to-LDAP branch from b0f709f to 575650c Compare August 10, 2026 11:26

keycloak-github-bot Bot reviewed Aug 10, 2026

View reviewed changes

ruchikajha95 force-pushed the feature-50958/operation-type-and-error-name-to-LDAP branch from 1af4829 to f32440f Compare August 11, 2026 13:01

pruivo reviewed Aug 11, 2026

View reviewed changes

ruchikajha95 force-pushed the feature-50958/operation-type-and-error-name-to-LDAP branch from f32440f to d98ec23 Compare August 12, 2026 10:30

ruchikajha95 added 4 commits August 12, 2026 12:09

Added the LDAP operation type and error name in the metric

613060d

Closes keycloak#50958 Signed-off-by: Ruchika <ruchika.jha1@ibm.com>

Resolved the pr review comments

114b241

Closes keycloak#50958 Signed-off-by: Ruchika <ruchika.jha1@ibm.com>

Resolved the pr review comments

536daed

Closes keycloak#50958 Signed-off-by: Ruchika <ruchika.jha1@ibm.com>

changed the operation tags

dc3ed3d

Closes keycloak#50958 Signed-off-by: Ruchika <ruchika.jha1@ibm.com>

ruchikajha95 force-pushed the feature-50958/operation-type-and-error-name-to-LDAP branch from d98ec23 to dc3ed3d Compare August 12, 2026 11:09

pruivo approved these changes Aug 12, 2026

View reviewed changes

ruchikajha95 marked this pull request as ready for review August 12, 2026 12:36

Copilot AI review requested due to automatic review settings August 12, 2026 12:36

ruchikajha95 requested a review from a team as a code owner August 12, 2026 12:36

Copilot started reviewing on behalf of ruchikajha95 August 12, 2026 12:37 View session

Copilot AI reviewed Aug 12, 2026

View reviewed changes

stianst approved these changes Aug 12, 2026

View reviewed changes

stianst enabled auto-merge (squash) August 12, 2026 12:58

stianst merged commit eba869e into keycloak:main Aug 12, 2026
98 of 100 checks passed

Uh oh!

Conversation

ruchikajha95 commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keycloak-github-bot Bot commented Aug 7, 2026

Unreported flaky test detected

org.keycloak.testsuite.actions.TermsAndConditionsTest#termsAccepted

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

pruivo commented Aug 10, 2026

Uh oh!

ruchikajha95 commented Aug 10, 2026

Uh oh!

pruivo commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ruchikajha95 commented Aug 10, 2026

Uh oh!

pruivo commented Aug 10, 2026

Uh oh!

keycloak-github-bot Bot commented Aug 10, 2026

Unreported flaky test detected

org.keycloak.testsuite.adapter.servlet.SAMLServletAdapterTest#salesPostEncSignedAssertionsOnlyTest

Uh oh!

keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

pruivo left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ruchikajha95 commented Aug 6, 2026 •

edited

Loading

pruivo commented Aug 10, 2026 •

edited

Loading