Skip to content
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.

Sign up for GitHub

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

Allow multiple Inst or resource for export #92

Merged
merged 1 commit into from
Mar 17, 2024
Merged

Allow multiple Inst or resource for export #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 48 additions & 3 deletions lib/opentelemetry/trace/exporter/otlp.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ local function call_collector(exporter, pb_encoded_body)
return false, res_error or "unknown"
end

function _M.export_spans(self, spans)
function _M.encode_spans(self, spans)
assert(spans[1])

local body = {
Expand All @@ -96,12 +96,57 @@ function _M.export_spans(self, spans)
}
}
}
local tracers = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is like tracer_ils_idx_map? 1:1 relationship between tracer and instrumentation library span index?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

local providers = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto here - provider_resource_span_idx_map right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the short names for clarity, but not a blocking opinion for sure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

tracers[spans[1].tracer] = 1
providers[spans[1].tracer.provider] = 1
for _, span in ipairs(spans) do
local rs_idx = providers[span.tracer.provider]
local ils_idx = tracers[span.tracer]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is ils short for here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instrumentation library, maybe this should become il_idx and rs_idx should become r_idx

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh because rs is for resource spans and ils is for instrumentation library spans so this makes sense

if not rs_idx then
rs_idx = #body.resource_spans + 1
ils_idx = 1
providers[span.tracer.provider] = rs_idx
tracers[span.tracer] = ils_idx
table.insert(
body.resource_spans,
{
resource = {
attributes = span.tracer.provider.resource.attrs,
dropped_attributes_count = 0,
},
instrumentation_library_spans = {
{
instrumentation_library = {
name = span.tracer.il.name,
version = span.tracer.il.version,
},
spans = {}
},
},
})
elseif not ils_idx then
ils_idx = #body.resource_spans[rs_idx].instrumentation_library_spans + 1
tracers[span.tracer] = ils_idx
table.insert(
body.resource_spans[rs_idx].instrumentation_library_spans,
{
instrumentation_library = {
name = span.tracer.il.name,
version = span.tracer.il.version,
},
spans = {}
})
end
table.insert(
body.resource_spans[1].instrumentation_library_spans[1].spans,
body.resource_spans[rs_idx].instrumentation_library_spans[ils_idx].spans,
encoder.for_otlp(span))
end
return call_collector(self, pb.encode(body))
return body
end

function _M.export_spans(self, spans)
return call_collector(self, pb.encode(self:encode_spans(spans)))
end

function _M.shutdown(self)
Expand Down
77 changes: 77 additions & 0 deletions spec/trace/exporter/otlp_spec.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,85 @@ local exporter = require "opentelemetry.trace.exporter.otlp"
local client = require "opentelemetry.trace.exporter.http_client"
local context = require "opentelemetry.context"
local tp = Global.get_tracer_provider()
local tracer_provider_new = require("opentelemetry.trace.tracer_provider").new
local tracer = tp:tracer("test")

describe("encode_spans", function()
it("one resource span and one ils for one span", function()
local span
local ctx = context.new()
ctx, span = tracer:start(ctx, "test span")
span:finish()
local cb = exporter.new(nil)
local encoded = cb:encode_spans({span, other_spans})
-- One resource, one il, one span
assert(#encoded.resource_spans == 1)
local resource = encoded.resource_spans[1]
assert(#resource.instrumentation_library_spans == 1)
assert(#resource.instrumentation_library_spans[1].spans == 1)
end)

it("one resource span and one ils for multiple span same tracer", function()
local span
local ctx = context.new()
local spans = {}
for i=10,1,-1 do
ctx, span = tracer:start(ctx, "test span" .. i)
span:finish()
table.insert(spans, span)
end
local cb = exporter.new(nil)
local encoded = cb:encode_spans(spans)
-- One resource, one il, 10 spans
assert(#encoded.resource_spans == 1)
local resource = encoded.resource_spans[1]
assert(#resource.instrumentation_library_spans == 1)
assert(#resource.instrumentation_library_spans[1].spans == 10)
end)

it("one resource span and two ils for spans from distinct tracers", function()
local span
local ctx = context.new()
local spans = {}
ctx, span = tracer:start(ctx, "test span")
span:finish()
table.insert(spans, span)
local other_tracer = tp:tracer("exam")
ctx, other_span = other_tracer:start(ctx, "exam span")
table.insert(spans, other_span)
local cb = exporter.new(nil)
local encoded = cb:encode_spans(spans)
-- One resource, two il, 1 span each
assert(#encoded.resource_spans == 1)
local resource = encoded.resource_spans[1]
assert(#resource.instrumentation_library_spans == 2)
assert(#resource.instrumentation_library_spans[1].spans == 1)
assert(#resource.instrumentation_library_spans[2].spans == 1)
end)
it("distinct trace providers provide distinct resources", function()
local span
local ctx = context.new()
local spans = {}
ctx, span = tracer:start(ctx, "test span")
span:finish()
table.insert(spans, span)
local op = tracer_provider_new(nil, nil)
local other_tracer = op:tracer("exam")
ctx, other_span = other_tracer:start(ctx, "exam span")
table.insert(spans, other_span)
local cb = exporter.new(nil)
local encoded = cb:encode_spans(spans)
-- two resources with one il, 1 span each
assert(#encoded.resource_spans == 2)
local resource = encoded.resource_spans[1]
assert(#resource.instrumentation_library_spans == 1)
assert(#resource.instrumentation_library_spans[1].spans == 1)
resource = encoded.resource_spans[2]
assert(#resource.instrumentation_library_spans == 1)
assert(#resource.instrumentation_library_spans[1].spans == 1)
end)
end)

describe("export_spans", function()
it("invokes do_request when there are no failures", function()
local span
Expand Down
Loading