feat(jans-fido2): add dropOffRate and completionRate to metrics error…#13360
Open
imran-ishaq wants to merge 6 commits intomainfrom
Open
feat(jans-fido2): add dropOffRate and completionRate to metrics error…#13360imran-ishaq wants to merge 6 commits intomainfrom
imran-ishaq wants to merge 6 commits intomainfrom
Conversation
…s analytics endpoint Signed-off-by: imran <imranishaq7071@gmail.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds two new error-analytics metrics (completionRate, dropOffRate), changes successRate/failureRate denominators to ATTEMPT (started) counts, updates service logic to compute and normalize rates with legacy fallback, and updates documentation/OpenAPI schema and metric constants. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
mo-auto
added
area-documentation
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java`:
- Around line 512-515: The analysis map is using hardcoded keys "completionRate"
and "dropOffRate" in Fido2MetricsService.put calls; add new constants (e.g.,
COMPLETION_RATE and DROP_OFF_RATE) to Fido2MetricsConstants and replace the
hardcoded strings in Fido2MetricsService (where analysis.put("completionRate",
...) and analysis.put("dropOffRate", ...)) to use
Fido2MetricsConstants.COMPLETION_RATE and Fido2MetricsConstants.DROP_OFF_RATE so
all metric keys are centralized and consistent.
- Around line 505-511: In Fido2MetricsService (around the block computing
successRate/failureRate), guard completionRate/dropOffRate against out-of-range
values caused by mixing legacy completion-only records: after computing
successRate and failureRate (using successfulOperations, failedOperations,
totalStarted), compute completionRate = successRate + failureRate and then clamp
it to [0.0, 1.0] (e.g., completionRate = Math.max(0.0, Math.min(1.0,
completionRate))); recompute dropOffRate = 1.0 - completionRate and similarly
ensure dropOffRate stays within [0.0, 1.0]. Ensure these clamped values are used
for subsequent metric reporting.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
jans-fido2/docs/METRICS_WIKI.mdjans-fido2/docs/jansFido2Swagger.yamljans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java
jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java
Show resolved
Hide resolved
jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java
Outdated
Show resolved
Hide resolved
…s analytics endpoint 1 Signed-off-by: imran <imranishaq7071@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java`:
- Around line 507-516: The computation in Fido2MetricsService currently clamps
completionRate independently, causing completionRate != successRate +
failureRate; fix by computing rawSum = successRate + failureRate, then clamp
completionRate = Math.min(1.0, rawSum) and if rawSum > 0 and rawSum !=
completionRate scale successRate and failureRate proportionally (successRate *=
completionRate / rawSum; failureRate *= completionRate / rawSum) so their sum
equals completionRate before putting them into analysis (use the existing
successfulOperations, failedOperations, totalStarted, and Fido2MetricsConstants
names to locate the code).
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
jans-fido2/model/src/main/java/io/jans/fido2/model/metric/Fido2MetricsConstants.javajans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java
jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java
Show resolved
Hide resolved
…s analytics endpoint 2 Signed-off-by: imran <imranishaq7071@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java`:
- Around line 523-533: The fallback branch in Fido2MetricsService currently
skips adding SUCCESS_RATE, FAILURE_RATE, COMPLETION_RATE, and DROP_OFF_RATE to
the analysis map when totalCompleted == 0, causing missing keys for empty
datasets; modify the else branch that computes totalCompleted from
successfulOperations and failedOperations so it always puts those four keys into
the analysis map even when totalCompleted == 0 — compute successRate and
failureRate as 0.0 when totalCompleted == 0, set completionRate to 0.0 and
dropOffRate to 0.0 in that case, and continue to use the existing constants
Fido2MetricsConstants.SUCCESS_RATE, FAILURE_RATE, COMPLETION_RATE, DROP_OFF_RATE
and the analysis map to ensure stable response shape.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java
jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java
Show resolved
Hide resolved
…s analytics endpoint 3 Signed-off-by: imran <imranishaq7071@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java`:
- Around line 494-503: The three separate streams counting
ATTEMPT/SUCCESS/FAILURE in Fido2MetricsService (producing totalStarted,
successfulOperations, failedOperations) should be collapsed into a single pass
to avoid traversing entries multiple times: iterate once (or use
entries.stream().collect(Collectors.groupingBy(e -> e.getStatus(),
Collectors.counting()))) to produce counts keyed by Fido2MetricsConstants values
and then assign totalStarted/successfulOperations/failedOperations from that
single tally; update any downstream code to use these consolidated counts.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java
jans-fido2/server/src/main/java/io/jans/fido2/service/metric/Fido2MetricsService.java
Outdated
Show resolved
Hide resolved
…s analytics endpoint 4 Signed-off-by: imran <imranishaq7071@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds dropOffRate and completionRate to the GET /metrics/analytics/errors response. Rates are based on started operations (ATTEMPT count): completionRate = successRate + failureRate, dropOffRate = 1 - completionRate. Includes a fallback when no ATTEMPT entries exist (e.g. legacy data)
Target issue
Adds explicit drop-off and completion metrics to the FIDO2 metrics errors analytics endpoint so administrators can track abandonment and completion rates (fixes #[issue-number] if you have one).
closes #13351
Implementation Details
Test and Document the changes
Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with
docs:to indicate documentation changes or if the below checklist is not selected.Summary by CodeRabbit
New Features
Documentation