If the "service" tag exists in the span, send it to ElasticSearch and

[LakerBaker]

use it in the auxiliary tags when merging spans, instead of the span's
service name. Also clean up pom.xml dependencies to fix some warnings.

[codefromthecrypt]

ps be careful with this. The only thing setting this tag is very new opentracing instrumentation... there's no api to ensure it is consistent for local child spans. This might not hit you yet due to little code using this tag, or only code that has one span, but it can easily mess up your dependency graph.

[codefromthecrypt]

for example, assuming brackets define process boundaries and words represent spans:

A call is made from profile service to cache via two internal hops on a proxy

... profile] -> [ proxy -> proxy ] -> [ cache ...

you might not want to see a service link profile -> proxy, proxy -> cache!

If one wanted to overlay by some override, they might want something like this.. so that the intermediate looks invisible:

... profile] -> [ profile -> profile ] -> [ cache ...

this could flatten using some heuristic

however, if non-propagating tags are used to carry the overlay, it would only affect the first span

... profile] -> [ profile -> proxy ] -> [ cache ...

that would cause some links hard to flatten, worst case not any better than the original. To fix this is processing pipeline work, to guess that the service name intended to inherit.

That said, if there is only one intermediate span, it doesn't matter as there's nothing to inherit. A naive rename can work

... profile] -> [ profile ] -> [ cache ...

I've raised concern offline with an opentracing person to help correct the api and/or docs so that they don't cause this problem.

The reason we have no api to make a tag like this in Brave yet, is we didn't want to cause this problem with a half-measure. In the rare scenario of proxying with service names simply using multiple tracers would work (or in the case of linkerd, they can use existing things present in finagle to get a consistent downstream name). There's so far only one scenario in shared library of proxying names (camel), it is possibly enough to design an api for.

In any case, knowing something is a proxied service name vs an authoritative name could be useful. In other cases, people just remap based on IP address https://developers.soundcloud.com/blog/using-kubernetes-pod-metadata-to-improve-zipkin-traces

[LakerBaker]

Thanks for the feedback; I'll keep your concerns in mind. There's a team at Expedia that wants to use Haystack to display the progress of a CICD pipeline, and the OT (mis?)use of tags to override service name allowed the Haystack team to handle their use case without having to invent another mechanism.

[codefromthecrypt]

OT doesn't define a format rather an api. this is why it is muddy. It isnt a problem to need to override things just warning against doing the api part incorrectly. The problem I mentioned was fairly usual in Twitter finagle data until they added a function to scope a service label. I have heard from others that they implicitly copy a service name downwards when someone overrides. none of this was discussed in OT until I mentioned it.. the details of how to override a service safely effectively a tracer specific feature. I may not impact you at all for CI/CD overrides.. no idea how that is coded. just watch out for service instrumentation who are accidentally buggy as now one tag ("service") is special and should be inherited while all others aren't! To support this before data gets to the pipeline implies a tracer change one way or another.. one way is them fixing the spec (to detail how this tag is special and caveat that old tracers will break because they dont know to inherit) or to switch the api to me more like your data model ex span.serviceName and in this case this forces users to upgrade to a tracer that inherits the data properly I'm mainly talking here about shared libraries as yeah we were here before a few years back and this caused problems so as shared library authors we shouldn't repeat mistakes

…

On Fri, 12 Oct 2018, 07:34 Jeff Baker, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In indexer/src/main/scala/com/expedia/www/haystack/trace/indexer/writers/cassandra/ServiceMetadataStatementBuilder.scala <#183 (comment)> : > @@ -72,8 +73,9 @@ class ServiceMetadataStatementBuilder(cassandra: CassandraSession, def getAndUpdateServiceMetadata(spans: Iterable[Span]): Seq[Statement] = { this.synchronized { spans.foreach(span => { - if (StringUtils.isNotEmpty(span.getServiceName) && StringUtils.isNotEmpty(span.getOperationName)) { - val operationsList = serviceMetadataMap.getOrElseUpdate(span.getServiceName, mutable.Set[String]()) + val serviceName = SpanUtils.getEffectiveServiceName(span) Thanks for the feedback; I'll keep your concerns in mind. There's a team at Expedia that wants to use Haystack to display the progress of a CICD pipeline, and the OT (mis?)use of tags to override service name allowed the Haystack team to handle their use case without having to invent another mechanism. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#183 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD610aShiIRB7jJD-R0u1MaALLi7_7rks5uj9WOgaJpZM4XCsay> .