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

drpcmetadata: encode and decode metadata #2

Merged
merged 15 commits into from
Mar 20, 2020
Merged

drpcmetadata: encode and decode metadata #2

merged 15 commits into from
Mar 20, 2020

Conversation

VinozzZ
Copy link
Contributor

@VinozzZ VinozzZ commented Mar 18, 2020

Provide interface for adding/retrieving metadata from context across the wire.

VinozzZ added 8 commits March 12, 2020 17:33
in order to send tracing information across the wire, we need to extend
drpc invoke message to contain key/value pairs.
We will set the first two bytes of the message to be empty so we know
the message is using the new format and may contain meatadata.
Then we will proto buff to encode our invoke message so we can send it
across the wire.

Change-Id: I03a5af62c5b41e0855dd101faacd8d12c66c2cdb
Change-Id: I7c3d526fdd091c0db7233ec54e0c36f8da634745
Change-Id: I3410aad28ccef787183b74cc537aa71b7d9b973b
Change-Id: Idfeaa81111d63775b8cd8fc41baa4628ff8c6d23
Change-Id: I3e6265735ae6f5859b23cbe1d585931ecff8a69b
Change-Id: Ia9551ba0db65fb4d27ea09ec9847696b6738fb8d
@VinozzZ
Copy link
Contributor Author

VinozzZ commented Mar 18, 2020

I still need to work on docs and tests for the new package but I thought it would nice to get the interface reviewed before I do that work.

Change-Id: I256df930c969c4c81850d66d85a72488abd5529b
Copy link
Collaborator

@zeebo zeebo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the drpcmetadata/proto package name yet, but I don't have a suggestion, either. Maybe it can be named drpcmetadata/invoke?

drpcconn/conn.go Outdated
Comment on lines 88 to 91
data, err := proto.Marshal(in)
if err != nil {
return errs.Wrap(err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this back up to where it was? I prefer to handle this error before we interact with the manager. The idea is to get all of the locally erroring things done before potentially doing network erroring things.

VinozzZ added 4 commits March 18, 2020 15:33
Change-Id: I8b5854318fa0bb4f0320c330528f5ba5a05c19c5
Change-Id: If6a25354a8956ede9c938f4706e6f7296ca89da6
Change-Id: I49a449e8b5127e30e54d50b30e44fc5fc0293007
Change-Id: I5f6b3d6857e9eb71aa503b2ba85b2f506fdf0a81
@VinozzZ VinozzZ requested a review from zeebo March 18, 2020 20:11
Copy link
Collaborator

@zeebo zeebo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nitpicks left.

Change-Id: I91feb4861693eaa70687378f04576de0876df46a
@VinozzZ VinozzZ requested a review from zeebo March 18, 2020 20:35
Change-Id: I783fc8ff13d160eed58708d8d670d6718af49b95
@VinozzZ VinozzZ merged commit 3311ebb into master Mar 20, 2020
@VinozzZ VinozzZ deleted the yz/drpcmetadata branch March 20, 2020 15:06
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.

3 participants