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

[Logs] ObjectDisposedException when using ParseStateValues #2905

Closed
glucaci opened this issue Feb 16, 2022 · 4 comments · Fixed by #3385
Closed

[Logs] ObjectDisposedException when using ParseStateValues #2905

glucaci opened this issue Feb 16, 2022 · 4 comments · Fixed by #3385
Assignees
Labels
bug Something isn't working logs Logging signal related pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package

Comments

@glucaci
Copy link
Contributor

glucaci commented Feb 16, 2022

When using ParseStateValues option for logging the app crash

Unhandled exception. System.ObjectDisposedException: IFeatureCollection has been disposed.
Object name: 'Collection'.

image

  • TargetFramework: net6.0
  • "OpenTelemetry.Exporter.OpenTelemetryProtocol.Logs" Version="1.0.0-rc9"
  • "OpenTelemetry.Instrumentation.AspNetCore" Version="1.0.0-rc9"
@cijothomas
Copy link
Member

Quick note on root cause:

  1. OTel is storing the state from ILogger.Log into LogRecord.
  2. When iterating over "state", if the state is tied to context, which is already disposed, we'll get this exception.
  3. In this particular case, the HttpRequest is powering the state as shown here: https://github.com/dotnet/aspnetcore/blob/c85baf8db0c72ae8e68643029d514b2e737c9fae/src/Hosting/Hosting/src/Internal/HostingRequestStartingLog.cs#L23-L35
  4. When used in BatchProcessor, where Request is already gone, this throws.

@CodeBlanch fixed the issue for Scopes, by copying the Scope, before we go out of the Logging scope. We need to do something similar.

Irrespective of the above, OTLP LogExporter should not throw and cause app crash - this is a quick fix and can be addressed in coming release. The fix for the above issue (with accessing state after the "state" is disposed) need more time.

@reyang reyang added pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package and removed pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package labels Feb 17, 2022
@glucaci
Copy link
Contributor Author

glucaci commented Feb 17, 2022

Thanks for update.

When will be a new release? A new RC will be nice, because are already 2 crashing bugs for Logs.

@cijothomas
Copy link
Member

Waiting for the 3rd crash 😆 ...
Not really - There will be a release in ~1 week, as soon as some OTLP Configuration (metrics related) changes are merged. And of course the crash bug needs to be fixed as well.

(Release process is still manual, so want to do one when there is a reasonable amount of payload.)

@cijothomas
Copy link
Member

The fix for the above issue (with accessing state after the "state" is disposed) need more time.

The fix for the above issue (with accessing state after the "state" is disposed) need more time.

^ @CodeBlanch agreed to look at addressing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working logs Logging signal related pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package
Projects
None yet
4 participants