Stabilize Logs

[jeremydvoss]

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

• [x] Confirm LogRecord emit does not overwrite unspecified timestamp https://github.com/open-telemetry/opentelemetry-specification/issues/3386
• [x] Drop log API include_trace_context parameter https://github.com/open-telemetry/opentelemetry-specification/pull/3397
• [x] Clarify how ObservedTimestamp field is set if unspecified #3385
• [ ] https://github.com/open-telemetry/opentelemetry-specification/pull/3383

Is your feature request related to a problem? Leaving Logs unstable enabled breaking changes like https://github.com/open-telemetry/opentelemetry-python/pull/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 https://github.com/open-telemetry/opentelemetry-python/issues/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: https://github.com/open-telemetry/opentelemetry-python/issues/3193

[jeremydvoss]

Take a look at https://github.com/open-telemetry/opentelemetry-python/pull/3608

[jeremydvoss]

Take a look at https://github.com/open-telemetry/opentelemetry-python/pull/3810