Add ability to detect virtual nodes in the servicegraph processor #2365
Conversation
mapno
requested review from
knylander-grafana,
joe-elliott,
annanay25,
mdisibio,
kvrhdn and
zalegrala
as code owners
| func (p *Processor) upsertPeerNode(e *store.Edge, spanAttr []*v1_common.KeyValue) { | ||
| for _, peerKey := range p.Cfg.PeerAttributes { | ||
| if v, ok := processor_util.FindAttributeValue(peerKey, spanAttr); ok { | ||
| e.PeerNode = v |
There was a problem hiding this comment.
Looking at this logic - is it correct to say that the order of PeerAttributes is their priority? If net.sock.peer.addr exists it will always be used first. If so let's add a note to the documentation snippet in service_graphs.md
There was a problem hiding this comment.
If the order of peerAttributes is the priority, I would suggest that names be preferred over addresses in the default list. Also, does the code look at peer.service anywhere? There is an otel way to map host name or ip address to peer.service: https://opentelemetry.io/docs/instrumentation/java/automatic/agent-config/#peer-service-name and see https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/span-general/#general-remote-service-attributes as well.
There was a problem hiding this comment.
Nice suggestions. Applied both.
| @@ -258,6 +278,17 @@ func (p *Processor) onComplete(e *store.Edge) { | |||
|
|
|||
| func (p *Processor) onExpire(e *store.Edge) { | |||
| p.metricExpiredEdges.Inc() | |||
|
|
|||
| e.ConnectionType = store.VirtualNode | |||
There was a problem hiding this comment.
I think this logic is detecting an unmatched edge and registering the virtual node if conditions are right. Could we add a comment? Should e.ConnectionType only be set if one of the conditions is true? I think always setting it (even if not needed) is a little unclear.
There was a problem hiding this comment.
Added comments to clarify what this is doing.
Should e.ConnectionType only be set if one of the conditions is true?
Doesn't really matter, as if it doesn't match one of the two conditions to be considered a virtual node, it won't be collected.
Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com>
… peer-node-service-graph
|
Hi. Can i know please when this feature will be released? |
|
This will be in Tempo 2.2. There is no set release date, but due to our normal cadence it will likely be cut late July/early August |
What this PR does:
Adds the ability to detect virtual nodes in the servicegraph processor.
Virtual nodes are nodes that form part of the lifecycle of a trace, but spans for them are not being collected because they're outside of the user's reach (eg. an external service for payment processing) or are not instrumented (eg. a frontend application).
Virtual nodes can be detected in two different ways
span.kindset toserver. This indicates that the request has initiated by an external system that's not instrumented, like a frontend application or an engineer viacurlclientspan does not have its matchingserverspan, but has a peer attribute present. In this case we make the assumption that a call was made to an external service, for which Tempo won't receive spans.peer.service,net.peer.name,net.sock.peer.name,rpc.service.key,net.sock.peer.addr,http.url,http.target.PRs in the OTel collector implementation
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]