-
-
Notifications
You must be signed in to change notification settings - Fork 964
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: add missing SessionIssued event for api flows #3348
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for getting on this quickly! Not sure if we hit all places yet, though.
Codecov Report
@@ Coverage Diff @@
## master #3348 +/- ##
==========================================
- Coverage 78.04% 78.04% -0.01%
==========================================
Files 324 324
Lines 21013 21021 +8
==========================================
+ Hits 16400 16406 +6
- Misses 3384 3385 +1
- Partials 1229 1230 +1
|
I agree that this is not very well instrumented yet. The instrumentation should probably happen in the persister. We can merge this as is though IMO. Up to you @jonas-jonas . Also possible that we missed a few spots for example in social sign in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Related issue(s)
Checklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security vulnerability, I
confirm that I got the approval (please contact
[email protected]) from the maintainers to push
the changes.
works.
Further Comments