-
Notifications
You must be signed in to change notification settings - Fork 7
Proposal: More flexible Logger.emit #155
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.
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
Changes from 2 commits
8cd2ca5
ed6788a
ae90f45
efa2e97
4818632
74f8422
9131040
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ const Attributes = @import("../../attributes.zig").Attributes; | |
| const InstrumentationScope = @import("../../scope.zig").InstrumentationScope; | ||
| const Context = @import("../context/context.zig").Context; | ||
| const EnabledParameters = @import("enabled_parameters.zig").EnabledParameters; | ||
| const trace = @import("../trace.zig"); | ||
|
|
||
| // Import configuration module | ||
| const Configuration = @import("../../sdk/config.zig").Configuration; | ||
|
|
@@ -290,37 +291,49 @@ pub const Logger = struct { | |
| self.allocator.destroy(self); | ||
| } | ||
|
|
||
| /// Emit a log record | ||
| pub const EmitOptions = struct { | ||
| /// Human-readable severity label (e.g. "WARN", "CRITICAL"). | ||
| /// Optional: backends can derive a standard label from `severity_number`. | ||
| /// Useful when bridging from a logging system that has its own level names. | ||
| severity_text: ?[]const u8 = null, | ||
|
|
||
| /// Key-value pairs attached to this log record. | ||
| attributes: ?[]const Attribute = null, | ||
|
|
||
| /// Span context to correlate this log record with an active trace. | ||
| /// Pass `span.span_context` to enable log-trace correlation in the backend. | ||
| span_context: ?trace.SpanContext = null, | ||
| }; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please add all optional parameters dicated by the API spec of I like the dedicated struct for options, it's also wide-spread practice in Zig codebases, so we can have all fields in the struct.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest to add for now Timestamp and Observed Timestamp which are actually optional and already supported by |
||
|
|
||
| /// Emit a log record. | ||
| pub fn emit( | ||
| self: *Self, | ||
| severity_number: ?u8, | ||
| severity_text: ?[]const u8, | ||
| body: ?[]const u8, | ||
| attributes: ?[]const Attribute, | ||
| severity_number: u8, | ||
| body: []const u8, | ||
| options: EmitOptions, | ||
|
inge4pres marked this conversation as resolved.
Outdated
|
||
| ) void { | ||
| if (self.provider.sdk_disabled or self.provider.is_shutdown.load(.acquire)) { | ||
| return; | ||
| } | ||
|
|
||
| // Create ReadWriteLogRecord | ||
| var log_record = ReadWriteLogRecord.init(self.scope); | ||
| defer log_record.deinit(self.allocator); | ||
|
|
||
| log_record.severity_number = severity_number; | ||
| log_record.severity_text = severity_text; | ||
| log_record.severity_text = options.severity_text; | ||
| log_record.body = body; | ||
| log_record.resource = self.provider.resource; | ||
| log_record.trace_id = if (options.span_context) |sc| sc.trace_id.toBinary() else null; | ||
| log_record.span_id = if (options.span_context) |sc| sc.span_id.toBinary() else null; | ||
|
|
||
| // Add attributes if provided | ||
| if (attributes) |attrs| { | ||
| if (options.attributes) |attrs| { | ||
| for (attrs) |attr| { | ||
| log_record.setAttribute(self.allocator, attr) catch |err| { | ||
| std.log.err("Failed to add attribute to log record: {}", .{err}); | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| // Call processors in order | ||
| const ctx = Context.init(); | ||
| self.provider.mutex.lockUncancelable(self.provider.io); | ||
| defer self.provider.mutex.unlock(self.provider.io); | ||
|
|
@@ -337,7 +350,7 @@ pub const Logger = struct { | |
| /// ```zig | ||
| /// if (logger.enabled(.{ .context = ctx, .severity = 9 })) { | ||
| /// const expensive_data = computeExpensiveDebugInfo(); | ||
| /// logger.emit(9, "INFO", expensive_data, null); | ||
| /// logger.emit(9, expensive_data, .{}); | ||
| /// } | ||
| /// ``` | ||
| /// | ||
|
|
@@ -443,7 +456,7 @@ test "LoggerProvider with processor" { | |
| const logger = try provider.getLogger(scope); | ||
|
|
||
| // Emit a log | ||
| logger.emit(9, "INFO", "test message", null); | ||
| logger.emit(9, "test message", .{ .severity_text = "INFO" }); | ||
|
|
||
| // Verify export was called | ||
| try std.testing.expectEqual(@as(usize, 1), mock_exporter.export_count); | ||
|
|
@@ -522,7 +535,7 @@ test "Logger log records inherit resource from provider" { | |
| const logger = try provider.getLogger(scope); | ||
|
|
||
| // Emit a log | ||
| logger.emit(9, "INFO", "test message", null); | ||
| logger.emit(9, "test message", .{ .severity_text = "INFO" }); | ||
|
|
||
| // Verify resource was passed to the log record | ||
| try std.testing.expect(mock_exporter.captured_resource != null); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.