Upgrading to v0.5.0 breaks writing some metrics #20
Comments
|
I will check it |
|
Thank you! I don't really get it but ok... 😐 |
|
@tomkerkhove I downgraded Microsoft.Extensions.DependencyInjection.Abstractions to 3.1.16 in v.0.6.0 version. It should be resolve problems |
|
Thank you! |
|
@tomkerkhove I downgraded Microsoft.Extensions.DependencyInjection.Abstractions to 3.1.13. I don't know how to test promitor in local machine, but I still think, that problem with version-conflict |
|
I don't get it either and v0.6.2 doesn't work as well for me but I don't see why :/ |
|
Another think, only about this: Before v0.5.0 services.AddSingleton(new CollectorRegistry() as ICollectorRegistry); // in code different, but should same behaviour
services.AddSingleton<IMetricFactory, MetricFactory>();After services.AddSingleton<ICollectorRegistry, CollectorRegistry>();
services.AddSingleton<IMetricFactory, MetricFactory>(); |
|
🤯 |
|
Hi @tomkerkhove! Is there any chance you start scrapping before configure/create all metrics? I've found a theoretical race condition, that could lead to a problems if a metric is being created while scrapping. |
|
@tomkerkhove @phnx47 I think I can reproduce some similar problem in scenario when multiple threads trying to create a metrics while scrape is happening on another thread. I've created a unit-test to reproduce the issue and prepare a fix soon. However the affected code was introduced more then a year ago, and this could be some similar but not the root cause for this exact problem. |
|
@tomkerkhove as we cannot reproduce the issue on our side, could you please help with investigation on the issue? It would be perfect to have something runnable to reproduce the issue on our side. Is there any way to create a promitor version configured to use some mocks so I can pull the version and debug a problem? Or could you please try to bump Prometheus.Client libraries one at the time so we can isolate a problem:
Also I suspect the problem might be somewhere around of multiple collector registry creation (I have no ideas how, but if the issue is consistently reproducible on your side, it could mean that different parts of code uses different collector registry... somehow). To ensure that there is only one could you please create a component that injects IEnumerable and check how many object is being injected. And finally please check if there is any collectors registered into Metrics.DefaultCollectorRegistry |
|
I'm able to succesfully bump the client & ASP.NET version through tomkerkhove/promitor#1691 with good results. However, when bumping the DI package to v0.5.0 or latest it is still failing. I'll try to dive deeper into this but the easiest way to run this yourself is through https://github.com/tomkerkhove/promitor/blob/master/development-guide.md#net-development |
|
I'm scratching my head on this one 😅
Inject |
|
HI @tomkerkhove , sorry it github trying to "help" me and hide tags in text :)
I'll try to run your code locally to debug the issue. |
|
I gave it a try and I always get 1 instance of the registry which eventually has multiple collectors. The difference is:
However, this is identical as how it was < v0.5 |
|
Magic =) @tomkerkhove could you please let me know how can I debug the Promitor? As I've got the following error when I run the Scraper project:
|
|
How did you run it? Through Docker compose? |
|
Just by running the project. Is there any way to run it without Docker? |
|
Yes, but then you'll have to configure a few things 😅 This should help you get started: https://github.com/tomkerkhove/promitor/blob/master/development-guide.md#running-promitor |
|
Hi @tomkerkhove ! Sorry for such long delay in updates, but I think I've finally found the root cause of the problem, that makes sense and perfectly explain what have happened. The problem in the Promitor code that explicitly makes a call to services.BuildServiceProvider(). According to the documentation this call creates a very new DI container on each call:
So it means the Promitor works at least with 2 DI containers: the one explicitly created by the code and the default one (created by asp.net runtime). It also means each singleton will be tracked separately in each container. So now it's obvious why simple changing from Register by istance to Register by type broke the metrics:
I hope this makes sense. So my suggestion here: remove the explicit creation of the container, as it affects not just Prometeus.Client infrastructure, but whole application lifetime logic. Update: Probably the bootstrap logic that schedules the syncs should be moved into some new service implementing IHostedService, so it will be automatically started by runtime. |
|
Good point, didn't think about that but did it for a reason. I'll check when I'm back from holiday and come back to it. |
Can you elaborate on what piece you are referring to here please? |
|
I meant the code under ScheduleMetricScraping method. Anyway, could you please double-check if my explanation is right, so we can close the issue on our side? And have a nice holiday =) |
I've had issues where only a subset of metrics were emitted:
So I reverted back to v0.4.0 which gave me the full metrics again:
However, when doing the diff I cannot spot the issue: v0.4.0...v0.5.0
The text was updated successfully, but these errors were encountered: