Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default value for ParseStateValues on OpenTelemetryLoggerOptions #4213

Closed
vishweshbankwar opened this issue Feb 21, 2023 · 3 comments
Closed
Assignees
Labels
enhancement New feature or request logs Logging signal related
Milestone

Comments

@vishweshbankwar
Copy link
Member

vishweshbankwar commented Feb 21, 2023

OTLP exporter sets this to true by default. Should we consider updating the default value on options to true? Any user who wants to disable it can do so by setting it false.

@CodeBlanch
Copy link
Member

I did some redesign on #3677 to try and make it more user friendly but it didn't end up making 1.4. I'm hoping to bring it back when we do the upcoming log updates. Specifically regarding ParseStateValues this is what I was planning:

State + StateValues + ParseStateValues

These things evolved to be strange. Exporters are having to check StateValues & State and some are forcing ParseStateValues = true. State itself is not safe to access beyond the log lifecycle. I tweaked things so there is a more deterministic behavior. If TState is IReadOnlyList<KeyValuePair<string, object>> (or IEnumerable<> equivalent) than LogRecord.Attributes will now always be populated. That will cover most messages written through ILogger using the source generator or the extensions. The only way to pass something that doesn't meet that requirement is calling ILogger.Log(...) directly. In that case if ParseStateValues = true than we will build Attributes dynamically using reflection.

This allows for the deprecation of LogRecord.State. Exporters should now be able to look at LogRecord.Attributes for everything and get a nice and consistent behavior.

If users don't care for export of attributes at all there is now an option IncludeState to turn off all operations against TState.

@CodeBlanch CodeBlanch self-assigned this Feb 22, 2023
@CodeBlanch CodeBlanch added this to the 1.6.0 milestone Feb 22, 2023
@vishweshbankwar
Copy link
Member Author

@CodeBlanch Can we close this one as part of #4334

@CodeBlanch
Copy link
Member

Yes thanks for the reminder @vishweshbankwar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request logs Logging signal related
Projects
None yet
Development

No branches or pull requests

3 participants