search

census-instrumentation / opencensus-python

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

Allow to configure the cloud role parameter #1052

Open se02035 opened 3 years ago

se02035 commented 3 years ago

It is unclear how to set the cloud role value for a log record. According to https://github.com/census-instrumentation/opencensus-python/blob/5ad2a7cc62fc9663d186e15636a7b0306c9a8d7a/contrib/opencensus-ext-azure/opencensus/ext/azure/common/utils.py#L27 the cloud is always 'ai.cloud.role': os.path.basename(sys.argv[0]) or 'Python Application' This is a problem when multiple applications send traces to a single AppInsights instance (and have the same entry point e.g. main.py)

A correct cloud role name is essential for Azure Application Insights Application Map. In order to use another role name (which is likely true for most of the cases) a custom callback has be used to update the role name. It would be better to have an parameter on "Options" https://github.com/census-instrumentation/opencensus-python/blob/1ca601584cbbd3b6019bddb9c7120c069ff4f819/contrib/opencensus-ext-azure/opencensus/ext/azure/common/__init__.py#L104 to set the cloud role (and instance) and pass use that value when creating the log/trace records - similar to the instrumentation key/connection string.

lzchen commented 3 years ago

You can modify this field via telemetry processors

se02035 commented 3 years ago

Yeah, I know but wouldn't it be nicer/cleaner to have a env var here? I mean I see a couple of benefits:

  1. other AI SDKs also allow to set the cloud role name via an env var.
  2. Similar approach is used for passing the AI instrumentation key
  3. the telemetry processor has to be configured for every AzureExporter/AzureLogHandler instance. E..g you need to do something like this (when using Azure Functions):
    • OpenCensusExtension._exporter.add_telemetry_processor(..) plus use the same for other AzureLogHandler (if available).
lzchen commented 3 years ago

@se02035

Those are excellent points and I agree with it. Feel free to open a PR and I will review it :)

se02035 commented 3 years ago

@lzchen , created a PR for allowing devs to set the cloud role name via an envvar. pls see https://github.com/census-instrumentation/opencensus-python/pull/1061