Stabilize Logs

[jeremydvoss]

After confirming all of the following have been taken care of, I think we can stabilized logs. Got these from open-telemetry/opentelemetry-specification#2911

• Confirm LogRecord emit does not overwrite unspecified timestamp Should "Emit a LogRecord" API set Timestamp if unspecified? opentelemetry-specification#3386
• Drop log API include_trace_context parameter Drop log API include_trace_context parameter opentelemetry-specification#3397
• Clarify how ObservedTimestamp field is set if unspecified #3385
• Clarify "Emit a LogRecord" API opentelemetry-specification#3383

Is your feature request related to a problem?
Leaving Logs unstable enabled breaking changes like #2943

Describe the solution you'd like
With the logging spec stable, we should confirm we have everything we need and stabilize logs as soon as possible.

[jeremydvoss]

For log records, here are some initial links:
https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py#L492
https://github.com/open-telemetry/opentelemetry-python/blob/main/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py#L530

Need to confirm logData does no defaulting.

[jeremydvoss]

Confirmed emit does not seem to default timestamp. This also means that trying to export that LogRecord to console fails because it cannot format timestamp. LogRecord.to_json() does not expect None. EX:

from opentelemetry.sdk.logs.export import ConsoleLogExporter, BatchLogRecordProcessor

lr = LogRecord()
print("timestamp: %s" % lr.timestamp)

# logger = Logger()
lp = LoggerProvider()
lp.add_log_record_processor(BatchLogRecordProcessor(ConsoleLogExporter()))
logger = lp.get_logger(__name__)

logger.emit(lr)

[jeremydvoss]

Looking into include_trace_context, it does not seem python has this to begin with. LoggingHandler.emit calls LoggingHandler._translate which uses get_current_span().get_span_context(). This may be an optional feature for if we want people to be able to EXPLICATELY set the context of a logger.

Related question: should a log's trace context fields be populated if include_trace_context=false and context is explicitly passed? I think yes. However, for languages that rely on explicit context because no implicit context is available, there's no point to having the include_trace_context parameter since it doesn't change any behavior
https://github.com/open-telemetry/opentelemetry-specification/pull/3387/files

[jeremydvoss]

Here is the observed_timestamp field

[jeremydvoss]

Seems we are NOT defaulting observed_timestamp to current time. We could change this in the sdk...

class LogRecord(APILogRecord):
    def __init__(
        ...
    ):
        if not observed_timestamp:
            observed_timestamp = int(time.time() * 1e9)
        super().__init__(
            ...

... or in the api:

class LogRecord(ABC):

    def __init__(
        ...
    ):
        self.timestamp = timestamp
        if observed_timestamp:
            self.observed_timestamp = observed_timestamp
        else:
            self.observed_timestamp = int(time.time() * 1e9)
        self.trace_id = trace_id
        ...

I am getting int(time.time() * 1e9) from how to translate native logs

[jeremydvoss]

Use time.time_ns()

[jenshnielsen]

Perhaps it would be worth including a fix for #3353 before stabilizing. This might not technically be a breaking change but it does significantly change the messages received

[lzchen]

@jenshnielsen

I don't believe the issue exists within the SDK itself so it wouldn't be a blocker.

[lzchen]

Might be another issue: #3193

[jeremydvoss]

Take a look at #3608

[jeremydvoss]

Take a look at #3810