Broker tracing conventions #1064
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ian-mi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Manually verified to produce the expected spans in Stackdriver. |
|
/hold needs fix for response spans |
ian-mi
force-pushed
the
broker-tracing-conventions
branch
from
88342b4
to
21c7f57
Compare
ian-mi
force-pushed
the
broker-tracing-conventions
branch
from
21c7f57
to
cafe654
Compare
|
/retest |
| @@ -127,15 +136,26 @@ func (h *Handler) ServeHTTP(response nethttp.ResponseWriter, request *nethttp.Re | |||
|
|
|||
| event.SetExtension(EventArrivalTime, cev2.Timestamp{Time: time.Now()}) | |||
|
|
|||
| ctx, span := trace.StartSpan(ctx, kntracing.BrokerMessagingDestination(broker)) | |||
There was a problem hiding this comment.
Should this check for an existing distributed trace? For example, if something has been replied back into the Broker. Or if the source that sent this already had started a trace.
There was a problem hiding this comment.
The existing trace context should always be set in the HTTP tracecontext so there is no need to check for the extension.
| span.AddAttributes( | ||
| kntracing.MessagingSystemAttribute, | ||
| tracing.PubSubProtocolAttribute, | ||
| kntracing.BrokerMessagingDestinationAttribute(broker), |
There was a problem hiding this comment.
What does this attribute represent?
For example, if this attribute is on a span, how can I interpret that span?
I ask because it looks like in the ingress we are annotating the span that sends the request to GCP PubSub with the broker as the destination. While in the filtering code I think the annotation states the trigger name while sending to the trigger's destination.
There was a problem hiding this comment.
This attribute considers each broker or trigger to be an event destination. On ingress a single broker span will be created and on fanout multiple trigger spans will be created identifiable by the messaging.destination attribute as well as the span name.
| span.AddAttributes( | ||
| kntracing.MessagingSystemAttribute, | ||
| tracing.PubSubProtocolAttribute, | ||
| kntracing.TriggerMessagingDestinationAttribute(trigger), |
There was a problem hiding this comment.
Should this include any form of type information? As-is, I can't tell the difference between different objects that have the same name and namespace (e.g. K8s service vs Broker vs KafkaChannel).
There was a problem hiding this comment.
messaging.destination is prefixed by either broker or trigger and will contain the namespaced name.
|
/unhold |
|
/unhold |
|
The following is the coverage report on the affected files.
|
|
/lgtm |
|
/retest |
11 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
Fixes #1063
Proposed Changes
Release Note