Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Make it easier for one process to report spans as multiple services #212

Closed
mabn opened this issue Jul 15, 2017 · 11 comments
Closed

Make it easier for one process to report spans as multiple services #212

mabn opened this issue Jul 15, 2017 · 11 comments

Comments

@mabn
Copy link
Contributor

mabn commented Jul 15, 2017

I have a process which listens on a queue with spans in our internal format and reports them to jaeger. Right now I create N tracers and it's acceptable because N is rather low (~50) - one per service, and I ignore other "process" tags like IP. And that was the simplest way.

I do that because UDPSender caches process tags and additionally process tags are taken from Tracer instance which adds them automatically.

Here's what could work:

  • disable sampling for a given span so that it is not reported when finished - that should work currently
  • somehow bypass caching of Process inside UDPSender that would allow to have just one UDPSender instance (or one per thread, one per consumer, etc.)
  • add new method to UDPSender which appends a span but takes additionally a Process and processBytesSize, something like:
  // old append delegates to the new method
  @Override
  public int append(Span span) throws SenderException {
    if (process == null) {
      process = new Process(span.getTracer().getServiceName());
      process.setTags(JaegerThriftSpanConverter.buildTags(span.getTracer().tags()));
      processBytesSize = getSizeOfSerializedThrift(process);
      byteBufferSize += processBytesSize;
    }
    append(span, process, processBytesSize);
}

public int append(Span span, Process process, int processBytesSize) throws SenderException {
    (...)
}

What I don't like here is the risk that passed processBytesSize won't match the real process size. But maybe it would be possible to create a separate class which contains Process and caches its size.

What do you think?

@yurishkuro
Copy link
Member

There was a discussion in OT spec repo about introducing a service tag for a very similar reason of creating spans for multiple services in one process (e.g. a routing mesh). I started thinking then how we could support it in Jaeger clients, given that we made a point of extracting process tags out of spans to avoid sending the same ones over the wire. However, once a batch reaches the collectors, each span is processed independently of the others, and process tags are currently stored in the db every time for each span.

So I think the simplest way to support multiple services per Tracer is to add a preprocessing step to the collectors that will look for a service tag on the span and if found will replace span.Process with a new Process representing that service name, e.g.

span.process = span.process.copy(service=serviceTag)

No changes will be required in the clients in this case.

@mabn
Copy link
Contributor Author

mabn commented Jul 15, 2017

Right, but that handles only the case of the "service" tag and there could be other process tags too. UI presents them differently so there would need to be some way to differentiate them from regular ones.

@yurishkuro
Copy link
Member

Sorry, I misunderstood your use case. If the spans are produced by another instrumentation, is it worth going through Jaeger client at all? Jaeger collectors have tchannel and http ports that accept spans in jaeger.thrift format, perhaps it would be a better option to just convert your spans to that format and submit directly to collectors? You would still need to do batching by different services in your queue processor, since both endpoints expect a Batch representing spans from the same service.

@mabn
Copy link
Contributor Author

mabn commented Jul 17, 2017

Well, now that you suggested it - it sounds like a better approach.

I used Jaeger client because it was convenient. Just new Span(..), report it, done (plus map of tracers per service name).

@jpkrohling
Copy link
Collaborator

@mabn it's now possible to use your own senders via the Sender API. Would it solve your original problem? You could have one custom Sender that sends each reported Span to multiple locations.

@mabn
Copy link
Contributor Author

mabn commented Jun 18, 2018

I tried to upgrade to most recent version and use Senders - but it's problematic.

A) I can use Sender.send(Process, List<io.jaegertracing.thriftjava.Span>) but then I need to implement my own batching/flushing - so basically something like RemoteReporter.

B) On the other hand it would be nice to use Sender.append(Span) but it has more issues:

  • need to create Span instance with a particular context (ideally via: Build a Span with a given SpanContext opentracing/specification#81). The only way as far as I know is to use package-protected Span constructor - which leads to ugly workaround with "io.jaegertracing" package
  • each Span has a reference to Tracer instance from which Sender extracts process tags and service name. So if I want to specify process tags then I need a pool of Tracers, one for each process tags combination.

So the A) option seems fine, but basically avoids whole opentracing API and requires some extra coding (custom pooling+batching+flushing)

@jpkrohling
Copy link
Collaborator

@mabn sorry, I should have been clearer :-/ I meant a change that has not been released yet, #449. With that, you should be able to just implement your SenderResolver/Sender and you'll have the chance of changing the Span before passing it down to an actual Sender (UdpSender/HttpSender from jaeger-thrift). Or completely bypass the jaeger-thrift Senders.

@mabn
Copy link
Contributor Author

mabn commented Jun 19, 2018

I looked at #449 but I don't think it helps here because the problems mentioned in B) remain - Span has a package-protected constructor and ThriftSender retrieves tags from a Tracer obtained from a Span.

But using Sender.send(Process, List<io.jaegertracing.thriftjava.Span>) without any opentracing types is good enough

@jpkrohling
Copy link
Collaborator

We are currently thinking about having a better public API (#396). If there's anything you'd like to see changed as part of that task, do let me know.

@jpkrohling
Copy link
Collaborator

Is the original request reported by this issue still relevant? Should this issue be closed?

@yurishkuro
Copy link
Member

I think it's the same use case as why the "service" tag is being proposed. But I think the simplest way for us to support service tag is by adding a rewrite rule to the collector which will split the batch and assign different Process objects to spans.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants