Drop log API include_trace_context parameter#3397
Drop log API include_trace_context parameter#3397jack-berg merged 5 commits intoopen-telemetry:mainfrom
Conversation
|
FYI @martinkuba. If this merges it would supersede #3387. WDYT? |
@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. |
|
@martinkuba you can achieve the goal of having |
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. |
|
@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? |
I think the sections are still necessary to explain how it is supposed to work. This PR makes the necessary changes in those sections. |
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 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. |
Resolves #3394.