-
Notifications
You must be signed in to change notification settings - Fork 19
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
ADR for usage of gRPC for networking between services #479
base: main
Are you sure you want to change the base?
Conversation
No New Or Fixed Issues Found |
Deploying contributing-docs with Cloudflare Pages
|
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.
I think a pivot on the contents will be beneficial here. I think I can get behind the end outcome based on my own experiences, but the way you got there isn't vetted as-is.
fast inter-service communication. It offers several advantages over traditional REST APIs and other | ||
HTTP/1.1 technologies: | ||
|
||
1. **Performance**: gRPC uses Protocol Buffers as its interface definition language and binary |
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.
⛏️ These don't need to be numbered; just use bullets.
fast inter-service communication. It offers several advantages over traditional REST APIs and other | ||
HTTP/1.1 technologies: | ||
|
||
1. **Performance**: gRPC uses Protocol Buffers as its interface definition language and binary |
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.
1. **Performance**: gRPC uses Protocol Buffers as its interface definition language and binary | |
1. **Performance** - gRPC uses Protocol Buffers as its interface definition language and binary |
Like the considered options, dash these.
|
||
## Context and Problem Statement | ||
|
||
As Bitwarden's microservices architecture continues to evolve, the need for efficient, reliable, and |
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.
🎨 Please drop any and all references to "microservices" in favor of "service-oriented architecture" or just "services".
- **Adopt gRPC for all inter-service communication** - Completely replace REST with gRPC for all | ||
service-to-service interactions. | ||
- **Hybrid approach** - Implement gRPC for all new services while maintaining existing solutions. | ||
- **Evaluate alternative RPC frameworks** - Consider other RPC frameworks like Apache Thrift or |
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.
💭 This is really what the ADR is about. While we often have a proposed solution when the ADR is written, gRPC is explained above before options are even laid out. What I'd like to see adjusted here is to move the gRPC content below and into the decision outcome as well as pros and cons. These additional RPC frameworks should be listed as options, with the decision outcome explaining why they were not a good fit. Your background above shouldn't be anything about gRPC but the problem at hand and desire to optimize the channel, security, etc. generically and not aligned with a specific choice.
🎟️ Tracking
Originated out of https://bitwarden.atlassian.net/wiki/spaces/EN/pages/809828356/Proposal+The+Pricing+Plans+Service
📔 Objective
This PR introduces a new ADR titled "Adopting gRPC for Inter-Service Communication". The purpose is to document the decision-making process and outcomes regarding adopting gRPC for inter-service communication within Bitwarden's microservices architecture.
⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes