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

v2.1.0-beta2 breaks WindowsAzure.Storage CreateIfNotExistsAsync #416

Closed
ksstott opened this Issue Apr 30, 2017 · 15 comments

Comments

Projects
None yet
9 participants
@ksstott
Copy link

ksstott commented Apr 30, 2017

Not sure if this is the correct repo to raise this issue however i reasonably certain that some code in the Application insights is the culprit.

When upgrading from version 2.0.0 of package Microsoft.ApplicationInsights.AspNetCore to version v2.1.0-beta2 calls to the CreateIfNotExistsAsync method on the CloudQueue object in WindowsAzure.Storage. Start failing with the following error.

Microsoft.WindowsAzure.Storage.StorageException: Server failed to authenticate the request. Make sure the value of Authorization header is formed correctly including the signature.

@henkmollema

This comment has been minimized.

Copy link
Contributor

henkmollema commented May 1, 2017

I'm seeing the same behavior on calling ExistsAsync on a CloudBlob instance after updating the NuGet package to 2.1.0-beta2.

@SergeyKanzhelev SergeyKanzhelev added the bug label May 1, 2017

@michielpost

This comment has been minimized.

Copy link

michielpost commented May 1, 2017

Same issue here. I'm using services.AddDataProtection().PersistKeysToAzureBlobStorage from Microsoft.AspNetCore.DataProtection.AzureStorage, it throws the same StorageException when rendering a form with an AntiForgeryToken.

@SergeyKanzhelev

This comment has been minimized.

Copy link
Member

SergeyKanzhelev commented May 1, 2017

Hm, it seems that DependencyTrackingTelemetryModule was not initialized correctly. Azure queue should be exempt from headers modifications. See Microsoft/ApplicationInsights-dotnet-server#198

I haven't tried this workaround, but I think you can get DependencyTrackingTelemetryModule from the list of services as ITelemetryModule and configure excluded domains. Something like this:

var modules = serviceProvider.GetServices<ITelemetryModule>();
var dependencyModule = services.FirstOrDefault<ServiceDescriptor>(t => t.ImplementationType == typeof(DependencyTrackingTelemetryModule));

var domains = dependencyModule .ExcludeComponentCorrelationHttpHeadersOnDomains;
domains.Add("core.windows.net");
domains.Add("core.chinacloudapi.cn");
domains.Add("core.cloudapi.de");
domains.Add("core.usgovcloudapi.net");
@SergeyKanzhelev

This comment has been minimized.

Copy link
Member

SergeyKanzhelev commented May 1, 2017

@yantang-msft @lmolkova can you please take a look and confirm that this is the case

@maganuk

This comment has been minimized.

Copy link

maganuk commented May 1, 2017

I have the same issue. Went back to beta 1 and it works fine

yantang-msft added a commit to yantang-msft/ApplicationInsights-aspnetcore that referenced this issue May 1, 2017

@DamianEdwards

This comment has been minimized.

Copy link
Member

DamianEdwards commented May 2, 2017

Just confirming I'm seeing this too trying to use 2.1.0-beta2 in https://github.com/aspnet/live.asp.net/ here.

@DamianEdwards

This comment has been minimized.

Copy link
Member

DamianEdwards commented May 2, 2017

The following workaround indeed works (put this in your Startup.Configure method):

var modules = app.ApplicationServices.GetServices<ITelemetryModule>();
var dependencyModule = modules.OfType<DependencyTrackingTelemetryModule>().FirstOrDefault();

if (dependencyModule != null)
{
    var domains = dependencyModule.ExcludeComponentCorrelationHttpHeadersOnDomains;
    domains.Add("core.windows.net");
    domains.Add("core.chinacloudapi.cn");
    domains.Add("core.cloudapi.de");
    domains.Add("core.usgovcloudapi.net");
}
@SergeyKanzhelev

This comment has been minimized.

Copy link
Member

SergeyKanzhelev commented May 2, 2017

@DamianEdwards thanks for confirmation.

@cijothomas when can we ship the update? This bug is nasty!

@henkmollema

This comment has been minimized.

Copy link
Contributor

henkmollema commented May 2, 2017

@SergeyKanzhelev applying your workaround resolves the issue for me as well. Thanks.

cijothomas added a commit that referenced this issue May 3, 2017

Merge pull request #418 from yantang-msft/Fix416
Fix issue #416. Add certain domains to dependency collector's exclude list
@cijothomas

This comment has been minimized.

Copy link
Contributor

cijothomas commented May 3, 2017

Included this in the 2.1 Beta3 release to be released sometime today.

@cijothomas cijothomas closed this May 3, 2017

@cijothomas

This comment has been minimized.

Copy link
Contributor

cijothomas commented May 3, 2017

#418 FIX PR

@joergjo

This comment has been minimized.

Copy link

joergjo commented Jul 1, 2017

Please reopen this issue--you're still missing 127.0.0.1 (i.e. Azure Storage Emulator) as exception. The workaround is the same as above:

var dependencyTrackingTelemetryModule = telemetryModules.OfType<DependencyTrackingTelemetryModule>().FirstOrDefault();
if (dependencyTrackingTelemetryModule != null)
{
    dependencyTrackingTelemetryModule.ExcludeComponentCorrelationHttpHeadersOnDomains.Add("127.0.0.1");
}
@yantang-msft

This comment has been minimized.

Copy link
Contributor

yantang-msft commented Jul 3, 2017

I'm not sure if adding localhost to the exclusion list by default is a good idea. You can at least add you custom domains in client code to workaround this issue.

@joergjo

This comment has been minimized.

Copy link

joergjo commented Jul 5, 2017

You're right--I forgot about scenarios where a service may call another service on localhost and it should be tracked by AI. It would still be nice though to have this interdependency documented explictly (unless I missed that), or maybe even exclude endpoints instead of domains.

@cijothomas

This comment has been minimized.

Copy link
Contributor

cijothomas commented Jul 7, 2017

#488 - for emulator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment