apm-agent-dotnet
apm-agent-dotnet copied to clipboard
Don't use static Agent API on APM middleware registration
UseElasticApm
function relies on static Agent.Setup(config)
, which throws if registering the middleware was already done. But this is not considering the scenario where more than one IHost
or IWebHost
instances can be running on the same process.
I think the common practice is registering a new IApmAgent
instance as singleton to IServiceCollection
and injecting it into the middleware. AutoMapper had the same issue previously and they fixed it following this approach and they just removed the static API for good in version 9.
That said, having IApmAgent
registered in DI allows injecting into controllers/services instead of relying on statics as documentation recommends.
Furthermore, current configuration approach only allows configuring the Agent form IConfiguration
and would be flexible to have an Options Builder approach like EF Core has.
I agree that UseElasticApm shouldn't rely on static fields and static API should be deprecated.
We already have an issue with integration tests in our Web application that uses APM agent. We are using WebApplicationFactory as explained here: https://docs.microsoft.com/en-us/aspnet/core/test/integration-tests?view=aspnetcore-2.2#customize-webapplicationfactory
When we inject сustomized WebApplicationFactory into different test classes (via IClassFixture as explained in article) then multiple instances created and method UseElasticApm called multiple times. So we catch exception: "The singleton APM agent has already been instantiated and can no longer be configured".
We have that problem only with APM agent and other middlewares work just fine.
What's the progress on this issue? To me this is a pretty big issue, as it bascally means all integration tests fail.
Are there any known work-arounds?
This is a dumb workaround, but it works:
try
{
app.UseAllElasticApm(_configuration);
}
catch (Exception ex)
{
// ignore this error
}
You could, of course, make it more sophisticated.
Thanks. But that only enables it for the first integration test, not the rest. Also it can hide errors occurring in production. What I did now was to run integration tests with Environment=Test, and then use that to exclude registering ElasticApm
@mterwoord, as legitimate workaround for xUnit integration tests is using AssemblyFixture
. It doesn't affect you source code of host configuration. However, AssemblyFixture
isn't suitable for all.
In case of unit tests, you can use some portion of magic instead of static API.
P.S.: I don't justify static api, only suggest workaround to not block using Elastic.Apm
as it's awesome product.
Hey :) Any news on that? I'm mostly interested in using non-static API since we have not only http but also async events with rabbitmq and I'm not sure if static API will be working properly in that scenario.
@Mpdreamz, do you know if there is any plan to fix the issue?
We released the Elastic.Apm.Extensions.Hosting
package which I believe will make this better - it's currently beta, in the next release we can remove the beta flag. That package has an extension method for IHostBuilder
and you don't need to register a middleware anymore, it basically creates the agent and adds its components to IServiceCollection
.
It's also part of the NetCoreAll
package that most of the people use.
Just do this in when you create your hostbuilder
public static IHostBuilder CreateHostBuilder(string[] args) =>
Host.CreateDefaultBuilder(args)
.ConfigureWebHostDefaults(webBuilder => { webBuilder.UseStartup<Startup>(); })
.UseAllElasticApm(); //<- this is from `Elastic.Apm.NetCoreAll` and uses the Elastic.Apm.Extensions.Hosting package
Once you do that, you'll be able to dependency inject a tracer - for example:
public class HomeController : Controller
{
private readonly ITracer _tracer;
public HomeController(ITracer tracer) => _tracer = tracer; //you get the tracer from DI
public IActionResult SomeMethod()
{
//now you can use the non static API
var span = _tracer.CurrentSpan?.StartSpan("MySpan", "SampleSpanType");
try
{
//...app code
}
finally
{
span?.End();
}
return View();
}
}
Sometimes I see people wanting the whole agent instance - I don't understand why people want that (happy to hear what you want to do with it), nevertheless, you can also inject an IApmAgent
instance into your class. I believe most people want to manually create spans and transactions and for that I'd suggest injecting an ITracer
- the tracer is the one that let's you create those.
Now, in the latest beta release of this package I believe we don't have ASP.NET Core monitoring (which is a huge missing thing), but that's already on master, so in the next release that'll be there and you can try it already on master.
Also, I'd like to point out that there is an Agent.IsConfigured
property which lets you avoid the double initialization problem.
On the "static Agent API" part: I understand everyone's concerns, I hope the approach above satisfies peoples needs - about the reasons why we can't drop it I wrote here and maybe this here is also related.
Let me know if this would help with everyone's problem here. Also maybe give a try to this new package.
That package has an extension method for IHostBuilder and you don't need to register a middleware anymore
@gregkalapos I just transitioned my current project to it, this resolves any double initialization problems (race conditions.. one in 4 startups is lucky) I've had with multiple IHostedService
s next to the ASP.NET Core stack but then no HTTP traces are created.
After playing around, this is the only working single configuration I could come up with that works under all cirumcstances:
public static class HostBuilderExtensions
{
public static IHostBuilder UseApm(this IHostBuilder hostBuilder)
{
hostBuilder
.UseElasticApm(
new HttpDiagnosticsSubscriber(),
new EfCoreDiagnosticsSubscriber(),
new SqlClientDiagnosticSubscriber(),
new ElasticsearchDiagnosticsSubscriber(),
new AspNetCoreDiagnosticsSubscriber()) // not included when calling hostBuilder.UseAllElasticApm()
.ConfigureServices(svc =>
{
// custom project-specific helpers and extensions for working with transactions & spans and custom log context enrichment
svc.AddSingleton<IApmCorrelationHelper, ApmCorrelationHelper>();
svc.AddSingleton<MvcActionObservabilityFilter>();
svc.Configure<MvcOptions>(
mvcOptions => mvcOptions.Filters.Add(
new ServiceFilterAttribute(typeof(MvcActionObservabilityFilter)) { IsReusable = true }));
// fix missing HTTP transactions when using Elastic.Apm.Extensions.Hosting APIs
svc.AddTransient<IStartupFilter, ApmStartupFilter>();
if (Type.GetType("Elastic.Apm.Api.Tracer, Elastic.Apm") is { } tracerType
&& Type.GetType("Elastic.Apm.ApmAgent, Elastic.Apm") is { } agentType)
{
svc.AddSingleton(tracerType, sp => sp.GetRequiredService<ITracer>());
svc.AddSingleton(agentType, sp => sp.GetRequiredService<IApmAgent>());
}
});
return hostBuilder;
}
private class ApmStartupFilter : IStartupFilter
{
public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next)
{
return builder =>
{
if (Type.GetType("Elastic.Apm.AspNetCore.ApmMiddleware, Elastic.Apm.AspNetCore") is { } apmMiddleware)
{
builder.UseMiddleware(apmMiddleware);
}
next(builder);
};
}
}
}
Am I missing some configuration option that provides a single configuration and allows for ASP.NET Core + IHostedService usage without startup race issues? (plus I really don't want to miss some transactions that happen at the app start in these IHostedService
s)
Since this needs reflection to access internal types, I'd be happy for a version that works with public API only.
With regards to workarounds with Xunit integration tests and double initialization of Agent
, one workaround is to ensure that each Xunit project only has a single class that takes a WebApplicationFactory
. I had to do this because I'm using a library that consumes APM and I cannot control how it gets bootstrapped.
Sometimes I see people wanting the whole agent instance - I don't understand why people want that (happy to hear what you want to do with it), nevertheless, you can also inject an IApmAgent instance into your class. I believe most people want to manually create spans and transactions and for that I'd suggest injecting an ITracer - the tracer is the one that let's you create those.
One use case where it seems you cannot use ITracer is StackExchengeRedisCache for IDistributedCache.
options.ConnectionMultiplexerFactory = async () =>
{
var connection = await ConnectionMultiplexer.ConnectAsync(options.ConfigurationOptions.ToString());
connection.UseElasticApm();
return connection;
};
You need to pass IApmAgent instance to .UseElasticApm method.
https://discuss.elastic.co/t/how-do-i-apply-elastic-apm-stackexchange-redis-to-microsofts-idistributedcache/292324