Azure / azure-sdk-for-python

This repository is for active development of the Azure SDK for Python. For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/python/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-python.
MIT License
4.37k stars 2.71k forks source link

Make `BaseExporter` fork-safe. #36364

Open avramdj opened 4 days ago

avramdj commented 4 days ago

Is your feature request related to a problem? Please describe. AzureMonitorLogExporter gives lots of Envelopes could not be exported and are not retryable when logging from lots of forks.

Describe the solution you'd like Make BaseExporter fork-safe by having an os.register_at_fork(after_in_child=...) hook. Maybe in BaseExporter.__init__ so it actually changes the state of the object by giving it another client connection.

Describe alternatives you've considered Currently, I'm solving this by reinitializing my entire opentelemetry context (tracer provider, logger provider, exporters, handlers) on forks, because they all depend on the exporter which has to be reinitialized.

Additional context I could be wrong about the mechanics of what causes the issue for me, but it feels like BaseExporters connection (or any other unsafe resource) to azure is being shared among forks. This would explain why the problem disappears for me when I reinitialize it every time.

pvaneck commented 3 days ago

Hey, thanks for the issue and explanation.

@lzchen @jeremydvoss Any ideas here?

github-actions[bot] commented 3 days ago

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @hectorhdzg @jeremydvoss @lzchen.

lzchen commented 15 hours ago

Make BaseExporter fork-safe

I am not sure if there is anything about the exporter that isn't "fork safe". Our recommendation to users is to user name-spaced loggers so the logging calls emitted by the exporter are not captured as telemetry and exported (causing recursion and a tremendous amount of logs). If you are receiving Envelopes could not be exported and are not retryable logging statements and have configured the LoggingHandler to be attached to your root logger, this is actually expected behavior.

avramdj commented 14 hours ago

Make BaseExporter fork-safe

I am not sure if there is anything about the exporter that isn't "fork safe". Our recommendation to users is to user name-spaced loggers so the logging calls emitted by the exporter are not captured as telemetry and exported (causing recursion and a tremendous amount of logs). If you are receiving Envelopes could not be exported and are not retryable logging statements and have configured the LoggingHandler to be attached to your root logger, this is actually expected behavior.

Yeah the handler is attached to a namespaced logger, not root. My only workaround that doesn't cause Envelopes could not be exported and are not retryable was to reinit the exporter in the fork.

If anyone has ideas regarding what I can do to provide more info on the issue, I'm willing to do it.

lzchen commented 11 hours ago

@avramdj

Interesting, could you share a snippet of your code in how you instrument with the exporter? Also, does this behavior occur if you are not using forks? What library are you using to generate this behavior?