Drop log API include_trace_context parameter

[jack-berg]

Resolves #3394.

[jack-berg]

FYI @martinkuba. If this merges it would supersede #3387. WDYT?

[jack-berg]

FYI @scheler I believe include_trace_context was originally added by you. See my arguments here for its removal. The same accomplished with include_trace_context=false is still possible with this change.

[martinkuba]

FYI @scheler I believe include_trace_context was originally added by you. See my arguments here for its removal. The same accomplished with include_trace_context=false is still possible with this change.

@jack-berg If I am reading this change correctly, the trace context will always be populated (it says MUST). So, if I did not want the trace context populated, I would have to call logger.emit() with empty context parameter. But I think that means that processors would also get empty context? I don't think that's a side effect that we want.

With that said, I don't know if there is a use case for not wanting to set trace context.

[jack-berg]

@martinkuba you can achieve the goal of having LogProcessor#onEmit(Context,LogRecord) have a useful context while LogRecord has empty trace context fields by setting explicit context when calling logger.emit, but first removing the trace context. In java, I'd accomplish this with Context.current().with(Span.getInvalid()).

[martinkuba]

@martinkuba you can achieve the goal of having LogProcessor#onEmit(Context,LogRecord) have a useful context while LogRecord has empty trace context fields by setting explicit context when calling logger.emit, but first removing the trace context. In java, I'd accomplish this with Context.current().with(Span.getInvalid()).

I think that's a good workaround but more inconvenient than having a logger instance that does not add the trace context to any logs it emits. I am just pointing out the difference but do not feel strongly about keeping this configuration.

[scheler]

@jack-berg this is fine. When the logs api was introduced it did not list out the parameters for logger operations explicitly. With the section on implicit context injection, I felt the need for a setting to avoid setting context on the log record.

Now with the explicit Context parameter in logger.emit, is there still a need to have these two sections in the spec?

Implicit Context Injection

Explicit Context Injection

[tigrannajaryan]

Now with the explicit Context parameter in logger.emit, is there still a need to have these two sections in the spec?

Implicit Context Injection

Explicit Context Injection

I think the sections are still necessary to explain how it is supposed to work. This PR makes the necessary changes in those sections.

[jack-berg]

I think that's a good workaround but more inconvenient than having a logger instance that does not add the trace context to any logs it emits.

Luckily its log appenders that call this API so they only need to write the code that excludes trace context context in one place.

I think the sections are still necessary to explain how it is supposed to work. This PR makes the necessary changes in those sections.

I took another look at those sections and wasn't satisfied with how their language had diverged from language we use elsewhere in the document. I rephrased it for consistency.