feat: Update Logger#on_emit to create LogRecords #1611
Conversation
Co-authored-by: James Bunch <fallwith@gmail.com>
kaylareopelle
requested review from
fbogsany,
mwear,
robertlaurin,
dazuma,
ericmustin,
arielvalentin,
ahayworth,
plantfansam and
robbkidd
as code owners
|
👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the |
Previously, trace_id, span_id, and trace_flags were assigned based on the OpenTelemetry::Trace.current_span's SpanContext. The specification requires a context be accepted as an argument on emit. If no context is provided, OpenTelemetry::Context.current will be used. If there isn't a current span on OpenTelemetry::Context.current trace_id, span_id, and trace_flags will be nil unless they are passed as arguments. Also, remaining #emit methods are renamed to #on_emit.
| # | ||
| # @api public | ||
| def emit( | ||
| def on_emit( |
There was a problem hiding this comment.
When we renamed the method in #1517, I forgot to go back and change the references in the API.
This code was leftover and should have been removed. Now, the specfic `trace_id`, `span_id`, and `trace_flags` attributes are used instead.
…lemetry-ruby into logs-sdk-log-record
| @span_id = span_id | ||
| @trace_flags = trace_flags | ||
| @resource = logger&.resource | ||
| @instrumentation_scope = logger&.instrumentation_scope |
There was a problem hiding this comment.
Is it possible for a log record to get created from an api logger? Otherwise I would expect instrumentation scope to always be present (do we need the safe operator here?).
| @trace_id = trace_id | ||
| @span_id = span_id | ||
| @trace_flags = trace_flags | ||
| @resource = logger&.resource |
There was a problem hiding this comment.
I know there's a method added to the sdk logger here to support this but it still reads like logger.logger_provider.resource to me.
I wonder if there's any merit in following the same pattern we did with the tracer https://github.com/open-telemetry/opentelemetry-ruby/blob/main/sdk/lib/opentelemetry/sdk/trace/tracer.rb#L35
| :resource, # optional OpenTelemetry::SDK::Resources::Resource | ||
| :instrumentation_scope, # OpenTelemetry::SDK::InstrumentationScope | ||
| :total_recorded_attributes) do # Integer | ||
| def unix_nano_timestamp |
There was a problem hiding this comment.
nit: is that possible to do alias_method :unix_nano_observed_timestamp, :unix_nano_timestamp?
This PR creates a
LogRecordclass that is read/writeable to start with, and becomes read-only when it transitions toLogRecordData.Additionally, it provides the SDK implementation of the Logs Bridge API
Logger#on_emitmethod to emitLogRecords to the processing pipeline.Relates to the following specs:
Resolves: #1484