Implement Connection Strings for Azure Exporters #767
Conversation
lzchen
changed the title
contrib/opencensus-ext-azure/opencensus/ext/azure/common/__init__.py
Outdated
Show resolved
Hide resolved
contrib/opencensus-ext-azure/opencensus/ext/azure/common/__init__.py
Outdated
Show resolved
Hide resolved
| endpoint = code_cs.get(INGESTION_ENDPOINT) \ | ||
| or env_cs.get(INGESTION_ENDPOINT) \ | ||
| or DEFAULT_BREEZE_ENDPOINT | ||
| options.endpoint = endpoint + '/v2/track' |
There was a problem hiding this comment.
If this endpoint encoded?
Do we do canonicalization?
There was a problem hiding this comment.
Can you explain more about how the endpoint would be encoded?
There was a problem hiding this comment.
For example, do you expect the endpoint to be something like https://zh.wikipedia.org/wiki/%E6%A3%89%E5%B0%BE%E5%85%94%E5%B1%9E ?
There was a problem hiding this comment.
I do not believe that an encoded endpoint would be intentionally passed in. According to Mothra, none of the current endpoint configurations handle strings in any special way, so I believe it's safe to leave the url string as is.
contrib/opencensus-ext-azure/opencensus/ext/azure/common/__init__.py
Outdated
Show resolved
Hide resolved
contrib/opencensus-ext-azure/opencensus/ext/azure/common/__init__.py
Outdated
Show resolved
Hide resolved
| if result.get(INGESTION_ENDPOINT) is None: | ||
| endpoint_suffix = '' | ||
| location_prefix = '' | ||
| suffix = result.get('EndpointSuffix') |
There was a problem hiding this comment.
I think the keys are supposed to be case-insensitive. Probably not a blocker since I don't see this actually being tested in other SDKs, but it could lead to some confusing errors for users that manually modify their conn strs 🤔
There was a problem hiding this comment.
Good point. It'll be safer to be case-insensitive.
| * Install the `logging integration package <../opencensus-ext-logging>`_ using ``pip install opencensus-ext-logging``. | ||
| * Put the instrumentation key in ``APPINSIGHTS_INSTRUMENTATIONKEY`` environment variable. | ||
| * You can also specify the instrumentation key explicitly in the code, which will take priority over a set environment variable. | ||
| * Place your instrumentation key in a connection string and into ``APPLICATIONINSIGHTS_CONNECTION_STRING`` environment variable. |
There was a problem hiding this comment.
Need a link to the definition of connection string.
There was a problem hiding this comment.
Good point. I included the connection string in the example code snippet, which is the bare minimum that a user needs to get started (so it is simple). The more formal definition will be included in the TSG for CSS engineers. This will help them troubleshoot any irregularities found with a user's connection string.
| * Put the instrumentation key in ``APPINSIGHTS_INSTRUMENTATIONKEY`` environment variable. | ||
| * You can also specify the instrumentation key explicitly in the code, which will take priority over a set environment variable. | ||
| * Place your instrumentation key in a connection string and into ``APPLICATIONINSIGHTS_CONNECTION_STRING`` environment variable. | ||
| * You can also put the instrumentation key in ``APPINSIGHTS_INSTRUMENTATIONKEY`` environment variable. |
There was a problem hiding this comment.
Do we want to keep this in the doc? I guess we want to remove it from the doc, and leave the logic in the codebase until we decided to bump major version.
There was a problem hiding this comment.
As discussed, we should probably remove the method of instrumentation_key from the docs, as we do not want to keep supporting legacy methods. As well, having multiple ways to instrument is confusing to the user.
Implements connection strings as defined by the spec.
We allow the user to provide an instrumentation_key value and connection_string value explicitly, as well as have the environment variable counterparts to these.
Connection strings currently provide another way to derive the instrumentation key as well as define an ingestion endpoint.
The priority for instrumentation key:
The priority for endpoint:
TODO:
Link to documentation for connection strings when created