Skip to content
This repository has been archived by the owner. It is now read-only.

Use default appinsights environment variable name #114

Merged
merged 1 commit into from Aug 21, 2017

Conversation

Projects
None yet
3 participants
@c-w
Copy link
Member

commented Aug 21, 2017

As per conversation on Slack: we decide to use a single appinsights account for all services so might as well use the same environment variable name everywhere.

@c-w c-w requested a review from erikschlegel Aug 21, 2017

@erikschlegel
Copy link
Collaborator

left a comment

LGTM

@jcjimenez
Copy link
Contributor

left a comment

LGTM with question

@@ -18,7 +18,7 @@ TRANSLATION_SERVICE_ACCOUNT_KEY="..."
FACEBOOK_AUTH_TOKEN="..."
FORTIS_FEATURE_SERVICE_HOST="..."
FORTIS_SERVICES_APPINSIGHTS_KEY="..."
APPINSIGHTS_INSTRUMENTATIONKEY="..."

This comment has been minimized.

Copy link
@jcjimenez

jcjimenez Aug 21, 2017

Contributor

I noticed that in project-fortis-spark the environment variable is APPLICATION_INSIGHTS_IKEY. Do we have a PR for that project as well? Or should we just make this project use that key?

This comment has been minimized.

Copy link
@c-w

c-w Aug 21, 2017

Author Member

This is amazing. The AppInsights SDKs are inconsistent between each other. Java uses APPLICATION_INSIGHTS_IKEY and all the others use APPINSIGHTS_INSTRUMENTATIONKEY. The latter is recommended, so I think we can stick with that.

@c-w c-w merged commit fba2222 into master Aug 21, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@c-w c-w deleted the appinsights-env-var branch Aug 21, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.