Allow multiple Inst or resource for export #92
Conversation
rdooley
commented
Mar 13, 2024
rdooley
commented
- Currently if 2 tracer providers or 2 tracers are exporting, there is a first one in batch wins condition for exporting spans
- This change moves encoding the body into its own function, and adds unittests for spans from different tracers and from different trace providers to ensure resource and il parts of the otlp export are set properly
| providers[spans[1].tracer.provider] = 1 | ||
| for i, span in ipairs(spans) do | ||
| local rs_idx = providers[span.tracer.provider] | ||
| local ils_idx = tracers[span.tracer] |
There was a problem hiding this comment.
what is ils short for here?
There was a problem hiding this comment.
instrumentation library, maybe this should become il_idx and rs_idx should become r_idx
There was a problem hiding this comment.
oh because rs is for resource spans and ils is for instrumentation library spans so this makes sense
* Currently if 2 tracer providers or 2 tracers are exporting, there is a first one in batch wins condition for exporting spans * This change moves encoding the body into its own function, and adds unittests for spans from different tracers and from different trace providers to ensure resource and il parts of the otlp export are set properly
| @@ -96,12 +96,57 @@ function _M.export_spans(self, spans) | |||
| } | |||
| } | |||
| } | |||
| local tracers = {} | |||
There was a problem hiding this comment.
I think this is like tracer_ils_idx_map? 1:1 relationship between tracer and instrumentation library span index?
There was a problem hiding this comment.
this is the set of all tracers in the incomoing set of spans, and the value stored is the index of that span's il on the ils of the span's provider's resource
| @@ -96,12 +96,57 @@ function _M.export_spans(self, spans) | |||
| } | |||
| } | |||
| } | |||
| local tracers = {} | |||
| local providers = {} | |||
There was a problem hiding this comment.
Ditto here - provider_resource_span_idx_map right?
There was a problem hiding this comment.
I like the short names for clarity, but not a blocking opinion for sure
There was a problem hiding this comment.
ln 103-104 make pretty clear how this is being used in my mind which is already polluted with knowledge of the underlying otlp proto structure
