Uh oh!
There was an error while loading. Please reload this page.
Conversation
Uh oh!
There was an error while loading. Please reload this page.
lcian
left a comment
There was a problem hiding this comment.
I think the current span is supposed to live on the current scope
And transaction probably on the isolation scope (not 100% sure)
Here we're making a separate storage for it
lcian
commented
Aug 20, 2025
Can we add the example? |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| try jw.objectField("span_id"); | ||
| try jw.write(span_id); | ||
| } | ||
| if (self.parent_span_id) |parent_span_id| { |
There was a problem hiding this comment.
We will need to update the init/deinits for this
| } | ||
| self.spans.deinit(); | ||
| self.allocator.destroy(self); |
There was a problem hiding this comment.
Why are we explicitly calling destroy self here and not in other structs ?
Uh oh!
There was an error while loading. Please reload this page.
| pub fn setClient(client: *SentryClient) void { | ||
| if (getGlobalScope() catch null) |scope| { | ||
| scope.bindClient(client); | ||
| } else { | ||
| std.log.err("Failed to get global scope for setting client", .{}); | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: I think this should return !void instead of failing silently
| const client = scope.getClient() orelse { | ||
| std.log.debug("No client available, cannot start transaction", .{}); | ||
| return null; | ||
| }; |
There was a problem hiding this comment.
Should this return an error instead of null if it fails because no client? It seems like the function returns null if it's sampled
| const client = scope.getClient() orelse { | ||
| std.log.debug("No client available, cannot start transaction from header", .{}); | ||
| return null; | ||
| }; |
There was a problem hiding this comment.
I think this should also error instead of returning null
| .{ .name = "Content-Type", .value = "application/x-sentry-envelope" }, | ||
| .{ .name = "Content-Length", .value = content_length }, | ||
| .{ .name = "X-Sentry-Auth", .value = auth_header }, | ||
| .{ .name = "User-Agent", .value = "sentry-zig/0.1.0" }, |
There was a problem hiding this comment.
We could make this a constant and add it to the auth header too
| pub const TraceId = @import("types/TraceId.zig").TraceId; | ||
| pub const SpanId = @import("types/SpanId.zig").SpanId; | ||
| pub const PropagationContext = @import("types/PropagationContext.zig").PropagationContext; | ||
| pub const Transaction = @import("types/Transaction.zig").Transaction; |
There was a problem hiding this comment.
implement span streaming and span first pls
| // Set transaction name and trace context | ||
| try transaction.setTransactionName(name); | ||
| transaction.trace_id = propagation_context.trace_id; |
There was a problem hiding this comment.
Why not check shouldSample before allocating ?
No description provided.