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

chore(deps): update prometheus.client #2136

Merged
merged 5 commits into from
Oct 5, 2022
Merged

chore(deps): update prometheus.client #2136

merged 5 commits into from
Oct 5, 2022

Conversation

phnx47
Copy link
Contributor

@phnx47 phnx47 commented Sep 28, 2022

Fixes #1772

@tomkerkhove I can't remove services.BuildServiceProvider(); without completely rewrite schedule logic. Right now I just updated Prometheus.Client and use DefaultCollectorRegistry.

Next step is rewrite scheduler or we can save it as is... For rewrite we should register Scheduler Job in runtime, Configure section instead of ConfigureServices section, but I never used CronScheduler.AspNetCore. I found this kdcllc/CronScheduler.AspNetCore#29, maybe you know how to register in runtime better.

@trafico-bot trafico-bot bot added the 🔍 Ready for Review Pull Request is not reviewed yet label Sep 28, 2022
@tomkerkhove
Copy link
Owner

Thanks for jumping in! Would you mind fixing the CI please?

@phnx47
Copy link
Contributor Author

phnx47 commented Oct 4, 2022

CI - Code / Verify Codebase (pull_request): Seems this error - dorny/test-reporter#168, but permissions already added.

I ran it in my fork and all tests passed:
https://github.com/phnx47/promitor/actions/runs/3183407827/jobs/5190580488
https://github.com/phnx47/promitor/runs/8703462355

So, something weird, I can't re-run it here...

@phnx47
Copy link
Contributor Author

phnx47 commented Oct 4, 2022

Promitor CI - Resource Discovery Agent and Promitor CI - Scraper Agent: The job running on agent Azure Pipelines 3 ran longer than the maximum time of 60 minutes

Step: Run Integration Tests

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
Current metric type is not a gauge, ignoring.
...
Current metric type is not a gauge, ignoring.
##[error]The operation was canceled.

@tomkerkhove
Copy link
Owner

CI - Code / Verify Codebase (pull_request): Seems this error - dorny/test-reporter#168, but permissions already added.

I ran it in my fork and all tests passed: phnx47/promitor/actions/runs/3183407827/jobs/5190580488 phnx47/promitor/runs/8703462355

So, something weird, I can't re-run it here...

Re-triggered, let's see as it's odd indeed.

@tomkerkhove
Copy link
Owner

Promitor CI - Resource Discovery Agent and Promitor CI - Scraper Agent: The job running on agent Azure Pipelines 3 ran longer than the maximum time of 60 minutes

Step: Run Integration Tests

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
Current metric type is not a gauge, ignoring.
...
Current metric type is not a gauge, ignoring.
##[error]The operation was canceled.

Yes, I've just noticed so that must mean something is wrong with publishing the metrics. Quickly did #2144, let's see if it works.

Copy link
Owner

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@trafico-bot trafico-bot bot added ✅ Approved Pull Request has been approved and can be merged and removed 🔍 Ready for Review Pull Request is not reviewed yet labels Oct 5, 2022
@tomkerkhove tomkerkhove merged commit 83ba66f into tomkerkhove:master Oct 5, 2022
@trafico-bot trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed ✅ Approved Pull Request has been approved and can be merged labels Oct 5, 2022
@phnx47 phnx47 deleted the update-prom-client branch October 6, 2022 00:16
@phnx47
Copy link
Contributor Author

phnx47 commented Oct 10, 2022

@tomkerkhove What about rewrite Scheduler to register jobs in runtime? I Can find time for this. It is not only for prometheus client https://learn.microsoft.com/en-us/dotnet/core/extensions/dependency-injection-guidelines#recommendations

@tomkerkhove
Copy link
Owner

Good callout - I forgot to open an issue for that. Would you mind doing that with some context please?

@phnx47
Copy link
Contributor Author

phnx47 commented Oct 10, 2022

@tomkerkhove Sure, I will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Merged Pull Request has been merged successfully
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to latest versions of Prometheus.Client packages 📦
2 participants