Skip to content
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

Adding tracing + attributes to all resolvers #1331

Merged
merged 24 commits into from
Feb 5, 2024
Merged

Conversation

willvedd
Copy link
Contributor

@willvedd willvedd commented Jan 31, 2024

Description

Improving observability of the check resolution process by adding tracing and an appropriate set of attributes for each resolver. Previously, only the local check resolver had tracing but now that we have three separate resolvers, it makes sense to observe those too.

Admittedly, the resolver_type attribute is not a best practice and should have been accomplished by the built-in package attribute but that would have required a messy refactor.

Demo:
Note: traces show that nested resolver delegation doesn't work as intended. Behavior currently depends if cached is enabled or not. This will be fixed in future PR. These are just to prove that traces are working.

Cache disabled:
Screenshot 2024-02-02 at 11 29 25 AM

Cache enabled:
Screenshot 2024-02-02 at 11 16 33 AM

References

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

internal/graph/check.go Outdated Show resolved Hide resolved
willvedd and others added 3 commits February 1, 2024 12:30
Co-authored-by: Jonathan Whitaker <jon.b.whitaker@gmail.com>
Co-authored-by: Jonathan Whitaker <jon.b.whitaker@gmail.com>
Co-authored-by: Jonathan Whitaker <jon.b.whitaker@gmail.com>
Copy link

codecov bot commented Feb 1, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (60f457e) 0.00% compared to head (7655730) 83.83%.

Files Patch % Lines
internal/graph/cached_resolver.go 88.89% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           main    #1331       +/-   ##
=========================================
+ Coverage      0   83.83%   +83.83%     
=========================================
  Files         0       85       +85     
  Lines         0     9850     +9850     
=========================================
+ Hits          0     8257     +8257     
- Misses        0     1239     +1239     
- Partials      0      354      +354     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@willvedd willvedd marked this pull request as ready for review February 1, 2024 22:40
@willvedd willvedd requested a review from a team as a code owner February 1, 2024 22:40
miparnisari
miparnisari previously approved these changes Feb 5, 2024
@miparnisari miparnisari merged commit aef0911 into main Feb 5, 2024
13 of 14 checks passed
@miparnisari miparnisari deleted the resolver-tracing branch February 5, 2024 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants