census-instrumentation / opencensus-python

A stats collection and distributed tracing framework
Apache License 2.0
668 stars 250 forks source link

Disable temp file creation if enable_local_storage set to be False #1208

Open liyakun opened 1 year ago

liyakun commented 1 year ago

Describe your environment.

Steps to reproduce. Running application with AzureEventHandler integrated with Python logging, logging configuration file has the following related structure

"azure_event_handler": {
      "level": "INFO",
      "formatter": "info",
      "()": "opencensus.ext.azure.log_exporter.AzureEventHandler",
      "enable_local_storage": 0,
      "storage_path": "not_exist"
}

The reason that I want to disable the local storage is because my container has read only system, any writing should be disabled. However, the checking in https://github.com/census-instrumentation/opencensus-python/blob/3a2d8dfe1db4e0129dc691c35901a0d12127afc1/contrib/opencensus-ext-azure/opencensus/ext/azure/common/__init__.py#L69 does not consider whether enable_local_storage is disable. The root cause is in https://github.com/census-instrumentation/opencensus-python/blob/3a2d8dfe1db4e0129dc691c35901a0d12127afc1/contrib/opencensus-ext-azure/opencensus/ext/azure/log_exporter/__init__.py#L267 which triggers https://github.com/census-instrumentation/opencensus-python/blob/3a2d8dfe1db4e0129dc691c35901a0d12127afc1/contrib/opencensus-ext-azure/opencensus/ext/azure/statsbeat/statsbeat.py#L43 and from here, it calls the process_options function again which will create the temp file.

What is the expected behavior? No temp file should be created if enable_local_storage is set to False.

What is the actual behavior? Temp file created even if enable_local_storage is set to False

jeremydvoss commented 1 year ago

Looking into this.

jeremydvoss commented 1 year ago

An easy workaround for read-only scenarios is to pass in a storage_path. Even though it won't be used, it will prevent the temp folder from being created.

liyakun commented 1 year ago

@jeremydvoss thanks for sharing this workaround, I actually tested it before, but it is not working. The reason basically is that https://github.com/census-instrumentation/opencensus-python/blob/3a2d8dfe1db4e0129dc691c35901a0d12127afc1/contrib/opencensus-ext-azure/opencensus/ext/azure/statsbeat/statsbeat.py#L43 won't pass this storage_path along, only the enable_local_storage is kept.

My issue description seems not really precise, actually once enable_local_storage is checked at https://github.com/census-instrumentation/opencensus-python/blob/3a2d8dfe1db4e0129dc691c35901a0d12127afc1/contrib/opencensus-ext-azure/opencensus/ext/azure/common/__init__.py#L69, we don't really have to worry about the passing of storage_path argument in statsbeat.py.

jeremydvoss commented 1 year ago

I see. Thanks for explaining. OpenCensus has been deprecated. I suggest migrating to our new OpenTelemetry Distro.

In the meantime, since the statsbeat exporter assumes it has the ability to create a temp file, you can try disabling statsbeat but setting the environment variable APPLICATIONINSIGHTS_STATSBEAT_DISABLED_ALL=true