Closed jeqo closed 5 years ago
@adriancole Got a new (old) idea about this concern:
If we use spans
topic instead of traces
topic to source traces, service names stores, then we don't need to be aware of the suppress/windowing issue, as it will be only used by dependencies (this is how it worked at the beginning)
The effect of this decision is that stores will now consume spans
"firehose" topic instead of traces topic, been (much) more intensive on the consumption side.
Would an option like: "storage based on completed spans" make sense? Or which topic will be a good default? probably spans instead of traces?
Anyway, I think this is a good example of how exposing spans and traces as streams enable different store use cases. I can imagine an scenario where we have 2 different store instances, one consuming from spans
with eagerly built view of all traces (supporting debugging purposes), and another instance with a filtered/sampled view of important traces.
wdyt?
if I understand correctly, we should consider if we aren't better off swallowing the complete ingress path? (ex spans aka normal zipkin ListOfSpans?) I think in recent thoughts it is better to have the thing that has firehose do the sampling vs split streams in each app. While incomplete, there are ideas about this here https://github.com/openzipkin/brave/pull/958
In other words, since we have everything here anyway, we probably ought to process everything and the kitchen sink. any processing behind will then get everything we have to offer with least copying (iiuc)
limitation is that you do actually cause more overhead this way. not everything needs complete trace. a scenario with "skeleton trace" could still make sense in-app. OTOH we have to weigh pros/cons and maybe decide which behavior to optimize towards.
will have to check limitations on more detail.
But I'd say for the current refactoring: we will consume "spans" topic directly to source our storage; and let the option to use "traces" topic as experimental via configuration. (this will be the easiest way for early adopters as well IMO)
Traces topic will anyhow be in place as it is required for dependency links counting.
main thing then is to differentiate if "spans" must be different than normal zipkin (ex paritioned on span id). if so, then adding the first layer of that topic should be visible in discussions and flow charts, as migrating all code to partition on trace ID won't be easy in some sites (while certainly would be easy in zipkin-reporter-java as we can read array offset to get traceID)
On Mon, Aug 19, 2019 at 6:51 PM Jorge Quilcate Otoya < notifications@github.com> wrote:
will have to check limitations on more detail.
But I'd say for the current refactoring: we will consume "spans" topic directly to source our storage; and let the option to use "traces" topic as experimental via configuration. (this will be the easiest way for early adopters as well IMO)
Traces topic will anyhow be in place as it is required for dependency links counting.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jeqo/zipkin-storage-kafka/issues/27?email_source=notifications&email_token=AAAPVV33JBZDOGTD4366ISLQFJ3JTA5CNFSM4IMKRKYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4SPWXA#issuecomment-522517340, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAPVVY2UXZHITSE45PNNC3QFJ3JTANCNFSM4IMKRKYA .
sorry easier. not necessarily easy :P
"zipkin" topic used by sender is not partition by span id but with null key, that means batches will be round-robin between partitions. Storage's "span" topic is keyed by trace id.
If we manage to move partitioning by traceId to the kafka-sender, it will remove a lot of overload and simplify the deployment. Then sites with current "zipkin" topic can enable the "KafkaSpanConsumer" here and the ones using the potential kafka-sender with partitioning enabled, could disable it and have only aggregation/stores in place.
PS I hope to create an issue today there to discuss what would need to be changed in order to support this in zipkin-reporter.
"zipkin" topic used by sender is not partition by span id but with null key, that means batches will be round-robin between partitions. Storage's "span" topic is keyed by trace id. sorry you probably pasted this somewhere before, if no where else answering me :) probably needs to be a pinned note heh.
If we manage to move partitioning by traceId to the kafka-sender, it will remove a lot of overload and simplify the deployment. Then sites with current "zipkin" topic can enable the "KafkaSpanConsumer" here and the ones using the potential kafka-sender with partitioning enabled, could disable it and have only aggregation/stores in place.
PS I hope to create an issue today there to discuss what would need to be changed in order to support this in zipkin-reporter. neat.
note depending on how ambitious you are.. in brave we could make a FinishedSpanHandler which not only does paritioning by trace ID, but also closing over by local root. It could be kafka-specific, no problem.
From comments on PR #25
adriancole: how do people normally accommodate this? would a synthetic span heartbeat be a bad idea (that could be disabled)
jeqo: The SO comment mentions that as the stream keeps moving, the windows will get closed eventually, so in a production context you will get "completed" traces constantly.
Another option could be to remove suppress operator conditionally (https://github.com/jeqo/zipkin-storage-kafka/pull/25/files#diff-e1fbe07c15a6efece3ac2119c54eefa7R85) But this will lead every change on the window to be emitted downstream, leading to incorrect results in dependencies stream (e.g. counting same links many times); as well as not helping on reducing the load downstream.
Also windowing timeouts are configurable. I reduce sizes on docker composes to make testing easier.
The idea of a heartbeat sounds interesting; but right now I don't see where it could be plugged: (thinking out loud) we could add another KStraem app consuming from spans and schedule emitting this syntactic span every inactivity gap + 1, and only emit these to the same topic to kick out the supressing in the another streams. Anyhow, will keep this in mind.
Would make sense to position this storage as "eventual consistent" and focus on enable processing, instead of supporting "dev" use-cases like post a trace and query immediately, like mem db?