-
Notifications
You must be signed in to change notification settings - Fork 58
Support for traces in sessions #604
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
Conversation
c257f4f
to
b67df0f
Compare
a8ac53d
to
9aa5b61
Compare
@@ -23,6 +24,7 @@ type Message struct { | |||
FromPKID []byte // PK identifier of the caller | |||
Status int32 // Message Status (OK, ERROR) | |||
Payload []byte // Payload | |||
Ctx context.Context |
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.
is this guaranteed to be serializable?
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.
it isn't, but it doesn't have to be
@@ -53,9 +55,15 @@ type Session interface { | |||
// Send sends the payload to the endpoint | |||
Send(payload []byte) error | |||
|
|||
// SendWithContext sends the payload to the endpoint with the passed context |
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.
does this mean that the context is part of the message or the context is used to control the send operation itself?
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.
no it's part of the message. it carries some information. in this case the span context
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 would clarify in the comment then
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.
No but the context doesn't have to do only with the span. The span uses the context but the context has more functions in general, e.g. canceling the context, or adding baggage values
be708a5
to
880a2f5
Compare
Signed-off-by: Alexandros Filios <[email protected]>
880a2f5
to
ed0ee4e
Compare
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
No description provided.