Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use WAS-DEFAULT-HOSTNAME HttpHeader to get slot name #1368

Closed
debugthings opened this issue May 24, 2019 · 20 comments
Closed

Use WAS-DEFAULT-HOSTNAME HttpHeader to get slot name #1368

debugthings opened this issue May 24, 2019 · 20 comments
Milestone

Comments

@debugthings
Copy link

Issue

Currently we use the WEBSITE_HOSTNAME to determine the App Service slot the code is deployed to.

This environment variable is passed in upon process creation by the App Service sandbox and is not updated. This requires a reboot of the App Service to get the correct value.

Result

The result of this is telemetry will have an incorrect cloud_RoleName until this happens which can skew customer metrics.

Proposal

The proposed fix is to read the WAS-DEFAULT-HOSTNAME http header instead which is sent by the App Service front end (ARR) servers and will always have the correct app service host name.

@cijothomas
Copy link
Contributor

@debugthings
Copy link
Author

Other references: microsoft/ApplicationInsights-dotnet-server#872, microsoft/ApplicationInsights-dotnet-server#871, microsoft/ApplicationInsights-dotnet-server#314

@lmolkova
Copy link
Member

We need to think about worker roles and telemetry reported in background threads.

@cijothomas
Copy link
Contributor

  1. Yes we need a way to solve this for worker roles. But it can be a separate enhancement than this.
  2. If we use the same TI as before, but modify it to periodically update the slot name from Request headers, then it should automatically benefit telemetry reported from background thread as well. (Assuming that this enhancement will be implemented NOT by reading incoming headers in every request, but once every min or so)

@aarondandy
Copy link

aarondandy commented Jul 24, 2019

How would this work for something using IHostedService? Would telemetry from it just use the cloud role name from the last HTTP request, assuming it would be cached? I know it won't be a common case but it could be a strange corner case and possibly complicated further with percentage based traffic allocations.

@lmolkova
Copy link
Member

Pushing it to the next milestone as this is a relatively big change and we don't have the bandwidth to take it in this cycle.

@cijothomas
Copy link
Contributor

Update:
We got confirmation from AppService team that was-default-hostname is correct way to get the role name we want, and its guaranteed to be correct.

There is no update yet on how to get this on webjobs or background jobs, so they'll continue to rely on the env variable.

@TimothyMothra TimothyMothra transferred this issue from microsoft/ApplicationInsights-dotnet-server Dec 4, 2019
@Expecho
Copy link
Contributor

Expecho commented May 12, 2020

Update:
We got confirmation from AppService team that was-default-hostname is correct way to get the role name we want, and its guaranteed to be correct.

There is no update yet on how to get this on webjobs or background jobs, so they'll continue to rely on the env variable.

I stumbled upon this issue in our web apps as well. What is the status of this issue? At least we can get it right for the web apps, and create a seperate issue for the webjobs?

@cijothomas
Copy link
Contributor

Update:
We got confirmation from AppService team that was-default-hostname is correct way to get the role name we want, and its guaranteed to be correct.
There is no update yet on how to get this on webjobs or background jobs, so they'll continue to rely on the env variable.

I stumbled upon this issue in our web apps as well. What is the status of this issue? At least we can get it right for the web apps, and create a seperate issue for the webjobs?

We have this fixed already for web apps, as they can rely on hostname header. For webjobs, there is no request or header, so not sure how to fix it there. So its unfixed for WebJobs and there are no clear guidance on how to fix it as well. (We can close this and open a new issue just to track this for webjobs)

@Expecho
Copy link
Contributor

Expecho commented May 15, 2020

... So its unfixed for WebJobs and there are no clear guidance on how to fix it as well. (We can close this and open a new issue just to track this for webjobs)

Maybe using the newly announced Event Grid events the environment variable can be updated, it might solve the issue as defined in this Issue:

Currently we use the WEBSITE_HOSTNAME to determine the App Service slot the code is deployed to.

This environment variable is passed in upon process creation by the App Service sandbox and is not updated. This requires a reboot of the App Service to get the correct value.

@cijothomas
Copy link
Contributor

... So its unfixed for WebJobs and there are no clear guidance on how to fix it as well. (We can close this and open a new issue just to track this for webjobs)

Maybe using the newly announced Event Grid events the environment variable can be updated, it might solve the issue as defined in this Issue:

Currently we use the WEBSITE_HOSTNAME to determine the App Service slot the code is deployed to.

This environment variable is passed in upon process creation by the App Service sandbox and is not updated. This requires a reboot of the App Service to get the correct value.

Looks promising.

One of the thing we are trying to avoid is SDKs having deep knowledge about all the Azure platforms - and instead have all Azure environments expose the information is a standardized way, so SDKs don't have to have specialized solution for each compute platform. (There was not enough bandwidth to pursue this, so I expect this issue to be unresolved for short-term)

@cijothomas
Copy link
Contributor

@Expecho If you use the EventGrid approach to solve you issue, please post back the approach/sample code, so other customers can leverage it, until SDK itself gets this capability (which will take some time)

@Expecho
Copy link
Contributor

Expecho commented May 18, 2020

@Expecho If you use the EventGrid approach to solve you issue, please post back the approach/sample code, so other customers can leverage it, until SDK itself gets this capability (which will take some time)

To be honest, we do not use webjobs so that part is not affecting us. So I won't probably use the EventGrid approach myself but it could be interesting for future readers of this issue. If we do have the need we will post a sample.

@IgnacioAlvarezArenas
Copy link

IgnacioAlvarezArenas commented Aug 12, 2020

Hi all. I was wondering if this issue was actually resolved for App Services. Let me explain my situation:

I have two slots, the production and another called csharp in my App Service called ignacio app. I have Agent Based App Insights so it is running these versions:

image

When I run this query the following happens:
requests
| where timestamp >= datetime(2020-08-05 08:00:00) and timestamp <= datetime(2020-08-05 09:20:00)
| where url !contains "csharp"
| project timestamp,cloud_RoleName,url
| order by timestamp

image

but then I do the swap:
image

So as you can see, the URL remains the same (www2.ignacioapps.com) but the Cloud role name changed. I restarted the App Service (which is also implicitly done by the swap).

Can you guys confirm this is the expected behaviour? I see in previous comments that the issue was fixed for webapps, but it seems that the Agent based App Insights client is old. In which version exactly the issue has been fixed?

@cijothomas cijothomas added this to the 2.12 milestone Aug 12, 2020
@cijothomas
Copy link
Contributor

Its fixed in 2.12 version of the SDK.
Are you using Asp.Net or Asp.Net Core in agent based enablement?

@IgnacioAlvarezArenas
Copy link

IgnacioAlvarezArenas commented Aug 12, 2020 via email

@cijothomas
Copy link
Contributor

@IgnacioAlvarezArenas Asp.Net agent is on an old version of SDK. You need to use SDK based approach to benefit from this.

@IgnacioAlvarezArenas
Copy link

IgnacioAlvarezArenas commented Aug 12, 2020 via email

@cijothomas
Copy link
Contributor

Asp.Net Core agent is somewhat new. I don't know exact version, but I think its newer than 2.12 so you will be fine in Asp.Net core.
(You can verify sdk version from any telemetry field to confirm which version is used by the agent)

@IgnacioAlvarezArenas
Copy link

Thanks for the clarification Cijo, really helpful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants