Add the simplest example for trace API #13
Conversation
Drumato
force-pushed
the
trace-smallest-example
branch
from
6a6c587 to
0da665b
Compare
src/api/trace.zig
Outdated
| pub fn isValid(self: Self) bool { | ||
| return Self.isValidValue(self.value); | ||
| } | ||
|
|
||
| pub fn isValidValue(value: [16]u8) bool { | ||
| for (value) |item| { | ||
| if (item != 0) { | ||
| return true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Q: why do we need two functions?
If it's part of the OTel specification, can you add a comment referencing the specification link that mandates having them?
There was a problem hiding this comment.
isValidValue() is just needed because the check function i need before transforming the raw array to a trace id.
so I will remove it, thanks!
src/api/trace.zig
Outdated
| pub fn isValid(self: Self) bool { | ||
| return Self.isValidValue(self.value); | ||
| } | ||
|
|
||
| pub fn isValidValue(value: [8]u8) bool { | ||
| for (value) |item| { | ||
| if (item != 0) { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } |
src/api/trace.zig
Outdated
| for (value) |item| { | ||
| if (item != 0) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Do I understand correctly that the only invalid value is an array of 16 u8s with all elemets set to 0?
There was a problem hiding this comment.
yes.
the specification about it is defined in https://opentelemetry.io/docs/specs/otel/trace/api/#isvalid.
src/sdk/trace/span_exporter.zig
Outdated
| /// but it is not defined in the OpenTelemetry specification, so for now we don't use it. | ||
| exportSpansFn: *const fn ( | ||
| ctx: *anyopaque, | ||
| spans: []pb.Span, |
There was a problem hiding this comment.
I think I made a mistake in the metrics, see my reasoning in #11.
I am not sure coupling to the protobuf data structures here is a goo idea either.
Ideally, we would have an internal representation of Spans, and then only convert to protobuf in the otlp exporter, which is self-contained.
Making protobuf tyoes part of the interface method here will force us to couple the entire SDK to them, which is not desirable.
WDYT?
There was a problem hiding this comment.
I strongly agree.
I will define another representation of Span.
| }; | ||
| } | ||
|
|
||
| pub fn exportSpans(ctx: *anyopaque, spans: []pb.Span) anyerror!void { |
There was a problem hiding this comment.
What is ctx and what does it represent?
Is it a generic structure that has no well-defined field?
There was a problem hiding this comment.
The *anyopaque indicates that any SpanExporter 's implementor can be the receiver of the exportSpans method.
It is usually used in the standard library.
https://github.com/ziglang/zig/blob/5647dd84c1930a70422fd80a7c2207c81cf858d8/lib/std/mem/Allocator.zig#L14
Signed-off-by: drumato <[email protected]>
Signed-off-by: drumato <[email protected]>
Signed-off-by: drumato <[email protected]>
Signed-off-by: drumato <[email protected]>
Drumato
force-pushed
the
trace-smallest-example
branch
from
2a8dc5a to
18f1079
Compare
Signed-off-by: drumato <[email protected]>
|
The test failure is with MetricReader 🙈 I think that' fixed in my next PR, apologies for the noise |
This PR achieves
Roadmap
std.ArrayList(T))How to implement generic interface
I think the
std.mem.Allocator's style is the best practice.because the Zig users already know how to use it and implement it.
all interfaces in this PR follows the style.