-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Discussion of Consistent Routing by TraceID #1724
Discussion of Consistent Routing by TraceID #1724
Comments
There was a desire to keep routing concerns out of the collector expressed in this pr As an alternative proposal, I think it would be useful to have a mechanism (processor) that would allow the collector to add a http header to the http calls that the exporters make. This would allow an external system (like Envoy) to perform the consistent hashing and route each span to a consistent destination.
I have a working prototype using this design but I don't think it works with all exporters currently (it works for OTLP in my testing). The questionable piece is func (proc *traceheaderprocessor) ConsumeTraceData(ctx context.Context, td consumerdata.TraceData) error {
spansByBucket := make(map[uint32][]*tracepb.Span, 0)
for _, span := range td.Spans {
// from probabilistic sampler
var numHashBuckets uint32 = 0x4000
var bitMaskHashBuckets = numHashBuckets - 1
bucket := hash(span.TraceId, 22) & bitMaskHashBuckets
if _, ok := spansByBucket[bucket]; !ok {
spansByBucket[bucket] = make([]*tracepb.Span, 0)
}
spansByBucket[bucket] = append(spansByBucket[bucket], span)
}
for bucket, spans := range spansByBucket {
newTraceData := consumerdata.TraceData{
Node: td.Node,
Resource: td.Resource,
SourceFormat: td.SourceFormat,
}
newTraceData.Spans = spans
bucketStr := strconv.FormatUint(uint64(bucket), 10)
grpCtx := metadata.AppendToOutgoingContext(ctx, "OTel-TraceBucket", bucketStr)
if err := proc.nextConsumer.ConsumeTraceData(grpCtx, newTraceData); err != nil {
return err
}
}
return nil
} While this will allow consistent routing to work, there are still questions about how to safely update the fleet of Tail Samplers without data loss/suboptimal sampling decisions |
Glad to see this! As mentioned in the call yesterday, I had this in my radar for some time now and my initial idea was very close to what @annanay25 implemented before, except that it would be an exporter instead of a processor and it wouldn't redirect spans to peers, but rather, to backends. This way, there could be 20 backend instances and only one loadbalancer.
The idea is that an otelcol instance would act alone(*) as a TraceID-aware loadbalancer, getting the list of possible backends from a service discovery mechanism (DNS at first). Using a consistent hashing mechanism to determine the backend was also in my mind, but there's one problem that even consistent hashing can't solve: when the ring changes, 1/n of the spectrum would be shuffled, which means that spans that previously reached a backend might now reach a different backend. This is one of the reasons for the The About the backends: I don't think it's feasible to allow it to have its receivers configurable. I would require the backend's receiver to be OTLP, so that we can use gRPC between the load balancer and the backend. This would offload a bunch of concerns to the gRPC client (retry, backoff, ...). And finally: note that if your use case is to apply tail-based sampling, you don't need the load balancer when you have the (*) The tricky part is that the loadbalancer would be seen as a critical piece of infra, and as such, needs to be highly-available itself. And with that, synchronization of the ring becomes another problem entirely (leader election, consistent view of the data source, ...) This is something I was expecting to deal with in a future iteration, once we gain more knowledge about how good everything else works. |
This is how I see this, having a LBExporter that can start multiple instances of a "delegated" exporter and route requests based on some rules:
This way we don't have to change the current pipeline design |
@bogdandrutu how would the exporter #.1 know to which host to send the data to? |
Our needs are exactly in line with what @jpkrohling is describing. Dynamic discovery of backends and maintaining a hash ring for minimal disruption during scaling events. I also had not considered, but really like the idea of relying on the
Agree this needs some attention. We are seeing success using both gossip and external key value stores to maintain a ring depending on the durability requirements. |
@jpkrohling "exporter # 1" is an instance of an exporter (otlp, etc.) created by the lb exporter. The lb exporter is a composed exporter which does the handling of the config changes and creates / deletes instances of real exporters as well as routes requests to the specific instance. So in the pipeline you connect only the lb exporter which then does routing to the right instance "exporter # X" which is just a simple exporter like today |
When the list of backend collectors changes, what happens? I'm trying to wrap my head around your proposal. |
Depends how we want to implement the "configuration watch":
I think you try to implement the first option, but I gave you both options to make sure we are on the same page. |
In a first implementation, the list of available backends should come from DNS ("A" records), as is usual for this kind of workload, especially when considering cloud-native platforms, like Kubernetes. This is also what gRPC client-side load balancing does. A stretch goal would be to have the possibility of reading it from
This is what I'm not getting. Did you mean that the whole collector's configuration would come from an HTTP endpoint? Or just the list of backends? How would the collector's configuration look like in your opinion? |
The |
It means that there's another process, somewhere, calculating and rebalancing which backends will receive each trace ID range? We'd still have the leader election/data sync problem that we have with DNS... A "batteries included" approach would probably make users lives easier in my opinion. If I understood your proposal correctly, the load balancer exporter would have to know details about the underlying exporters, as the "endpoint" is configured differently depending on the exporter. See the Kafka exporter, for example. I think we should really do the easy solution, at least for the first implementation:
Once we have a solution out there, let's see what people need. If they need to recalculate the ring by themselves, we can add support for the endpoint configuration. |
@joe-elliott, @bogdandrutu would you be open to review a proposal as a PR with the ideas that I have in mind? It might be easier to discuss by looking at a concrete code. |
I'm still working on this one, but I have some code to show already: https://github.com/jpkrohling/opentelemetry-collector-contrib/tree/jpkrohling/1724-loadbalancing-processor Each commit in this branch is a step in the direction for the full solution, and I believe it's easier to check them individually.
The code is pretty much complete for a first draft, but I'm yet to run some e2e tests. |
Opened open-telemetry/opentelemetry-collector-contrib#1231 with the proposal. |
I don't know what the bar for merging to contrib is, but I'm confident that @jpkrohling has something with merging if we're talking about it. Grafana is swamped right now preparing for an upcoming conference but if this is in contrib once things settle we will experiment with it and give feedback. |
* Added the backend resolver * Added the metrics definitions **Link to tracking Issue:** Partially solves open-telemetry/opentelemetry-collector#1724, next step after #1349 **Testing:** unit tests **Documentation:** godoc Signed-off-by: Juraci Paixão Kröhling <[email protected]>
…tlp (open-telemetry#1724) * Bump google.golang.org/protobuf from 1.25.0 to 1.26.0 in /exporters/otlp Bumps [google.golang.org/protobuf](https://github.com/protocolbuffers/protobuf-go) from 1.25.0 to 1.26.0. - [Release notes](https://github.com/protocolbuffers/protobuf-go/releases) - [Changelog](https://github.com/protocolbuffers/protobuf-go/blob/master/release.bash) - [Commits](protocolbuffers/protobuf-go@v1.25.0...v1.26.0) Signed-off-by: dependabot[bot] <[email protected]> * Auto-fix go.sum changes in dependent modules Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: MrAlias <[email protected]>
Need
Creating this issue to continue a discussion from the Otel/Agent Collector meeting. We have need for a custom exporter with the following attributes. Our need is specifically for an OTLP exporter, but this could be done in a way that is exporter agnostic.
It appears that there is some work in related areas such as batching by traceid and routing to specific exporters. Our needs, however, are different and I would like to have a discussion about the feasibility of including them in the OTel Collector.
Proposal
We have no specific proposal at the moment. These needs are fairly complex and individual pieces might need specific proposals. This issue exists to discuss if the OTel Collector maintainers are willing to go down this path and next steps to accomplish this.
cc @annanay25 @jpkrohling
The text was updated successfully, but these errors were encountered: