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

Injecting TelemetryConfiguration no longer works after updating from v3-Preview to v3 #5353

Closed
andreasohlund opened this issue Dec 9, 2019 · 39 comments · Fixed by #5551
Closed

Comments

@andreasohlund
Copy link

Following the instructions in https://docs.microsoft.com/en-us/azure/azure-functions/functions-monitoring#version-2x-3 used to work in the v3-preview but when updating to v3 + netcoreapp3.1 TelemetryConfiguration is no longer available and function blow up with:

Microsoft.Extensions.DependencyInjection.Abstractions: Unable to resolve service for type 'Microsoft.ApplicationInsights.Extensibility.TelemetryConfiguration'

This commit shows the details of the upgrade attempt:

andreasohlund/PublicApi@2783a2b

"APPINSIGHTS_INSTRUMENTATIONKEY": "..." is set in local.setting.json

Could it be the the v3 host no longer enable telemetry when running locally?

Investigative information

Please provide the following:

  • Timestamp:
  • Function App version (1.0 or 2.0): v3
  • Function App name: PublicApiBackend
  • Function name(s) (as appropriate): N/A
  • Invocation ID:N/A
  • Region:N/A

Repro steps

Provide the steps required to reproduce the problem:

  1. In v3-Preview inject a TelemetryClient or TelemetryConfiguration
  2. Update to v3 and see it fail when running locally

Expected behavior

Should inject the TelemetryConfiguration according to https://docs.microsoft.com/en-us/azure/azure-functions/functions-monitoring#version-2x-3

Actual behavior

Injection fails with

Microsoft.Extensions.DependencyInjection.Abstractions: Unable to resolve service for type 'Microsoft.ApplicationInsights.Extensibility.TelemetryConfiguration'

Known workarounds

None at the stage

Related information

@JAugustusSmith
Copy link

JAugustusSmith commented Dec 10, 2019

I get the same error, worked in 2.2, but doesn't work in 3

private readonly TelemetryClient _telemetryClient;

    public Function1(TelemetryConfiguration telemetryConfiguration)
    {
        _telemetryClient = new TelemetryClient(telemetryConfiguration);
    }

Executed 'Function1' (Failed, Id=e836d513-5157-432f-9b85-11566f864c5e)
[10/12/2019 7:37:51 AM] Microsoft.Extensions.DependencyInjection.Abstractions: Unable to resolve service for type 'Microsoft.ApplicationInsights.Extensibility.TelemetryConfiguration' while attempting to activate 'FunctionApp4.Function1'.
[10/12/2019 7:37:52 AM] An unhandled host error has occurred.
[10/12/2019 7:37:52 AM] Microsoft.Extensions.DependencyInjection.Abstractions: Unable to resolve service for type 'Microsoft.ApplicationInsights.Extensibility.TelemetryConfiguration' while attempting to activate 'FunctionApp4.Function1'.

@andreasohlund
Copy link
Author

This might be related to Azure/azure-functions-dotnet-extensions#5 but I can confirm that my

[assembly: FunctionsStartup(typeof(PublicAPI.Functions.Startup))]

namespace PublicAPI.Functions
{
    public class Startup : FunctionsStartup
    {
        public override void Configure(IFunctionsHostBuilder builder)
        {

is being invoked. Not sure how the telemetry setup is applied though.

@eliashdezr
Copy link

eliashdezr commented Dec 12, 2019

Related issue too: Azure/azure-functions-dotnet-extensions#32

@msft-mw
Copy link

msft-mw commented Dec 14, 2019

I'm seeing something similar running locally where I can't resolve the TelemetryClient either, it just injects a null reference into my class.

@JAugustusSmith
Copy link

I got it to work by Injecting the TelemetryClient manually. This is against what the documentation says in v2

[assembly: FunctionsStartup(typeof(FunctionApp7.Startup))]

namespace FunctionApp7
{
public class Startup : FunctionsStartup
{
public override void Configure(IFunctionsHostBuilder builder)
{
builder.Services.AddApplicationInsightsTelemetry();
}
}
}

@pksorensen
Copy link

@JAugustusSmith What you are doing is just re-adding it to the service collection. That's not how this is supposed to work. Functions are adding its own stuff that we want to resolve. So dont just blindly add the above code without confirmation from microsoft that this is how they want us to do it now. Its a breaking change from 2.0

@pksorensen
Copy link

The reason why its failing is:

Unable to resolve service for type 'Microsoft.Azure.WebJobs.Script.IEnvironment' while attempting to activate 'Microsoft.Azure.WebJobs.Script.Configuration.ApplicationInsightsLoggerOptionsSetup'.

proved by the following

 public class Startup : IWebJobsStartup
    {
         

        public void Configure(IWebJobsBuilder builder)
        {
            var tc = builder.Services.FirstOrDefault(t => t.ServiceType.Name.Contains("TelemetryConfiguration")).ServiceType;

            var a = builder.Services.BuildServiceProvider().GetRequiredService(tc);

        }
    }

as i am stepping though the linked source code.

@pksorensen
Copy link

and the error above is due to me doing the buildserviceprovider before IEnvironment is ready.

image

In the image, a is resolved correctly and b is null.

So its clearly due to wrong type version

@pksorensen
Copy link

typeof(TelemetryConfiguration).AssemblyQualifiedName
"Microsoft.ApplicationInsights.Extensibility.TelemetryConfiguration, Microsoft.ApplicationInsights, Version=2.11.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35"
Startup.TelemetryConfigurationType.AssemblyQualifiedName
"Microsoft.ApplicationInsights.Extensibility.TelemetryConfiguration, Microsoft.ApplicationInsights, Version=2.11.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35"

i am confused, its the same type but a is resolved, b is null.

But this is properly the reason.

Startup.TelemetryConfigurationType.Assembly.Location
"C:\\Users\\pks\\AppData\\Local\\AzureFunctionsTools\\Releases\\3.2.0\\cli_x64\\Microsoft.ApplicationInsights.dll"
typeof(TelemetryConfiguration).Assembly.Location
"C:\\Users\\pks\\source\\repos\\FunctionApp8\\FunctionApp8\\bin\\Debug\\netcoreapp3.1\\bin\\Microsoft.ApplicationInsights.dll"

How do i get that into my application that two dlls are loaded for the same ?

@brettsam
Copy link
Member

brettsam commented Dec 18, 2019

What version of App Insights are you referencing in your project? I'd recommend referencing the latest https://www.nuget.org/packages/Microsoft.Azure.WebJobs.Logging.ApplicationInsights/, rather than the App Insights package itself. Sometimes they move ahead of where we are (we are typically a couple weeks behind) and these odd type resolution issues hit.

@dcousino
Copy link

Using Microsoft.Azure.WebJobs.Logging.ApplicationInsights (v 3.0.14) did not resolve the issue for me.

@pksorensen
Copy link

@brettsam I am using the samme version, see the output from my previus post.

I am finding the type of Telemetry Configuration in the service collection that the framework injects and check the version and locaiton dll, its same version as application but the dll location is the framework and the other is the app location.

@JAugustusSmith
Copy link

Using Microsoft.Azure.WebJobs.Logging.ApplicationInsights also didn't help me. It's the first thing I tried.

@JAugustusSmith What you are doing is just re-adding it to the service collection. That's not how this is supposed to work. Functions are adding its own stuff that we want to resolve. So dont just blindly add the above code without confirmation from microsoft that this is how they want us to do it now. Its a breaking change from 2.0

It was the only way I could get it to work with the current bug. I'm not leaving it in as a solution, but it was either this, or go to 2.2

@pksorensen
Copy link

@JAugustusSmith - i get that, i was facing the same decision myself.

I had this

  public class FixNameProcessor : ITelemetryProcessor
    {
        private ITelemetryProcessor _next;

        public FixNameProcessor(ITelemetryProcessor next)
        {
            // Next TelemetryProcessor in the chain
            _next = next;


        }

        public void Process(ITelemetry item)
        {
            if (item is RequestTelemetry request)
            {
                request.Name = $"{request.Properties["HttpMethod"]} {request.Properties["HttpPath"]}";
                request.Context.Operation.Name = request.Name;
            }

            // Send the item to the next TelemetryProcessor
            _next.Process(item);
        }
    }

that i had added in 2.2 to get better operation names for my functions hosting aspnet core application. in 3.0 its i cant get the TelemetryContext and add it anymore.

But got it working now using reflection only instead and building the telemetry processor by hand at runtime using il code generation.

I can make a class that can be resolved with the type of the telemetry configuration.

 public class TeleetryConfigurationProvider
    {
        public Type serviceType;
        
        public TeleetryConfigurationProvider(Type serviceType)
        {
            this.serviceType = serviceType;
        }

and register it

builder.Services.AddSingleton(new TeleetryConfigurationProvider(builder.Services.FirstOrDefault(t => t.ServiceType.Name.Contains("TelemetryConfiguration"))?.ServiceType));

and add a telemetry processor

   internal void Initialize(IServiceProvider serviceProvider)
        {
            if (serviceType != null)
            {

                var tc = serviceProvider.GetService(serviceType);


                var chain = serviceType.GetProperty("TelemetryProcessorChainBuilder").GetValue(tc);
                var ITelemetryProcessor = serviceType.Assembly.GetType("Microsoft.ApplicationInsights.Extensibility.ITelemetryProcessor");
                 
                //  var func = Delegate.CreateDelegate(funcType, methodInfo);
                // var f = (object o) => methodInfo.Invoke(this, new[] { o });
                chain.GetType().GetMethod("Use").Invoke(chain, new object[] { this.GetType().GetMethod("GetMethod").MakeGenericMethod(ITelemetryProcessor).Invoke(this, null) });
                chain.GetType().GetMethod("Build").Invoke(chain, new object[] { });
            }
        }

that uses the fixrequest method to set operation name.

 public static void FixRequest(dynamic request)
        {
            if(request.GetType().Name == "RequestTelemetry")
            {
              //  var Properties = o.GetType().GetProperty("Properties").GetValue(o) as IDictionary<string,string>;
                request.Name = $"{request.Properties["HttpMethod"]} {request.Properties["HttpPath"]}";
                request.Context.Operation.Name = request.Name;
            }
        }

that is called from il generated code.

  private  TypeBuilder GetTypeBuilder(string name)
        {
            var ITelemetryProcessor = serviceType.Assembly.GetType("Microsoft.ApplicationInsights.Extensibility.ITelemetryProcessor");
            var ITelemetry = serviceType.Assembly.GetType("Microsoft.ApplicationInsights.Channel.ITelemetry");
            var RequestTelemetry = serviceType.Assembly.GetType("Microsoft.ApplicationInsights.DataContracts.RequestTelemetry");

            var typeSignature = name;
            var an = new AssemblyName(typeSignature);
            var assemblyBuilder = AssemblyBuilder.DefineDynamicAssembly(new AssemblyName(Guid.NewGuid().ToString()), AssemblyBuilderAccess.Run);
            ModuleBuilder moduleBuilder = assemblyBuilder.DefineDynamicModule("MainModule");
            TypeBuilder tb = moduleBuilder.DefineType(typeSignature,
                    TypeAttributes.Public |
                    TypeAttributes.Class |
                    TypeAttributes.AutoClass |
                    TypeAttributes.AnsiClass |
                    TypeAttributes.BeforeFieldInit |
                    TypeAttributes.AutoLayout,
                    null);
            tb.AddInterfaceImplementation(ITelemetryProcessor);

            var field = tb.DefineField("_next", ITelemetryProcessor, FieldAttributes.Private);
            var someMethod = field.FieldType.GetMethod("Process");

            var process = tb.DefineMethod("Process", MethodAttributes.Public| MethodAttributes.Virtual, null, parameterTypes: new Type[] { ITelemetry });

            var ilGen = process.GetILGenerator();
           // ilGen.Emit(OpCodes.Ldarg_0);
           
            //  ilGen.Emit(OpCodes.Ldarg_1);
            //  
            var b = this.GetType().GetMethod("FixRequest", BindingFlags.Static| BindingFlags.Public);
            ilGen.Emit(OpCodes.Ldarg_1);
            ilGen.Emit(OpCodes.Call,b );


            ilGen.Emit(OpCodes.Ldarg_0);
            ilGen.Emit(OpCodes.Ldfld, field);
            ilGen.Emit(OpCodes.Ldarg_1);

            ilGen.Emit(OpCodes.Callvirt, someMethod);
            ilGen.Emit(OpCodes.Ret);

            tb.DefineMethodOverride(process, someMethod);

            ConstructorBuilder constructor = tb.DefineConstructor(MethodAttributes.Public,
                CallingConventions.Standard,
                new Type[] { ITelemetryProcessor});

           
            var setIL = constructor.GetILGenerator();
             
            setIL.Emit(OpCodes.Ldarg_0);
            setIL.Emit(OpCodes.Call, typeof(object).GetConstructors().Single());
            setIL.Emit(OpCodes.Ldarg_0);
            setIL.Emit(OpCodes.Ldarg_1);
            setIL.Emit(OpCodes.Stfld, field);
            setIL.Emit(OpCodes.Ret);

          

           
           

            return tb;
        }

So thanks for making a breaking change here :) I would love to see @brettsam provide a minimal example where its resolved out of the box and it not being a breaking change. I assume that this was not testet. So lets not assume its the same as the other times we had these issues.

@fabiocav fabiocav added this to the Triaged milestone Dec 19, 2019
@fabiocav
Copy link
Member

Flagging this as triaged for investigation.

We'll provide an update once we have more information (this will be triaged as soon as possible for a sprint assignment).

@fabiocav
Copy link
Member

fabiocav commented Dec 19, 2019

One quick comment....

We took a very quick initial look at this and it doesn't look like a runtime regression. This issue is present in 2.0 and the preview releases as well, and what changed here was actually the default behavior of the SDK build process.

Adding the following project property is a workaround that will restore the previous behavior and work in 3.0:

<SkipFunctionsDepsCopy>true</SkipFunctionsDepsCopy>

This should unblock you until a permanent fix is made.

@dcousino
Copy link

@fabiocav while adding <SkipFunctionsDepsCopy>true</SkipFunctionsDepsCopy> does fix the TelemetryConfiguration issue, it introduces a new one Microsoft.Azure.WebJobs.Host: Can't bind BlobTrigger to type 'Microsoft.WindowsAzure.Storage.Blob.CloudBlockBlob'

@fabiocav
Copy link
Member

@dcousino, yes, removing that file has potential side effects like what you're seeing before, but this wouldn't be a change in behavior from 2.0.

Are you using the storage SDK supported by the storage extension you're referencing? There will be a version of the extension coming up soon targeting newer storage releases, but in some of those cases, there were significant changes and they are essentially completely different SDKs, which may lead to what you're seeing above. You can follow the progress on that here: Azure/azure-webjobs-sdk#2065 (comment)

@briandunnington
Copy link

@fabiocav I can also confirm that <SkipFunctionsDepsCopy>true</SkipFunctionsDepsCopy> works for us. But since there is no documentation about that property or what it does, I am hesitant to use it so that I don't run into side effects similar to @dcousino (and more importantly, to avoid any other unknown unknowns). Is there any estimation on a more permanent long term solution?

@kylepope-ge
Copy link

I can also confirm that <SkipFunctionsDepsCopy>true</SkipFunctionsDepsCopy> works. In my case, I had multiple project files, so I needed to add the SkipFunctionsDepsCopy property to more than one place. Took me forever-1 to figure out how to get the workaround to take effect. Hopefully we can get a permanent solution soon...

@metoule
Copy link

metoule commented Jan 16, 2020

Using <SkipFunctionsDepsCopy>true</SkipFunctionsDepsCopy> allows TelemetryConfiguration to be properly injected, however it triggers Azure/Azure-Functions#1370 because the function.deps.json file is not properly copied to the output or publish folder..

Is there a timeline for a permanent fix to this issue?

@ngg
Copy link

ngg commented Jan 20, 2020

Might be the same as #4952

@jabbera
Copy link

jabbera commented Jan 22, 2020

I just ran into this.

@Bpflugrad
Copy link

Similar to @jabbera this also started affecting one of my V2 functions. I've spent about 4 hours switching the version of Microsoft.ApplicationInsights but no version from 2.9.1+ will work.
2.11.0 was working prior to 1/22, so something must have changed in the Function host, but this is proving quite difficult to fix.

@galvesribeiro
Copy link

To avoid break the function code or to have to deal with obscure MSBuild props, we went to the simplest route for now:

builder.Services.AddSingleton<TelemetryConfiguration>(sp =>
            {
                var key = Environment.GetEnvironmentVariable("APPINSIGHTS_INSTRUMENTATIONKEY");
                if (!string.IsNullOrWhiteSpace(key))
                {
                    return new TelemetryConfiguration(key);
                }
                return new TelemetryConfiguration();
            });

Although it is a ridiculous solution, it at least works for now and don't have the side-effects other had mentioned here...

This is a critical problem that should be catch on the release tests as it is one of the built-in features provided by the host... I hope some fix would come soon...

@dougxwok2
Copy link

@fabiocav You've closed this with the merge of the fix, but when can we expect the release train to push out updated packages that actually resolve this for end users?

@Adrian10988
Copy link

Where is <SkipFunctionsDepsCopy>true</SkipFunctionsDepsCopy> supposed to be placed?

@andrewleader
Copy link

andrewleader commented Jan 31, 2020

@Adrian10988 I placed it in the top level property group (and it goes in your MyProjectName.csproj project file), so...

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>netcoreapp3.0</TargetFramework>
    <AzureFunctionsVersion>v3</AzureFunctionsVersion>
    <SkipFunctionsDepsCopy>true</SkipFunctionsDepsCopy>
  </PropertyGroup>
  <ItemGroup>
    ....

And I can confirm that it worked for my basic function.

@brettsam
Copy link
Member

@dougxwok2 -- this will be in the next release payload which should start within the next couple of business days. I'd tentatively say it'll be everywhere within 2 weeks.

@deyanp
Copy link

deyanp commented Feb 3, 2020

@brettsam , can this patch be expedited? The workaround is causing other issues, and the inability to inject TelemetryConfiguration in v3 is affecting customer projects and deliverables ...

@Adrian10988
Copy link

Adrian10988 commented Feb 14, 2020

Any update on when this will be released? We cannot keep operation id between calls that go through our azure functions so debugging and bench marking is a nightmare right now

@brettsam
Copy link
Member

It's part of https://github.com/Azure/azure-functions-host/releases/tag/v2.0.12998. The rollout has started. Unless we hit snags it should be everywhere later next week.

@richardszalay
Copy link

richardszalay commented Feb 22, 2020

@brettsam Is there a timeline for the VS tools to be updated to use 2.0.12998 / 3.0.13130? Alternatively, is there a supported way to update VS's tools to use a newer runtime version?

@galvesribeiro
Copy link

@brettsam any update? 9 days passed and I don't see any update neither the tools or the packages. Thanks

@richardszalay
Copy link

@galvesribeiro I believe @brettsam was referring to the rollout to Azure Function instances, which has indeed happened for us.

@galvesribeiro
Copy link

@richardszalay what you mean? the service runtime?

If that is the case, we still need to get tool/package updates otherwise we still have the same problem on our dev machines or on Azure IoT Edge and/or kubernetes based Azure Function deployments...

@richardszalay
Copy link

richardszalay commented Feb 24, 2020

@galvesribeiro Correct.

Here's what I'm using to work around the issue in local environments which is enough for us, but you'll want to test it with your IoT / Kubernetes environments to ensure it doesn't clobber any platform AI data.

public override void Configure(IFunctionsHostBuilder builder)
{
    if (IsAppInsightsRegistrationRequired(builder.Services))
    {
        builder.Services.AddSingleton(sp =>
        {
            var key = Environment.GetEnvironmentVariable("APPINSIGHTS_INSTRUMENTATIONKEY");
            var telemetryConfiguration = (!string.IsNullOrWhiteSpace(key))
                ? new TelemetryConfiguration(key)
                : new TelemetryConfiguration();

            telemetryConfiguration.TelemetryInitializers.Add(new OperationCorrelationTelemetryInitializer());
            return new TelemetryClient(telemetryConfiguration);
        });
    }
}

// Temporary workaround until the client tools (ie. VS support) includes the fixes from 3.0.13130
static bool IsAppInsightsRegistrationRequired(IEnumerable<ServiceDescriptor> services) =>
    !IsServiceRegistered<TelemetryClient>(services);

static bool IsServiceRegistered<T>(IEnumerable<ServiceDescriptor> services) =>
    services.Any(descriptor => descriptor.ServiceType == typeof(T));

@galvesribeiro
Copy link

Yes, as I mentioned here #5353 (comment) We're doing something similar to inject the config type. It works both locally and on the hosts but it is far from ideal.

To fix that, we need everything updated with the fix...

@MatisseHack
Copy link

@brettsam any idea when version 3.0.13130 (with this fix) will hit Azure Gov?

@Azure Azure locked as resolved and limited conversation to collaborators Apr 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.