search

DataDog / pg_tracing

Distributed Tracing for PostgreSQL
MIT License
22 stars 2 forks source link

Question: redundant computation of span_id in TracedPlanstate #34

Closed cwang9208 closed 2 weeks ago

cwang9208 commented 2 weeks ago

Hi @bonnefoa , I'm recently reading through the code and would love to contribute to this project.

However, the logic in pg_tracing_planstate.c is quite complicated. I noticed that you override the ExecProcNode function to record the start time of each node. Besides a span_id is alse generated for each node in ExecProcNodeFirstPgTracing. But at the end of the query, when you call create_spans_from_planstate to walk through the planstate tree, you compute a random span id for each node again (line 439 in file pg_tracing_planstate.c).

I'm just wondering why don't we just reuse the span ids generated in traced_planstate?

Thanks for your time.

bonnefoa commented 2 weeks ago

Hey,

However, the logic in pg_tracing_planstate.c is quite complicated.

Yeah, it's definitely a bit of a hack but that's the only way I've found to be able to get the start of a node (node instrumentation only provides a duration).

But at the end of the query, when you call create_spans_from_planstate to walk through the planstate tree, you compute a random span id for each node again (line 439 in file pg_tracing_planstate.c).

I'm just wondering why don't we just reuse the span ids generated in traced_planstate?

Good point, the span_id from the traced_planstate should be usable. I will push a fix for that.