ddtrace/tracer: implement peer.service #1975
Conversation
BenchmarksBenchmark execution time: 2023-06-14 09:26:42 Comparing candidate commit e101abc in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 23 metrics, 0 unstable metrics. scenario:BenchmarkConcurrentTracing-24
|
rarguelloF
changed the title
There was a problem hiding this comment.
I don't see DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED discussed anywhere in the RFCs/docs. Can you add this to the specification first? I'm not so sure about the naming.
Or maybe we don't even want this at all. Perhaps using this is contingent on setting DD_TRACE_SPAN_ATTRIBUTE_SCHEMA=v1, with no other way to configure it. Let's discuss that offline.
There was a problem hiding this comment.
I don't see DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED discussed anywhere in the RFCs/docs. Can you add this to the specification first? I'm not so sure about the naming.
Or maybe we don't even want this at all. Perhaps using this is contingent on setting DD_TRACE_SPAN_ATTRIBUTE_SCHEMA=v1, with no other way to configure it. Let's discuss that offline.
ddtrace/tracer/option.go
Outdated
| // peer.service tag is always enabled if using attribute schema >= 1 | ||
| c.peerServiceDefaultsEnabled = true | ||
| if c.spanAttributeSchemaVersion == int(namingschema.SchemaV0) { | ||
| c.peerServiceDefaultsEnabled = internal.BoolEnv("DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED", false) | ||
| } | ||
| c.peerServiceMappings = make(map[string]string) | ||
| if v := os.Getenv("DD_TRACE_PEER_SERVICE_MAPPING"); v != "" { | ||
| internal.ForEachStringTag(v, func(key, val string) { c.peerServiceMappings[key] = val }) | ||
| } |
There was a problem hiding this comment.
Can we add a unit test to cover this?
ddtrace/tracer/spancontext.go
Outdated
| @@ -414,10 +415,86 @@ func (t *trace) finishedOne(s *span) { | |||
| if hn := tr.hostname(); hn != "" { | |||
| s.setMeta(keyTracerHostname, hn) | |||
| } | |||
| setPeerService(s, tr.config) | |||
There was a problem hiding this comment.
I just tested this for a service in dd-go, and this never get executed. In almost all cases the peer.service needs to be set on a leaf span, but because of the condition at line 404 if len(t.spans) != t.finished { return }, this function is never reached.
rarguelloF
force-pushed
the
rarguelloF/add-peer-service
branch
from
13bb529 to
dc44b3e
Compare
Co-authored-by: Kyle Nusbaum <kyle@datadog.com>
What does this PR do?
peer.servicedefaults calculation in the tracer.DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED: default value isfalseand only applies whenDD_TRACE_SPAN_ATTRIBUTE_SCHEMA == 0. In other case,peer.servicedefaults calculation is always enabled.DD_TRACE_PEER_SERVICE_MAPPING: same behavior asDD_SERVICE_MAPPINGbut forpeer.service._dd.peer.service.source: the precursor tag that was used to setpeer.service._dd.peer.service.remapped_from: the previous value before remapping happened (if applies).Motivation
Implement
peer.serviceas it will be used in different parts of the product.Describe how to test/QA your changes
Reviewer's Checklist