-
Notifications
You must be signed in to change notification settings - Fork 94
Add Resource to Span #174
Add Resource to Span #174
Conversation
fixes #169 |
@@ -290,6 +291,11 @@ message Span { | |||
// Status.Ok (code = 0). | |||
Status status = 11; | |||
|
|||
// An optional resource that this span describes. If not set, this span | |||
// should be part of a batch that does include the span information, unless resource |
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.
"that does include the span information" or "that does include the resource information"?
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.
ah you are right, should be resource. -- fixing
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
@@ -290,6 +291,11 @@ message Span { | |||
// Status.Ok (code = 0). | |||
Status status = 11; | |||
|
|||
// An optional resource that this span describes. If not set, this span |
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.
span doesn't describe resource. May be this should be rephrased to something like "An optional resource to which this span is associated with".
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.
Reworded. Please take another look
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
As discussed in the wider meeting, adds Resource information to individual spans.