census-instrumentation / opencensus-python

A stats collection and distributed tracing framework
Apache License 2.0
669 stars 250 forks source link

Store core span properties in their raw forms #106

Open thefotios opened 6 years ago

thefotios commented 6 years ago

(Somewhat related to #105)

I would like to propose that some of the span properties (like start_time/end_time, span_id/trace_id) are stored in a more canonical format (eg, raw datetime.datetime and uuid.UUID respectively) inside the Span object. We can then defer the formatting of this value to whichever exporter is being used.

For example, right now start_time undergoes the following transformations when using the ZipkinExporter:

If we stored it as a raw datetime.datetime object, we could simply convert it to an int when reporting and subtract. It would be simple to add the conversion to ISO format when reporting for StackDriver as well. I'm sure the core classes were designed with StackDriver as the first class citizen, but some changes like this would make for easier addition of other Exporters.

Another possible solution is to store the start_time and end_time separately from the duration. This could allow us to use something higher res like time.perf_counter() (at least in Python3) for calculating the duration.

liyanhui1228 commented 6 years ago

Hi @thefotios, yeah that's correct the core classes were originally designed with StackDriver, so there are some conversions back and forth in other exporters. And storing the start_time and end_time as raw datetime.datetime object sounds reasonable, but then we will need to convert it to str or int in each exporter. As for the span_id/trace_id, I would prefer leave it as it is for now. Because the probability_sampler is using the trace_id to make the sampling decision and we will need to convert trace_id to str in the sampler if we store it as the raw format.

thefotios-enigma commented 6 years ago

Sorry for the delay in the response, work responsibilities pulled me in a few different directions. I'll see if I can take a crack at this during the week. Converting the datetime stuff shouldn't be that hard and it will probably make it easier in the long run for other converters/formatters.

As for the UUID, I agree it might reach a little deeper into the system than I first realized. I'll keep them separate so we can get the dates standardized first then maybe l'll look deeper into the UUID stuff.